[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20260129215225.33cce6dc@pumpkin>
Date: Thu, 29 Jan 2026 21:52:25 +0000
From: David Laight <david.laight.linux@...il.com>
To: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@...ux.intel.com>
Cc: "Rafael J . Wysocki" <rafael@...nel.org>, Daniel Lezcano
<daniel.lezcano@...aro.org>, Zhang Rui <rui.zhang@...el.com>, Lukasz Luba
<lukasz.luba@....com>, Srinivas Pandruvada
<srinivas.pandruvada@...ux.intel.com>, linux-pm@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 3/9] powercap: intel_rapl: Use GENMASK() and BIT()
macros
On Thu, 29 Jan 2026 10:36:40 -0800
Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@...ux.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;
While using BIT() for bit patterns is resonable, wholesale substition
isn't really right - and that isn't a bit pattern.
David
Powered by blists - more mailing lists