Re: [PATCH] iio: mlx90614: fix missing GPIO direction return value checks

[Crt Mori]

On Tue, 28 Apr 2026 at 18:04, Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>
> On Tue, 28 Apr 2026 09:38:47 +0100
> Salah Triki <salah.triki@xxxxxxxxx> wrote:
>
> > On Tue, Apr 28, 2026 at 11:00:04AM +0300, Andy Shevchenko wrote:
> > > On Mon, Apr 27, 2026 at 10:58:00PM +0100, Salah Triki wrote:
> > > > The functions gpiod_direction_output() and gpiod_direction_input() can
> > > > fail, but their return values were previously ignored.
> > > >
> > > > If an error occurs during the GPIO configuration, the function should
> > > > abort the wake-up sequence and return the error code. More importantly,
> > > > failing to check these values could lead to the I2C bus remaining
> > > > locked if an error occurs after i2c_lock_bus() is called.
> > > >
> > > > Add return value checks and ensure the I2C bus is properly unlocked
> > > > via a goto label in case of failure.
> > >
> > > ...
> > >
> > > > - gpiod_direction_output(data->wakeup_gpio, 0);
> > > > +
> > > > + ret = gpiod_direction_output(data->wakeup_gpio, 0);
> > > > + if (ret)
> > > > +         goto out_unlock;
> > > > +
> > > >   msleep(chip_info->wakeup_delay_ms);
> > > > - gpiod_direction_input(data->wakeup_gpio);
> > > > +
> > > > + ret = gpiod_direction_input(data->wakeup_gpio);
> > > > + if (ret)
> > > > +         goto out_unlock;
> > >
> > > While technically it sounds correct, the potential problem here is that you may
> > > fail this in case CONFIG_GPIOLIB=n. Is this GPIO optional?
> > > What may happen if GPIO is not optional, but for some reason setting it fails?
> > >
> > > TL;DR:
> > > I am not sure about this patch. At least I'm not comfortable to take it without
> > > testing on real HW.
> > >
> > > --
> > > With Best Regards,
> > > Andy Shevchenko
> > >
> > >
> >
> > Thank you for your feedback. Since I don't have the physical hardware to
> > perform the necessary tests and ensure there are no regressions, I agree
> > that it is safer to drop this patch. I don't want to risk breaking the
> > driver for a minor cleanup.
> >
> Maybe Crt has the hardware to test. For now I'll assume this is not
> going anywhere wrt to tracking in patchwork.
>
I will dig the hardware to test as it was off my desk for some time
now. It seems like backwards compatible, but I am wondering why would
we want to be informed of this failing? The problem is that sensor
uses wakeup line (same as for i2c) to receive wakeup pulse, so in
theory this is already a workaround. @Salah did you see any issues in
the wild?

> > Best regards,
> > --
> > Salah Triki
>