Re: [bug report] mfd: lp87565: Handle optional reset pin

Lee Jones
2021-11-18T08:38:18Z

On Wed, 17 Nov 2021, Dan Carpenter wrote:

> Hello Luca Ceresoli,
>
> The patch 50e4d7a2a667: "mfd: lp87565: Handle optional reset pin"
> from Feb 26, 2021, leads to the following Smatch static checker
> warning:
>
> drivers/mfd/lp87565.c:76 lp87565_probe()
> warn: 'lp87565->reset_gpio' could be an error pointer
>
> drivers/mfd/lp87565.c
> 46 static int lp87565_probe(struct i2c_client *client,
> 47 const struct i2c_device_id *ids)
> 48 {
> 49 struct lp87565 *lp87565;
> 50 const struct of_device_id *of_id;
> 51 int ret;
> 52 unsigned int otpid;
> 53
> 54 lp87565 = devm_kzalloc(&client->dev, sizeof(*lp87565), GFP_KERNEL);
> 55 if (!lp87565)
> 56 return -ENOMEM;
> 57
> 58 lp87565->dev = &client->dev;
> 59
> 60 lp87565->regmap = devm_regmap_init_i2c(client, &lp87565_regmap_config);
> 61 if (IS_ERR(lp87565->regmap)) {
> 62 ret = PTR_ERR(lp87565->regmap);
> 63 dev_err(lp87565->dev,
> 64 "Failed to initialize register map: %d\n", ret);
> 65 return ret;
> 66 }
> 67
> 68 lp87565->reset_gpio = devm_gpiod_get_optional(lp87565->dev, "reset",
> 69 GPIOD_OUT_LOW);
> 70 if (IS_ERR(lp87565->reset_gpio)) {
> 71 ret = PTR_ERR(lp87565->reset_gpio);
> 72 if (ret == -EPROBE_DEFER)
> 73 return ret;
>
> Only "ret = -EPROBE_DEFER" is handled. Other error pointer will lead to
> a crash.
>
> 74 }
> 75
> --> 76 if (lp87565->reset_gpio) {

I guess this should be:

if (lp87565->reset_gpio >= 0)

If 0 is valid, or '>' if it's not.

> 77 gpiod_set_value_cansleep(lp87565->reset_gpio, 1);
> 78 /* The minimum assertion time is undocumented, just guess */
> 79 usleep_range(2000, 4000);
> 80
> 81 gpiod_set_value_cansleep(lp87565->reset_gpio, 0);
> 82 /* Min 1.2 ms before first I2C transaction */
> 83 usleep_range(1500, 3000);
> 84 }
> 85
> 86 ret = regmap_read(lp87565->regmap, LP87565_REG_OTP_REV, &otpid);
> 87 if (ret) {
> 88 dev_err(lp87565->dev, "Failed to read OTP ID\n");
> 89 return ret;
> 90 }
> 91
> 92 lp87565->rev = otpid & LP87565_OTP_REV_OTP_ID;
> 93
> 94 of_id = of_match_device(of_lp87565_match_table, &client->dev);
> 95 if (of_id)
> 96 lp87565->dev_type = (enum lp87565_device_type)of_id->data;
> 97
> 98 i2c_set_clientdata(client, lp87565);
> 99
> 100 return devm_mfd_add_devices(lp87565->dev, PLATFORM_DEVID_AUTO,
> 101 lp87565_cells, ARRAY_SIZE(lp87565_cells),
> 102 NULL, 0, NULL);
> 103 }
>
> regards,
> dan carpenter

--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog