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

[Jean Delvare]

On Tue, 6 Nov 2007 11:01:08 -0800, David Brownell wrote:
> On Tuesday 06 November 2007, Jean Delvare wrote:
> > On Sun, 4 Nov 2007 12:38:35 -0700, David Brownell wrote:
> > > Don't use yield().  It can have nasty scheduling properties; and in any
> > > case this code could be interrupted or otherwise preempted at any time.
> > 
> > Can you please be more specific? I can't see any problem here. We're
> > between two I2C transactions, how could being rescheduled be an issue?
> 
> Use (== abuse) of yield() is one of Andrew Mortons pet bugaboos.
> 
> I forget the details -- I think they related to trashing state
> that kept the scheduler well-behaved -- but the short version of
> the story was that it shouldn't really be used.

If yield() shouldn't be used, then it should be removed from the driver
API. Or at least be documented as such.

> Turn it around:  why use it at all?

Because bit-banged I2C causes a high latency and we have an opportunity
to let other parts of the kernel run at this point. Otherwise the
latency is even worse.

> Or why use yield() instead of cond_resched()?

That I can't say, I have no idea what the difference is. I see yield()
as cooperative preemption, that's about all I know. If cond_resched()
is better and you can explain to me why, I'm fine changing yield() for
cond_resched().

> > > This address retry mechanism should be removed from the I2C stack at some
> > > point.  Meanwhile, there's no point in having its primary implementation
> > > misbehave.
> > 
> > Well, I seem to remember we agreed on that and nobody objected, so
> > rather than trying to fix this code, what about sending a patch that
> > rips the whole thing off?
> 
> NYET.  We also wanted to try a less bloodthirsty approach
> than just ripping it out ... in case anyone relied on it.  :)

I don't remember that part (must be my selective memory). Really, I
don't care. 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.

-- 
Jean Delvare