On Thu, 18 Nov 2021, Dan Carpenter wrote:
> 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.
Ah, I assumed this was a GPIO #. I missed the gpioD part.
> There are two options. The first is to just "return ret;" for all
> errors.
No, don't do that. This GPIO is optional.
> 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;
> }
Works for me.
Or:
if (!IS_ERR(lp87565->reset_gpio))
Or stick it in the else.
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
