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

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

On Tue, Aug 12, 2025 at 09:32:36PM +0200, Marek Vasut wrote:
> On 8/12/25 3:26 PM, 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 ?

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.

-- 
Regards,

Laurent Pinchart