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