Re: [PATCH v1 3/9] powercap: intel_rapl: Use GENMASK() and BIT() macros

[Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>]

On 1/29/2026 1:52 PM, David Laight wrote:
> On Thu, 29 Jan 2026 10:36:40 -0800
> Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> wrote:
> 
>> Replace hardcoded bitmasks and bit shift operations with standard
>> GENMASK(), GENMASK_ULL(), BIT(), and BIT_ULL() macros for better
>> readability and to follow kernel coding conventions.
>>
>> No functional changes.
> 
> Assuming that changing values to 'unsigned long' doesn't have any
> subtle side effects.
> 
> ...
>>  	value = (ra.value & ENERGY_UNIT_MASK) >> ENERGY_UNIT_OFFSET;
>> -	rd->energy_unit = ENERGY_UNIT_SCALE * 1000000 / (1 << value);
>> +	rd->energy_unit = ENERGY_UNIT_SCALE * 1000000 / BIT(value);
> 
> That should really be:
> 	rd->energy_unit = ENERGY_UNIT_SCALE * 1000000 >> value;

Type change to unsigned long will not have any impact for this use case. 
However, as you noted, using right shift (>>) instead of division by 
BIT() is the correct approach here anyway.

> 
> While using BIT() for bit patterns is resonable, wholesale substition
> isn't really right - and that isn't a bit pattern.

Agreed. I will use right shift for 1 / (1 << value) use cases.


> 
> 	David

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer