Re: [RESEND RFC PATCH 10/12] serial/sh-sci: Fix earlyprintk / earlycon.

Hi Sato-san,

On Thu, Aug 31, 2023 at 11:23 AM Yoshinori Sato
<[email protected]> wrote:
> - earlyprintk having fault.
> - fix shortname conflict.
> - fix SCI regshift in SH4.
>
> Signed-off-by: Yoshinori Sato <[email protected]>

Thanks for your patch!

> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -2721,7 +2721,7 @@ static int sci_remap_port(struct uart_port *port)
>         if (port->membase)
>                 return 0;
>
> -       if (port->dev->of_node || (port->flags & UPF_IOREMAP)) {
> +       if ((port->dev && port->dev->of_node) || (port->flags & UPF_IOREMAP)) {

Why is this needed? It works fine on arm32, arm64, and riscv.

>                 port->membase = ioremap(port->mapbase, sport->reg_size);
>                 if (unlikely(!port->membase)) {
>                         dev_err(port->dev, "can't remap port#%d\n", port->line);
> @@ -3507,6 +3507,10 @@ static int __init early_console_setup(struct earlycon_device *device,
>         if (!device->port.membase)
>                 return -ENODEV;
>
> +       if (type == PORT_SCI &&
> +           (IS_ENABLED(CONFIG_CPU_SH3) || IS_ENABLED(CONFIG_CPU_SH4))) {
> +               device->port.regshift = 2;
> +       }

How do you run into this, given you don't have any SCI ports enabled
in your DTS, only SCIF?

Anyway, I think this should be fixed in sci_init_single(), by extending
the driver to support the more-or-less standard "reg-shift" DT property
(as I said before in
https://lore.kernel.org/all/CAMuHMdW2gxDzYbP_0Z90o8mHdUm_BV6e+gMHpELJx_g=ezAbdw@mail.gmail.com).

>         device->port.serial_in = sci_serial_in;
>         device->port.serial_out = sci_serial_out;
>         device->port.type = type;
> @@ -3556,8 +3560,8 @@ static int __init hscif_early_console_setup(struct earlycon_device *device,
>
>  OF_EARLYCON_DECLARE(sci, "renesas,sci", sci_early_console_setup);
>  OF_EARLYCON_DECLARE(scif, "renesas,scif", scif_early_console_setup);
> -OF_EARLYCON_DECLARE(scif, "renesas,scif-r7s9210", rzscifa_early_console_setup);
> -OF_EARLYCON_DECLARE(scif, "renesas,scif-r9a07g044", rzscifa_early_console_setup);
> +OF_EARLYCON_DECLARE(rzscif, "renesas,scif-r7s9210", rzscifa_early_console_setup);
> +OF_EARLYCON_DECLARE(rzscif, "renesas,scif-r9a07g044", rzscifa_early_console_setup);

Why?

>  OF_EARLYCON_DECLARE(scifa, "renesas,scifa", scifa_early_console_setup);
>  OF_EARLYCON_DECLARE(scifb, "renesas,scifb", scifb_early_console_setup);
>  OF_EARLYCON_DECLARE(hscif, "renesas,hscif", hscif_early_console_setup);

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds