Re: [PATCH] fsldma: fsl_ioread64*() do not need lower_32_bits()

From: Luc Van Oostenryck
Date: Sun Aug 30 2020 - 08:12:31 EST


On Sat, Aug 29, 2020 at 10:29:55AM -0700, Linus Torvalds wrote:
> On Sat, Aug 29, 2020 at 5:46 AM Luc Van Oostenryck
> <luc.vanoostenryck@xxxxxxxxx> wrote:
> >
> > But the pointer is already 32-bit, so simply cast the pointer to u32.
>
> Yeah, that code was completely pointless. If the pointer had actually
> been 64-bit, the old code would have warned too.
>
> The odd thing is that the fsl_iowrite64() functions make sense. It's
> only the fsl_ioread64() functions that seem to be written by somebody
> who is really confused.
>
> That said, this patch only humors the confusion. The cast to 'u32' is
> completely pointless. In fact, it seems to be actively wrong, because
> it means that the later "fsl_addr + 1" is done entirely incorrectly -
> it now literally adds "1" to an integer value, while the iowrite()
> functions will add one to a "u32 __iomem *" pointer (so will do
> pointer arithmetic, and add 4).
>
My bad. I had noticed the '+ 1' and so automatically assumed
'OK, pointer arithmetic now' without noticing that the cast was
done only after the addition. Grrr.

FWIW, the version you committed looks much better to me.

-- Luc