Re: [PATCH v0 04/14] iio: magnetometer: ak8975: Inline timeout constants

[Andy Shevchenko]

On Wed, Apr 29, 2026 at 03:59:11PM +0200, Joshua Crofts wrote:
> On Tue, 28 Apr 2026 at 19:06, Andy Shevchenko
> <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> > On Tue, Apr 28, 2026 at 05:22:12PM +0100, Jonathan Cameron wrote:
> > > On Mon, 27 Apr 2026 22:09:49 +0200
> > > Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:

...

> > > >     /* Wait for the conversion to complete. */
> > > >     ret = read_poll_timeout(i2c_smbus_read_byte_data, val, val != 0,
> > > > -                           AK8975_CONVERSION_DONE_POLL_TIME * USEC_PER_MSEC,
> > > > -                           AK8975_MAX_CONVERSION_TIMEOUT * USEC_PER_MSEC,
> > > > +                           10 * USEC_PER_MSEC, 500 * USEC_PER_MSEC,
> > >
> > > I'm not that keen on these being repeated in two places given they take the same values.
> > > Perhaps we should push them up to the caller?  That is, add them as
> > > parameters of wait_completion_complete_gpio() and wait_completion_polled()
> > >
> > > I don't mind if we have them hard coded into those two calls as they are right next
> > > to each other so it should be obvious they values are the same.
> > >
> > > Ideally I'd like to see a comment on why these numbers next to those calls.
> > >
> > > Fine to pass them in msecs and do the scaling here.  Just name the variables to
> > > make that clear.  poll_msecs timeout_msecs maybe?
> >
> > I think we are fine with _ms, so poll_ms, timeout_ms.
> >
> > For the completeness we may also give a timeout_ms to the third one
> > (and convert it to jiffies there).
> 
> Simple enough, however should I do this as a separate patch or
> just a fixup into this one?

I am not sure, but since Jonathan doesn't like the numbers be copied and split
far from each other, I think the fixup sounds better. The commit message needs
to be amended accordingly.

-- 
With Best Regards,
Andy Shevchenko