RE: [PATCH 1/7] Revert "tty: serial: fsl_lpuart: drop EARLYCON_DECLARE"

[Peng Fan]

> Subject: Re: [PATCH 1/7] Revert "tty: serial: fsl_lpuart: drop
> EARLYCON_DECLARE"
> 
> Hi,
> 
> Am 2020-02-21 02:30, schrieb Peng Fan:
> >> Subject: [PATCH 1/7] Revert "tty: serial: fsl_lpuart: drop
> >> EARLYCON_DECLARE"
> >>
> >> This reverts commit a659652f6169240a5818cb244b280c5a362ef5a4.
> >>
> >> This broke the earlycon on LS1021A processors because the order of
> >> the
> >> earlycon_setup() functions were changed. Before the commit the normal
> >> lpuart32_early_console_setup() was called. After the commit the
> >> lpuart32_imx_early_console_setup() is called instead.
> >
> > How do you pass earlycon args to kernel?
> 
> earlycon=lpuart32,mmio32be,0x2950000,115200
> 
> please note that there are two possible declarations: (1) an OF/ACPI based
> earlycon, eg just "earlycon" on the bootargs and (2) an elaborate one where
> you can give the offset and access method yourself, eg. the one from above.
> 
> (1) will still work even with the EARLYCON_DECLARE() removed. But (2) will
> search through all possible
>    OF_DELARE_EARLYCON(lpuart32,..)
>    EARLYCON_DECLARE(lpuart32,..)
> 
> and doesn't take the compatible into account. So which setup function is
> actually called depends on (a) the order of the OF_DECLARE_EARLYCON() and
> EARLYCON_DECLARE() statements and (b) on the compiler (thats just a guess!).
> For me, the order in which it will actually end up in the __earlycon_table is
> reversed, eg. the last one is called. So now that you've removed the
> EARLYCON_DECLARE() the last one is the imx setup function which will add
> the reg offset and doesn't work on LS1021A.

You mean the OF_DECLARD_EARLYCON for i.MX7ULP will be put before LS1021A
in Image? I am not sure about this. If this is true, you could try below diff, to see
whether it works. i.MX always use little endian.

diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index 91e2805e6441..1b0aa3b836c5 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -2381,8 +2381,10 @@ static int __init lpuart32_imx_early_console_setup(struct earlycon_device *devic
        if (!device->port.membase)
                return -ENODEV;

-       device->port.iotype = UPIO_MEM32;
-       device->port.membase += IMX_REG_OFF;
+       if (device->port.iotype != UPIO_MEM32BE) {
+               device->port.iotype = UPIO_MEM32;
+               device->port.membase += IMX_REG_OFF;
+       }
        device->con->write = lpuart32_early_write;

        return 0;

Thanks,
Peng.

> 
> I've proposed a fix of the underlying problem [1]. But that fix also requires the
> EARLYCON_DECLARE() in this driver.
> 
> 
> -michael
> 
> [1]
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.ke
> rnel.org%2Flinux-serial%2F20200220174607.24285-1-michael%40walle.cc%
> 2F&data=02%7C01%7Cpeng.fan%40nxp.com%7Cc411bf1d5d45435be2a
> 308d7b6b14a2c%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63
> 7178744883261681&sdata=gjl8rie%2FXhUQ0pxcAbbDI4NGqGDYj1jpqfR
> mRi%2FAgFk%3D&reserved=0