| Message ID | 20241204155806.3781200-1-claudiu.beznea.uj@bp.renesas.com |
|---|---|
| Headers | show |
| Series | serial: sh-sci: Fixes for earlycon and keep_bootcon | expand |
Hi, Wolfram, On 04.12.2024 23:38, Wolfram Sang wrote: > Hi Claudiu, > >> 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. Thank you for checking it, Claudiu [1] https://lore.kernel.org/all/20241115134401.3893008-6-claudiu.beznea.uj@bp.renesas.com/ > > Happy hacking, > > Wolfram >
Hi, Wolfram, On 05.12.2024 10:39, Claudiu Beznea wrote: > Hi, Wolfram, > > On 04.12.2024 23:38, Wolfram Sang wrote: >> Hi Claudiu, >> >>> 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"? Sorry, I noticed I missed to provide a clear answer your question: if boot works for this scenarios we should be OK. > > 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. If you can also check: - it boots in this case and - the serial device with alias zero and the debug serial are both working (tx, rx are working) after boot then we can declare it OK as well. Thank you, Claudiu > > Thank you for checking it, > Claudiu > > [1] > https://lore.kernel.org/all/20241115134401.3893008-6-claudiu.beznea.uj@bp.renesas.com/ > >> >> Happy hacking, >> >> Wolfram >>
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. 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? - Alternatively, can we use a separate sci_port structure instead of abusing sci_ports[0]? 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
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
From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Hi, This series adds fixes for earlycon and keep_bootcon on sh-sci driver. Patch 1/6 was initially part of [1], then posted as standalone fix at [2]. Patch 5/6 was integrated but then reverted as issues were identified after that with it as standalone patch. I added it in this series to keep all the sh-sci fixes in the same place. All these fixes are prerequisites for the Renesas RZ/G3S SCI support. Series was tested on the boards with the following device trees binaries: - r8a7742-iwg21d-q7.dtb - r8a7743-iwg20d-q7.dtb - r8a7745-iwg22d-sodimm.dtb - r8a77470-iwg23s-sbc.dtb - r8a774a1-hihope-rzg2m-ex.dtb - r8a774b1-hihope-rzg2n-ex.dtb - r8a774e1-hihope-rzg2h-ex.dtb - r9a07g043u11-smarc.dtb - r9a07g044c2-smarc.dtb - r9a07g044l2-smarc.dtb - r9a07g054l2-smarc.dtb - r9a08g045s33-smarc.dtb - r9a08g045s33-smarc-pmod.dtb (not integrated in the latest kernel tree, but the device tree was posted at [3]) 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 1, 2, 3 were tested also with renesas_defconfig on r9a08g045s33-smarc-pmod.dtb. Please give it a try on your devices as well. Thank you, Claudiu Beznea [1] https://lore.kernel.org/all/20241115134401.3893008-1-claudiu.beznea.uj@bp.renesas.com/ [2] https://lore.kernel.org/all/20241125115856.513642-1-claudiu.beznea.uj@bp.renesas.com/ [3] https://lore.kernel.org/all/20241115134401.3893008-9-claudiu.beznea.uj@bp.renesas.com/ Claudiu Beznea (6): serial: sh-sci: Check if TX data was written to device in .tx_empty() serial: sh-sci: Drop __initdata macro for port_cfg serial: sh-sci: Move runtime PM enable to sci_probe_single() serial: sh-sci: Do not probe the serial port if its slot in sci_ports[] is in use serial: sh-sci: Clean sci_ports[0] after at earlycon exit serial: sh-sci: Increment the runtime usage counter for the earlycon device drivers/tty/serial/sh-sci.c | 121 ++++++++++++++++++++++++++++++------ 1 file changed, 102 insertions(+), 19 deletions(-)