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

Joshua Crofts
2026-04-29T13:59:24Z

On Tue, 28 Apr 2026 at 19:06, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> 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@linux.intel.com> 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?

--
Kind regards

CJD