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

[Marek Vasut]

On 8/13/25 8:59 AM, Geert Uytterhoeven wrote:
> On Tue, 12 Aug 2025 at 22:05, Laurent Pinchart
> <laurent.pinchart at ideasonboard.com> wrote:
>> On Tue, Aug 12, 2025 at 09:32:36PM +0200, Marek Vasut wrote:
>>> On 8/12/25 3:26 PM, Tomi Valkeinen wrote:
>>>>> 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 ?
>>
>> For what it's worth, for single-bit register fields, I usually define a
>> single macro. I understand it's usually a coding style preference.
> 
> An alternative would be to define 3 macros:
> 
>      #define TXVMVPRMSET0R_HSPOL        BIT(17)
>      #define TXVMVPRMSET0R_HSPOL_HIG    0
>      #define TXVMVPRMSET0R_HSPOL_LOW    1
> 
> and use FIELD_PREP(TXVMVPRMSET0R_HSPOL, TXVMVPRMSET0R_HSPOL_{HIG,LOW}).
> But I agree a single definition is fine for a single-bit register field.

I think single bit macro for single register bit is just about the right 
amount of complexity .