Re: [PATCH v2 1/2] iio: dac: adi-axi-dac: fix bus read

[Jonathan Cameron <jic23@kernel.org>]

On Wed, 9 Apr 2025 19:49:25 +0300
Andy Shevchenko <andy@kernel.org> wrote:

> On Wed, Apr 09, 2025 at 11:16:54AM +0200, Angelo Dureghello wrote:
> > From: Angelo Dureghello <adureghello@baylibre.com>
> > 
> > Fix bus read function.
> > 
> > Testing the driver, on a random basis, wrong reads was detected, mainly
> > by a wrong DAC chip ID read at first boot.
> > Before reading the expected value from the AXI regmap, need always to
> > wait for busy flag to be cleared.  
> 
> ...
> 
> > +	ret = regmap_read_poll_timeout(st->regmap,
> > +				AXI_DAC_UI_STATUS_REG, ival,
> > +				FIELD_GET(AXI_DAC_UI_STATUS_IF_BUSY, ival) == 0,
> > +				10, 100 * KILO);  
> 
> It's timeout, we have special constants for that, I believe you wanted to have
> USEC_PER_MSEC here.

This is an odd corner case.  If we had a define for that 100 along the lines
of X_TIMEOUT_MSEC then I'd agree that using USEC_PER_MSEC makes complete sense.
All we have is a bare number which has no defined units.  I'd just go with 100000 and
not use the units.h defines at all.  They make sense when lots of zeros are involved
or for standard conversions, but to me not worth it here.

Jonathan

> 
> > +	if (ret)
> > +		return ret;  
>