[i2c] [patch 2.6.23-rc5] i2c_msg kerneldoc

[Jean Delvare]

Hi David,

On Sat, 08 Sep 2007 15:33:35 -0700, David Brownell wrote:
> Actually as I commented earlier, I think that the arbitrary limitation
> that the buffers be "fixed/small size" is problematic and error prone.
> 
> That's experience talking.  When implementations have such limits, it's
> only a matter of time before those limits are found to be too small.
> (Unfortunately, with this I2C stack removing that limit looks tricky.)

The I2C stack itself doesn't have such limitations. Only the SMBus
helpers do, and for a good reason: the SMBus protocol itself limits the
block size to 32 bytes. So I do not plan to change anything on that
front until someone points out a real-world problem with the current
implementation.

> > This suggests that our interface is less than optimal and we should let
> > the caller pass the buffer size? If we want to do this, now is probably
> > the right time, as there is a single user of this function in-tree.
> 
> Ideally we'd have both TX and RX block paths able to take buffers of
> any size physically possible with the encoding (1-255), and to report
> just how many bytes were transferred.  Like most I/O interfaces ...

What makes you think that 255 is the maximum size?

> Agreed that callers should be able to specify how many bytes they
> will accept; that fixes one of the problems in this area.  As part
> of this patch??

Of course not, that would be a different patch. It might not be as easy
as it sounds, BTW, because of the i2c-dev side of things.

> > > + * Returns the number of bytes read in the slave's response, else a
> > > + * negative number to indicate some kind of error.
> 
> Actually that irks me.  The kernel convention is "returns negative
> errno", and that's what should be done here and elsewhere in I2C.
> Without some convention for error codes, callers can only guess
> about what went wrong and what recovery is appropriate.

Agreed, this was a rather bad idea. There were some complaints about
that choice already:
  http://www.lm-sensors.org/ticket/2203
I would welcome patches improving the situation. You'll have to pay
attention to the fact that some drivers are explicitly checking for -1,
that would have to be fixed first.

> (...)
> By the way, did you consider how this affects I2C_RDWR from userspace?

No, and I do not really care. I2C_M_RECV_LEN is meant for SMBus block
reads, and SMBus block reads should be done from userspace using
I2C_SMBUS, not I2C_RDWR.

-- 
Jean Delvare