On Thu, Nov 18, 2021 at 08:38:12AM +0000, Lee Jones wrote:
> 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.
>
lp87565->reset_gpio is a pointer so that's not going to work.
There are two options. The first is to just "return ret;" for all
errors. The second option is to say something like:
if (IS_ERR(lp87565->reset_gpio)) {
ret = dev_err_probe(lp87565->dev, PTR_ERR(lp87565->reset_gpio),
"no GPIO error for ->reset_gpio");
if (ret == -EPROBE_DEFER)
return;
lp87565->reset_gpio = NULL;
}
regards,
dan carpenter
