[PATCH v3 3/4] pwm: meson: change clk/pwm gate from mask to bit

[Heiner Kallweit]

On 13.04.2023 11:06, Thierry Reding wrote:
> On Wed, Apr 12, 2023 at 10:47:05PM +0200, Martin Blumenstingl wrote:
>> Hi Heiner,
>>
>> On Wed, Apr 12, 2023 at 9:23 PM Heiner Kallweit <hkallweit1 at gmail.com> wrote:
>>>
>>> Change single-bit values from mask to bit. This facilitates
>>> CCF initialization for the clock gate in a follow-up patch.
>>>
>>> Signed-off-by: Heiner Kallweit <hkallweit1 at gmail.com>
>> Tested-by: Martin Blumenstingl <martin.blumenstingl at googlemail.com> #
>> meson8b-odroidc1, sm1-x96-air
>>
>> [...]
>>>  #define REG_MISC_AB            0x8
>>> -#define MISC_B_CLK_EN          BIT(23)
>>> -#define MISC_A_CLK_EN          BIT(15)
>>> +#define MISC_B_CLK_EN          23
>>> +#define MISC_A_CLK_EN          15
>>>  #define MISC_CLK_DIV_MASK      0x7f
>>>  #define MISC_B_CLK_DIV_SHIFT   16
>>>  #define MISC_A_CLK_DIV_SHIFT   8
>>>  #define MISC_B_CLK_SEL_SHIFT   6
>>>  #define MISC_A_CLK_SEL_SHIFT   4
>>>  #define MISC_CLK_SEL_MASK      0x3
>>> -#define MISC_B_EN              BIT(1)
>>> -#define MISC_A_EN              BIT(0)
>>> +#define MISC_B_EN              1
>>> +#define MISC_A_EN              0
>> Personally I'm fine with this change but it's not how I would have done it:
>> - I would have kept the BIT() macro for MISC_{A,B}_EN
>> - then I would have renamed MISC_{A,}_CLK_EN to
>> MISC_{A,B}_CLK_EN_SHIFT (to be consistent with _SHIFT of the mux and
>> divider) and drop the BIT() macro there (like you did)
>>
>> This is possibly/likely personal preference, so my suggestion is to
>> wait for some more feedback.
> 
> It looks like these aren't used outside the meson_pwm_per_channel_data
> array, so why bother with a #define (and any potential inconsistencies)
> in the first place?
> 
I think we follow a common pattern here and first define constants for all
registers and bits/fields. Having the register layout defined in one place
makes it easier to check it against the chip datasheet.
However I'm not sure whether this is what you were referring to.

> Thierry