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

[Jon Smirl <jonsmirl@gmail.com>]

On Tue, May 26, 2009 at 1:03 PM, Timur Tabi <timur@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 be=
havior. =A0I also think returning the actual value of the expression is bet=
ter than a return code. =A0Remember, the primary purpose of this macro is t=
o wait for a hardware register to change. =A0Contrast this to wait_event_xx=
x, which usually queries a variable. =A0Therefore, the hardware register ma=
y set multiple bits. =A0For instance, you could do this:
>
> ret =3D spin_event_timeout(in_be32(x) & 0x14, ...);
>
> if (ret & 0x10)
> =A0 =A0 =A0 =A0do something here
>
> if (ret & 0x04)
> =A0 =A0 =A0 =A0do something else here

If (ret =3D=3D 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 =3D spin_event_timeout(result =3D (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 =3D spin_event_timeout(in_be32(x) & 0x14, ...);
if (rc)
   timeout_happened;



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



--=20
Jon Smirl
jonsmirl@gmail.com