Re: [PATCH] i2c/busses: (mpc) Add support for SMBUS_READ_BLOCK_DATA

[York Sun <yorksun-KZfg59tc24xl57MIdRCFDg@public.gmane.org>]

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