On 8/13/25 8:59 AM, Geert Uytterhoeven wrote:
On Tue, 12 Aug 2025 at 22:05, Laurent Pinchart
<laurent.pinch...@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 .
