Hi Wolfgang,
On Thu, Jun 16, 2011 at 9:46 PM, Wolfgang Denk wd@denx.de wrote:
Dear Graeme Russ,
In message 4DF9E409.1060600@gmail.com you wrote:
is equivalent except that, as already pointed out, clrsetbits and friends:
a) Are not portable because only ARM and PPC define them which makes them, by definition, non-standard
Agreed - As mentioned in another post, I will look to implement clrsetbits and friends in x86 at some point
They should be added to _any_ arch/<arch>/include/asm/io.h that doesn't use them yet.
b) Each invocation results in a read barrier plus a write barrier
...which is intentionally (actually mandatory) when accessing I/O memory, and negligable overhead on plain memory.
Agreed - The raw performance overhead of the barriers is most likely negligible, although is there possibly a cache flush/sync operation or flush of the predictive branch cache that could have a more significant impact? I don't see this as such a huge problem since clrsetbits is (usually) used to update hardware registers, any flush will happen anyway
c) If the hardware register is sensitive to partial updates (i.e. requires all bit-fields to be updated in on operation) this requires a read into a local variable, calls to clrsetbits against that variable and finally a write-back - Lots of memory barriers
Wrong. The macro does this automatically. There is only a single read and a single write per call.
I mean: clrsetbits(&foo_reg, foo_val_1, foo_val_1_mask); clrsetbits(&foo_reg, foo_val_2, foo_val_2_mask); clrsetbits(&foo_reg, foo_val_3, foo_val_3_mask);
This could cause side-effects if foo_reg is sensitive to the three foo_val's being set independently. To ensure all three values are updated in a single write to foo_reg you would need to do:
foo_tmp = readl(&foo_reg);
clrsetbits(&foo_tmp, foo_val_1, foo_val_1_mask); clrsetbits(&foo_tmp, foo_val_2, foo_val_2_mask); clrsetbits(&foo_tmp, foo_val_3, foo_val_3_mask);
foo_tmp = writel(foo_tmp, &foo_reg);
Regards,
Graeme