Hi Ben, hi David, On Mon, 2 Jun 2008 23:19:23 +0100, Ben Dooks wrote: > On Mon, Jun 02, 2008 at 02:03:46PM -0700, David Brownell wrote: > > On Monday 02 June 2008, Jean Delvare wrote: > > > > > > > Sorry, I meant what happens if a NAK is received after the address > > > > part of the i2c_msg has been sent, when sending the msg data? Is this > > > > a case for an error like EPIPE, ENOLINK or EREMOTEIO? > > > > > > Our current error codes document suggests EIO. > > > > And also says it's sufficiently vague as to make avoiding it > > be worthwhile. :) > > > > > > > EPIPE and ENOLINK do not > > > make much sense IMHO. EREMOTEIO would make some sense, but I suspect > > > that many drivers won't be able to isolate this specific error and thus > > > will still return EIO. David, what do you think? > > > > This is one of those cases where the I2C programming interface is > > insufficient. As noted in the comment added to i2c_transfer() by > > the patch updating fault code passthrough handling in i2c-core.c: > > > > > > * - When we get a NAK after transmitting N bytes to a slave, > > > > * there is no way to report "N" ... or to let the master > > > > * continue executing the rest of this combined message, if > > > > * that's the appropriate response. > > I thought the i2c_msg flag to ignore NAK was the way to let the master > continue, although this does mean that you loose the fact that a NAK > actually happened. > > The case of letting the master continue after such an error still > makes reporting the amount of data written difficult, how do you > indicate I wrote m bytes, got an error, and then continued on to > write all n bytes of the i2c_msgs provided? Can anyone think of > an actual example of where this would be useful?
The "ignore nack" flag is a hack, it's a divergence from the I2C standard to support a couple broken devices out there. We don't give a damn to reporting fine-grained errors when using it. > > > As I mentioned earlier, the best way to address this issue would > > be to carve a 16 bit field out of the existing struct padding and > > use that from updated i2c_adapter drivers to report N (actual_len). > > So you are suggesting adding a done field to the i2c_msg structure > that is updated by the bus driver as it goes along? For arm, this > should fit, but are there any other architectures out there that are > going to be affected by this change in structure (it does, iirc, get > exported to userspace). Yes, it is exported to user-space. But I guess that the binary compatibility constraints are the same for user-space and kernel-space. > > If you add a __u16 done field to the i2c_msg, then you either have to > find a special value for 'not filled in' and have the i2c core fill > in the i2c_msg array at start time? > > > The standard way to report short writes is not via errno values... > > but by reporting how many bytes were written. See "man 2 write". > > It's not what the I2C stack does now, though. > > Yes, that does make sense to return the amount of data written to > the user, however having a quick look through all users of > i2c_transfer() all just want to know if all the messages where > sent (which is an easier check that to compute the amount of > data in all the messages). > > Would it better to add an new i2c_transfer-alike call which would > return the amount of data written, instead of going around modifying > all users of i2c_transfer() ? This would also make transfering from > one call to the other, with i2c_transfer becoming deprecated? > > I was just wondering if it would be useful to add an extra field > after the buffer to indicate the error that happened to the message > such as NAK, bus arbitration, etc. The main question here is: who needs this? I2C errors don't happen frequently, and when they happen, how many devices are able to recover with a partial transaction based on how far the initial transaction went? I suspect that most devices will need to restart the transaction from scratch anyway, and for those who could spare a few bytes, it it really worth it? Driver authors most certainly won't bother. Given the compatibility constraints, this all looks like a lot of work and pain for a benefit which is essentially aesthetic. I agree that the current API isn't nice, but very much doubt that there's anything to win by reworking it in practice. So I think my time will be much better used for other matters. > > > That could be done if i2c-core got some updates, at least with > > any i2c adapters that get updated to better report this fault > > (using that new field living in what's now struct padding). > > As noted above, is it padding on _all_ architectures? > > > Failing such updates, I'd avoid EREMOTEIO since that's what USB > > uses to indicate short *reads* (not writes) when they're flagged > > for reporting as errors (not the default). EFBIG (too big) > > would make more sense if short writes must be reported as some > > kind of fault. > -- Jean Delvare _______________________________________________ i2c mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/i2c
