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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAHp75Vc_DjOYFCExi=h5Qd1+ACgiBzYo1Q-UHSyZVSNJOwbvXQ@mail.gmail.com>
Date: Sat, 12 Apr 2025 21:03:14 +0300
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Jonathan Cameron <jic23@...nel.org>
Cc: Andy Shevchenko <andy@...nel.org>, Angelo Dureghello <adureghello@...libre.com>, 
	Nuno Sa <nuno.sa@...log.com>, Lars-Peter Clausen <lars@...afoo.de>, 
	Michael Hennerich <Michael.Hennerich@...log.com>, David Lechner <dlechner@...libre.com>, 
	linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Jonathan Cameron <Jonathan.Cameron@...wei.com>
Subject: Re: [PATCH v2 1/2] iio: dac: adi-axi-dac: fix bus read

On Sat, Apr 12, 2025 at 4:00 PM Jonathan Cameron <jic23@...nel.org> wrote:
> On Wed, 9 Apr 2025 19:49:25 +0300
> Andy Shevchenko <andy@...nel.org> wrote:
> > On Wed, Apr 09, 2025 at 11:16:54AM +0200, Angelo Dureghello wrote:

...

> > > +   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.

I still think it's slightly better for at least two reasons:
1) the named constant makes it easier to understand the units (esp.
for the unprepared reader who doesn't know well the kernel internal
APIs);
2) educational and similar cases when somebody will copy'n'paste
(okay, copy, paste, and modify) this with thinking that this is a good
practice, which may lead to bugs when it will be more zeroes that easy
to miscount.

I prefer the constant.

-- 
With Best Regards,
Andy Shevchenko