[PATCH v4] media: tuner: check return value of I2C transfers in set_type()

[PATCH v4] media: tuner: check return value of I2C transfers in set_type()

[Wenyuan Li]

In set_type(), several I2C transfers are performed to initialize
specific tuners (e.g. FMD1216ME, FMD1216MEX, TD1316), but the return
value of i2c_master_send() is not checked.

If an I2C transfer fails, the initialization sequence may be
incomplete, potentially leaving the tuner in an inconsistent state
without any error being reported.

Check the return value of i2c_master_send() and propagate failures
to the attach_failed path. A small helper is introduced to reduce
duplication and provide consistent error reporting.

This ensures that I2C communication errors during tuner
initialization are properly detected and handled.

Fixes: 93df3413f1b4 ("[PATCH] v4l: 655: added support for the philips td1316 tuner")
Cc: stable@vger.kernel.org
Signed-off-by: Wenyuan Li <2063309626@qq.com>

---
v5:
- Reword commit message to better explain rationale
- Clarify error handling approach
- No functional changes

v4:
- Added Cc: stable@vger.kernel.org
- Updated Fixes tag
---
 drivers/media/v4l2-core/tuner-core.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/media/v4l2-core/tuner-core.c b/drivers/media/v4l2-core/tuner-core.c
index 004ec4d7beea..01f28436a1f8 100644
--- a/drivers/media/v4l2-core/tuner-core.c
+++ b/drivers/media/v4l2-core/tuner-core.c
@@ -280,6 +280,19 @@ static const struct analog_demod_ops tuner_analog_ops = {
  * Functions to select between radio and TV and tuner probe/remove functions
  */
 
+static int tuner_i2c_send(struct i2c_client *c, u8 *buf, int len)
+{
+	int ret = i2c_master_send(c, buf, len);
+
+	if (ret != len) {
+		int err = ret < 0 ? ret : -EIO;
+
+		dev_err(&c->dev, "I2C send failed: %pe\n", ERR_PTR(err));
+		return err;
+	}
+	return 0;
+}
+
 /**
  * set_type - Sets the tuner type for a given device
  *
@@ -351,11 +364,13 @@ static void set_type(struct i2c_client *c, unsigned int type,
 		buffer[1] = 0xdc;
 		buffer[2] = 0x9c;
 		buffer[3] = 0x60;
-		i2c_master_send(c, buffer, 4);
+		if (tuner_i2c_send(c, buffer, 4))
+			goto attach_failed;
 		mdelay(1);
 		buffer[2] = 0x86;
 		buffer[3] = 0x54;
-		i2c_master_send(c, buffer, 4);
+		if (tuner_i2c_send(c, buffer, 4))
+			goto attach_failed;
 		if (!dvb_attach(simple_tuner_attach, &t->fe,
 				t->i2c->adapter, t->i2c->addr, t->type))
 			goto attach_failed;
@@ -365,7 +380,8 @@ static void set_type(struct i2c_client *c, unsigned int type,
 		buffer[1] = 0xdc;
 		buffer[2] = 0x86;
 		buffer[3] = 0xa4;
-		i2c_master_send(c, buffer, 4);
+		if (tuner_i2c_send(c, buffer, 4))
+			goto attach_failed;
 		if (!dvb_attach(simple_tuner_attach, &t->fe,
 				t->i2c->adapter, t->i2c->addr, t->type))
 			goto attach_failed;
-- 
2.43.0

Re: [PATCH v4] media: tuner: check return value of I2C transfers in set_type()

[Markus Elfring]

…
> ---
> v5:
> - Reword commit message to better explain rationale
…

How does does the prefix “[PATCH v4]” fit to such information?

Regards,
Markus

Re: [PATCH v4] media: tuner: check return value of I2C transfers in set_type()

[Hans Verkuil]

On 3/29/26 12:51, Wenyuan Li wrote:
> In set_type(), several I2C transfers are performed to initialize
> specific tuners (e.g. FMD1216ME, FMD1216MEX, TD1316), but the return
> value of i2c_master_send() is not checked.
> 
> If an I2C transfer fails, the initialization sequence may be
> incomplete, potentially leaving the tuner in an inconsistent state
> without any error being reported.
> 
> Check the return value of i2c_master_send() and propagate failures
> to the attach_failed path. A small helper is introduced to reduce
> duplication and provide consistent error reporting.
> 
> This ensures that I2C communication errors during tuner
> initialization are properly detected and handled.
> 
> Fixes: 93df3413f1b4 ("[PATCH] v4l: 655: added support for the philips td1316 tuner")
> Cc: stable@vger.kernel.org
> Signed-off-by: Wenyuan Li <2063309626@qq.com>
> 
> ---
> v5:
> - Reword commit message to better explain rationale
> - Clarify error handling approach
> - No functional changes
> 
> v4:
> - Added Cc: stable@vger.kernel.org
> - Updated Fixes tag
> ---
>  drivers/media/v4l2-core/tuner-core.c | 22 +++++++++++++++++++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/tuner-core.c b/drivers/media/v4l2-core/tuner-core.c
> index 004ec4d7beea..01f28436a1f8 100644
> --- a/drivers/media/v4l2-core/tuner-core.c
> +++ b/drivers/media/v4l2-core/tuner-core.c
> @@ -280,6 +280,19 @@ static const struct analog_demod_ops tuner_analog_ops = {
>   * Functions to select between radio and TV and tuner probe/remove functions
>   */
>  
> +static int tuner_i2c_send(struct i2c_client *c, u8 *buf, int len)
> +{
> +	int ret = i2c_master_send(c, buf, len);
> +
> +	if (ret != len) {
> +		int err = ret < 0 ? ret : -EIO;
> +
> +		dev_err(&c->dev, "I2C send failed: %pe\n", ERR_PTR(err));
> +		return err;
> +	}
> +	return 0;
> +}
> +

I think this is overkill, and I also like to keep changes to this old code
to a minimum.

>  /**
>   * set_type - Sets the tuner type for a given device
>   *
> @@ -351,11 +364,13 @@ static void set_type(struct i2c_client *c, unsigned int type,
>  		buffer[1] = 0xdc;
>  		buffer[2] = 0x9c;
>  		buffer[3] = 0x60;
> -		i2c_master_send(c, buffer, 4);
> +		if (tuner_i2c_send(c, buffer, 4))
> +			goto attach_failed;

Just keep it simple:

		if (i2c_master_send(c, buffer, 4) != 4)
			goto attach_failed;

Ditto below.

>  		mdelay(1);
>  		buffer[2] = 0x86;
>  		buffer[3] = 0x54;
> -		i2c_master_send(c, buffer, 4);
> +		if (tuner_i2c_send(c, buffer, 4))
> +			goto attach_failed;
>  		if (!dvb_attach(simple_tuner_attach, &t->fe,
>  				t->i2c->adapter, t->i2c->addr, t->type))
>  			goto attach_failed;
> @@ -365,7 +380,8 @@ static void set_type(struct i2c_client *c, unsigned int type,
>  		buffer[1] = 0xdc;
>  		buffer[2] = 0x86;
>  		buffer[3] = 0xa4;
> -		i2c_master_send(c, buffer, 4);
> +		if (tuner_i2c_send(c, buffer, 4))
> +			goto attach_failed;
>  		if (!dvb_attach(simple_tuner_attach, &t->fe,
>  				t->i2c->adapter, t->i2c->addr, t->type))
>  			goto attach_failed;

Regards,

	Hans