[i2c] [patch 2.6.23-rc5 2/2] i2c-algo-bit read block data bugfix

Jean Delvare khali at linux-fr.org
Sun Sep 9 15:20:41 CEST 2007


On Sat, 08 Sep 2007 16:53:51 -0700, David Brownell wrote:
> > The I2C stack would have been better off if "i2c_msg" included both
> > a "len" (caller provides) and an "actual_len" (reported by adapter).
> 
> I think it could do so with little trouble.  The current layout is:
> 
> 	u16	addr
> 	u16	flags
> 	u16	len
> 	PADDING
> 	u8	*buf
> 
> Where PADDING is 2 bytes for both 32-bit and 64-bit kernels.  That's
> just the right size for a new field reporting actual length ...
> 
> 
> > That way for block RX the caller could provide as much (or as little)
> > as it needed but the actual length would still be reported ... and
> > for multibyte TX you'd be able to tell if the device choked before
> > accepting all the bytes you were trying to feed it.
> 
> The problem would of course be how to phase in support for such a
> new i2c_msg.actual_len field.
> 
> Probably the simplest approach is to have i2c_transfer() initialize
> that to a value that adapters won't set.  Then if the returned value
> is anything else, the caller knows the adapter which changed it.
> Given that approach:
> 
>  - Zero could be used ... if it weren't the actual_len value
>    to return when the address doesn't get ACKed.
>    
>  - Another obvious candidate is 0xffff, implying an upper limit
>    for "len".  If 0xffef were that limit, returning values from
>    0xfff0-0xfffe could indicate specific faults.  (Then name that
>    new field "status".)
> 
>  - Those could be combined.  Initialize to zero; have the limit
>    anyway; and the adapter would use (say) 0xfff0 to indicate
>    the "device didn't ACK" fault.
> 
> All without losing binary compatibility with userspace, since
> those padding bits are currently available (but unused).  Lengths
> over 8K are already rejected by i2cdev_ioctl(), and I have a hard
> time imagining many drivers topping even 63K...
> 
> Comments?  There's no evident rush on this issue, but it would
> be good to fix this reporting glitch in the I2C interface.

What real-world problem are you trying to solve? In traditional I2C
messaging, the master decides how many bytes it wants to write to or
read from the slave, so buffer sizing is not an issue. The only case
where the slave decides how many bytes it wants to return is the SMBus
block read, and my original plan was to fix this in
i2c_smbus_read_block_data(). This is much smaller in scope that the
change you propose, and as far as I can see, it addresses the problem at
hand.

-- 
Jean Delvare



More information about the i2c mailing list