Re: [PATCH v5] introduce macro spin_event_timeout()

[Scott Wood <scottwood@freescale.com>]

Josh Boyer wrote:
> On Tue, Mar 10, 2009 at 05:33:08PM -0500, Scott Wood wrote:
>> Timur Tabi wrote:
>>> The macro spin_event_timeout() takes a condition and timeout value
>>> (in microseconds) as parameters.  It spins until either the condition is true
>>> or the timeout expires.  It returns zero if the timeout expires first, non-zero
>>> otherwise.
>>>
>>> This primary purpose of this macro is to poll on a hardware register until a
>>> status bit changes.  The timeout ensures that the loop still terminates if the
>>> bit doesn't change as expected.  This macro makes it easier for driver
>>> developers to perform this kind of operation properly.
>>>
>>> Signed-off-by: Timur Tabi <timur@freescale.com>
>>> ---
>>>
>>> v5: ported to arch/powerpc, made it powerpc-specific, eliminated udelay
>> Why make it powerpc-specific?  This would be nice to have in  
>> arch-independent code.
> 
> That's just mean.  He already posted it to lkml and was told to make it
> powerpc specific by Alan.

Well, that's what happens when a discussion hops mailing lists with no 
backreference. :-P

I don't see anywhere where he says it should be architecture dependent, 
but rather a general "I don't like this, get off my lawn!" response.

I cannot agree with the "we shouldn't be encouraging this" sentiment; 
people don't generally do spin loops because they're lazy[1], but rather 
because the hardware demands it -- and it's hardly only on powerpc (much 
less just "some Freescale drivers") that I've encountered hardware that 
demands it, typiclally during reset/initialization or similarly non-hot 
paths.  Why not provide something less likely to have bugs (the timeout 
case is unlikely to be well tested), more easily seen when reviewing a 
patch, and more likely to result in spin loops *with* a timeout rather 
than without?

-Scott

[1] Or rather, those that do should be smacked down during patch review.