[PATCH 1/2 v8] powerpc: introduce macro spin_event_timeout()

Jon Smirl jonsmirl at gmail.com
Wed May 27 03:56:31 EST 2009


On Tue, May 26, 2009 at 1:03 PM, Timur Tabi <timur at freescale.com> wrote:
> Geoff Thorpe wrote:
>
>> So from this user's perspective (FWIW), it would come as a surprise if
>> the return value reflected the evaluated expression rather than what
>> happened w.r.t. the spin/timeout.
>
> It shouldn't come as a surprise because I've thoroughly documented the behavior.  I also think returning the actual value of the expression is better than a return code.  Remember, the primary purpose of this macro is to wait for a hardware register to change.  Contrast this to wait_event_xxx, which usually queries a variable.  Therefore, the hardware register may set multiple bits.  For instance, you could do this:
>
> ret = spin_event_timeout(in_be32(x) & 0x14, ...);
>
> if (ret & 0x10)
>        do something here
>
> if (ret & 0x04)
>        do something else here

If (ret == 0)
    timeout_happened;

That's the part that looks wrong.

>
> I think the ability to do this is more important than making the code as similar as possible to wait_event_xxx.

Why not this?

rc = spin_event_timeout(result = (in_be32(x) & 0x14), ...);
if (rc)
   timeout_happened;

if (result & 0x10)
       do something here

if (result & 0x04)
       do something else here


Then if I don't care about the result (which I think is the common case)...

rc = spin_event_timeout(in_be32(x) & 0x14, ...);
if (rc)
   timeout_happened;



>
> --
> Timur Tabi
> Linux kernel developer at Freescale
>



-- 
Jon Smirl
jonsmirl at gmail.com



More information about the Linuxppc-dev mailing list