Regards Shashank
On 10/10/2015 4:17 AM, Emil Velikov wrote: > Hi Shashank, > > On 9 October 2015 at 20:28, Shashank Sharma <shashank.sharma at 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. > Regards, > Emil >