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

Jon Smirl jonsmirl at gmail.com
Wed May 27 05:04:50 EST 2009


On Tue, May 26, 2009 at 2:17 PM, Timur Tabi <timur at freescale.com> wrote:
> Geoff Thorpe wrote:
>
>> rc = spin_event_timeout((ret = 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.  That is, you're sneaking in an assignment in the hopes that the code will behave properly.  You see that the current code evaluates the condition only once, so it works.  The code will look like this:
>
> 2) Unlike the wait_event_xxx macros, I believe that the actual evaluated expression is important to the caller.  So 90% of the time, the caller is going to pass in "ret = (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 at ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>



-- 
Jon Smirl
jonsmirl at gmail.com



More information about the Linuxppc-dev mailing list