[PATCH v2 0/3] staging: media: max96712: a few, mainly cosmetic, changes

[PATCH v2 0/3] staging: media: max96712: a few, mainly cosmetic, changes

[Laurentiu Palcu]

Hi,

This is in preparation for bringing our downstream max96724 work into
the max96712 driver. The changes in this patch-set are mainly cosmetic
to increase readability, with one exception related to the PCLK debug
register which appears to exist in MAX96724 as well(see VPG section in
users guide [1]).

Unfortunately, I couldn't find any max96712 reference manual anywhere to
make sure the registers match between the 2 chips, nor I have a max96712
to test with.

However, since the max96724 compatible was added here, according to the
commit

6c872b782ae ("media: staging: max96712: Add support for MAX96724")

the chips are "almost identical". So, I used max96724 register and
field descriptions instead. Someone with a max96712 RM may want to
double-check though.

Thanks,
Laurentiu

[1] https://www.analog.com/media/en/technical-documentation/user-guides/max96724fr-user-guide.pdf

Laurentiu Palcu (3):
  staging: media: max96712: fix typo
  staging: media: max96712: remove extra debug register flag
  staging: media: max96712: add some register and field definitions

 drivers/staging/media/max96712/max96712.c | 213 +++++++++++++++++-----
 1 file changed, 164 insertions(+), 49 deletions(-)

-- 
2.34.1

[PATCH v2 1/3] staging: media: max96712: fix typo

[Laurentiu Palcu]

Fix a typo in VS_LOW expression.

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
Reviewed-by: Niklas S??derlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/staging/media/max96712/max96712.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/max96712/max96712.c b/drivers/staging/media/max96712/max96712.c
index 0751b2e048958..5228f9ec60859 100644
--- a/drivers/staging/media/max96712/max96712.c
+++ b/drivers/staging/media/max96712/max96712.c
@@ -182,7 +182,7 @@ static void max96712_pattern_enable(struct max96712_priv *priv, bool enable)
 	max96712_write_bulk_value(priv, 0x1052, 0, 3);
 	max96712_write_bulk_value(priv, 0x1055, v_sw * h_tot, 3);
 	max96712_write_bulk_value(priv, 0x1058,
-				  (v_active + v_fp + + v_bp) * h_tot, 3);
+				  (v_active + v_fp + v_bp) * h_tot, 3);
 	max96712_write_bulk_value(priv, 0x105b, 0, 3);
 	max96712_write_bulk_value(priv, 0x105e, h_sw, 2);
 	max96712_write_bulk_value(priv, 0x1060, h_active + h_fp + h_bp, 2);
-- 
2.34.1

[PATCH v2 2/3] staging: media: max96712: remove extra debug register flag

[Laurentiu Palcu]

The current implementation has the register disabled since it is not
documented in the MAX96724 RM. However, in the chip's user's guide [1],
in the Video Pattern Generator section, the register is documented and
it appears to work just fine. Before the change I was experiencing
approx 10fps when streaming, after: 30.3fps.

Hence, since both max96712 and max96724 have the register, we can remove
the flag.

[1] https://www.analog.com/media/en/technical-documentation/user-guides/max96724fr-user-guide.pdf

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
---
v2:
 * addressed Niklas' comments and removed the extra debug register flag;

 drivers/staging/media/max96712/max96712.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/staging/media/max96712/max96712.c b/drivers/staging/media/max96712/max96712.c
index 5228f9ec60859..ed4d5db1ff8d1 100644
--- a/drivers/staging/media/max96712/max96712.c
+++ b/drivers/staging/media/max96712/max96712.c
@@ -27,7 +27,6 @@ enum max96712_pattern {
 
 struct max96712_info {
 	unsigned int dpllfreq;
-	bool have_debug_extra;
 };
 
 struct max96712_priv {
@@ -174,9 +173,7 @@ static void max96712_pattern_enable(struct max96712_priv *priv, bool enable)
 		return;
 	}
 
-	/* Set PCLK to 75MHz if device have DEBUG_EXTRA register. */
-	if (priv->info->have_debug_extra)
-		max96712_write(priv, DEBUG_EXTRA_REG, DEBUG_EXTRA_PCLK_75MHZ);
+	max96712_write(priv, DEBUG_EXTRA_REG, DEBUG_EXTRA_PCLK_75MHZ);
 
 	/* Configure Video Timing Generator for 1920x1080 @ 30 fps. */
 	max96712_write_bulk_value(priv, 0x1052, 0, 3);
@@ -457,7 +454,6 @@ static void max96712_remove(struct i2c_client *client)
 
 static const struct max96712_info max96712_info_max96712 = {
 	.dpllfreq = 1000,
-	.have_debug_extra = true,
 };
 
 static const struct max96712_info max96712_info_max96724 = {
-- 
2.34.1

[PATCH v2 3/3] staging: media: max96712: add some register and field definitions

[Laurentiu Palcu]

Most of the registers in the driver, and their values, are hard-coded
which makes the code rather difficult to read and maintain. Add register
and field definitions and use them to replace the hard-coded values
where possible.

This patch does not change the driver functionality or its structure.

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
Reviewed-by: Niklas S??derlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/staging/media/max96712/max96712.c | 207 +++++++++++++++++-----
 1 file changed, 163 insertions(+), 44 deletions(-)

diff --git a/drivers/staging/media/max96712/max96712.c b/drivers/staging/media/max96712/max96712.c
index ed4d5db1ff8d1..47842facec125 100644
--- a/drivers/staging/media/max96712/max96712.c
+++ b/drivers/staging/media/max96712/max96712.c
@@ -16,9 +16,118 @@
 #include <media/v4l2-fwnode.h>
 #include <media/v4l2-subdev.h>
 
-#define DEBUG_EXTRA_REG			0x09
-#define DEBUG_EXTRA_PCLK_25MHZ		0x00
-#define DEBUG_EXTRA_PCLK_75MHZ		0x01
+/* TOP_CTRL */
+#define MAX96712_DEBUG_EXTRA_REG			0x0009
+#define   DEBUG_EXTRA_PCLK_25MHZ			0x00
+#define   DEBUG_EXTRA_PCLK_75MHZ			0x01
+#define MAX96724_TOP_CTRL_PWR1				0x0013
+#define   RESET_ALL					BIT(6)
+
+/* BACKTOP0 */
+#define MAX96712_BACKTOP0_12				0x040b
+#define   CSI_OUT_EN					BIT(1)
+#define   SOFT_BPP_0_MASK				GENMASK(7, 3)
+#define   SOFT_BPP_0_SHIFT				3
+#define MAX96712_BACKTOP0_22				0x0415
+#define MAX96712_BACKTOP0_25				0x0418
+#define   PHY_CSI_TX_DPLL_PREDEF_FREQ_MASK		GENMASK(4, 0)
+#define   PHY_CSI_TX_DPLL_PREDEF_FREQ_SHIFT		0
+#define   PHY_CSI_TX_DPLL_FB_FRACTION_PREDEF_EN		BIT(5)
+#define   OVERRIDE_BPP_VC_DT_0_2			BIT(6)
+#define   OVERRIDE_BPP_VC_DT_1_3			BIT(7)
+
+/* MIPI_PHY */
+#define MAX96712_MIPI_PHY_0				0x08a0
+#define   PHY_4X2					BIT(0)
+#define   PHY_2X4					BIT(2)
+#define   PHY_1X4A_22					BIT(3)
+#define   PHY_1X4B_22					BIT(4)
+#define   FORCE_CLK0_EN					BIT(5)
+#define   FORCE_CLK3_EN					BIT(6)
+#define   FORCE_CSI_OUT_EN				BIT(7)
+#define MAX96712_MIPI_PHY_2				0x08a2
+#define   T_HS_TRAIL_MASK				GENMASK(1, 0)
+#define   T_HS_TRAIL_SHIFT				0
+#define   T_LPX_MASK					GENMASK(3, 2)
+#define   T_LPX_SHIFT					2
+#define   PHY_STDBY_N_MASK				GENMASK(7, 4)
+#define   PHY_STDBY_N_SHIFT				4
+#define   PHY0_EN					BIT(4)
+#define   PHY1_EN					BIT(5)
+#define   PHY2_EN					BIT(6)
+#define   PHY3_EN					BIT(7)
+#define MAX96712_MIPI_PHY_3				0x08a3
+#define   PHY0_LANE_MAP_MASK				GENMASK(3, 0)
+#define   PHY0_LANE_MAP_SHIFT				0
+#define   PHY1_LANE_MAP_MASK				GENMASK(7, 4)
+#define   PHY1_LANE_MAP_SHIFT				4
+#define MAX96712_MIPI_PHY_5				0x08a5
+#define   PHY0_POL_MAP_MASK				GENMASK(2, 0)
+#define   PHY0_POL_MAP_SHIFT				0
+#define   PHY1_POL_MAP_MASK				GENMASK(5, 3)
+#define   PHY1_POL_MAP_SHIFT				3
+#define   T_CLK_PREP_MASK				GENMASK(7, 6)
+#define   T_CLK_PREP_SHIFT				6
+#define MAX96712_MIPI_PHY_13				0x08ad
+#define   T_T3_PREBEGIN_MASK				GENMASK(5, 0)
+#define   T_T3_PREBEGIN_SHIFT				0
+#define MAX96712_MIPI_PHY_14				0x08ae
+#define   T_T3_PREP_MASK				GENMASK(1, 0)
+#define   T_T3_PREP_SHIFT				0
+#define   T_T3_PREP_40NS				0
+#define   T_T3_PREP_55NS				1
+#define   T_T3_PREP_66_7NS				2
+#define   T_T3_PREP_86_7NS				3
+#define   T_T3_POST_MASK				GENMASK(6, 2)
+#define   T_T3_POST_SHIFT				2
+
+/* MIPI_TX: 0 <= phy < 4 */
+#define MAX96712_MIPI_TX_10(phy)			(0x090a + (phy) * 0x40)
+#define   CSI2_TWAKEUP_H_MASK				GENMASK(2, 0)
+#define   CSI2_TWAKEUP_H_SHIFT				0
+#define   CSI2_VCX_EN					BIT(4)
+#define   CSI2_CPHY_EN					BIT(5)
+#define   CSI2_LANE_CNT_MASK				GENMASK(7, 6)
+#define   CSI2_LANE_CNT_SHIFT				6
+
+/* VRX_PATGEN */
+#define MAX96712_VRX_PATGEN_0				0x1050
+#define   VTG_MODE_MASK					GENMASK(1, 0)
+#define   VTG_MODE_SHIFT				0
+#define   VTG_MODE_VS_TRACKING				0
+#define   VTG_MODE_VS_TRIGGER				1
+#define   VTG_MODE_AUTO_REPEAT				2
+#define   VTG_MODE_FREE_RUNNING				3
+#define   DE_INV					BIT(2)
+#define   HS_INV					BIT(3)
+#define   VS_INV					BIT(4)
+#define   GEN_DE					BIT(5)
+#define   GEN_HS					BIT(6)
+#define   GEN_VS					BIT(7)
+#define MAX96712_VRX_PATGEN_1				0x1051
+#define   VS_TRIG					BIT(0)
+#define   PATGEN_MODE_MASK				GENMASK(5, 4)
+#define   PATGEN_MODE_SHIFT				4
+#define   PATGEN_MODE_CHECKERBOARD			(1 << PATGEN_MODE_SHIFT)
+#define   PATGEN_MODE_GRADIENT				(2 << PATGEN_MODE_SHIFT)
+#define   GRAD_MODE					BIT(7)
+#define MAX96712_VRX_PATGEN_VS_DLY			0x1052
+#define MAX96712_VRX_PATGEN_VS_HIGH			0x1055
+#define MAX96712_VRX_PATGEN_VS_LOW			0x1058
+#define MAX96712_VRX_PATGEN_V2H				0x105b
+#define MAX96712_VRX_PATGEN_HS_HIGH			0x105e
+#define MAX96712_VRX_PATGEN_HS_LOW			0x1060
+#define MAX96712_VRX_PATGEN_HS_CNT			0x1062
+#define MAX96712_VRX_PATGEN_V2D				0x1064
+#define MAX96712_VRX_PATGEN_DE_HIGH			0x1067
+#define MAX96712_VRX_PATGEN_DE_LOW			0x1069
+#define MAX96712_VRX_PATGEN_DE_CNT			0x106b
+#define MAX96712_VRX_PATGEN_GRAD_INCR			0x106d
+#define MAX96712_VRX_PATGEN_CHKR_COLOR_A		0x106e
+#define MAX96712_VRX_PATGEN_CHKR_COLOR_B		0x1071
+#define MAX96712_VRX_PATGEN_CHKR_RPT_A			0x1074
+#define MAX96712_VRX_PATGEN_CHKR_RPT_B			0x1075
+#define MAX96712_VRX_PATGEN_CHKR_ALT			0x1076
 
 enum max96712_pattern {
 	MAX96712_PATTERN_CHECKERBOARD = 0,
@@ -96,18 +205,18 @@ static int max96712_write_bulk_value(struct max96712_priv *priv,
 
 static void max96712_reset(struct max96712_priv *priv)
 {
-	max96712_update_bits(priv, 0x13, 0x40, 0x40);
+	max96712_update_bits(priv, MAX96724_TOP_CTRL_PWR1, RESET_ALL, RESET_ALL);
 	msleep(20);
 }
 
 static void max96712_mipi_enable(struct max96712_priv *priv, bool enable)
 {
 	if (enable) {
-		max96712_update_bits(priv, 0x40b, 0x02, 0x02);
-		max96712_update_bits(priv, 0x8a0, 0x80, 0x80);
+		max96712_update_bits(priv, MAX96712_BACKTOP0_12, CSI_OUT_EN, CSI_OUT_EN);
+		max96712_update_bits(priv, MAX96712_MIPI_PHY_0, FORCE_CSI_OUT_EN, FORCE_CSI_OUT_EN);
 	} else {
-		max96712_update_bits(priv, 0x8a0, 0x80, 0x00);
-		max96712_update_bits(priv, 0x40b, 0x02, 0x00);
+		max96712_update_bits(priv, MAX96712_MIPI_PHY_0, FORCE_CSI_OUT_EN, 0x00);
+		max96712_update_bits(priv, MAX96712_BACKTOP0_12, CSI_OUT_EN, 0x00);
 	}
 }
 
@@ -119,39 +228,47 @@ static void max96712_mipi_configure(struct max96712_priv *priv)
 	max96712_mipi_enable(priv, false);
 
 	/* Select 2x4 mode. */
-	max96712_write(priv, 0x8a0, 0x04);
+	max96712_write(priv, MAX96712_MIPI_PHY_0, PHY_2X4);
 
 	/* TODO: Add support for 2-lane and 1-lane configurations. */
 	if (priv->cphy) {
-		/* Configure a 3-lane C-PHY using PHY0 and PHY1. */
-		max96712_write(priv, 0x94a, 0xa0);
+		/* Configure a 3-lane C-PHY using PHY1. */
+		max96712_write(priv, MAX96712_MIPI_TX_10(1),
+			       (2 << CSI2_LANE_CNT_SHIFT) | CSI2_CPHY_EN);
 
 		/* Configure C-PHY timings. */
-		max96712_write(priv, 0x8ad, 0x3f);
-		max96712_write(priv, 0x8ae, 0x7d);
+		max96712_write(priv, MAX96712_MIPI_PHY_13, 0x3f);
+		max96712_write(priv, MAX96712_MIPI_PHY_14,
+			       (0x1f << T_T3_POST_SHIFT) | T_T3_PREP_55NS);
 	} else {
-		/* Configure a 4-lane D-PHY using PHY0 and PHY1. */
-		max96712_write(priv, 0x94a, 0xc0);
+		/* Configure a 4-lane D-PHY using PHY1. */
+		max96712_write(priv, MAX96712_MIPI_TX_10(1), 3 << CSI2_LANE_CNT_SHIFT);
 	}
 
 	/* Configure lane mapping for PHY0 and PHY1. */
 	/* TODO: Add support for lane swapping. */
-	max96712_write(priv, 0x8a3, 0xe4);
+	max96712_write(priv, MAX96712_MIPI_PHY_3, 0xe4);
 
 	/* Configure lane polarity for PHY0 and PHY1. */
 	for (i = 0; i < priv->mipi.num_data_lanes + 1; i++)
 		if (priv->mipi.lane_polarities[i])
 			phy5 |= BIT(i == 0 ? 5 : i < 3 ? i - 1 : i);
-	max96712_write(priv, 0x8a5, phy5);
+	max96712_write(priv, MAX96712_MIPI_PHY_5, phy5);
 
 	/* Set link frequency for PHY0 and PHY1. */
-	max96712_update_bits(priv, 0x415, 0x3f,
-			     ((priv->info->dpllfreq / 100) & 0x1f) | BIT(5));
-	max96712_update_bits(priv, 0x418, 0x3f,
-			     ((priv->info->dpllfreq / 100) & 0x1f) | BIT(5));
+	max96712_update_bits(priv, MAX96712_BACKTOP0_22,
+			     PHY_CSI_TX_DPLL_FB_FRACTION_PREDEF_EN |
+			     PHY_CSI_TX_DPLL_PREDEF_FREQ_MASK,
+			     PHY_CSI_TX_DPLL_FB_FRACTION_PREDEF_EN |
+			     ((priv->info->dpllfreq / 100) & 0x1f));
+	max96712_update_bits(priv, MAX96712_BACKTOP0_25,
+			     PHY_CSI_TX_DPLL_FB_FRACTION_PREDEF_EN |
+			     PHY_CSI_TX_DPLL_PREDEF_FREQ_MASK,
+			     PHY_CSI_TX_DPLL_FB_FRACTION_PREDEF_EN |
+			     ((priv->info->dpllfreq / 100) & 0x1f));
 
 	/* Enable PHY0 and PHY1 */
-	max96712_update_bits(priv, 0x8a2, 0xf0, 0x30);
+	max96712_update_bits(priv, MAX96712_MIPI_PHY_2, PHY_STDBY_N_MASK, PHY0_EN | PHY1_EN);
 }
 
 static void max96712_pattern_enable(struct max96712_priv *priv, bool enable)
@@ -169,49 +286,51 @@ static void max96712_pattern_enable(struct max96712_priv *priv, bool enable)
 	const u32 v_tot = v_active + v_fp + v_sw + v_bp;
 
 	if (!enable) {
-		max96712_write(priv, 0x1051, 0x00);
+		max96712_write(priv, MAX96712_VRX_PATGEN_1, 0x00);
 		return;
 	}
 
-	max96712_write(priv, DEBUG_EXTRA_REG, DEBUG_EXTRA_PCLK_75MHZ);
+	max96712_write(priv, MAX96712_DEBUG_EXTRA_REG, DEBUG_EXTRA_PCLK_75MHZ);
 
 	/* Configure Video Timing Generator for 1920x1080 @ 30 fps. */
-	max96712_write_bulk_value(priv, 0x1052, 0, 3);
-	max96712_write_bulk_value(priv, 0x1055, v_sw * h_tot, 3);
-	max96712_write_bulk_value(priv, 0x1058,
+	max96712_write_bulk_value(priv, MAX96712_VRX_PATGEN_VS_DLY, 0, 3);
+	max96712_write_bulk_value(priv, MAX96712_VRX_PATGEN_VS_HIGH, v_sw * h_tot, 3);
+	max96712_write_bulk_value(priv, MAX96712_VRX_PATGEN_VS_LOW,
 				  (v_active + v_fp + v_bp) * h_tot, 3);
-	max96712_write_bulk_value(priv, 0x105b, 0, 3);
-	max96712_write_bulk_value(priv, 0x105e, h_sw, 2);
-	max96712_write_bulk_value(priv, 0x1060, h_active + h_fp + h_bp, 2);
-	max96712_write_bulk_value(priv, 0x1062, v_tot, 2);
-	max96712_write_bulk_value(priv, 0x1064,
+	max96712_write_bulk_value(priv, MAX96712_VRX_PATGEN_V2H, 0, 3);
+	max96712_write_bulk_value(priv, MAX96712_VRX_PATGEN_HS_HIGH, h_sw, 2);
+	max96712_write_bulk_value(priv, MAX96712_VRX_PATGEN_HS_LOW, h_active + h_fp + h_bp, 2);
+	max96712_write_bulk_value(priv, MAX96712_VRX_PATGEN_HS_CNT, v_tot, 2);
+	max96712_write_bulk_value(priv, MAX96712_VRX_PATGEN_V2D,
 				  h_tot * (v_sw + v_bp) + (h_sw + h_bp), 3);
-	max96712_write_bulk_value(priv, 0x1067, h_active, 2);
-	max96712_write_bulk_value(priv, 0x1069, h_fp + h_sw + h_bp, 2);
-	max96712_write_bulk_value(priv, 0x106b, v_active, 2);
+	max96712_write_bulk_value(priv, MAX96712_VRX_PATGEN_DE_HIGH, h_active, 2);
+	max96712_write_bulk_value(priv, MAX96712_VRX_PATGEN_DE_LOW, h_fp + h_sw + h_bp, 2);
+	max96712_write_bulk_value(priv, MAX96712_VRX_PATGEN_DE_CNT, v_active, 2);
 
 	/* Generate VS, HS and DE in free-running mode. */
-	max96712_write(priv, 0x1050, 0xfb);
+	max96712_write(priv, MAX96712_VRX_PATGEN_0,
+		       GEN_VS | GEN_HS | GEN_DE | VS_INV | HS_INV |
+		       (VTG_MODE_FREE_RUNNING << VTG_MODE_SHIFT));
 
 	/* Configure Video Pattern Generator. */
 	if (priv->pattern == MAX96712_PATTERN_CHECKERBOARD) {
 		/* Set checkerboard pattern size. */
-		max96712_write(priv, 0x1074, 0x3c);
-		max96712_write(priv, 0x1075, 0x3c);
-		max96712_write(priv, 0x1076, 0x3c);
+		max96712_write(priv, MAX96712_VRX_PATGEN_CHKR_RPT_A, 0x3c);
+		max96712_write(priv, MAX96712_VRX_PATGEN_CHKR_RPT_B, 0x3c);
+		max96712_write(priv, MAX96712_VRX_PATGEN_CHKR_ALT, 0x3c);
 
 		/* Set checkerboard pattern colors. */
-		max96712_write_bulk_value(priv, 0x106e, 0xfecc00, 3);
-		max96712_write_bulk_value(priv, 0x1071, 0x006aa7, 3);
+		max96712_write_bulk_value(priv, MAX96712_VRX_PATGEN_CHKR_COLOR_A, 0xfecc00, 3);
+		max96712_write_bulk_value(priv, MAX96712_VRX_PATGEN_CHKR_COLOR_B, 0x006aa7, 3);
 
 		/* Generate checkerboard pattern. */
-		max96712_write(priv, 0x1051, 0x10);
+		max96712_write(priv, MAX96712_VRX_PATGEN_1, PATGEN_MODE_CHECKERBOARD);
 	} else {
 		/* Set gradient increment. */
-		max96712_write(priv, 0x106d, 0x10);
+		max96712_write(priv, MAX96712_VRX_PATGEN_GRAD_INCR, 0x10);
 
 		/* Generate gradient pattern. */
-		max96712_write(priv, 0x1051, 0x20);
+		max96712_write(priv, MAX96712_VRX_PATGEN_1, PATGEN_MODE_GRADIENT);
 	}
 }
 
-- 
2.34.1

Re: [PATCH v2 2/3] staging: media: max96712: remove extra debug register flag

[Niklas Söderlund]

Hi Laurentiu,

Thanks for your work.

On 2024-12-20 12:49:37 +0200, Laurentiu Palcu wrote:
> The current implementation has the register disabled since it is not
> documented in the MAX96724 RM. However, in the chip's user's guide [1],
> in the Video Pattern Generator section, the register is documented and
> it appears to work just fine. Before the change I was experiencing
> approx 10fps when streaming, after: 30.3fps.
> 
> Hence, since both max96712 and max96724 have the register, we can remove
> the flag.
> 
> [1] https://www.analog.com/media/en/technical-documentation/user-guides/max96724fr-user-guide.pdf
> 
> Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
> v2:
>  * addressed Niklas' comments and removed the extra debug register flag;
> 
>  drivers/staging/media/max96712/max96712.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/media/max96712/max96712.c b/drivers/staging/media/max96712/max96712.c
> index 5228f9ec60859..ed4d5db1ff8d1 100644
> --- a/drivers/staging/media/max96712/max96712.c
> +++ b/drivers/staging/media/max96712/max96712.c
> @@ -27,7 +27,6 @@ enum max96712_pattern {
>  
>  struct max96712_info {
>  	unsigned int dpllfreq;
> -	bool have_debug_extra;
>  };
>  
>  struct max96712_priv {
> @@ -174,9 +173,7 @@ static void max96712_pattern_enable(struct max96712_priv *priv, bool enable)
>  		return;
>  	}
>  
> -	/* Set PCLK to 75MHz if device have DEBUG_EXTRA register. */
> -	if (priv->info->have_debug_extra)
> -		max96712_write(priv, DEBUG_EXTRA_REG, DEBUG_EXTRA_PCLK_75MHZ);
> +	max96712_write(priv, DEBUG_EXTRA_REG, DEBUG_EXTRA_PCLK_75MHZ);
>  
>  	/* Configure Video Timing Generator for 1920x1080 @ 30 fps. */
>  	max96712_write_bulk_value(priv, 0x1052, 0, 3);
> @@ -457,7 +454,6 @@ static void max96712_remove(struct i2c_client *client)
>  
>  static const struct max96712_info max96712_info_max96712 = {
>  	.dpllfreq = 1000,
> -	.have_debug_extra = true,
>  };
>  
>  static const struct max96712_info max96712_info_max96724 = {
> -- 
> 2.34.1
> 

-- 
Kind Regards,
Niklas Söderlund

Re: [PATCH v2 1/3] staging: media: max96712: fix typo

[Niklas Söderlund]

Hi Laurentiu,

On 2024-12-20 12:49:36 +0200, Laurentiu Palcu wrote:
> Fix a typo in VS_LOW expression.
> 
> Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
> Reviewed-by: Niklas S??derlund <niklas.soderlund+renesas@ragnatech.se>

I think your mail setup mangled my döts ;-) The Content-Type header for 
this mail is set to 'text/plain; charset="us-ascii"' while the patch is 
likely utf-8 encoded. Same in patch 3/3.

> ---
>  drivers/staging/media/max96712/max96712.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/media/max96712/max96712.c b/drivers/staging/media/max96712/max96712.c
> index 0751b2e048958..5228f9ec60859 100644
> --- a/drivers/staging/media/max96712/max96712.c
> +++ b/drivers/staging/media/max96712/max96712.c
> @@ -182,7 +182,7 @@ static void max96712_pattern_enable(struct max96712_priv *priv, bool enable)
>  	max96712_write_bulk_value(priv, 0x1052, 0, 3);
>  	max96712_write_bulk_value(priv, 0x1055, v_sw * h_tot, 3);
>  	max96712_write_bulk_value(priv, 0x1058,
> -				  (v_active + v_fp + + v_bp) * h_tot, 3);
> +				  (v_active + v_fp + v_bp) * h_tot, 3);
>  	max96712_write_bulk_value(priv, 0x105b, 0, 3);
>  	max96712_write_bulk_value(priv, 0x105e, h_sw, 2);
>  	max96712_write_bulk_value(priv, 0x1060, h_active + h_fp + h_bp, 2);
> -- 
> 2.34.1
> 

-- 
Kind Regards,
Niklas Söderlund

Re: [PATCH v2 1/3] staging: media: max96712: fix typo

[Laurentiu Palcu]

Hi Niklas,

On Fri, Dec 20, 2024 at 11:59:34AM +0100, Niklas Söderlund wrote:
> Hi Laurentiu,
> 
> On 2024-12-20 12:49:36 +0200, Laurentiu Palcu wrote:
> > Fix a typo in VS_LOW expression.
> > 
> > Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
> > Reviewed-by: Niklas S??derlund <niklas.soderlund+renesas@ragnatech.se>
> 
> I think your mail setup mangled my döts ;-) The Content-Type header for 
> this mail is set to 'text/plain; charset="us-ascii"' while the patch is 
> likely utf-8 encoded. Same in patch 3/3.

Sorry about that. I didn't mean to mess-up your name... :/ Apparently,
this how 'git format-patch' generated the patch header:

===
From 19c513feb2003dff89c28ef9e54c0e06929efb0a Mon Sep 17 00:00:00 2001
From: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
Date: Thu, 19 Dec 2024 09:28:54 +0000
Subject: [PATCH v2 1/3] staging: media: max96712: fix typo
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Content-Type: text/plain; charset="us-ascii"
===

It looks like it generated 2 Content-Type lines. And the last one
took precedence... 

Looks like a git bug?!? I'm no expert in this area though.

Thanks,
Laurentiu

> 
> > ---
> >  drivers/staging/media/max96712/max96712.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/media/max96712/max96712.c b/drivers/staging/media/max96712/max96712.c
> > index 0751b2e048958..5228f9ec60859 100644
> > --- a/drivers/staging/media/max96712/max96712.c
> > +++ b/drivers/staging/media/max96712/max96712.c
> > @@ -182,7 +182,7 @@ static void max96712_pattern_enable(struct max96712_priv *priv, bool enable)
> >  	max96712_write_bulk_value(priv, 0x1052, 0, 3);
> >  	max96712_write_bulk_value(priv, 0x1055, v_sw * h_tot, 3);
> >  	max96712_write_bulk_value(priv, 0x1058,
> > -				  (v_active + v_fp + + v_bp) * h_tot, 3);
> > +				  (v_active + v_fp + v_bp) * h_tot, 3);
> >  	max96712_write_bulk_value(priv, 0x105b, 0, 3);
> >  	max96712_write_bulk_value(priv, 0x105e, h_sw, 2);
> >  	max96712_write_bulk_value(priv, 0x1060, h_active + h_fp + h_bp, 2);
> > -- 
> > 2.34.1
> > 
> 
> -- 
> Kind Regards,
> Niklas Söderlund

Re: [PATCH v2 1/3] staging: media: max96712: fix typo

[Laurentiu Palcu]

ok, I figured it out... I had 'format' set in my ~/.gitconfig to
us-ascii. :/ My bad after all. :)

Thanks for the heads-up.

Do you think it's worth sending this patch-set again to fix that? I
suppose it could also be fixed before pushing...

Thanks,
Laurentiu

On Fri, Dec 20, 2024 at 01:09:59PM +0200, Laurentiu Palcu wrote:
> Hi Niklas,
> 
> On Fri, Dec 20, 2024 at 11:59:34AM +0100, Niklas Söderlund wrote:
> > Hi Laurentiu,
> > 
> > On 2024-12-20 12:49:36 +0200, Laurentiu Palcu wrote:
> > > Fix a typo in VS_LOW expression.
> > > 
> > > Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
> > > Reviewed-by: Niklas S??derlund <niklas.soderlund+renesas@ragnatech.se>
> > 
> > I think your mail setup mangled my döts ;-) The Content-Type header for 
> > this mail is set to 'text/plain; charset="us-ascii"' while the patch is 
> > likely utf-8 encoded. Same in patch 3/3.
> 
> Sorry about that. I didn't mean to mess-up your name... :/ Apparently,
> this how 'git format-patch' generated the patch header:
> 
> ===
> From 19c513feb2003dff89c28ef9e54c0e06929efb0a Mon Sep 17 00:00:00 2001
> From: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
> Date: Thu, 19 Dec 2024 09:28:54 +0000
> Subject: [PATCH v2 1/3] staging: media: max96712: fix typo
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> Content-Type: text/plain; charset="us-ascii"
> ===
> 
> It looks like it generated 2 Content-Type lines. And the last one
> took precedence... 
> 
> Looks like a git bug?!? I'm no expert in this area though.
> 
> Thanks,
> Laurentiu
> 
> > 
> > > ---
> > >  drivers/staging/media/max96712/max96712.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/staging/media/max96712/max96712.c b/drivers/staging/media/max96712/max96712.c
> > > index 0751b2e048958..5228f9ec60859 100644
> > > --- a/drivers/staging/media/max96712/max96712.c
> > > +++ b/drivers/staging/media/max96712/max96712.c
> > > @@ -182,7 +182,7 @@ static void max96712_pattern_enable(struct max96712_priv *priv, bool enable)
> > >  	max96712_write_bulk_value(priv, 0x1052, 0, 3);
> > >  	max96712_write_bulk_value(priv, 0x1055, v_sw * h_tot, 3);
> > >  	max96712_write_bulk_value(priv, 0x1058,
> > > -				  (v_active + v_fp + + v_bp) * h_tot, 3);
> > > +				  (v_active + v_fp + v_bp) * h_tot, 3);
> > >  	max96712_write_bulk_value(priv, 0x105b, 0, 3);
> > >  	max96712_write_bulk_value(priv, 0x105e, h_sw, 2);
> > >  	max96712_write_bulk_value(priv, 0x1060, h_active + h_fp + h_bp, 2);
> > > -- 
> > > 2.34.1
> > > 
> > 
> > -- 
> > Kind Regards,
> > Niklas Söderlund

Re: [PATCH v2 1/3] staging: media: max96712: fix typo

[Niklas Söderlund]

On 2024-12-20 13:23:37 +0200, Laurentiu Palcu wrote:
> ok, I figured it out... I had 'format' set in my ~/.gitconfig to
> us-ascii. :/ My bad after all. :)
> 
> Thanks for the heads-up.

No worries, thanks for figuring it out.

> 
> Do you think it's worth sending this patch-set again to fix that? I
> suppose it could also be fixed before pushing...

I see no harm in resending, you could collect my R-b on 2/3 as well.

> 
> Thanks,
> Laurentiu
> 
> On Fri, Dec 20, 2024 at 01:09:59PM +0200, Laurentiu Palcu wrote:
> > Hi Niklas,
> > 
> > On Fri, Dec 20, 2024 at 11:59:34AM +0100, Niklas Söderlund wrote:
> > > Hi Laurentiu,
> > > 
> > > On 2024-12-20 12:49:36 +0200, Laurentiu Palcu wrote:
> > > > Fix a typo in VS_LOW expression.
> > > > 
> > > > Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
> > > > Reviewed-by: Niklas S??derlund <niklas.soderlund+renesas@ragnatech.se>
> > > 
> > > I think your mail setup mangled my döts ;-) The Content-Type header for 
> > > this mail is set to 'text/plain; charset="us-ascii"' while the patch is 
> > > likely utf-8 encoded. Same in patch 3/3.
> > 
> > Sorry about that. I didn't mean to mess-up your name... :/ Apparently,
> > this how 'git format-patch' generated the patch header:
> > 
> > ===
> > From 19c513feb2003dff89c28ef9e54c0e06929efb0a Mon Sep 17 00:00:00 2001
> > From: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
> > Date: Thu, 19 Dec 2024 09:28:54 +0000
> > Subject: [PATCH v2 1/3] staging: media: max96712: fix typo
> > MIME-Version: 1.0
> > Content-Type: text/plain; charset=UTF-8
> > Content-Transfer-Encoding: 8bit
> > Content-Type: text/plain; charset="us-ascii"
> > ===
> > 
> > It looks like it generated 2 Content-Type lines. And the last one
> > took precedence... 
> > 
> > Looks like a git bug?!? I'm no expert in this area though.
> > 
> > Thanks,
> > Laurentiu
> > 
> > > 
> > > > ---
> > > >  drivers/staging/media/max96712/max96712.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/staging/media/max96712/max96712.c b/drivers/staging/media/max96712/max96712.c
> > > > index 0751b2e048958..5228f9ec60859 100644
> > > > --- a/drivers/staging/media/max96712/max96712.c
> > > > +++ b/drivers/staging/media/max96712/max96712.c
> > > > @@ -182,7 +182,7 @@ static void max96712_pattern_enable(struct max96712_priv *priv, bool enable)
> > > >  	max96712_write_bulk_value(priv, 0x1052, 0, 3);
> > > >  	max96712_write_bulk_value(priv, 0x1055, v_sw * h_tot, 3);
> > > >  	max96712_write_bulk_value(priv, 0x1058,
> > > > -				  (v_active + v_fp + + v_bp) * h_tot, 3);
> > > > +				  (v_active + v_fp + v_bp) * h_tot, 3);
> > > >  	max96712_write_bulk_value(priv, 0x105b, 0, 3);
> > > >  	max96712_write_bulk_value(priv, 0x105e, h_sw, 2);
> > > >  	max96712_write_bulk_value(priv, 0x1060, h_active + h_fp + h_bp, 2);
> > > > -- 
> > > > 2.34.1
> > > > 
> > > 
> > > -- 
> > > Kind Regards,
> > > Niklas Söderlund

-- 
Kind Regards,
Niklas Söderlund