Re: [Qemu-ppc] [PATCH] ppc: Fix sam460ex devicetree when booting the Linux kernel

[Guenter Roeck]

On 06/23/2018 01:55 PM, BALATON Zoltan wrote:
> On Fri, 22 Jun 2018, Guenter Roeck wrote:
> > The problem is this (from the kernel diffs provided by aCube):
> >
> > #if defined(CONFIG_PPC32)
> > -#define smc501_readl(addr)             ioread32be((addr))
> > -#define smc501_writel(val, addr)       iowrite32be((val), (addr))
> > +#define smc501_readl(addr)             ioread32((addr))
> > +#define smc501_writel(val, addr)       iowrite32((val), (addr))
> > #else
> > #define smc501_readl(addr)             readl(addr)
> > #define smc501_writel(val, addr)       writel(val, addr)
> >
> > This is a bit fishy since the cpu is big endian and iowrite32be()
> > should be identical to iowrite32(), but apparently that is not the
> > case here. I don't think I'll have time to track this down, though.
>
> Thanks for finding this, it helps even if you don't have time to track it down 
> completely. Could this be related to using little endian PCI device on a big 
> endian CPU? Not sure which implementation will this use but in 
> arch/powerpc/kernel/iomap.c:
>
> unsigned int ioread32(void __iomem *addr)
> {
>          return readl(addr);
> }
> unsigned int ioread32be(void __iomem *addr)
> {
>          return readl_be(addr);
> }
>
> so they don't look identical.

Sure, but normally I would assume that readl_be() matches readl() on a big 
endian system.
This is not the case here - it specifically maps to an assembler instruction 
which
afaics always swaps the byte order. Other architectures typically have 
something like

#define ioread32be(p) be32_to_cpu(ioread32(p))

or similar. FWIW, using ioread32() instead of ioread32be() does improve the 
situation:

sm501 0002:00:06.0: enabling device (0000 -> 0002)
sm501 0002:00:06.0: SM501 At (ptrval): Version 050100a0, 64 Mb, IRQ 0
sm501 0002:00:06.0: setting M1XCLK to 144000000
sm501 0002:00:06.0: setting MCLK to 72000000
sm501-usb[0] [mem 0xd84040000-0xd8405ffff]
sm501-usb[1] [mem 0xd83fc0000-0xd83ffffff]
sm501-usb[2] [irq 0]
sm501-fb[0] [mem 0xd84080000-0xd8408ffff]
sm501-fb[1] [mem 0xd84100000-0xd8414ffff]
sm501-fb[2] [mem 0xd80000000-0xd83fbffff]
sm501-fb[3] [irq 0]

even though irq 0 looks fishy. This may be related to:

qemu-system-ppc: ppc440_pcix_set_irq: PCI irq -1

seen when starting qemu.

Also:

sm501-usb sm501-usb.80: cannot declare coherent memory
sm501-usb: probe of sm501-usb.80 failed with error -38

This is as far as I got. Fixing this may require a new configuration option -
something like CONFIG_MFD_SM501_NATIVE_ENDIAN which would be set to false
by default. I think this should be tested on real HW, though, not only on
sam460ex but also on some other PPC32 system with SM501. Unfortunately
I have neither the hardware nor the time to do all the testing.

Guenter