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

[BALATON Zoltan]

On Sat, 23 Jun 2018, Guenter Roeck wrote:
> 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.

I think sm501 irq is not emulated currently so it's not expected to work 
anyway.

> Also:
>
> sm501-usb sm501-usb.80: cannot declare coherent memory

I think I've seen this in logs from real hardware too but not sure what it 
is but may not be emulation related.

> sm501-usb: probe of sm501-usb.80 failed with error -38

USB part is only emulated on the sysbus version used on SH, the PCI 
version does not create it because AFAIK it's unused on sam460ex so this 
is expected too. Sam460ex has other USB ports in SoC which is emulated.

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

I don't have real hardware either nor know about any other system that 
have SM501/SM502 besides sam460ex so I only hope this conversation in the 
list archives might help someone in the future who has a way to test it 
and submit a patch for Linux to fix it.

Regards,
BALATON Zoltan