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

David Brownell david-b at pacbell.net
Sun Sep 9 18:47:40 CEST 2007


> > > 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 ...
> >	[ later: or similar i2c_msg status ]
> > 
> > ...
>
> What real-world problem are you trying to solve?

None just now, in the sense that I don't have anything that can't
proceed without such interface bugfixes.

The change would be more to ensure that when problems in this area
crop up later, some key fixes will already be in place.  


>	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.

This isn't the buffer sizing issue; it's a flakey-status-reporting issue.

Sure the host may decide it wants to send 200 bytes, but when the slave
NAKs the thirtieth byte (stopping that transfer) robust drivers would
need to know and compensate ... rather than assuming the slave has seen
the critical data in bytes 87-133 (or whatever) just because the I2C
stack interface is unable to report that particular shortfall.


>	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().

The only place in a "standardized" protocol, maybe ... there's also the
write block data then read block data proc call, which is so similar
that I suspect your same approach can be applied there too.

But there's no reason to expect that other devices out there won't work
differently.  SMBus is mostly just one possible set of policies on top
of I2C ... while products can (and do!) easily use different policies.
In fact, with firmware-based devices, it's especially easy to do so.


>	This is much smaller in scope that the
> change you propose, and as far as I can see, it addresses the problem at
> hand.

Not really.  There are several issues I was commenting on.  One was
the way read_block_data() can't cope with "actual size" buffers, and
expects always to be able to fit 32 bytes there.  That's clearly worth
fixing in its own right.  Another is a broader issue, of which that
could be said to be just one facet:  the way that the i2c_msg layer is
hiding many faults.

- Dave




More information about the i2c mailing list