Re: [PATCH v5 07/18] iio: magnetometer: ak8975: modernize polling loops with iopoll() macros

[Joshua Crofts]

On Wed, 6 May 2026 at 08:53, Andy Shevchenko
<andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
>
> On Tue, May 05, 2026 at 01:46:03PM +0200, Joshua Crofts via B4 Relay wrote:
>
> > The driver currently uses while loops and msleep() for polling during
> > conversion waits.
> >
> > Replace the custom polling loops with readx_poll_timeout() and
> > read_poll_timeout() macros from <linux/iopoll.h>. This reduces
> > boilerplate, standardizes timeout handling and improves overall code
> > readability, keeping the original timing and error behaviour. Add
> > <linux/time.h> for USEC_PER_MSEC macro instead of using magic numbers.
>
> ...
>
> > +     ret = read_poll_timeout(i2c_smbus_read_byte_data, val, val != 0,
> > +                             poll_ms * USEC_PER_MSEC,
> > +                             timeout_ms * USEC_PER_MSEC,
> > +                             true,
> > +                             client, data->def->ctrl_regs[ST1]);
> > +     if (ret)
> > +             return ret;
> > +     if (val < 0) {
> > +             dev_err(&client->dev, "Error in reading ST1\n");
> > +             return val;
> >       }
> > -     if (!timeout_ms)
> > -             return -ETIMEDOUT;
> >
> > -     return read_status;
> > +     return val;
>
> Besides the unneeded return val duplication, I think this is the wrong location
> for this check and it changes behaviour (really subtle change!).
>
> Before if the last iteration gives an error from the device, we return
> -ETIMEDOUT instead of the whatever the i2c_smbus_read_byte_data() returns.

Yeah, that is pretty subtle, but I agree.

-- 
Kind regards

CJD