-----Original Message-----
From: Linus Torvalds <torvalds@linux-foundation.org>
Sent: Saturday, August 29, 2020 4:20 PM
To: Guenter Roeck <linux@roeck-us.net>
Cc: Luc Van Oostenryck <redacted>; Herbert Xu
[off-list ref]; Andrew Morton <akpm@linux-
foundation.org>; Joerg Roedel [off-list ref]; Leo Li
[off-list ref]; Zhang Wei [off-list ref]; Dan Williams
[off-list ref]; Vinod Koul [off-list ref]; linuxppc-dev
[off-list ref]; dma [off-list ref]; Linux
Kernel Mailing List [off-list ref]
Subject: Re: [PATCH] fsldma: fsl_ioread64*() do not need lower_32_bits()

On Sat, Aug 29, 2020 at 1:40 PM Guenter Roeck [off-list ref] wrote:

quoted

Except for

CHECK: spaces preferred around that '+' (ctx:VxV)
#29: FILE: drivers/dma/fsldma.h:223:
+       u32 val_lo = in_be32((u32 __iomem *)addr+1);

Added spaces.

quoted

I don't see anything wrong with it either, so

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Since I didn't see the real problem with the original code, I'd take
that with a grain of salt, though.

Well, honestly, the old code was so confused that just making it build is
clearly already an improvement even if everything else were to be wrong.

So I committed my "fix". If it turns out there's more wrong in there and
somebody tests it, we can fix it again. But now it hopefully compiles, at least.

My bet is that if that driver ever worked on ppc32, it will continue to work
whatever we do to that function.

I _think_ the old code happened to - completely by mistake - get the value
right for the case of "little endian access, with dma_addr_t being 32-bit".
Because then it would still read the upper bits wrong, but the cast to
dma_addr_t would then throw those bits away. And the lower bits would be
right.

But for big-endian accesses or for ARCH_DMA_ADDR_T_64BIT it really looks
like it always returned a completely incorrect value.

And again - the driver may have worked even with that completely incorrect
value, since the use of it seems to be very incidental.

In either case ("it didn't work before" or "it worked because the value
doesn't really matter"), I don't think I could possibly have made things worse.

Famous last words.

Thanks for the patch.  

Acked-by: Li Yang <redacted>

We are having periodical auto regression tests covering ppc32 platforms.  But looks like it missed this issue.  I will ask the test team to investigate on why the test cases are not sufficient.

Regards,
Leo