Dear Aneesh V,
In message 1305202276-27784-3-git-send-email-aneesh@ti.com you wrote:
add utility macros for:
- bit field operations
- log2n functions
...
+/* extract a bit field from a bit vector */ +#define get_bit_field(nr, start, mask)\
- (((nr) & (mask)) >> (start))
+/* Set a field in a bit vector */ +#define set_bit_field(nr, start, mask, val)\
- do { \
(nr) = ((nr) & ~(mask)) | (((val) << (start)) & (mask));\- } while (0);
I really dislike such "useful" helpers, because they make the code unreadable. Being nonstandrd, nbody knows what they are doing, so you always will have to look up the implementation before you can continue reading the code. It's a waste of time an resources.
+/*
- Utility macro for read-modify-write of a hardware register
- addr - address of the register
- shift - starting bit position of the field to be modified
- msk - mask for the field
- val - value to be shifted masked and written to the field
- */
+#define modify_reg_32(addr, shift, msk, val) \
- do {\
writel(((readl(addr) & ~(msk))|(((val) << (shift)) & (msk))),\(addr));\- } while (0);
NAK again, for the same reasons.
Note that there are some semi-standardized I/O accessro macros available, at least for some architectures, like clrbits.*(), setbits_(), or clrsetbits().
See for example "arch/arm/include/asm/io.h", "arch/powerpc/include/asm/io.h" for reference.
Instead of reinventing the wheel (just differently shaped) we should rather try and use a single, standardized set of such helpers.
So please use the existing macros instead of inventing new, non-standard ones.
Best regards,
Wolfgang Denk