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

[Lee Jones <lee.jones@linaro.org>]

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