[PATCH v5] introduce macro spin_event_timeout()
Scott Wood
scottwood at freescale.com
Wed Mar 11 09:58:58 EST 2009
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 at 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.
More information about the Linuxppc-dev
mailing list