Re: [PATCH v5 08/18] iio: magnetometer: ak8975: check if gpiod read was successful

[Maxwell Doose]

On Tue, May 5, 2026 at 8:09 PM Maxwell Doose <m32285159@xxxxxxxxx> wrote:
>
> On Tue May 5, 2026 at 6:46 AM CDT, Joshua Crofts via B4 Relay wrote:
> > From: Joshua Crofts <joshua.crofts1@xxxxxxxxx>
> >
> > Add a check that ensures that valid data has been read from GPIOD. If
> > not, log an error and return the negative read value.
> >
> > Suggested-by: Jonathan Cameron <jic23@xxxxxxxxxx>
> > Signed-off-by: Joshua Crofts <joshua.crofts1@xxxxxxxxx>
> > ---
> >  drivers/iio/magnetometer/ak8975.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
> > index 129c98c97071b4cf8efa3abf2364a3abbde129ba..b3caf5f3ebf29bf701bb214aa480fa7afb2651c6 100644
> > --- a/drivers/iio/magnetometer/ak8975.c
> > +++ b/drivers/iio/magnetometer/ak8975.c
> > @@ -658,6 +658,10 @@ static int wait_conversion_complete_gpio(struct ak8975_data *data,
> >                                timeout_ms * USEC_PER_MSEC);
> >       if (ret)
> >               return ret;
> > +     if (val < 0) {
> > +             dev_err(&client->dev, "Error in reading GPIOD\n");
> > +             return val;
> > +     }
> >
> >       ret = i2c_smbus_read_byte_data(client, data->def->ctrl_regs[ST1]);
> >       if (ret < 0)
>
> Small nit but I wonder if we can change:
>
>         if (val < 0) {
>                 dev_err(&client->dev, "Error in reading GPIOD\n");
>                 return val;
>         }
>
> to something like:
>
>         if (val < 0)
>                 return dev_err_probe(&client->dev, val, "Error in reading GPIOD\n");
>
> Would be good for code density.
>
> My rb will apply either way.
>
> Reviewed-by: Maxwell Doose <m32285159@xxxxxxxxx>
>
> best regards,
> max

Actually, timeout. Sashiko had a good potential catch here:

https://sashiko.dev/#/patchset/20260505-magnetometer-fixes-v5-0-831b9b5550fc%40gmail.com

"Does this new error path leak the pm runtime reference?
In ak8975_read_axis(), a pm runtime reference is acquired via
pm_runtime_get_sync(&data->client->dev). If this new error path returns
val back up the call chain through ak8975_start_read_axis() to
ak8975_read_axis(), the code jumps to the exit label.
The exit block unlocks the mutex and returns the error code, but it skips
calling pm_runtime_put_autosuspend()."

I'm going to withdraw my reviewed-by until I can confirm that this
isn't a problem, sorry about that.

best regards,
max