Re: [PATCHv3 1/3] net: fec: do not use readl()/writel() for ColdFire

From: Jakub Kicinski

Date: Fri Sep 11 2026 - 19:05:51 EST


On Fri, 11 Sep 2026 23:03:56 +1000 Greg Ungerer wrote:
> On 11/9/26 10:42, Jakub Kicinski wrote:
> > On Mon, 7 Sep 2026 23:37:09 +1000 Greg Ungerer wrote:
> >> The FEC driver works today because the m68k architecture io.h has a
> >> kludge in the definitions of the readl() and writel() functions for
> >> ColdFire that allow big-endian access if the address of the register to
> >> access is within the SoC's internal peripheral registers. This is being
> >> fixed in the near future to define readl() and writel() correctly - with
> >> no byte swapping. Thus the motivation for this fix here.
> >
> > What is the motivation for this cleanup?
>
> Sorry, maybe you were not in the CC list of the cover email. From that:
>
> This collection of patches is aimed at fixing the non-standard ColdFire
> set of readX()/writeX() IO access functions. Instead switching to using the
> asm-generic definitions in include/asm-generic/io.h. The difficulty comes
> in trying not to break any drivers with this change.
>
> The implementation of the readX()/writeX() family of IO access functions
> is non-standard on ColdFire platforms. They either return big-endian (that
> is native endian) data, or on platforms with PCI bus support check the
> supplied address and return either big or little endian data based on that
> check. This is non-standard, they are expected to always return
> little-endian byte ordered data. Unfortunately this behavior also means
> that ioreadX()/iowroteX() and their big-endian counter parts
> ioreadXbe()/iowriteXbe() are currently broken because they are implemented
> using the readX()/writeX() functions.
>
> Patches 1 and 2 in this series are specific net driver changes that can be
> applied independently of the final ColdFire readX()/writeX() change.

I read it, it doesn't really answer the question of priority.

> > Is someone still making
> > new SKUs of Coldfire boards? Or (and please don't take this the wrong
> > way) it was a long standing TODO that was tempting to feed to an LLM?
>
> It is a long standing todo/fix, but there is nothing LLM anywhere here.
> This was all manually coded and tested. This patch is 95% textual
> substitution, so it wasn't exactly very difficult.
>
> > IMO keeping the hacks in m68k is a better choice. FEC was used on more
> > modern SoCs, definitely on PPC ones. So we'll be able to get rid of
> > m68k before we can get rid of FEC. Sprinkling m68k workarounds in
> > the FEC driver is backwards.
>
> These changes are not work arounds, they are made to make the FEC driver
> explicitly aware of endianess of the underlying processor and FEC hardware
> block. The code as it is now, that is carrying hacks inside readX and writeX
> for m68k, is trying to hide that from the driver.

Whatever, let me not waste any more time on this.
If Arnd thinks its worth it:

Acked-by: Jakub Kicinski <kuba@xxxxxxxxxx>