[PATCH AUTOSEL 6.16-5.4] media: tc358743: Check I2C succeeded during probe

[Sasha Levin <sashal@kernel.org>]

From: Dave Stevenson <dave.stevenson@raspberrypi.com>

[ Upstream commit 303d81635e1d9c949b370215cc94526ed81f2e3d ]

The probe for the TC358743 reads the CHIPID register from
the device and compares it to the expected value of 0.
If the I2C request fails then that also returns 0, so
the driver loads thinking that the device is there.

Generally I2C communications are reliable so there is
limited need to check the return value on every transfer,
therefore only amend the one read during probe to check
for I2C errors.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

**Backport Status: YES**

This commit should be backported to stable kernel trees for the
following reasons:

## Bug Fix Analysis

1. **Critical Probe Bug**: The commit fixes a serious bug where the
   TC358743 driver incorrectly loads even when the device is not
   present. The original code reads the CHIPID register and compares it
   to 0, but if the I2C communication fails, `i2c_rd16()` also returns
   0, causing a false positive detection.

2. **User-Visible Impact**: This bug causes the driver to incorrectly
   bind to non-existent hardware, which can lead to:
   - System instability when attempting to use the non-existent device
   - Incorrect driver loading that prevents proper hardware detection
   - Potential crashes or errors when userspace tries to interact with
     the phantom device

## Code Changes Analysis

The fix is minimal and surgical:

1. **Modified I2C read functions** to return error status:
   - Changed `i2c_rd()` from void to int, returning error status
   - Added `i2c_rd16_err()` wrapper that captures error state
   - Added `i2c_rdreg_err()` to propagate errors through the stack

2. **Fixed probe logic** (lines 2134-2135):
  ```c
  -if ((i2c_rd16(sd, CHIPID) & MASK_CHIPID) != 0) {
  +if (i2c_rd16_err(sd, CHIPID, &chipid) ||
  +    (chipid & MASK_CHIPID) != 0) {
  ```
  Now properly checks for I2C errors before validating the chip ID.

## Stable Tree Criteria

The commit meets stable tree requirements:

1. **Fixes a real bug**: Prevents incorrect driver loading on I2C
   failures
2. **Small and contained**: Changes are limited to error handling in the
   probe path
3. **Low regression risk**: Only adds error checking; doesn't change
   successful probe behavior
4. **No new features**: Pure bug fix, no functionality additions
5. **Clear fix**: The problem and solution are straightforward

The commit message explicitly states "Generally I2C communications are
reliable so there is limited need to check the return value on every
transfer, therefore only amend the one read during probe" - showing
careful consideration to minimize changes while fixing the critical
issue.

This is a textbook example of a stable-worthy fix: it addresses a
specific hardware detection bug with minimal, safe changes that have no
impact on properly functioning systems.

 drivers/media/i2c/tc358743.c | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index 3d6703b75bfa..8c269e28fd5f 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -114,7 +114,7 @@ static inline struct tc358743_state *to_state(struct v4l2_subdev *sd)
 
 /* --------------- I2C --------------- */
 
-static void i2c_rd(struct v4l2_subdev *sd, u16 reg, u8 *values, u32 n)
+static int i2c_rd(struct v4l2_subdev *sd, u16 reg, u8 *values, u32 n)
 {
 	struct tc358743_state *state = to_state(sd);
 	struct i2c_client *client = state->i2c_client;
@@ -140,6 +140,7 @@ static void i2c_rd(struct v4l2_subdev *sd, u16 reg, u8 *values, u32 n)
 		v4l2_err(sd, "%s: reading register 0x%x from 0x%x failed: %d\n",
 				__func__, reg, client->addr, err);
 	}
+	return err != ARRAY_SIZE(msgs);
 }
 
 static void i2c_wr(struct v4l2_subdev *sd, u16 reg, u8 *values, u32 n)
@@ -196,15 +197,24 @@ static void i2c_wr(struct v4l2_subdev *sd, u16 reg, u8 *values, u32 n)
 	}
 }
 
-static noinline u32 i2c_rdreg(struct v4l2_subdev *sd, u16 reg, u32 n)
+static noinline u32 i2c_rdreg_err(struct v4l2_subdev *sd, u16 reg, u32 n,
+				  int *err)
 {
+	int error;
 	__le32 val = 0;
 
-	i2c_rd(sd, reg, (u8 __force *)&val, n);
+	error = i2c_rd(sd, reg, (u8 __force *)&val, n);
+	if (err)
+		*err = error;
 
 	return le32_to_cpu(val);
 }
 
+static inline u32 i2c_rdreg(struct v4l2_subdev *sd, u16 reg, u32 n)
+{
+	return i2c_rdreg_err(sd, reg, n, NULL);
+}
+
 static noinline void i2c_wrreg(struct v4l2_subdev *sd, u16 reg, u32 val, u32 n)
 {
 	__le32 raw = cpu_to_le32(val);
@@ -233,6 +243,13 @@ static u16 i2c_rd16(struct v4l2_subdev *sd, u16 reg)
 	return i2c_rdreg(sd, reg, 2);
 }
 
+static int i2c_rd16_err(struct v4l2_subdev *sd, u16 reg, u16 *value)
+{
+	int err;
+	*value = i2c_rdreg_err(sd, reg, 2, &err);
+	return err;
+}
+
 static void i2c_wr16(struct v4l2_subdev *sd, u16 reg, u16 val)
 {
 	i2c_wrreg(sd, reg, val, 2);
@@ -2061,6 +2078,7 @@ static int tc358743_probe(struct i2c_client *client)
 	struct tc358743_platform_data *pdata = client->dev.platform_data;
 	struct v4l2_subdev *sd;
 	u16 irq_mask = MASK_HDMI_MSK | MASK_CSI_MSK;
+	u16 chipid;
 	int err;
 
 	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
@@ -2092,7 +2110,8 @@ static int tc358743_probe(struct i2c_client *client)
 	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;
 
 	/* i2c access */
-	if ((i2c_rd16(sd, CHIPID) & MASK_CHIPID) != 0) {
+	if (i2c_rd16_err(sd, CHIPID, &chipid) ||
+	    (chipid & MASK_CHIPID) != 0) {
 		v4l2_info(sd, "not a TC358743 on address 0x%x\n",
 			  client->addr << 1);
 		return -ENODEV;
-- 
2.39.5