[PATCH] media: i2c: imx: Add note to prevent buggy code re-use

[PATCH] media: i2c: imx: Add note to prevent buggy code re-use

[Krzysztof Kozlowski]

Multiple Sony IMX sensor drivers have mixed up logical and line level
for XCLR signal.  They call it a reset signal (it indeed behaves like
that), but drivers assert the reset to operate which is clearly
incorrect and relies on incorrect DTS.

People in discussions copy existing poor code and claim they can repeat
same mistake, so add a note to prevent that.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/media/i2c/imx219.c | 4 ++++
 drivers/media/i2c/imx274.c | 2 ++
 drivers/media/i2c/imx334.c | 4 ++++
 3 files changed, 10 insertions(+)

diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
index 3b4f68543342..9857929a3321 100644
--- a/drivers/media/i2c/imx219.c
+++ b/drivers/media/i2c/imx219.c
@@ -1034,6 +1034,10 @@ static int imx219_power_on(struct device *dev)
 		goto reg_off;
 	}
 
+	/*
+	 * Note: Misinterpreation of reset assertion - do not re-use this code.
+	 * XCLR pin is using incorrect (for reset signal) logical level.
+	 */
 	gpiod_set_value_cansleep(imx219->reset_gpio, 1);
 	usleep_range(IMX219_XCLR_MIN_DELAY_US,
 		     IMX219_XCLR_MIN_DELAY_US + IMX219_XCLR_DELAY_RANGE_US);
diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c
index a2b824986027..385a3df74b66 100644
--- a/drivers/media/i2c/imx274.c
+++ b/drivers/media/i2c/imx274.c
@@ -826,6 +826,8 @@ static int imx274_start_stream(struct stimx274 *priv)
  * if rst = 0, keep it in reset;
  * if rst = 1, bring it out of reset.
  *
+ * Note: Misinterpreation of reset assertion - do not re-use this code.
+ * XCLR pin is using incorrect (for reset signal) logical level.
  */
 static void imx274_reset(struct stimx274 *priv, int rst)
 {
diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c
index 846b9928d4e8..72e020d1d335 100644
--- a/drivers/media/i2c/imx334.c
+++ b/drivers/media/i2c/imx334.c
@@ -1070,6 +1070,10 @@ static int imx334_power_on(struct device *dev)
 	struct imx334 *imx334 = to_imx334(sd);
 	int ret;
 
+	/*
+	 * Note: Misinterpreation of reset assertion - do not re-use this code.
+	 * XCLR pin is using incorrect (for reset signal) logical level.
+	 */
 	gpiod_set_value_cansleep(imx334->reset_gpio, 1);
 
 	ret = clk_prepare_enable(imx334->inclk);
-- 
2.48.1

Re: [PATCH] media: i2c: imx: Add note to prevent buggy code re-use

[Krzysztof Kozlowski]

On 17/08/2025 10:48, Krzysztof Kozlowski wrote:
> Multiple Sony IMX sensor drivers have mixed up logical and line level
> for XCLR signal.  They call it a reset signal (it indeed behaves like
> that), but drivers assert the reset to operate which is clearly
> incorrect and relies on incorrect DTS.
> 
> People in discussions copy existing poor code and claim they can repeat
> same mistake, so add a note to prevent that.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  drivers/media/i2c/imx219.c | 4 ++++
>  drivers/media/i2c/imx274.c | 2 ++
>  drivers/media/i2c/imx334.c | 4 ++++
>  3 files changed, 10 insertions(+)
> 
> diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
> index 3b4f68543342..9857929a3321 100644
> --- a/drivers/media/i2c/imx219.c
> +++ b/drivers/media/i2c/imx219.c
> @@ -1034,6 +1034,10 @@ static int imx219_power_on(struct device *dev)
>  		goto reg_off;
>  	}
>  
> +	/*
> +	 * Note: Misinterpreation of reset assertion - do not re-use this code.


Typo here: Misinterpretation

I will send a v2.

Best regards,
Krzysztof