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
