On Thu, Feb 27, 2025 at 05:23:48PM +0200, Andy Shevchenko wrote:

On Thu, Feb 27, 2025 at 03:17:27PM +0100, Arnd Bergmann wrote:

quoted

From: Arnd Bergmann <arnd@arndb.de>

While reviewing a patch to the ioread64_hi_lo() helpers, I noticed
that there are several PTP drivers that use multiple register reads
to access a 64-bit hardware register in a racy way.

There are usually safe ways of doing this, but at least these four
drivers do that.  A third register read obviously makes the hardware
access 50% slower. If the low word counds nanoseconds and a single
register read takes on the order of 1µs, the resulting value is
wrong in one of 4 million cases, which is pretty rare but common
enough that it would be observed in practice.

If the hardware does NOT latch the registers together, then the driver must do:

  1. hi1 = read hi
  2. low = read lo
  3. hi2 = read h1
  4. if (hi2 == hi1 return (hi1 << 32) | low;
  5. goto step 1.

This for correctness, and correctness > performance.

quoted

Sorry I hadn't sent this out as a proper patch so far. Any ideas
what we should do here?

Need to have driver authors check the data sheet because ...

Actually this reminds me one of the discussion where it was some interesting
HW design that latches the value on the first read of _low_ part (IIRC), but
I might be mistaken with the details.

That said, it's from HW to HW, it might be race-less in some cases.

... of this.

Thanks,
Richard