Re: [PATCH] iomap: Fix -Wmissing-prototypes on UM

[Arnd Bergmann]

On Mon, Feb 3, 2025, at 14:23, Andy Shevchenko wrote:
> On Mon, Feb 03, 2025 at 02:08:35PM +0100, Arnd Bergmann wrote:
>
>> > I like the lib/* and include/* cleanup but PTP probably should stay as is.
>> > OTOH WWAN case most likely had been tested on 64-bit platforms only and
>> > proves that readq()/writeq() works there, so it's okay to unify.
>> 
>> Ok, I'll try to split it up into sensible patches then. For ptp
>> (both ixp46x and pch), these are the options I see:

Update: While splitting out the wwan patch, I see that this would
revert 7d5a7dd5a358 ("net: wwan: t7xx: Split 64bit accesses to fix
alignment issues"), so that driver also needs to keep using the
split accessors, at least for some of the registers. From what
I can tell, the problem here is that REG_CLDMA_UL_START_ADDRL_0
is not a multiple of 8, and arm64 CPUs require MMIO registers
to be naturally aligned.

If that is the cause of the problem, this means that on x86-64
the unaligned readq() is still used but happens to work.
Once I change linux/io-64-nonatomic-lo-hi.h, it will split
the access on x86-64 and other using GENERIC_IOMAP as well.

The accesses to ATR_PCIE_WIN0_T0_TRSL_ADDR and
ATR_PCIE_WIN0_T0_ATR_PARAM_SRC_ADDR can probably use ioread64()
because they are on aligned addresses, but it should be safe
to just leave this driver alone and keep all the split
accesses.

>> - leave it unchanged since nobody cares any more
>> - add some comments about being racy and possibly broken on 64-bit
>
> Any combination of these two I would prefer.
>
>> - revert your pch patch d09adf61002f/8664d49a815e3 to make it have 32-
>>   bit accesses again and fix the theoretical 64-bit issue but not the
>>   race
>
> Definitely not this. I assume that _hi_lo and _lo_hi semantics of IO
> APIs implies non-atomicity access and hence always splits the IO in
> 64-bit case. These helpers make code much less verbose and actually
> (due to naming) clearer about the sequence of the reads or writes.
> I prefer to have them stay (in the drivers).

Right, after my change to the headers, it will at least behave
consistently according to the documentation, so there is no
additional problem if someone reuses that driver on (problem
nonexistent) 64-bit hardware.

       Arnd