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

[Jon Smirl <jonsmirl@gmail.com>]

On Tue, May 26, 2009 at 2:17 PM, Timur Tabi <timur@freescale.com> wrote:
> Geoff Thorpe wrote:
>
>> rc =3D spin_event_timeout((ret =3D in_be32(x) & 0x14), ...);
>
> It's an interesting idea, but I have two problems with it:
>
> 1) This approach is that it depends on the internals of the macro. =A0Tha=
t is, you're sneaking in an assignment in the hopes that the code will beha=
ve properly. =A0You see that the current code evaluates the condition only =
once, so it works. =A0The code will look like this:
>
> 2) Unlike the wait_event_xxx macros, I believe that the actual evaluated =
expression is important to the caller. =A0So 90% of the time, the caller is=
 going to pass in "ret =3D (condition)", which means it makes more sense to=
 have this as a built-in feature of the macro.

The only time you need the result is when you are waiting on multiple
bits. I don't think looping while waiting on multiple bits is the
common case.

> So if you're okay with v9 of the patch, please ACK it.

I'll ACK ths so that I can get my driver in. But every time I used
this function I got it wrong on the first try. This is going to be a
source of bugs.


>
> --
> Timur Tabi
> Linux kernel developer at Freescale
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>



--=20
Jon Smirl
jonsmirl@gmail.com