mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Arnd Bergmann" <arnd@kernel.org>
To: "Greg Ungerer" <gerg@linux-m68k.org>, "Daniel Palmer" <daniel@0x0f.com>
Cc: "Geert Uytterhoeven" <geert@linux-m68k.org>,
	christoph.plattner@gmx.at, linux-m68k@lists.linux-m68k.org,
	linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH] m68k: nommu: Fix behaviour of io macros on non-CF
Date: Wed, 15 Apr 2026 16:54:49 +0200	[thread overview]
Message-ID: <4ec2b9f3-8420-4f77-9373-7ae4df83404d@app.fastmail.com> (raw)
In-Reply-To: <25448303-a816-429d-851b-d827fc8f26fb@linux-m68k.org>

On Wed, Apr 15, 2026, at 15:52, Greg Ungerer wrote:
> On 14/4/26 23:24, Arnd Bergmann wrote:
>> On Tue, Apr 14, 2026, at 14:55, Daniel Palmer wrote:
>>
>> There are other coldfire specific drivers (fec, spi, ...)
>> that would need a similar change.
>
> Right, and this is really the work that needs to be done first before
> we will be able to remove the io_no.h specific readl/write code.
> This is what I see:
>
> Drivers that are used on ColdFire targets that use some of
> readb/readw/readl and writeb/writew/writel:
>
>      drivers/net/ethernet/freescale/fec*
>      drivers/net/ethernet/smsc/smc9x.[hc]
>      drivers/net/ethernet/davicom/dm9000.c
>      drivers/spi/spi-coldfire-qspi.c

> Obviously spi-coldfire-qspi.c is ColdFire specific, so easy to change
> and not affect anything else.

> The FEC ethernet driver is widely used on
> ARM and ColdFire, though the change to a register reading helper looks
> pretty strait forward but will be a largish patch.

Agreed.

> Changing the smc91x driver looks easy enough, the ColdFire
> multi-byte access macros in the header are specific to ColdFire.

smc91x is a bit weird because this uses a readw() loop
for byte streams but ioread16be() for register accesses.
I think what is going on here is that this is in fact
a little-endian device, and ioread16be() ends up working
because this does a byteswap on coldfire while readl()
does not. With the normal io.h semantics, these should be
readsw() or ioread16_rep() for the bytestream and readw()
or ioread16() for the LE register instead, same as the
#else path in the header.

> Need to give some thought to changes
> to the dm9000 driver.

The dm9000.c driver should not need any changes, as it only
deals with 8-bit registers and iowrite32_rep()/ioread32_rep()
byte stream transfers, but no 16-bit or 32-bit register
accesses, so no byteswap is needed here.

> Drivers that are used on ColdFire that use only 8-bit readb and writeb:
>
>      drivers/tty/serial/mcf.c
>      drivers/i2c/busses/i2c-imx.c
>
> We could ignore these for now I guess, since readb and writeb will always
> work the same. Though it might be nice to clean them up with proper helpers.
> mcf.c is only used on ColdFire so won't affect anything else.

Right

> Drivers that are used on ColdFire that use iowrite32/iowrite32be:
>
>      drivers/net/can/flexcan/flexcan-core.c
>
> Although this driver has support for differentiating big and little endian
> access I think for ColdFire it is relying on the kludgy overrides in io_no.h
> to actually work right. So will need a fix too.

Makes sense. So the FLEXCAN_QUIRK_DEFAULT_BIG_ENDIAN flag
should be flipped at the same time as flipping the endianess
of iowrite32be()/iowrite32().

> Did I miss any?

I test-built all the defconfigs, with readX/writeX/ioreadXX/iowriteXX
commented out and found three more for mcf5441x:

drivers/mmc/host/sdhci-esdhc-mcf.c

 -> needs to be flipped to explicit big-endian accessors

drivers/dma/mcf-edma-main.c

 -> same, but confusingly this also has edma_readl()/edma_writel()
    helpers that try to handle this. As far as I can tell, this
    is currently broken because coldfire sets edma->big_endian=true
    but then uses ioread32be() that appears to flip the BE
    register contents to LE. I'm either missing something here,
    or the upstream version never worked.

drivers/spi/spi-fsl-dspi.c

  -> this one uses regmap_mmio without specifying an endianess,
     so this should get the default LE helpers everywhere, which
     would be wrong on coldfire after the change. I think the
     correct solution here is to set explict endianess in the
     regmap setup for coldfire.

       Arnd

  reply	other threads:[~2026-04-15 14:55 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-04  2:46 Daniel Palmer
2026-04-13 16:52 ` Arnd Bergmann
2026-04-14  7:14   ` Geert Uytterhoeven
2026-04-14 12:18 ` Greg Ungerer
2026-04-14 12:55   ` Daniel Palmer
2026-04-14 13:24     ` Arnd Bergmann
2026-04-15 13:52       ` Greg Ungerer
2026-04-15 14:54         ` Arnd Bergmann [this message]
2026-04-17 14:11           ` Greg Ungerer
2026-04-17 16:36             ` Arnd Bergmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4ec2b9f3-8420-4f77-9373-7ae4df83404d@app.fastmail.com \
    --to=arnd@kernel.org \
    --cc=christoph.plattner@gmx.at \
    --cc=daniel@0x0f.com \
    --cc=geert@linux-m68k.org \
    --cc=gerg@linux-m68k.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-m68k@lists.linux-m68k.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®