Thanks for the review Emil. Please find my comments inline
Regards Shashank On 10/13/2015 6:29 PM, Emil Velikov wrote:
On 10 October 2015 at 05:55, Sharma, Shashank shashank.sharma@intel.com wrote:
On 10/10/2015 4:17 AM, Emil Velikov wrote:
Hi Shashank,
On 9 October 2015 at 20:28, Shashank Sharma shashank.sharma@intel.com wrote: [snip]
+/* Color management bit utilities */ +#define GET_BIT_MASK(n) ((1 << n) - 1)
+/* Read bits of a word from bit no. 'start'(lsb) till 'n' bits */ +#define GET_BITS(x, start, nbits) ((x >> start) & GET_BIT_MASK(nbits))
+/* Round off by adding 1 to the immediate lower bit */ +#define GET_BITS_ROUNDOFF(x, start, nbits) \
((GET_BITS(x, start, (nbits + 1)) + 1) >> 1)+/* Clear bits of a word from bit no. 'start' till nbits */ +#define CLEAR_BITS(x, start, nbits) ( \
x &= ~((GET_BIT_MASK(nbits) << start)))+/* Write bit_pattern of no_bits bits in a target word */ +#define SET_BITS(target, bit_pattern, start_bit, no_bits) \
do { \CLEAR_BITS(target, start_bit, no_bits); \target |= (bit_pattern << start_bit); \} while (0)It feels suspicious that the kernel does not have macros for either one of these.
I would invite you to take a look at include/linux/bitops.h and other kernel headers.
Thanks for pointing this out, but if you closely observe, these macros are well tested, and created for color management operations, which have specific requirements, like:
- pick 8 bits from 16th bit onwards, make them LSB, and give result:
GET_BITS
- take these 8 bits and move to bit 17th of the word, clearing the existing
ones: SET_BITS For core register programming, this was required, so we created it. I would still have a look at the existing ones which you pointed out to avoid any duplication, if they fall directly in the implementation, else I would like to continue with these.
Unless I'm missing something, these are generic bit manipulation macros, are they not ? As such I'd imagine we have some of these already available, but I cannot say which ones off-hand.
If you closely observe, what set_bit does is picks up the bit pattern from nth to n+reqd bit, moves it to LSB and returns it. similarly set bits clears the bits, then copy the bit pattern in the respective bits and manipulates the shifts. I could not find any such examples which I can directly use from suggested macros.
Regards, Emil