On Wed Jun 11 19:37:15 2025 +0100, Dave Stevenson wrote:
> 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.steven...@raspberrypi.com>
> Signed-off-by: Hans Verkuil <hverk...@xs4all.nl>

Patch committed.

Thanks,
Hans Verkuil

 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 ddba8c392ead..4d3dc61a9b8b 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);
@@ -2092,6 +2109,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))
@@ -2123,7 +2141,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;

Reply via email to