Re: [PATCH RFT 0/6] serial: sh-sci: Fixes for earlycon and keep_bootcon

[Claudiu Beznea <claudiu.beznea@tuxon.dev>]

Hi, Geert,

On 19.12.2024 17:11, Geert Uytterhoeven wrote:
> Hi Claudiu,
> 
> On Thu, Dec 5, 2024 at 9:39 AM Claudiu Beznea <claudiu.beznea@tuxon.dev> wrote:
>> On 04.12.2024 23:38, Wolfram Sang wrote:
>>>> in the following scenarios:
>>>>
>>>> 1/ "earlycon keep_bootcon" were present in bootargs
>>>> 2/ only "earlycon" was present in bootargs
>>>> 3/ none of the "earlycon" or "earlycon keep_bootcon" were present in
>>>>    bootargs
>>> ...
>>>> Please give it a try on your devices as well.
>>>
>>> Will happily do so. Is there something to look for? Except for "it
>>> works"?
>>
>> As this code touches the earlycon functionality, of interest are the 3
>> cases highlighted above:
>>
>> 1/ "earlycon keep_bootcon" are both present in bootargs
>> 2/ only "earlycon" is present in bootargs
>> 3/ none of the "earlycon" or "earlycon keep_bootcon" are present in
>>    bootargs
>>
>> One other thing, that I was currently able to test only on RZ/G3S, is to
>> see how it behaves when the debug serial is described in DT with an alias
>> other than zero. E.g., on [1] the debug serial alias on RZ/G3S was changed
>> from 0 to 3. With the new alias (3) there were issues that I've tried to
>> fix with this series.
> 
> I gave this a try on Koelsch, which has two easily-accessible usb-serial
> ports, for all three cases above.  Originally, I had CONFIG_VT_CONSOLE=y
> (tty0 takes over from earlycon rather early), but I had to disable
> that to exercise all code paths (ttySC0 takes over much later).
> 
>   A. CONFIG_VT_CONSOLE=y: OK
>   B. CONFIG_VT_CONSOLE=y earlycon: OK
>        early_console_setup: mapbase 0x00000000e6e60000
>        earlycon: scif0 at MMIO 0x00000000e6e60000 (options '115200n8')
>        printk: legacy bootconsole [scif0] enabled
>        printk: legacy console [tty0] enabled
>        printk: legacy bootconsole [scif0] disabled
>        early_console_exit: Clearing sci_ports[0]
>   C. CONFIG_VT_CONSOLE=n earlycon: OK
>        early_console_setup: mapbase 0x00000000e6e60000
>        earlycon: scif0 at MMIO 0x00000000e6e60000 (options '115200n8')
>        printk: legacy bootconsole [scif0] enabled
>        printk: legacy console [ttySC0] enabled
>        printk: legacy bootconsole [scif0] disabled
>        early_console_exit: Not clearing sci_ports[0]
>   D. CONFIG_VT_CONSOLE=y earlycon keep_bootcon: OK
>        early_console_setup: mapbase 0x00000000e6e60000
>        earlycon: scif0 at MMIO 0x00000000e6e60000 (options '115200n8')
>        printk: legacy bootconsole [scif0] enabled
>        printk: legacy console [tty0] enabled
> 
> So all good, but note that these cases worked fine without your
> series, too.
> 
> The real troublesome cases involve using earlycon on a different
> serial port than serial0.  As I don't have any Renesas boards where
> chosen/stdout-path does not use serial0, I tried exchanging the serial0
> and serial1 DT aliases, and updating chosen/stdout-path accordingly.
> 
>   E. CONFIG_VT_CONSOLE=y: OK
>   F. CONFIG_VT_CONSOLE=y earlycon: OK
>        early_console_setup: mapbase 0x00000000e6e60000
>        earlycon: scif0 at MMIO 0x00000000e6e60000 (options '115200n8')
>        printk: legacy bootconsole [scif0] enabled
>        printk: legacy console [tty0] enabled
>        printk: legacy bootconsole [scif0] disabled
>        early_console_exit: Clearing sci_ports[0]
>   G. CONFIG_VT_CONSOLE=y earlycon keep_bootcon: SCIF1 missing
>        early_console_setup: mapbase 0x00000000e6e60000
>        earlycon: scif0 at MMIO 0x00000000e6e60000 (options '115200n8')
>        printk: legacy bootconsole [scif0] enabled
>        printk: legacy console [tty0] enabled
>        sh-sci e6e68000.serial: error -EBUSY: sci_port[0] is used by earlycon!
>   H. CONFIG_VT_CONSOLE=n earlycon: SCIF1 missing
>        early_console_setup: mapbase 0x00000000e6e60000
>        earlycon: scif0 at MMIO 0x00000000e6e60000 (options '115200n8')
>        printk: legacy bootconsole [scif0] enabled
>        printk: legacy console [ttySC1] enabled
>        printk: legacy bootconsole [scif0] disabled
>        early_console_exit: Not clearing sci_ports[0]
>        sh-sci e6e68000.serial: error -EBUSY: sci_port[0] is used by earlycon!
> 
> Case G gives a missing SCIF1, because sci_port[0] is still
> used for earlycon, as expected.
> Case H also gives a missing SCIF1, but should succeed IMHO, as earlycon
> is no longer active.  I think early_console_exit() should clear the
> earlycon flag regardless.

I'll double check it.

> 
> Note that before your series, cases E-F worked too, but cases G-H gave
> an initialized but broken SCIF1 instead.
> 
> Now, can we improve?
>   - Can we use a proper id instead of zero for earlycon, e.g.
>     sci_probe_earlyprintk() does fill in early_serial_console.index?

I looked into that but, as of my investigation, index zero is the one used
in the earlyprintk initialization process. sci_probe_earlyprintk() is
called from sci_probe(). I'll double checked it though, anyway.


>   - Alternatively, can we use a separate sci_port structure instead
>     of abusing sci_ports[0]?

I explored this too, but didn't manage to make it work.

Thank you for running all these tests,
Claudiu

> 
> Thanks!
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> 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