[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

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

[Angelo Dureghello]

Hi all,

sorry that i could check this now only, hope is not too late.

On Thu, May 07, 2026 at 10:43:01PM +1000, Greg Ungerer wrote:
> 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?
>

I was some years far from this driver (and the kernel itself), but i have
seen there are several changes in the edma common part from that time.
I remember i reviewed/tested some of them, some other unfortunately not.

I realized last month, that i was back working on this board (stmark2)
that somehting related to edma or dspi looks broken in mainline now:

[    2.270000] fsl-dspi fsl-dspi.0: Not able to get desc for DMA xfer
[    2.280000] fsl-dspi fsl-dspi.0: DMA transfer failed
[    2.280000] spi_master spi0: failed to transfer one message from queue
[    2.290000] spi_master spi0: noqueue transfer failed
[    2.290000] spi-nor spi0.1: probe with driver spi-nor failed with error -5

DSPI is using edma, i will try to understand where the issue is asap.

About how it works:
- for accesses to edma module (IP) mmio registers, must be native
big_endian, so using the "be" suffix in "mcf"_edma looks ok for me.
- for accessing the "tcd" memory structure, that must be, from what i
remember, anyway in little endian, independently from the cpu core
endiannes, this is the reason that big_endian flag is needed, it is
used for tcd area accesses, so the IP module was built.
The tcd area may be similar to pci accesses (see mcf54415 RM 19.4.16).

Anyway, tested the patch, nothing looks changed nor in better or worst.
Will look into this in the next days.

>
> > > 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
>

Regards,
angelo

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

[Arnd Bergmann]

On Sun, May 17, 2026, at 21:43, Angelo Dureghello wrote:
> On Thu, May 07, 2026 at 10:43:01PM +1000, Greg Ungerer wrote:
>> On 7/5/26 05:12, Arnd Bergmann wrote:
>> > On Wed, May 6, 2026, at 16:26, Greg Ungerer wrote:
>
> [    2.270000] fsl-dspi fsl-dspi.0: Not able to get desc for DMA xfer
> [    2.280000] fsl-dspi fsl-dspi.0: DMA transfer failed
> [    2.280000] spi_master spi0: failed to transfer one message from queue
> [    2.290000] spi_master spi0: noqueue transfer failed
> [    2.290000] spi-nor spi0.1: probe with driver spi-nor failed with error -5
>
> DSPI is using edma, i will try to understand where the issue is asap.
>
> About how it works:
> - for accesses to edma module (IP) mmio registers, must be native
> big_endian, so using the "be" suffix in "mcf"_edma looks ok for me.

The twist here is that with the way that readl() is defined on
coldfire as a non-swapping operation, and the generic
definition assuming the opposite in

static inline u32 ioread32be(const void __iomem *addr)
{
        return swab32(readl(addr));
}

the function called ioread32be() actually tries to access
the registers as little-endian. I can see two possible ways
we got here, but don't know which one is currect:

a) the device actually has little-endian registers (like it
   does on i.MX, but unlike all other coldfire devices), and
   you just never noticed because using ioread32be() worked
   as you expected.

b) you tested the driver using an ioread32be() definition that
   did not have a byteswap and it correctly accessed big-endian
   registers at the time, but the version in mainline today does
   not.

> - for accessing the "tcd" memory structure, that must be, from what i
> remember, anyway in little endian, independently from the cpu core
> endiannes, this is the reason that big_endian flag is needed, it is
> used for tcd area accesses, so the IP module was built.
> The tcd area may be similar to pci accesses (see mcf54415 RM 19.4.16).

edma_read_tcdreg() calls into edma_readl(), which is the same function
that is used for normal register access, so from what I can tell,
they always use the same endianess here.

      Arnd

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

[Angelo Dureghello]

Hi Arnd,

On Sun, May 17, 2026 at 10:08:22PM +0200, Arnd Bergmann wrote:
> On Sun, May 17, 2026, at 21:43, Angelo Dureghello wrote:
> > On Thu, May 07, 2026 at 10:43:01PM +1000, Greg Ungerer wrote:
> >> On 7/5/26 05:12, Arnd Bergmann wrote:
> >> > On Wed, May 6, 2026, at 16:26, Greg Ungerer wrote:
> >
> > [    2.270000] fsl-dspi fsl-dspi.0: Not able to get desc for DMA xfer
> > [    2.280000] fsl-dspi fsl-dspi.0: DMA transfer failed
> > [    2.280000] spi_master spi0: failed to transfer one message from queue
> > [    2.290000] spi_master spi0: noqueue transfer failed
> > [    2.290000] spi-nor spi0.1: probe with driver spi-nor failed with error -5
> >
> > DSPI is using edma, i will try to understand where the issue is asap.
> >
> > About how it works:
> > - for accesses to edma module (IP) mmio registers, must be native
> > big_endian, so using the "be" suffix in "mcf"_edma looks ok for me.
>
> The twist here is that with the way that readl() is defined on
> coldfire as a non-swapping operation, and the generic
> definition assuming the opposite in
>
> static inline u32 ioread32be(const void __iomem *addr)
> {
>         return swab32(readl(addr));
> }
>
> the function called ioread32be() actually tries to access
> the registers as little-endian. I can see two possible ways
> we got here, but don't know which one is currect:
>
> a) the device actually has little-endian registers (like it
>    does on i.MX, but unlike all other coldfire devices), and
>    you just never noticed because using ioread32be() worked
>    as you expected.
>
> b) you tested the driver using an ioread32be() definition that
>    did not have a byteswap and it correctly accessed big-endian
>    registers at the time, but the version in mainline today does
>    not.

Ok. The ioread32be now works properly since i had applied Greg patches.
I generated an error in _probe on edma channel 2, reading status reg.
looks consistent:

	iowrite16(2121, regs->erqh);
	iowrite8(0x77, regs->serq);
	iowrite8(0x12, regs->ssrt);
	
	u32 status = ioread32be(regs->es);
	printk("%s() status: %04x\n", __func__, status);

[    0.140000] mcf_edma_probe() entering
[    0.140000] mcf_edma_probe(): allocating data
[    0.140000] mcf_edma_probe() status: 800012f8

If i am not loosing myself in this r/w labyrinth, the path should be:

1) Greg removed coldfire readl/writel, leaving now the standard LE r/w,
2) So the ioread32be swaps the standard LE read giving BE.

Am i correct ?


>
> > - for accessing the "tcd" memory structure, that must be, from what i
> > remember, anyway in little endian, independently from the cpu core
> > endiannes, this is the reason that big_endian flag is needed, it is
> > used for tcd area accesses, so the IP module was built.
> > The tcd area may be similar to pci accesses (see mcf54415 RM 19.4.16).
>
> edma_read_tcdreg() calls into edma_readl(), which is the same function
> that is used for normal register access, so from what I can tell,
> they always use the same endianess here.
>

If edma_readl was using

        if (edma->big_endian)
                val = ioread32be(addr);

and never changed, without Greg patch, it was likely returning little
endian for coldfire and correct LE for other arch ? :)

I remember something about tcd area was coded LE, but will investigate
better, now i am over midnight.

Regards,
angelo

>       Arnd

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

[Angelo Dureghello]

Hi,

On Sun, May 17, 2026 at 03:04:23PM -0700, Angelo Dureghello wrote:
> Hi Arnd,
>
> On Sun, May 17, 2026 at 10:08:22PM +0200, Arnd Bergmann wrote:
> > On Sun, May 17, 2026, at 21:43, Angelo Dureghello wrote:
> > > On Thu, May 07, 2026 at 10:43:01PM +1000, Greg Ungerer wrote:
> > >> On 7/5/26 05:12, Arnd Bergmann wrote:
> > >> > On Wed, May 6, 2026, at 16:26, Greg Ungerer wrote:
> > >
> > > [    2.270000] fsl-dspi fsl-dspi.0: Not able to get desc for DMA xfer
> > > [    2.280000] fsl-dspi fsl-dspi.0: DMA transfer failed
> > > [    2.280000] spi_master spi0: failed to transfer one message from queue
> > > [    2.290000] spi_master spi0: noqueue transfer failed
> > > [    2.290000] spi-nor spi0.1: probe with driver spi-nor failed with error -5
> > >

About this issue, it fails on dma_pool_alloc(), so tomorrow will check,
i probably lost some dma config option.

> > > DSPI is using edma, i will try to understand where the issue is asap.
> > >
> > > About how it works:
> > > - for accesses to edma module (IP) mmio registers, must be native
> > > big_endian, so using the "be" suffix in "mcf"_edma looks ok for me.
> >
> > The twist here is that with the way that readl() is defined on
> > coldfire as a non-swapping operation, and the generic
> > definition assuming the opposite in
> >
> > static inline u32 ioread32be(const void __iomem *addr)
> > {
> >         return swab32(readl(addr));
> > }
> >
> > the function called ioread32be() actually tries to access
> > the registers as little-endian. I can see two possible ways
> > we got here, but don't know which one is currect:
> >
> > a) the device actually has little-endian registers (like it
> >    does on i.MX, but unlike all other coldfire devices), and
> >    you just never noticed because using ioread32be() worked
> >    as you expected.
> >
> > b) you tested the driver using an ioread32be() definition that
> >    did not have a byteswap and it correctly accessed big-endian
> >    registers at the time, but the version in mainline today does
> >    not.
>
> Ok. The ioread32be now works properly since i had applied Greg patches.
> I generated an error in _probe on edma channel 2, reading status reg.
> looks consistent:
>
> 	iowrite16(2121, regs->erqh);
> 	iowrite8(0x77, regs->serq);
> 	iowrite8(0x12, regs->ssrt);
> 	
> 	u32 status = ioread32be(regs->es);
> 	printk("%s() status: %04x\n", __func__, status);
>
> [    0.140000] mcf_edma_probe() entering
> [    0.140000] mcf_edma_probe(): allocating data
> [    0.140000] mcf_edma_probe() status: 800012f8
>
> If i am not loosing myself in this r/w labyrinth, the path should be:
>
> 1) Greg removed coldfire readl/writel, leaving now the standard LE r/w,
> 2) So the ioread32be swaps the standard LE read giving BE.
>
> Am i correct ?
>
>
> >
> > > - for accessing the "tcd" memory structure, that must be, from what i
> > > remember, anyway in little endian, independently from the cpu core
> > > endiannes, this is the reason that big_endian flag is needed, it is
> > > used for tcd area accesses, so the IP module was built.
> > > The tcd area may be similar to pci accesses (see mcf54415 RM 19.4.16).
> >
> > edma_read_tcdreg() calls into edma_readl(), which is the same function
> > that is used for normal register access, so from what I can tell,
> > they always use the same endianess here.
> >
>
> If edma_readl was using
>
>         if (edma->big_endian)
>                 val = ioread32be(addr);
>
> and never changed, without Greg patch, it was likely returning little
> endian for coldfire and correct LE for other arch ? :)
>
> I remember something about tcd area was coded LE, but will investigate
> better, now i am over midnight.
>
> Regards,
> angelo
>
> >       Arnd

Regards,
angelo

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

[Angelo Dureghello]

Hi All,

On Sun, May 17, 2026 at 03:41:31PM -0700, Angelo Dureghello wrote:
> Hi,
>
> On Sun, May 17, 2026 at 03:04:23PM -0700, Angelo Dureghello wrote:
> > Hi Arnd,
> >
> > On Sun, May 17, 2026 at 10:08:22PM +0200, Arnd Bergmann wrote:
> > > On Sun, May 17, 2026, at 21:43, Angelo Dureghello wrote:
> > > > On Thu, May 07, 2026 at 10:43:01PM +1000, Greg Ungerer wrote:
> > > >> On 7/5/26 05:12, Arnd Bergmann wrote:
> > > >> > On Wed, May 6, 2026, at 16:26, Greg Ungerer wrote:
> > > >
> > > > [    2.270000] fsl-dspi fsl-dspi.0: Not able to get desc for DMA xfer
> > > > [    2.280000] fsl-dspi fsl-dspi.0: DMA transfer failed
> > > > [    2.280000] spi_master spi0: failed to transfer one message from queue
> > > > [    2.290000] spi_master spi0: noqueue transfer failed
> > > > [    2.290000] spi-nor spi0.1: probe with driver spi-nor failed with error -5
> > > >
>
> About this issue, it fails on dma_pool_alloc(), so tomorrow will check,
> i probably lost some dma config option.
>

so i worked on this open issue above:

- moved to master and rebased,
- crated a wip/edma branch,
- bisected and found the offending commit, before this, mcf-edma driver
  and connected spi-fsl-dspi (using edma) was both working correctly.

7a360df941a4bd60847208de59f1ac8b166265a2 is the first bad commit
commit 7a360df941a4bd60847208de59f1ac8b166265a2 (HEAD)
Author: Christoph Hellwig <hch@lst.de>
Date:   Thu Oct 12 09:52:27 2023 +0200

    m68k: don't provide arch_dma_alloc for nommu/coldfire

    Coldfire cores configured with a data cache can't provide coherent
    DMA allocations at all.

    Instead of returning non-coherent kernel memory in this case,
    return NULL and fail the allocation.

    The only driver that used to rely on the previous behavior (fec) has
    been switched to use non-coherent allocations for this case recently.

    Signed-off-by: Christoph Hellwig <hch@lst.de>
    Reviewed-by: Greg Ungerer <gerg@linux-m68k.org>
    Tested-by: Greg Ungerer <gerg@linux-m68k.org>

 arch/m68k/Kconfig      |  1 -
 arch/m68k/kernel/dma.c | 23 -----------------------
 2 files changed, 24 deletions(-)

So i can try next week a patch for edma looking what has been done
in fec, and since i am probably the only with mcf54415, will test it
here.

> > > > DSPI is using edma, i will try to understand where the issue is asap.
> > > >
> > > > About how it works:
> > > > - for accesses to edma module (IP) mmio registers, must be native
> > > > big_endian, so using the "be" suffix in "mcf"_edma looks ok for me.
> > >
> > > The twist here is that with the way that readl() is defined on
> > > coldfire as a non-swapping operation, and the generic
> > > definition assuming the opposite in
> > >
> > > static inline u32 ioread32be(const void __iomem *addr)
> > > {
> > >         return swab32(readl(addr));
> > > }
> > >
> > > the function called ioread32be() actually tries to access
> > > the registers as little-endian. I can see two possible ways
> > > we got here, but don't know which one is currect:
> > >
> > > a) the device actually has little-endian registers (like it
> > >    does on i.MX, but unlike all other coldfire devices), and
> > >    you just never noticed because using ioread32be() worked
> > >    as you expected.
> > >
> > > b) you tested the driver using an ioread32be() definition that
> > >    did not have a byteswap and it correctly accessed big-endian
> > >    registers at the time, but the version in mainline today does
> > >    not.
> >
> > Ok. The ioread32be now works properly since i had applied Greg patches.
> > I generated an error in _probe on edma channel 2, reading status reg.
> > looks consistent:
> >
> > 	iowrite16(2121, regs->erqh);
> > 	iowrite8(0x77, regs->serq);
> > 	iowrite8(0x12, regs->ssrt);
> > 	
> > 	u32 status = ioread32be(regs->es);
> > 	printk("%s() status: %04x\n", __func__, status);
> >
> > [    0.140000] mcf_edma_probe() entering
> > [    0.140000] mcf_edma_probe(): allocating data
> > [    0.140000] mcf_edma_probe() status: 800012f8
> >
> > If i am not loosing myself in this r/w labyrinth, the path should be:
> >
> > 1) Greg removed coldfire readl/writel, leaving now the standard LE r/w,
> > 2) So the ioread32be swaps the standard LE read giving BE.
> >
> > Am i correct ?
> >
> >
> > >
> > > > - for accessing the "tcd" memory structure, that must be, from what i
> > > > remember, anyway in little endian, independently from the cpu core
> > > > endiannes, this is the reason that big_endian flag is needed, it is
> > > > used for tcd area accesses, so the IP module was built.
> > > > The tcd area may be similar to pci accesses (see mcf54415 RM 19.4.16).
> > >
> > > edma_read_tcdreg() calls into edma_readl(), which is the same function
> > > that is used for normal register access, so from what I can tell,
> > > they always use the same endianess here.
> > >
> >
> > If edma_readl was using
> >
> >         if (edma->big_endian)
> >                 val = ioread32be(addr);
> >
> > and never changed, without Greg patch, it was likely returning little
> > endian for coldfire and correct LE for other arch ? :)
> >
> > I remember something about tcd area was coded LE, but will investigate
> > better, now i am over midnight.
> >
> > Regards,
> > angelo
> >
> > >       Arnd
>
> Regards,
> angelo

Regards,
angelo

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

[Angelo Dureghello]

On Sun, May 24, 2026 at 02:17:07PM -0700, Angelo Dureghello wrote:
> Hi All,
>
> On Sun, May 17, 2026 at 03:41:31PM -0700, Angelo Dureghello wrote:
> > Hi,
> >
> > On Sun, May 17, 2026 at 03:04:23PM -0700, Angelo Dureghello wrote:
> > > Hi Arnd,
> > >
> > > On Sun, May 17, 2026 at 10:08:22PM +0200, Arnd Bergmann wrote:
> > > > On Sun, May 17, 2026, at 21:43, Angelo Dureghello wrote:
> > > > > On Thu, May 07, 2026 at 10:43:01PM +1000, Greg Ungerer wrote:
> > > > >> On 7/5/26 05:12, Arnd Bergmann wrote:
> > > > >> > On Wed, May 6, 2026, at 16:26, Greg Ungerer wrote:
> > > > >
> > > > > [    2.270000] fsl-dspi fsl-dspi.0: Not able to get desc for DMA xfer
> > > > > [    2.280000] fsl-dspi fsl-dspi.0: DMA transfer failed
> > > > > [    2.280000] spi_master spi0: failed to transfer one message from queue
> > > > > [    2.290000] spi_master spi0: noqueue transfer failed
> > > > > [    2.290000] spi-nor spi0.1: probe with driver spi-nor failed with error -5
> > > > >
> >
> > About this issue, it fails on dma_pool_alloc(), so tomorrow will check,
> > i probably lost some dma config option.
> >
>
> so i worked on this open issue above:
>
> - moved to master and rebased,
> - crated a wip/edma branch,
> - bisected and found the offending commit, before this, mcf-edma driver
>   and connected spi-fsl-dspi (using edma) was both working correctly.
>
> 7a360df941a4bd60847208de59f1ac8b166265a2 is the first bad commit
> commit 7a360df941a4bd60847208de59f1ac8b166265a2 (HEAD)
> Author: Christoph Hellwig <hch@lst.de>
> Date:   Thu Oct 12 09:52:27 2023 +0200
>
>     m68k: don't provide arch_dma_alloc for nommu/coldfire
>
>     Coldfire cores configured with a data cache can't provide coherent
>     DMA allocations at all.
>
>     Instead of returning non-coherent kernel memory in this case,
>     return NULL and fail the allocation.
>
>     The only driver that used to rely on the previous behavior (fec) has
>     been switched to use non-coherent allocations for this case recently.
>
>     Signed-off-by: Christoph Hellwig <hch@lst.de>
>     Reviewed-by: Greg Ungerer <gerg@linux-m68k.org>
>     Tested-by: Greg Ungerer <gerg@linux-m68k.org>
>
>  arch/m68k/Kconfig      |  1 -
>  arch/m68k/kernel/dma.c | 23 -----------------------
>  2 files changed, 24 deletions(-)
>
> So i can try next week a patch for edma looking what has been done
> in fec, and since i am probably the only with mcf54415, will test it
> here.
>

Looking into this better, looks like the above commit was meant for the
majority on non-mmu ColdFire. I think mcf5441x and some other with mmu
enabled can flag pages as "page cache disabled".

So i would re-enabled that code only for such mmu families.

Please let me know if i am correct.
Thanks.

> > > > > DSPI is using edma, i will try to understand where the issue is asap.
> > > > >
> > > > > About how it works:
> > > > > - for accesses to edma module (IP) mmio registers, must be native
> > > > > big_endian, so using the "be" suffix in "mcf"_edma looks ok for me.
> > > >
> > > > The twist here is that with the way that readl() is defined on
> > > > coldfire as a non-swapping operation, and the generic
> > > > definition assuming the opposite in
> > > >
> > > > static inline u32 ioread32be(const void __iomem *addr)
> > > > {
> > > >         return swab32(readl(addr));
> > > > }
> > > >
> > > > the function called ioread32be() actually tries to access
> > > > the registers as little-endian. I can see two possible ways
> > > > we got here, but don't know which one is currect:
> > > >
> > > > a) the device actually has little-endian registers (like it
> > > >    does on i.MX, but unlike all other coldfire devices), and
> > > >    you just never noticed because using ioread32be() worked
> > > >    as you expected.
> > > >
> > > > b) you tested the driver using an ioread32be() definition that
> > > >    did not have a byteswap and it correctly accessed big-endian
> > > >    registers at the time, but the version in mainline today does
> > > >    not.
> > >
> > > Ok. The ioread32be now works properly since i had applied Greg patches.
> > > I generated an error in _probe on edma channel 2, reading status reg.
> > > looks consistent:
> > >
> > > 	iowrite16(2121, regs->erqh);
> > > 	iowrite8(0x77, regs->serq);
> > > 	iowrite8(0x12, regs->ssrt);
> > > 	
> > > 	u32 status = ioread32be(regs->es);
> > > 	printk("%s() status: %04x\n", __func__, status);
> > >
> > > [    0.140000] mcf_edma_probe() entering
> > > [    0.140000] mcf_edma_probe(): allocating data
> > > [    0.140000] mcf_edma_probe() status: 800012f8
> > >
> > > If i am not loosing myself in this r/w labyrinth, the path should be:
> > >
> > > 1) Greg removed coldfire readl/writel, leaving now the standard LE r/w,
> > > 2) So the ioread32be swaps the standard LE read giving BE.
> > >
> > > Am i correct ?
> > >
> > >
> > > >
> > > > > - for accessing the "tcd" memory structure, that must be, from what i
> > > > > remember, anyway in little endian, independently from the cpu core
> > > > > endiannes, this is the reason that big_endian flag is needed, it is
> > > > > used for tcd area accesses, so the IP module was built.
> > > > > The tcd area may be similar to pci accesses (see mcf54415 RM 19.4.16).
> > > >
> > > > edma_read_tcdreg() calls into edma_readl(), which is the same function
> > > > that is used for normal register access, so from what I can tell,
> > > > they always use the same endianess here.
> > > >
> > >
> > > If edma_readl was using
> > >
> > >         if (edma->big_endian)
> > >                 val = ioread32be(addr);
> > >
> > > and never changed, without Greg patch, it was likely returning little
> > > endian for coldfire and correct LE for other arch ? :)
> > >
> > > I remember something about tcd area was coded LE, but will investigate
> > > better, now i am over midnight.
> > >
> > > Regards,
> > > angelo
> > >
> > > >       Arnd
> >
> > Regards,
> > angelo
>
> Regards,
> angelo

Regards,
angelo

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

[Angelo Dureghello]

Hi Greg and all,

so i posted a patch this morning to have edma driver back working.
If approved, i can proceed testing this RFC (adjusting it as needed)
with edma and dspi driver.

Regards,
angelo

On Sun, May 24, 2026 at 04:34:03PM -0500, Angelo Dureghello wrote:
> On Sun, May 24, 2026 at 02:17:07PM -0700, Angelo Dureghello wrote:
> > Hi All,
> >
> > On Sun, May 17, 2026 at 03:41:31PM -0700, Angelo Dureghello wrote:
> > > Hi,
> > >
> > > On Sun, May 17, 2026 at 03:04:23PM -0700, Angelo Dureghello wrote:
> > > > Hi Arnd,
> > > >
> > > > On Sun, May 17, 2026 at 10:08:22PM +0200, Arnd Bergmann wrote:
> > > > > On Sun, May 17, 2026, at 21:43, Angelo Dureghello wrote:
> > > > > > On Thu, May 07, 2026 at 10:43:01PM +1000, Greg Ungerer wrote:
> > > > > >> On 7/5/26 05:12, Arnd Bergmann wrote:
> > > > > >> > On Wed, May 6, 2026, at 16:26, Greg Ungerer wrote:
> > > > > >
> > > > > > [    2.270000] fsl-dspi fsl-dspi.0: Not able to get desc for DMA xfer
> > > > > > [    2.280000] fsl-dspi fsl-dspi.0: DMA transfer failed
> > > > > > [    2.280000] spi_master spi0: failed to transfer one message from queue
> > > > > > [    2.290000] spi_master spi0: noqueue transfer failed
> > > > > > [    2.290000] spi-nor spi0.1: probe with driver spi-nor failed with error -5
> > > > > >
> > >
> > > About this issue, it fails on dma_pool_alloc(), so tomorrow will check,
> > > i probably lost some dma config option.
> > >
> >
> > so i worked on this open issue above:
> >
> > - moved to master and rebased,
> > - crated a wip/edma branch,
> > - bisected and found the offending commit, before this, mcf-edma driver
> >   and connected spi-fsl-dspi (using edma) was both working correctly.
> >
> > 7a360df941a4bd60847208de59f1ac8b166265a2 is the first bad commit
> > commit 7a360df941a4bd60847208de59f1ac8b166265a2 (HEAD)
> > Author: Christoph Hellwig <hch@lst.de>
> > Date:   Thu Oct 12 09:52:27 2023 +0200
> >
> >     m68k: don't provide arch_dma_alloc for nommu/coldfire
> >
> >     Coldfire cores configured with a data cache can't provide coherent
> >     DMA allocations at all.
> >
> >     Instead of returning non-coherent kernel memory in this case,
> >     return NULL and fail the allocation.
> >
> >     The only driver that used to rely on the previous behavior (fec) has
> >     been switched to use non-coherent allocations for this case recently.
> >
> >     Signed-off-by: Christoph Hellwig <hch@lst.de>
> >     Reviewed-by: Greg Ungerer <gerg@linux-m68k.org>
> >     Tested-by: Greg Ungerer <gerg@linux-m68k.org>
> >
> >  arch/m68k/Kconfig      |  1 -
> >  arch/m68k/kernel/dma.c | 23 -----------------------
> >  2 files changed, 24 deletions(-)
> >
> > So i can try next week a patch for edma looking what has been done
> > in fec, and since i am probably the only with mcf54415, will test it
> > here.
> >
>
> Looking into this better, looks like the above commit was meant for the
> majority on non-mmu ColdFire. I think mcf5441x and some other with mmu
> enabled can flag pages as "page cache disabled".
>
> So i would re-enabled that code only for such mmu families.
>
> Please let me know if i am correct.
> Thanks.
>
> > > > > > DSPI is using edma, i will try to understand where the issue is asap.
> > > > > >
> > > > > > About how it works:
> > > > > > - for accesses to edma module (IP) mmio registers, must be native
> > > > > > big_endian, so using the "be" suffix in "mcf"_edma looks ok for me.
> > > > >
> > > > > The twist here is that with the way that readl() is defined on
> > > > > coldfire as a non-swapping operation, and the generic
> > > > > definition assuming the opposite in
> > > > >
> > > > > static inline u32 ioread32be(const void __iomem *addr)
> > > > > {
> > > > >         return swab32(readl(addr));
> > > > > }
> > > > >
> > > > > the function called ioread32be() actually tries to access
> > > > > the registers as little-endian. I can see two possible ways
> > > > > we got here, but don't know which one is currect:
> > > > >
> > > > > a) the device actually has little-endian registers (like it
> > > > >    does on i.MX, but unlike all other coldfire devices), and
> > > > >    you just never noticed because using ioread32be() worked
> > > > >    as you expected.
> > > > >
> > > > > b) you tested the driver using an ioread32be() definition that
> > > > >    did not have a byteswap and it correctly accessed big-endian
> > > > >    registers at the time, but the version in mainline today does
> > > > >    not.
> > > >
> > > > Ok. The ioread32be now works properly since i had applied Greg patches.
> > > > I generated an error in _probe on edma channel 2, reading status reg.
> > > > looks consistent:
> > > >
> > > > 	iowrite16(2121, regs->erqh);
> > > > 	iowrite8(0x77, regs->serq);
> > > > 	iowrite8(0x12, regs->ssrt);
> > > > 	
> > > > 	u32 status = ioread32be(regs->es);
> > > > 	printk("%s() status: %04x\n", __func__, status);
> > > >
> > > > [    0.140000] mcf_edma_probe() entering
> > > > [    0.140000] mcf_edma_probe(): allocating data
> > > > [    0.140000] mcf_edma_probe() status: 800012f8
> > > >
> > > > If i am not loosing myself in this r/w labyrinth, the path should be:
> > > >
> > > > 1) Greg removed coldfire readl/writel, leaving now the standard LE r/w,
> > > > 2) So the ioread32be swaps the standard LE read giving BE.
> > > >
> > > > Am i correct ?
> > > >
> > > >
> > > > >
> > > > > > - for accessing the "tcd" memory structure, that must be, from what i
> > > > > > remember, anyway in little endian, independently from the cpu core
> > > > > > endiannes, this is the reason that big_endian flag is needed, it is
> > > > > > used for tcd area accesses, so the IP module was built.
> > > > > > The tcd area may be similar to pci accesses (see mcf54415 RM 19.4.16).
> > > > >
> > > > > edma_read_tcdreg() calls into edma_readl(), which is the same function
> > > > > that is used for normal register access, so from what I can tell,
> > > > > they always use the same endianess here.
> > > > >
> > > >
> > > > If edma_readl was using
> > > >
> > > >         if (edma->big_endian)
> > > >                 val = ioread32be(addr);
> > > >
> > > > and never changed, without Greg patch, it was likely returning little
> > > > endian for coldfire and correct LE for other arch ? :)
> > > >
> > > > I remember something about tcd area was coded LE, but will investigate
> > > > better, now i am over midnight.
> > > >
> > > > Regards,
> > > > angelo
> > > >
> > > > >       Arnd
> > >
> > > Regards,
> > > angelo
> >
> > Regards,
> > angelo
>
> Regards,
> angelo

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

[Greg Ungerer]

Hi Angelo,

(Adding Christoph to CC list)

On 25/5/26 07:34, Angelo Dureghello wrote:
> On Sun, May 24, 2026 at 02:17:07PM -0700, Angelo Dureghello wrote:
>> Hi All,
>>
>> On Sun, May 17, 2026 at 03:41:31PM -0700, Angelo Dureghello wrote:
>>> Hi,
>>>
>>> On Sun, May 17, 2026 at 03:04:23PM -0700, Angelo Dureghello wrote:
>>>> Hi Arnd,
>>>>
>>>> On Sun, May 17, 2026 at 10:08:22PM +0200, Arnd Bergmann wrote:
>>>>> On Sun, May 17, 2026, at 21:43, Angelo Dureghello wrote:
>>>>>> On Thu, May 07, 2026 at 10:43:01PM +1000, Greg Ungerer wrote:
>>>>>>> On 7/5/26 05:12, Arnd Bergmann wrote:
>>>>>>>> On Wed, May 6, 2026, at 16:26, Greg Ungerer wrote:
>>>>>>
>>>>>> [    2.270000] fsl-dspi fsl-dspi.0: Not able to get desc for DMA xfer
>>>>>> [    2.280000] fsl-dspi fsl-dspi.0: DMA transfer failed
>>>>>> [    2.280000] spi_master spi0: failed to transfer one message from queue
>>>>>> [    2.290000] spi_master spi0: noqueue transfer failed
>>>>>> [    2.290000] spi-nor spi0.1: probe with driver spi-nor failed with error -5
>>>>>>
>>>
>>> About this issue, it fails on dma_pool_alloc(), so tomorrow will check,
>>> i probably lost some dma config option.
>>>
>>
>> so i worked on this open issue above:
>>
>> - moved to master and rebased,
>> - crated a wip/edma branch,
>> - bisected and found the offending commit, before this, mcf-edma driver
>>    and connected spi-fsl-dspi (using edma) was both working correctly.
>>
>> 7a360df941a4bd60847208de59f1ac8b166265a2 is the first bad commit
>> commit 7a360df941a4bd60847208de59f1ac8b166265a2 (HEAD)
>> Author: Christoph Hellwig <hch@lst.de>
>> Date:   Thu Oct 12 09:52:27 2023 +0200
>>
>>      m68k: don't provide arch_dma_alloc for nommu/coldfire
>>
>>      Coldfire cores configured with a data cache can't provide coherent
>>      DMA allocations at all.
>>
>>      Instead of returning non-coherent kernel memory in this case,
>>      return NULL and fail the allocation.
>>
>>      The only driver that used to rely on the previous behavior (fec) has
>>      been switched to use non-coherent allocations for this case recently.
>>
>>      Signed-off-by: Christoph Hellwig <hch@lst.de>
>>      Reviewed-by: Greg Ungerer <gerg@linux-m68k.org>
>>      Tested-by: Greg Ungerer <gerg@linux-m68k.org>
>>
>>   arch/m68k/Kconfig      |  1 -
>>   arch/m68k/kernel/dma.c | 23 -----------------------
>>   2 files changed, 24 deletions(-)
>>
>> So i can try next week a patch for edma looking what has been done
>> in fec, and since i am probably the only with mcf54415, will test it
>> here.
>>
> 
> Looking into this better, looks like the above commit was meant for the
> majority on non-mmu ColdFire. I think mcf5441x and some other with mmu
> enabled can flag pages as "page cache disabled".

I don't think that is right. The way the underlying data cache is setup for
MMU ColdFire (via the ACR/CACR registers) means that individual pages cannot
be marked as non-cached. So coherent memory allocations are not possible -
at least the way things are today.

It would be possible to set aside a chunk of RAM at kernel startup time
to use as a pool for coherent allocations (since it could be marked as
non-cached via the ACR/CACR registers), but there is no code to support doing
that today.

Regards
Greg


> So i would re-enabled that code only for such mmu families.
> 
> Please let me know if i am correct.
> Thanks.
> 
>>>>>> DSPI is using edma, i will try to understand where the issue is asap.
>>>>>>
>>>>>> About how it works:
>>>>>> - for accesses to edma module (IP) mmio registers, must be native
>>>>>> big_endian, so using the "be" suffix in "mcf"_edma looks ok for me.
>>>>>
>>>>> The twist here is that with the way that readl() is defined on
>>>>> coldfire as a non-swapping operation, and the generic
>>>>> definition assuming the opposite in
>>>>>
>>>>> static inline u32 ioread32be(const void __iomem *addr)
>>>>> {
>>>>>          return swab32(readl(addr));
>>>>> }
>>>>>
>>>>> the function called ioread32be() actually tries to access
>>>>> the registers as little-endian. I can see two possible ways
>>>>> we got here, but don't know which one is currect:
>>>>>
>>>>> a) the device actually has little-endian registers (like it
>>>>>     does on i.MX, but unlike all other coldfire devices), and
>>>>>     you just never noticed because using ioread32be() worked
>>>>>     as you expected.
>>>>>
>>>>> b) you tested the driver using an ioread32be() definition that
>>>>>     did not have a byteswap and it correctly accessed big-endian
>>>>>     registers at the time, but the version in mainline today does
>>>>>     not.
>>>>
>>>> Ok. The ioread32be now works properly since i had applied Greg patches.
>>>> I generated an error in _probe on edma channel 2, reading status reg.
>>>> looks consistent:
>>>>
>>>> 	iowrite16(2121, regs->erqh);
>>>> 	iowrite8(0x77, regs->serq);
>>>> 	iowrite8(0x12, regs->ssrt);
>>>> 	
>>>> 	u32 status = ioread32be(regs->es);
>>>> 	printk("%s() status: %04x\n", __func__, status);
>>>>
>>>> [    0.140000] mcf_edma_probe() entering
>>>> [    0.140000] mcf_edma_probe(): allocating data
>>>> [    0.140000] mcf_edma_probe() status: 800012f8
>>>>
>>>> If i am not loosing myself in this r/w labyrinth, the path should be:
>>>>
>>>> 1) Greg removed coldfire readl/writel, leaving now the standard LE r/w,
>>>> 2) So the ioread32be swaps the standard LE read giving BE.
>>>>
>>>> Am i correct ?
>>>>
>>>>
>>>>>
>>>>>> - for accessing the "tcd" memory structure, that must be, from what i
>>>>>> remember, anyway in little endian, independently from the cpu core
>>>>>> endiannes, this is the reason that big_endian flag is needed, it is
>>>>>> used for tcd area accesses, so the IP module was built.
>>>>>> The tcd area may be similar to pci accesses (see mcf54415 RM 19.4.16).
>>>>>
>>>>> edma_read_tcdreg() calls into edma_readl(), which is the same function
>>>>> that is used for normal register access, so from what I can tell,
>>>>> they always use the same endianess here.
>>>>>
>>>>
>>>> If edma_readl was using
>>>>
>>>>          if (edma->big_endian)
>>>>                  val = ioread32be(addr);
>>>>
>>>> and never changed, without Greg patch, it was likely returning little
>>>> endian for coldfire and correct LE for other arch ? :)
>>>>
>>>> I remember something about tcd area was coded LE, but will investigate
>>>> better, now i am over midnight.
>>>>
>>>> Regards,
>>>> angelo
>>>>
>>>>>        Arnd
>>>
>>> Regards,
>>> angelo
>>
>> Regards,
>> angelo
> 
> Regards,
> angelo

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

[Christoph Hellwig]

On Sun, May 31, 2026 at 11:42:26PM +1000, Greg Ungerer wrote:
> I don't think that is right. The way the underlying data cache is setup for
> MMU ColdFire (via the ACR/CACR registers) means that individual pages cannot
> be marked as non-cached. So coherent memory allocations are not possible -
> at least the way things are today.
>
> It would be possible to set aside a chunk of RAM at kernel startup time
> to use as a pool for coherent allocations (since it could be marked as
> non-cached via the ACR/CACR registers), but there is no code to support doing
> that today.

With CONFIG_DMA_GLOBAL_POOL there is some generic code dealing with
most of this.  But if this driver worked on coldfire in the past,
it must have been fine with non-coherent memory and could use the
non-coherent allocator.