[PATCH 1/4] drm/rcar-du: dsi: Convert register bits to BIT() macro

[Marek Vasut]

On 8/13/25 9:42 AM, Tomi Valkeinen wrote:

Hi,

>>>> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h b/
>>>> drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h
>>>> index a6b276f1d6ee..b3e57217ae63 100644
>>>> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h
>>>> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h
>>
>> [...]
>>
>>>> @@ -51,11 +51,11 @@
>>>>      #define TXVMVPRMSET0R            0x1d0
>>>>    #define TXVMVPRMSET0R_HSPOL_HIG        (0 << 17)
>>>> -#define TXVMVPRMSET0R_HSPOL_LOW        (1 << 17)
>>>> +#define TXVMVPRMSET0R_HSPOL_LOW        BIT(17)
>>>
>>> I'm not sure about this (and below). We have two defines for the HSPOL,
>>> high and low. If one of them is (x << y), shouldn't the other one be of
>>> that style too?
>> It is inconsistent, but one macro describes bit set to 0 and the other
>> bit set to 1 (i.e. the actual bit) which is converted to BIT(n) macro. I
>> would be tempted to remove the bits set to 0, that's probably the real
>> discussion that should happen here. But that would also be a much bigger
>> patch. What do you think ?
> 
> In my mind if you have defines for both HIGH and LOW, you have a
> bitfield with a value, the values being 0 and 1, and for values you use
> (0 << 17) and (1 << 17). It just happens here that the bitfield value is
> only one bit long.

I am not a big fan of that, it seems overcomplicated, hence this clean up.

> But I'm also fine with having only "TXVMVPRMSET0R_HSPOL_LOW
> BIT(17)", and then the interpretation is that we have a enable/disable
> style bit.

I think this would work, yes.

> In the end, I'm fine with any of these, or the current one in the patch.
> Although the current one does rub me the wrong way just enough for me to
> comment about it =).
I can also drop this patch from the series and do full conversion of the 
driver to TXVMVPRMSET0R_HSPOL_LOW BIT(17) style afterward. This patch is 
not strictly necessary for the follow up patches.