On 24/03/20 10:54 am, Chris Packham wrote: > Hi Christophe, > > On Wed, 2020-02-05 at 12:03 +0000, Christophe Leroy wrote: >> [ 0.000000] ioremap() called early from >> find_legacy_serial_ports+0x3cc/0x474. Use early_ioremap() instead >> > I was just about to dig into this error message and found you patch. I > applied it to a v5.5 base. > >> find_legacy_serial_ports() is called early from setup_arch(), before >> paging_init(). vmalloc is not available yet, ioremap shouldn't be >> used that early. >> >> Use early_ioremap() and switch to a regular ioremap() later. >> >> Signed-off-by: Christophe Leroy <christophe.le...@c-s.fr> > On my system (Freescale T2080 SOC) this seems to cause a crash/hang in > early boot. Unfortunately because this is affecting the boot console I > don't get any earlyprintk output.
I've been doing a bit more digging into why Christophe's patch didn't work for me. I noticed the powerpc specific early_ioremap_range() returns addresses around ioremap_bot. Yet the generic early_ioremap() uses addresses around FIXADDR_TOP. If I try the following hack I can make Christophe's patch work diff --git a/arch/powerpc/include/asm/fixmap.h b/arch/powerpc/include/asm/fixmap.h index 2ef155a3c821..7bc2f3f73c8b 100644 --- a/arch/powerpc/include/asm/fixmap.h +++ b/arch/powerpc/include/asm/fixmap.h @@ -27,7 +27,7 @@ #include <asm/kasan.h> #define FIXADDR_TOP (KASAN_SHADOW_START - PAGE_SIZE) #else -#define FIXADDR_TOP ((unsigned long)(-PAGE_SIZE)) +#define FIXADDR_TOP (IOREMAP_END - PAGE_SIZE) #endif /* I'll admit to being out of my depth. It seems that the generic early_ioremap() is not quite correctly plumbed in for powerpc. >> --- >> arch/powerpc/kernel/legacy_serial.c | 33 +++++++++++++++++++++++++ >> ---- >> 1 file changed, 29 insertions(+), 4 deletions(-) >> >> diff --git a/arch/powerpc/kernel/legacy_serial.c >> b/arch/powerpc/kernel/legacy_serial.c >> index f061e06e9f51..8b2c1a8553a0 100644 >> --- a/arch/powerpc/kernel/legacy_serial.c >> +++ b/arch/powerpc/kernel/legacy_serial.c >> @@ -15,6 +15,7 @@ >> #include <asm/udbg.h> >> #include <asm/pci-bridge.h> >> #include <asm/ppc-pci.h> >> +#include <asm/early_ioremap.h> >> >> #undef DEBUG >> >> @@ -34,6 +35,7 @@ static struct legacy_serial_info { >> unsigned int clock; >> int irq_check_parent; >> phys_addr_t taddr; >> + void __iomem *early_addr; >> } legacy_serial_infos[MAX_LEGACY_SERIAL_PORTS]; >> >> static const struct of_device_id legacy_serial_parents[] __initconst >> = { >> @@ -325,17 +327,16 @@ static void __init >> setup_legacy_serial_console(int console) >> { >> struct legacy_serial_info *info = >> &legacy_serial_infos[console]; >> struct plat_serial8250_port *port = >> &legacy_serial_ports[console]; >> - void __iomem *addr; >> unsigned int stride; >> >> stride = 1 << port->regshift; >> >> /* Check if a translated MMIO address has been found */ >> if (info->taddr) { >> - addr = ioremap(info->taddr, 0x1000); >> - if (addr == NULL) >> + info->early_addr = early_ioremap(info->taddr, 0x1000); >> + if (info->early_addr == NULL) >> return; >> - udbg_uart_init_mmio(addr, stride); >> + udbg_uart_init_mmio(info->early_addr, stride); >> } else { >> /* Check if it's PIO and we support untranslated PIO */ >> if (port->iotype == UPIO_PORT && isa_io_special) >> @@ -353,6 +354,30 @@ static void __init >> setup_legacy_serial_console(int console) >> udbg_uart_setup(info->speed, info->clock); >> } >> >> +static int __init ioremap_legacy_serial_console(void) >> +{ >> + struct legacy_serial_info *info = >> &legacy_serial_infos[legacy_serial_console]; >> + struct plat_serial8250_port *port = >> &legacy_serial_ports[legacy_serial_console]; >> + void __iomem *vaddr; >> + >> + if (legacy_serial_console < 0) >> + return 0; >> + >> + if (!info->early_addr) >> + return 0; >> + >> + vaddr = ioremap(info->taddr, 0x1000); >> + if (WARN_ON(!vaddr)) >> + return -ENOMEM; >> + >> + udbg_uart_init_mmio(vaddr, 1 << port->regshift); >> + early_iounmap(info->early_addr, 0x1000); >> + info->early_addr = NULL; >> + >> + return 0; >> +} >> +early_initcall(ioremap_legacy_serial_console); >> + >> /* >> * This is called very early, as part of setup_system() or >> eventually >> * setup_arch(), basically before anything else in this file. This >> function