linux-kernel - Re: [PATCH] i2c/busses: (mpc) Add support for SMBUS_READ_BLOCK

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1321466135.7847.55.camel@oslab-l1>
Date:	Wed, 16 Nov 2011 09:55:35 -0800
From:	York Sun <yorksun@...escale.com>
To:	<guenter.roeck@...csson.com>
CC:	Tabi Timur-B04825 <B04825@...escale.com>,
	"linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"B29983@...escale.com" <B29983@...escale.com>
Subject: Re: [PATCH] i2c/busses: (mpc) Add support for SMBUS_READ_BLOCK_DATA

On Wed, 2011-11-16 at 09:36 -0800, Guenter Roeck wrote:
> On Wed, 2011-11-16 at 12:28 -0500, York Sun wrote:
> > I am not sure if this mail will reach the list as I am not subscribed.
> > 
> > In the v2 patch (actually both version), you wrote
> > 
> > > +		byte = readb(i2c->base + MPC_I2C_DR);
> > > +
> > > +		/* Adjust length if first received byte is length */
> > > +		if (i == 0 && recv_len) {
> > > +			if (byte == 0 || byte > I2C_SMBUS_BLOCK_MAX)
> > > +				return -EPROTO;
> > > +			length += byte;
> > > +			/*
> > > +			 * For block reads, generate txack here if data length
> > > +			 * is 1 byte (total length is 2 bytes).
> > > +			 */
> > > +			if (length == 2)
> > > +				writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_MSTA
> > > +					 | CCR_TXAK);
> > > +		}
> > > +		data[i] = byte;
> > 
> > I am wondering if the first byte should be left out for data[] if the
> > recv_len is non-zero. If I understand correctly, this patch is to
> > support of SMBus with multiple byte read. The SMBus is different from
> > I2C bus on multiple byte. The first data is byte count vs slave data for
> > I2C. So you will receive all data preceded by the byte count, which is
> > one more than your loop.
> > 
> York,
> 
> The calling code expects the data length in data[0], and the actual data
> in data[1] .. data[<byte_count>]. The initial value for length is 1; the
> byte count is added to it, so <byte count + 1> bytes are placed into the
> buffer.
> 
> Thanks,
> Guenter

Thanks for explanation. I am more confused by the length += byte now.
For I2C bus, if you need length of byte, just keep reading until you get
all of them. Of course you need to deal with the ACK. For SMBus, it is
similar but you shouldn't read more after the byte count which is in the
first data. If you want to read length of data but the block size is
bigger than length, you should call block read at first place. If the
block size is smaller than length, why increase the length? Does your
SMBus controller only support fixed block size and not support single
byte read? If it does, I would do

Block, Block, Block, byte, byte... until length of data

Regards,

York



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/