On Monday 16 May 2011 12:14 AM, Wolfgang Denk wrote:
Dear Aneesh V,
In message1305202276-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.
I will be very happy to use a standard one if one exists. I checked in bitops.h but couldn't find something that's equivalent to the above. Can you point me to a standard one that does something equivalent.
Yes, you may have to look-up the implementation, but maybe just once. That goes with any API, right?
On the other hand, doing shifting ORing, ANDing etc directly in the code is less readable in my opinion.
+/*
- 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.
clrsetbits will work for this need albeit not as clean as the above one. I will use that.
Best regards,
Wolfgang Denk