[RFC 4/4] m68k: coldfire: fix non-standard readX()/writeX() functions

[RFC 4/4] m68k: coldfire: fix non-standard readX()/writeX() functions

[Greg Ungerer]

From: Greg Ungerer <gerg@linux-m68k.org>

Remove the local ColdFire definitions of readb()/readw()/readl() and
writeb()/writew()/writel() and use the asm-generic versions of them.

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.

The change to use the asm-generic versions of readX()/writeX() itself is
quite strait forward, just remove the ColdFire local versions of them.  But
this of course has implications for any remaining drivers that use any of
these IO access functions. A number of drivers can be independently fixed,
before this final fix to readX()/writeX() for ColdFire. A small number of
drivers cannot easily be independently fixed and remain in a working
state. Those drivers are fixed here as well:

drivers/dma/mcf-edma-main.c
  Supports big-endian access by setting the big-endian flag of
  the drivers struct fsl_edma_engine. But locally should be using
  ioread32be() and iowrite32be() instead of ioread32() and iowrite32().

drivers/net/can/flexcan/flexcan-core.c
  Setting the driver quirks flag for big-endian access will force
  driver to use correct access functions.

drivers/spi/spi-fsl-dspi.c
  Setting the regmap format_endian flags to use native endian will
  force driver to use appropriate big or little endian access on
  whatever platform it is built for.

These drivers have only been compile tested.

Signed-off-by: Greg Ungerer <gerg@linux-m68k.org>
---
 arch/m68k/include/asm/io_no.h          | 68 +++-----------------------
 drivers/dma/mcf-edma-main.c            | 14 +++---
 drivers/net/can/flexcan/flexcan-core.c |  1 +
 drivers/spi/spi-fsl-dspi.c             |  2 +
 4 files changed, 16 insertions(+), 69 deletions(-)

diff --git a/arch/m68k/include/asm/io_no.h b/arch/m68k/include/asm/io_no.h
index 4f0f34b06e37..2f12f4ed0da5 100644
--- a/arch/m68k/include/asm/io_no.h
+++ b/arch/m68k/include/asm/io_no.h
@@ -3,15 +3,10 @@
 #define _M68KNOMMU_IO_H
 
 /*
- * Convert a physical memory address into a IO memory address.
- * For us this is trivially a type cast.
- */
-#define iomem(a)	((void __iomem *) (a))
-
-/*
- * The non-MMU m68k and ColdFire IO and memory mapped hardware access
- * functions have always worked in CPU native endian. We need to define
- * that behavior here first before we include asm-generic/io.h.
+ * Historically the raw native endian IO access macros for non-MMU m68k and
+ * ColdFire have accepted any integral value as the address argument.
+ * The asm-generic versions of these expect "void __iomem *" for the address,
+ * so define local permissive versions here.
  */
 #define __raw_readb(addr) \
     ({ u8 __v = (*(__force volatile u8 *) (addr)); __v; })
@@ -45,66 +40,15 @@
  * applies just the same of there is no MMU but something like a PCI bus
  * is present.
  */
-static int __cf_internalio(unsigned long addr)
+static inline int __cf_internalio(unsigned long addr)
 {
 	return (addr >= IOMEMBASE) && (addr <= IOMEMBASE + IOMEMSIZE - 1);
 }
 
-static int cf_internalio(const volatile void __iomem *addr)
+static inline int cf_internalio(const volatile void __iomem *addr)
 {
 	return __cf_internalio((unsigned long) addr);
 }
-
-/*
- * We need to treat built-in peripherals and bus based address ranges
- * differently. Local built-in peripherals (and the ColdFire SoC parts
- * have quite a lot of them) are always native endian - which is big
- * endian on m68k/ColdFire. Bus based address ranges, like the PCI bus,
- * are accessed little endian - so we need to byte swap those.
- */
-#define readw readw
-static inline u16 readw(const volatile void __iomem *addr)
-{
-	if (cf_internalio(addr))
-		return __raw_readw(addr);
-	return swab16(__raw_readw(addr));
-}
-
-#define readl readl
-static inline u32 readl(const volatile void __iomem *addr)
-{
-	if (cf_internalio(addr))
-		return __raw_readl(addr);
-	return swab32(__raw_readl(addr));
-}
-
-#define writew writew
-static inline void writew(u16 value, volatile void __iomem *addr)
-{
-	if (cf_internalio(addr))
-		__raw_writew(value, addr);
-	else
-		__raw_writew(swab16(value), addr);
-}
-
-#define writel writel
-static inline void writel(u32 value, volatile void __iomem *addr)
-{
-	if (cf_internalio(addr))
-		__raw_writel(value, addr);
-	else
-		__raw_writel(swab32(value), addr);
-}
-
-#else
-
-#define readb __raw_readb
-#define readw __raw_readw
-#define readl __raw_readl
-#define writeb __raw_writeb
-#define writew __raw_writew
-#define writel __raw_writel
-
 #endif /* IOMEMBASE */
 
 #if defined(CONFIG_COLDFIRE)
diff --git a/drivers/dma/mcf-edma-main.c b/drivers/dma/mcf-edma-main.c
index 9e1c6400c77b..4ed0ce644e37 100644
--- a/drivers/dma/mcf-edma-main.c
+++ b/drivers/dma/mcf-edma-main.c
@@ -21,9 +21,9 @@ static irqreturn_t mcf_edma_tx_handler(int irq, void *dev_id)
 	unsigned int ch;
 	u64 intmap;
 
-	intmap = ioread32(regs->inth);
+	intmap = ioread32be(regs->inth);
 	intmap <<= 32;
-	intmap |= ioread32(regs->intl);
+	intmap |= ioread32be(regs->intl);
 	if (!intmap)
 		return IRQ_NONE;
 
@@ -43,7 +43,7 @@ static irqreturn_t mcf_edma_err_handler(int irq, void *dev_id)
 	struct edma_regs *regs = &mcf_edma->regs;
 	unsigned int err, ch;
 
-	err = ioread32(regs->errl);
+	err = ioread32be(regs->errl);
 	if (!err)
 		return IRQ_NONE;
 
@@ -55,7 +55,7 @@ static irqreturn_t mcf_edma_err_handler(int irq, void *dev_id)
 		}
 	}
 
-	err = ioread32(regs->errh);
+	err = ioread32be(regs->errh);
 	if (!err)
 		return IRQ_NONE;
 
@@ -203,8 +203,8 @@ static int mcf_edma_probe(struct platform_device *pdev)
 		edma_write_tcdreg(mcf_chan, cpu_to_le32(0), csr);
 	}
 
-	iowrite32(~0, regs->inth);
-	iowrite32(~0, regs->intl);
+	iowrite32be(~0, regs->inth);
+	iowrite32be(~0, regs->intl);
 
 	ret = mcf_edma->drvdata->setup_irq(pdev, mcf_edma);
 	if (ret)
@@ -248,7 +248,7 @@ static int mcf_edma_probe(struct platform_device *pdev)
 	}
 
 	/* Enable round robin arbitration */
-	iowrite32(EDMA_CR_ERGA | EDMA_CR_ERCA, regs->cr);
+	iowrite32be(EDMA_CR_ERGA | EDMA_CR_ERCA, regs->cr);
 
 	return 0;
 }
diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
index f5d22c61503f..a682f02d2c43 100644
--- a/drivers/net/can/flexcan/flexcan-core.c
+++ b/drivers/net/can/flexcan/flexcan-core.c
@@ -296,6 +296,7 @@ static_assert(sizeof(struct flexcan_regs) ==  0x4 * 18 + 0xfb8);
 static const struct flexcan_devtype_data fsl_mcf5441x_devtype_data = {
 	.quirks = FLEXCAN_QUIRK_BROKEN_PERR_STATE |
 		FLEXCAN_QUIRK_NR_IRQ_3 | FLEXCAN_QUIRK_NR_MB_16 |
+		FLEXCAN_QUIRK_DEFAULT_BIG_ENDIAN |
 		FLEXCAN_QUIRK_SUPPORT_RX_MAILBOX |
 		FLEXCAN_QUIRK_SUPPORT_RX_FIFO,
 };
diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 9f2a7b8163b1..964ca6baf32f 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -203,6 +203,8 @@ static const struct regmap_config dspi_regmap_config[] = {
 		.volatile_table	= &dspi_volatile_table,
 		.rd_table	= &dspi_access_table,
 		.wr_table	= &dspi_access_table,
+		.reg_format_endian = REGMAP_ENDIAN_NATIVE,
+		.val_format_endian = REGMAP_ENDIAN_NATIVE,
 	},
 	[S32G_DSPI_REGMAP] = {
 		.reg_bits	= 32,
-- 
2.54.0

Re: [RFC 4/4] m68k: coldfire: fix non-standard readX()/writeX() functions

[Frank Li]

On Thu, May 07, 2026 at 12:26:48AM +1000, Greg Ungerer wrote:
> From: Greg Ungerer <gerg@linux-m68k.org>
>
> Remove the local ColdFire definitions of readb()/readw()/readl() and
> writeb()/writew()/writel() and use the asm-generic versions of them.
>
> 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.
>
> The change to use the asm-generic versions of readX()/writeX() itself is
> quite strait forward, just remove the ColdFire local versions of them.  But
> this of course has implications for any remaining drivers that use any of
> these IO access functions. A number of drivers can be independently fixed,
> before this final fix to readX()/writeX() for ColdFire. A small number of
> drivers cannot easily be independently fixed and remain in a working
> state. Those drivers are fixed here as well:
>
> drivers/dma/mcf-edma-main.c
>   Supports big-endian access by setting the big-endian flag of
>   the drivers struct fsl_edma_engine. But locally should be using
>   ioread32be() and iowrite32be() instead of ioread32() and iowrite32().
>
> drivers/net/can/flexcan/flexcan-core.c
>   Setting the driver quirks flag for big-endian access will force
>   driver to use correct access functions.
>
> drivers/spi/spi-fsl-dspi.c
>   Setting the regmap format_endian flags to use native endian will
>   force driver to use appropriate big or little endian access on
>   whatever platform it is built for.
>
> These drivers have only been compile tested.
>
> Signed-off-by: Greg Ungerer <gerg@linux-m68k.org>
> ---
>  arch/m68k/include/asm/io_no.h          | 68 +++-----------------------
>  drivers/dma/mcf-edma-main.c            | 14 +++---

Suppose it is correct, but I have not such platform to test it. it should
be correct if disassembly code is the same at access register.

Frank

>

Re: [RFC 4/4] m68k: coldfire: fix non-standard readX()/writeX() functions

[Arnd Bergmann]

On Wed, May 6, 2026, at 16:26, Greg Ungerer wrote:

> drivers/dma/mcf-edma-main.c
>   Supports big-endian access by setting the big-endian flag of
>   the drivers struct fsl_edma_engine. But locally should be using
>   ioread32be() and iowrite32be() instead of ioread32() and iowrite32().

I'm still a bit confused about how this works at the moment,
since the drivers/dma/fsl-edma-common.h file already contains
checks for the edma->big_endian flag, which is set in
mcf_edma_probe(). The version after your patch makes sense
to me, but it looks like the existing code cannot work.

> drivers/spi/spi-fsl-dspi.c
>   Setting the regmap format_endian flags to use native endian will
>   force driver to use appropriate big or little endian access on
>   whatever platform it is built for.
>
> These drivers have only been compile tested.

I would suggest marking these as explicit BIG_ENDIAN rather than
NATIVE_ENDIAN. The effect should be the same since coldfire CPUs
cannot run little-endian code, but the way that hardware usually
works is that the endianess is fixed at the bus level to one way
or the other. NATIVE_ENDIAN to me implies that the registers
have configurable endianess that is switched along with the CPU
mode.

       Arnd

Re: [RFC 4/4] m68k: coldfire: fix non-standard readX()/writeX() functions

[Greg Ungerer]

Hi Arnd,

On 7/5/26 05:12, Arnd Bergmann wrote:
> On Wed, May 6, 2026, at 16:26, Greg Ungerer wrote:
> 
>> drivers/dma/mcf-edma-main.c
>>    Supports big-endian access by setting the big-endian flag of
>>    the drivers struct fsl_edma_engine. But locally should be using
>>    ioread32be() and iowrite32be() instead of ioread32() and iowrite32().
> 
> I'm still a bit confused about how this works at the moment,
> since the drivers/dma/fsl-edma-common.h file already contains
> checks for the edma->big_endian flag, which is set in
> mcf_edma_probe(). The version after your patch makes sense
> to me, but it looks like the existing code cannot work.

Yes, it certainly doesn't look right to me either.

Angelo: you look to be the original author of this driver, can you shed any
light on its working status in mainline currently?


>> drivers/spi/spi-fsl-dspi.c
>>    Setting the regmap format_endian flags to use native endian will
>>    force driver to use appropriate big or little endian access on
>>    whatever platform it is built for.
>>
>> These drivers have only been compile tested.
> 
> I would suggest marking these as explicit BIG_ENDIAN rather than
> NATIVE_ENDIAN. The effect should be the same since coldfire CPUs
> cannot run little-endian code, but the way that hardware usually
> works is that the endianess is fixed at the bus level to one way
> or the other. NATIVE_ENDIAN to me implies that the registers
> have configurable endianess that is switched along with the CPU
> mode.

Ok, will change. I chose native endian in this case since the regmap config
entry used for the m5441x family is also used by the vf610 devce (which looks
to be an ARM imx SoC). So it will need a duplicate setup with those endian
flags set to BIG_ENDIAN. But that is no problem.

Thanks
Greg

Re: [RFC 4/4] m68k: coldfire: fix non-standard readX()/writeX() functions

[Arnd Bergmann]

On Thu, May 7, 2026, at 14:43, Greg Ungerer wrote:
> On 7/5/26 05:12, Arnd Bergmann wrote:
>> On Wed, May 6, 2026, at 16:26, Greg Ungerer wrote:
>> 
>> I would suggest marking these as explicit BIG_ENDIAN rather than
>> NATIVE_ENDIAN. The effect should be the same since coldfire CPUs
>> cannot run little-endian code, but the way that hardware usually
>> works is that the endianess is fixed at the bus level to one way
>> or the other. NATIVE_ENDIAN to me implies that the registers
>> have configurable endianess that is switched along with the CPU
>> mode.
>
> Ok, will change. I chose native endian in this case since the regmap config
> entry used for the m5441x family is also used by the vf610 devce (which looks
> to be an ARM imx SoC). So it will need a duplicate setup with those endian
> flags set to BIG_ENDIAN. But that is no problem.

Sounds good. In this case, splitting it up is technically even required,
because you can run a big-endian ARMv7 kernel on vf610, so the vf610
entry has to use little-endian registers rather than native.

I don't think anyone has run big-endian kernels on vf610, though I have
heard of users testing them successfully on i.MX6.

      Arnd

Re: [RFC 4/4] m68k: coldfire: fix non-standard readX()/writeX() functions

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1172 bytes --]

On 07.05.2026 00:26:48, Greg Ungerer wrote:
> diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
> index f5d22c61503f..a682f02d2c43 100644
> --- a/drivers/net/can/flexcan/flexcan-core.c
> +++ b/drivers/net/can/flexcan/flexcan-core.c
> @@ -296,6 +296,7 @@ static_assert(sizeof(struct flexcan_regs) ==  0x4 * 18 + 0xfb8);
>  static const struct flexcan_devtype_data fsl_mcf5441x_devtype_data = {
>  	.quirks = FLEXCAN_QUIRK_BROKEN_PERR_STATE |

Nitpick:
Can you move it here, so that the quirks stay sorted?

		FLEXCAN_QUIRK_DEFAULT_BIG_ENDIAN |

>  		FLEXCAN_QUIRK_NR_IRQ_3 | FLEXCAN_QUIRK_NR_MB_16 |
> +		FLEXCAN_QUIRK_DEFAULT_BIG_ENDIAN |
>  		FLEXCAN_QUIRK_SUPPORT_RX_MAILBOX |
>  		FLEXCAN_QUIRK_SUPPORT_RX_FIFO,
>  };

With that change:

Acked-by: Marc Kleine-Budde <mkl@pengutronix.de> # for drivers/net/can/flexcan

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [RFC 4/4] m68k: coldfire: fix non-standard readX()/writeX() functions

[Greg Ungerer]

On 7/5/26 23:30, Marc Kleine-Budde wrote:
> On 07.05.2026 00:26:48, Greg Ungerer wrote:
>> diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
>> index f5d22c61503f..a682f02d2c43 100644
>> --- a/drivers/net/can/flexcan/flexcan-core.c
>> +++ b/drivers/net/can/flexcan/flexcan-core.c
>> @@ -296,6 +296,7 @@ static_assert(sizeof(struct flexcan_regs) ==  0x4 * 18 + 0xfb8);
>>   static const struct flexcan_devtype_data fsl_mcf5441x_devtype_data = {
>>   	.quirks = FLEXCAN_QUIRK_BROKEN_PERR_STATE |
> 
> Nitpick:
> Can you move it here, so that the quirks stay sorted?

Yep, sure thing.


> 		FLEXCAN_QUIRK_DEFAULT_BIG_ENDIAN |
> 
>>   		FLEXCAN_QUIRK_NR_IRQ_3 | FLEXCAN_QUIRK_NR_MB_16 |
>> +		FLEXCAN_QUIRK_DEFAULT_BIG_ENDIAN |
>>   		FLEXCAN_QUIRK_SUPPORT_RX_MAILBOX |
>>   		FLEXCAN_QUIRK_SUPPORT_RX_FIFO,
>>   };
> 
> With that change:
> 
> Acked-by: Marc Kleine-Budde <mkl@pengutronix.de> # for drivers/net/can/flexcan

Thanks
Greg