On Tue, May 26, 2009 at 2:17 PM, Timur Tabi <ti...@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@ozlabs.org > https://ozlabs.org/mailman/listinfo/linuxppc-dev > -- Jon Smirl jonsm...@gmail.com _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
