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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20251230083435.26267-2-krzysztof.kozlowski@oss.qualcomm.com>
Date: Tue, 30 Dec 2025 09:34:36 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@....qualcomm.com>
To: Hans Verkuil <hverkuil@...nel.org>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Pavel Machek <pavel@...nel.org>,
        Sakari Ailus <sakari.ailus@...ux.intel.com>,
        linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@....qualcomm.com>
Subject: [PATCH] media: i2c: Add note to prevent buggy code re-use

adv7604 and et8ek8 sensor drivers have mixed up logical and line level
for reset/powerdown 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 wrong ACTIVE_HIGH flag in the 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@....qualcomm.com>

---

Similar to my commit 9d108d226224 ("media: i2c: imx: Add note to prevent
buggy code re-use"). I went through rest of i2c drivers and found only
these two doing it incorrectly.
---
 drivers/media/i2c/adv7604.c              | 8 +++++++-
 drivers/media/i2c/et8ek8/et8ek8_driver.c | 4 ++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index 516553fb17e9..67116a4ef134 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -3453,7 +3453,13 @@ static int configure_regmaps(struct adv76xx_state *state)
 static void adv76xx_reset(struct adv76xx_state *state)
 {
 	if (state->reset_gpio) {
-		/* ADV76XX can be reset by a low reset pulse of minimum 5 ms. */
+		/*
+		 * Note: Misinterpretation of reset assertion - do not re-use
+		 * this code.  The reset pin is using incorrect (for a reset
+		 * signal) logical level.
+		 *
+		 * ADV76XX can be reset by a low reset pulse of minimum 5 ms.
+		 */
 		gpiod_set_value_cansleep(state->reset_gpio, 0);
 		usleep_range(5000, 10000);
 		gpiod_set_value_cansleep(state->reset_gpio, 1);
diff --git a/drivers/media/i2c/et8ek8/et8ek8_driver.c b/drivers/media/i2c/et8ek8/et8ek8_driver.c
index 2cb7b718782b..50121c3e5b48 100644
--- a/drivers/media/i2c/et8ek8/et8ek8_driver.c
+++ b/drivers/media/i2c/et8ek8/et8ek8_driver.c
@@ -835,6 +835,10 @@ static int et8ek8_power_on(struct et8ek8_sensor *sensor)
 
 	udelay(10); /* I wish this is a good value */
 
+	/*
+	 * Note: Misinterpretation of reset assertion - do not re-use this code.
+	 * The reset pin is using incorrect (for a reset signal) logical level.
+	 */
 	gpiod_set_value(sensor->reset, 1);
 
 	msleep(5000 * 1000 / sensor->xclk_freq + 1); /* Wait 5000 cycles */
-- 
2.51.0