[i2c] [patch 2.6.24-rc1-git] i2c-algo-bit, don't yield()

[Jean Delvare]

On Thu, 15 Nov 2007 23:16:44 +0100, Jean Delvare wrote:
> Hi David,
> 
> On Thu, 15 Nov 2007 00:43:32 -0800, David Brownell wrote:
> > On Tuesday 06 November 2007, Jean Delvare wrote:
> > > 	This automatic retry thing is misimplemented in at least
> > > two bus drivers, and causes delays for everyone (e.g. when framebuffer
> > > drivers probe for EDID EEPROMs). That's enough harm to warrant a swift
> > > removal. If anybody really cares, we'll discuss the matter with them
> > > when they complain.
> > 
> > OK ... here's a patch removing it from i2c-algo-bit
> > (and thence its users).
> 
> Great, thanks.
> 
> > 
> > - Dave
> > 
> > =============
> > Remove the "retry address stage" mechanism from i2c-algo-bit.  That
> > mechanism is not something that upper level code can rely on; hardly
> > any I2C adapter drivers implement it, much less do it correctly.
> 
> Adapters which do not implement it, or do not implement it correctly,
> do _not_ use i2c-algo-bit, so that comment is hardly relevant for a
> patch that only touches i2c-algo-bit. It would be more suitable for the
> future patch that will remove the structure field.
> 
> > Plus the notion itself is dubious.
> 
> Not sure what you mean here. The intent is rather clear: as you
> underlined yourself below, I2C chips don't have to ack the address byte
> if they're busy.
> 
> > 
> > Return any actual fault code returned in preferece to -EREMOTEIO, when
> > trying to address a specific device.
> 
> Typo: preference.

Two more things. First, the good news: with your patch applied,
"modprobe eeprom" is down from 5.2 s to 1.4 s on my system. That's just
an example of the performance boost brought by your patch. The
difference is admittedly magnified by the fact that one of my
(radeonfb) i2c adapters is broken and all transactions timeout there,
but the principle still applies to the general case.

Second, about the returned error code: we are deleting the retry
mechanism from bus drivers on the basis that it's a chip-specific
attribute whether retries make sense or not. If so, shouldn't we give
chip drivers a way to differentiate between a slave not acking its
address and other transaction errors? I think that i2c-algo-bit (and
other bus drivers) should return a unique error code (-ENODEV?) for the
former case. What do you think?

Thanks,
-- 
Jean Delvare