Dear Alessandro Rubini,
In message 20101229231004.GA17999@mail.gnudd.com you wrote:
#define writeb(v,c) ({ __iowmb(); __arch_putb(v,c); v;})
(note the additional 'v;') should result in correct code, too.
Yes, that's good. Also "0" may work, and may be more readable, (or not, according to who reads it).
'0' looks wrong to me, as it changes behaviour. Keep in mind that the previous implemention usually looks like this:
#define writeb(val,addr) (*(volatile unsigned char *)(addr)) = (val)
The value of this should be "val", not 0.
#define writeb(v,c) do { __iowmb(); __arch_putb(v,c); } while (0)
do the same with a slightly different syntax, so these patches are fine, too.
It's not just different syntax, it's different semantics.
This is true. Thanks for pointing out.
The ({...}) trick turns statements into expressions, while the "do {...} while(0)" does not. I'd better not forbid statements like
while (reg = readb(addr), reg != VALUE) { .... }
or
if (readb(addr) == VALUE) { ... }
or swtich (readb(addr)) { ... }
Here you are mixing things up. There is no issue with the read*() code, only the write*() code is affected.
And while no person ion a sane mind of state should use write*() in such a context, I agree that for the sake of backward compatibility we should change the code. Patch follows.
While I agree they may in general not be clean, I can forsee meaningful uses. Moreover, I'd better allow expression-looking macros to really behave like expressions -- otherwise error messages are quite hard to understand for the unaquainted.
Agreed. But the write*() "functions" are considered to return void, so there should not be any such issues.
IMHO, the only reason to use "do {} while(0)" over statemente expressions is being portable but in u-boot we are gcc-specific anyways.
This is wrong interpretation, too; nothing in this construct is GCC specific.
Best regards,
Wolfgang Denk