[PATCH v2] mfd: tps65910: add error handling for dummy I2C transfer in probe

Wenyuan Li posted 1 patch 19 hours ago
There is a newer version of this series
drivers/mfd/tps65910.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
[PATCH v2] mfd: tps65910: add error handling for dummy I2C transfer in probe
Posted by Wenyuan Li 19 hours ago
In tps65910_i2c_probe(), a dummy I2C transfer is performed to work
around silicon erratum SWCZ010. However, the return value of
i2c_master_send() is not checked.

If this dummy transfer fails, the driver continues execution without
detecting the error. This may lead to subsequent I2C operations also
failing, but the driver would incorrectly report success.

Add proper return value checking for the dummy I2C transfer. If the
transfer fails, log the error and return an appropriate error code
to the caller.

Signed-off-by: Wenyuan Li <2063309626@qq.com>
---
v2:
- Use dev_err_probe() for error handling
- Replace ternary operator with if/else
- Add TPS65910_DUMMY_XFER_LEN macro
---
 drivers/mfd/tps65910.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/tps65910.c b/drivers/mfd/tps65910.c
index 6a7b7a697fb7..31e4726118b3 100644
--- a/drivers/mfd/tps65910.c
+++ b/drivers/mfd/tps65910.c
@@ -21,6 +21,9 @@
 #include <linux/of.h>
 #include <linux/property.h>
 
+/* Dummy I2C transfer length for SWCZ010 workaround */
+#define TPS65910_DUMMY_XFER_LEN 1
+
 static const struct resource rtc_resources[] = {
 	{
 		.start  = TPS65910_IRQ_RTC_ALARM,
@@ -472,7 +475,18 @@ static int tps65910_i2c_probe(struct i2c_client *i2c)
 	 * first I2C transfer. So issue a dummy transfer before the first
 	 * real transfer.
 	 */
-	i2c_master_send(i2c, "", 1);
+	ret = i2c_master_send(i2c, "", TPS65910_DUMMY_XFER_LEN);
+	if (ret != TPS65910_DUMMY_XFER_LEN) {
+		int err;
+
+		if (ret < 0)
+			err = ret;
+		else
+			err = -EIO;
+
+	return dev_err_probe(&i2c->dev, err, "dummy transfer failed\n");
+	}
+
 	tps65910->regmap = devm_regmap_init_i2c(i2c, &tps65910_regmap_config);
 	if (IS_ERR(tps65910->regmap)) {
 		ret = PTR_ERR(tps65910->regmap);
-- 
2.43.0
Re: [PATCH v2] mfd: tps65910: add error handling for dummy I2C transfer in probe
Posted by kernel test robot 5 hours ago
Hi Wenyuan,

kernel test robot noticed the following build warnings:

[auto build test WARNING on lee-leds/for-leds-next]
[also build test WARNING on lee-mfd/for-mfd-next lee-mfd/for-mfd-fixes linus/master v7.0-rc6 next-20260330]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Wenyuan-Li/mfd-tps65910-add-error-handling-for-dummy-I2C-transfer-in-probe/20260331-203820
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/leds.git for-leds-next
patch link:    https://lore.kernel.org/r/tencent_74C5C53384C579056ED161B70122D16ACF06%40qq.com
patch subject: [PATCH v2] mfd: tps65910: add error handling for dummy I2C transfer in probe
config: arm64-randconfig-r072-20260401 (https://download.01.org/0day-ci/archive/20260401/202604011034.5GOHWfcw-lkp@intel.com/config)
compiler: clang version 23.0.0git (https://github.com/llvm/llvm-project 2cd67b8b69f78e3f95918204320c3075a74ba16c)
smatch: v0.5.0-9004-gb810ac53

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202604011034.5GOHWfcw-lkp@intel.com/

smatch warnings:
drivers/mfd/tps65910.c:487 tps65910_i2c_probe() warn: inconsistent indenting

vim +487 drivers/mfd/tps65910.c

   439	
   440	static int tps65910_i2c_probe(struct i2c_client *i2c)
   441	{
   442		const struct i2c_device_id *id = i2c_client_get_device_id(i2c);
   443		struct tps65910 *tps65910;
   444		struct tps65910_board *pmic_plat_data;
   445		struct tps65910_board *of_pmic_plat_data = NULL;
   446		struct tps65910_platform_data *init_data;
   447		unsigned long chip_id = id->driver_data;
   448		int ret;
   449	
   450		pmic_plat_data = dev_get_platdata(&i2c->dev);
   451	
   452		if (!pmic_plat_data && i2c->dev.of_node) {
   453			pmic_plat_data = tps65910_parse_dt(i2c, &chip_id);
   454			of_pmic_plat_data = pmic_plat_data;
   455		}
   456	
   457		if (!pmic_plat_data)
   458			return -EINVAL;
   459	
   460		init_data = devm_kzalloc(&i2c->dev, sizeof(*init_data), GFP_KERNEL);
   461		if (init_data == NULL)
   462			return -ENOMEM;
   463	
   464		tps65910 = devm_kzalloc(&i2c->dev, sizeof(*tps65910), GFP_KERNEL);
   465		if (tps65910 == NULL)
   466			return -ENOMEM;
   467	
   468		tps65910->of_plat_data = of_pmic_plat_data;
   469		i2c_set_clientdata(i2c, tps65910);
   470		tps65910->dev = &i2c->dev;
   471		tps65910->i2c_client = i2c;
   472		tps65910->id = chip_id;
   473	
   474		/* Work around silicon erratum SWCZ010: the tps65910 may miss the
   475		 * first I2C transfer. So issue a dummy transfer before the first
   476		 * real transfer.
   477		 */
   478		ret = i2c_master_send(i2c, "", TPS65910_DUMMY_XFER_LEN);
   479		if (ret != TPS65910_DUMMY_XFER_LEN) {
   480			int err;
   481	
   482			if (ret < 0)
   483				err = ret;
   484			else
   485				err = -EIO;
   486	
 > 487		return dev_err_probe(&i2c->dev, err, "dummy transfer failed\n");
   488		}
   489	
   490		tps65910->regmap = devm_regmap_init_i2c(i2c, &tps65910_regmap_config);
   491		if (IS_ERR(tps65910->regmap)) {
   492			ret = PTR_ERR(tps65910->regmap);
   493			dev_err(&i2c->dev, "regmap initialization failed: %d\n", ret);
   494			return ret;
   495		}
   496	
   497		init_data->irq = pmic_plat_data->irq;
   498		init_data->irq_base = pmic_plat_data->irq_base;
   499	
   500		tps65910_irq_init(tps65910, init_data->irq, init_data);
   501		tps65910_ck32k_init(tps65910, pmic_plat_data);
   502		tps65910_sleepinit(tps65910, pmic_plat_data);
   503	
   504		if (pmic_plat_data->pm_off && !pm_power_off) {
   505			/*
   506			 * The PWR_OFF bit needs to be set separately, before
   507			 * transitioning to the OFF state. It enables the "sequential"
   508			 * power-off mode on TPS65911, it's a NO-OP on TPS65910.
   509			 */
   510			ret = regmap_set_bits(tps65910->regmap, TPS65910_DEVCTRL,
   511					      DEVCTRL_PWR_OFF_MASK);
   512			if (ret) {
   513				dev_err(&i2c->dev, "failed to set power-off mode: %d\n",
   514					ret);
   515				return ret;
   516			}
   517	
   518			tps65910_i2c_client = i2c;
   519			pm_power_off = tps65910_power_off;
   520		}
   521	
   522		ret = devm_mfd_add_devices(tps65910->dev, -1,
   523					   tps65910s, ARRAY_SIZE(tps65910s),
   524					   NULL, 0,
   525					   regmap_irq_get_domain(tps65910->irq_data));
   526		if (ret < 0) {
   527			dev_err(&i2c->dev, "mfd_add_devices failed: %d\n", ret);
   528			return ret;
   529		}
   530	
   531		return ret;
   532	}
   533	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki