[RFC] tty: serial: kgdboc: Fix 8250_* kgd over serial

Message ID 20231208212845.1679621-1-michael@amarulasolutions.com
State New
Headers
Series [RFC] tty: serial: kgdboc: Fix 8250_* kgd over serial |

Commit Message

Michael Nazzareno Trimarchi Dec. 8, 2023, 9:28 p.m. UTC
  Use late_initcall_sync insted of module init to be sure that
serial driver is really probed and get take handover from
early driver. The 8250 register the platform driver after
the 8250 core is initialized. As shown by kdbg

Thread 2 received signal SIGSEGV, Segmentation fault.
[Switching to Thread 1]
_outb (addr=<optimized out>, value=<optimized out>) at ./include/asm-generic/io.h:584
584		__raw_writeb(value, PCI_IOBASE + addr);
(gdb) bt

This section of the code is too early because in this case
the omap serial is not probed

Thread 2 received signal SIGSEGV, Segmentation fault.
[Switching to Thread 1]
_outb (addr=<optimized out>, value=<optimized out>) at ./include/asm-generic/io.h:584
584		__raw_writeb(value, PCI_IOBASE + addr);
(gdb) bt

Thread 2 received signal SIGSEGV, Segmentation fault.
[Switching to Thread 1]
_outb (addr=<optimized out>, value=<optimized out>) at ./include/asm-generic/io.h:584
584		__raw_writeb(value, PCI_IOBASE + addr);
(gdb) bt
0  _outb (addr=<optimized out>, value=<optimized out>) at ./include/asm-generic/io.h:584
1  logic_outb (value=0 '\000', addr=18446739675637874689) at lib/logic_pio.c:299
2  0xffff80008082dfcc in io_serial_out (p=0x0, offset=16760830, value=0) at drivers/tty/serial/8250/8250_port.c:416
3  0xffff80008082fe34 in serial_port_out (value=<optimized out>, offset=<optimized out>, up=<optimized out>)
    at ./include/linux/serial_core.h:677
4  serial8250_do_set_termios (port=0xffff8000828ee940 <serial8250_ports+1568>, termios=0xffff80008292b93c, old=0x0)
    at drivers/tty/serial/8250/8250_port.c:2860
5  0xffff800080830064 in serial8250_set_termios (port=0xfffffbfffe800000, termios=0xffbffe, old=0x0)
    at drivers/tty/serial/8250/8250_port.c:2912
6  0xffff80008082571c in uart_set_options (port=0xffff8000828ee940 <serial8250_ports+1568>, co=0x0, baud=115200, parity=110, bits=8, flow=110)
    at drivers/tty/serial/serial_core.c:2285
7  0xffff800080828434 in uart_poll_init (driver=0xfffffbfffe800000, line=16760830, options=0xffff8000828f7506 <config+6> "115200n8")
    at drivers/tty/serial/serial_core.c:2656
8  0xffff800080801690 in tty_find_polling_driver (name=0xffff8000828f7500 <config> "ttyS2,115200n8", line=0xffff80008292ba90)
    at drivers/tty/tty_io.c:410
9  0xffff80008086c0b0 in configure_kgdboc () at drivers/tty/serial/kgdboc.c:194
10 0xffff80008086c1ec in kgdboc_probe (pdev=0xfffffbfffe800000) at drivers/tty/serial/kgdboc.c:249
11 0xffff8000808b399c in platform_probe (_dev=0xffff000000ebb810) at drivers/base/platform.c:1404
12 0xffff8000808b0b44 in call_driver_probe (drv=<optimized out>, dev=<optimized out>) at drivers/base/dd.c:579
13 really_probe (dev=0xffff000000ebb810, drv=0xffff80008277f138 <kgdboc_platform_driver+48>) at drivers/base/dd.c:658
14 0xffff8000808b0d2c in __driver_probe_device (drv=0xffff80008277f138 <kgdboc_platform_driver+48>, dev=0xffff000000ebb810)
    at drivers/base/dd.c:800
15 0xffff8000808b0eb8 in driver_probe_device (drv=0xfffffbfffe800000, dev=0xffff000000ebb810) at drivers/base/dd.c:830
16 0xffff8000808b0ff4 in __device_attach_driver (drv=0xffff80008277f138 <kgdboc_platform_driver+48>, _data=0xffff80008292bc48)
    at drivers/base/dd.c:958
17 0xffff8000808ae970 in bus_for_each_drv (bus=0xfffffbfffe800000, start=0x0, data=0xffff80008292bc48,
    fn=0xffff8000808b0f3c <__device_attach_driver>) at drivers/base/bus.c:457
18 0xffff8000808b1408 in __device_attach (dev=0xffff000000ebb810, allow_async=true) at drivers/base/dd.c:1030
19 0xffff8000808b16d8 in device_initial_probe (dev=0xfffffbfffe800000) at drivers/base/dd.c:1079
20 0xffff8000808af9f4 in bus_probe_device (dev=0xffff000000ebb810) at drivers/base/bus.c:532
21 0xffff8000808ac77c in device_add (dev=0xfffffbfffe800000) at drivers/base/core.c:3625
22 0xffff8000808b3428 in platform_device_add (pdev=0xffff000000ebb800) at drivers/base/platform.c:716
23 0xffff800081b5dc0c in init_kgdboc () at drivers/tty/serial/kgdboc.c:292
24 0xffff800080014db0 in do_one_initcall (fn=0xffff800081b5dba4 <init_kgdboc>) at init/main.c:1236
25 0xffff800081b0114c in do_initcall_level (command_line=<optimized out>, level=<optimized out>) at init/main.c:1298
26 do_initcalls () at init/main.c:1314
27 do_basic_setup () at init/main.c:1333
28 kernel_init_freeable () at init/main.c:1551
29 0xffff8000810271ec in kernel_init (unused=0xfffffbfffe800000) at init/main.c:1441
30 0xffff800080015e80 in ret_from_fork () at arch/arm64/kernel/entry.S:857

Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
---
 drivers/tty/serial/kgdboc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Doug Anderson Dec. 11, 2023, 9:38 p.m. UTC | #1
Hi,

On Fri, Dec 8, 2023 at 1:28 PM Michael Trimarchi
<michael@amarulasolutions.com> wrote:
>
> Use late_initcall_sync insted of module init to be sure that
> serial driver is really probed and get take handover from
> early driver.

Awesome that you used the earlycon driver to debug problems with
registering the normal driver! :-P


> The 8250 register the platform driver after
> the 8250 core is initialized. As shown by kdbg
>
> Thread 2 received signal SIGSEGV, Segmentation fault.
> [Switching to Thread 1]
> _outb (addr=<optimized out>, value=<optimized out>) at ./include/asm-generic/io.h:584
> 584             __raw_writeb(value, PCI_IOBASE + addr);
> (gdb) bt
>
> This section of the code is too early because in this case
> the omap serial is not probed
>
> Thread 2 received signal SIGSEGV, Segmentation fault.
> [Switching to Thread 1]
> _outb (addr=<optimized out>, value=<optimized out>) at ./include/asm-generic/io.h:584
> 584             __raw_writeb(value, PCI_IOBASE + addr);
> (gdb) bt
>
> Thread 2 received signal SIGSEGV, Segmentation fault.
> [Switching to Thread 1]
> _outb (addr=<optimized out>, value=<optimized out>) at ./include/asm-generic/io.h:584
> 584             __raw_writeb(value, PCI_IOBASE + addr);
> (gdb) bt
> 0  _outb (addr=<optimized out>, value=<optimized out>) at ./include/asm-generic/io.h:584
> 1  logic_outb (value=0 '\000', addr=18446739675637874689) at lib/logic_pio.c:299
> 2  0xffff80008082dfcc in io_serial_out (p=0x0, offset=16760830, value=0) at drivers/tty/serial/8250/8250_port.c:416
> 3  0xffff80008082fe34 in serial_port_out (value=<optimized out>, offset=<optimized out>, up=<optimized out>)
>     at ./include/linux/serial_core.h:677
> 4  serial8250_do_set_termios (port=0xffff8000828ee940 <serial8250_ports+1568>, termios=0xffff80008292b93c, old=0x0)
>     at drivers/tty/serial/8250/8250_port.c:2860
> 5  0xffff800080830064 in serial8250_set_termios (port=0xfffffbfffe800000, termios=0xffbffe, old=0x0)
>     at drivers/tty/serial/8250/8250_port.c:2912
> 6  0xffff80008082571c in uart_set_options (port=0xffff8000828ee940 <serial8250_ports+1568>, co=0x0, baud=115200, parity=110, bits=8, flow=110)
>     at drivers/tty/serial/serial_core.c:2285
> 7  0xffff800080828434 in uart_poll_init (driver=0xfffffbfffe800000, line=16760830, options=0xffff8000828f7506 <config+6> "115200n8")
>     at drivers/tty/serial/serial_core.c:2656
> 8  0xffff800080801690 in tty_find_polling_driver (name=0xffff8000828f7500 <config> "ttyS2,115200n8", line=0xffff80008292ba90)
>     at drivers/tty/tty_io.c:410
> 9  0xffff80008086c0b0 in configure_kgdboc () at drivers/tty/serial/kgdboc.c:194
> 10 0xffff80008086c1ec in kgdboc_probe (pdev=0xfffffbfffe800000) at drivers/tty/serial/kgdboc.c:249
> 11 0xffff8000808b399c in platform_probe (_dev=0xffff000000ebb810) at drivers/base/platform.c:1404
> 12 0xffff8000808b0b44 in call_driver_probe (drv=<optimized out>, dev=<optimized out>) at drivers/base/dd.c:579
> 13 really_probe (dev=0xffff000000ebb810, drv=0xffff80008277f138 <kgdboc_platform_driver+48>) at drivers/base/dd.c:658
> 14 0xffff8000808b0d2c in __driver_probe_device (drv=0xffff80008277f138 <kgdboc_platform_driver+48>, dev=0xffff000000ebb810)
>     at drivers/base/dd.c:800
> 15 0xffff8000808b0eb8 in driver_probe_device (drv=0xfffffbfffe800000, dev=0xffff000000ebb810) at drivers/base/dd.c:830
> 16 0xffff8000808b0ff4 in __device_attach_driver (drv=0xffff80008277f138 <kgdboc_platform_driver+48>, _data=0xffff80008292bc48)
>     at drivers/base/dd.c:958
> 17 0xffff8000808ae970 in bus_for_each_drv (bus=0xfffffbfffe800000, start=0x0, data=0xffff80008292bc48,
>     fn=0xffff8000808b0f3c <__device_attach_driver>) at drivers/base/bus.c:457
> 18 0xffff8000808b1408 in __device_attach (dev=0xffff000000ebb810, allow_async=true) at drivers/base/dd.c:1030
> 19 0xffff8000808b16d8 in device_initial_probe (dev=0xfffffbfffe800000) at drivers/base/dd.c:1079
> 20 0xffff8000808af9f4 in bus_probe_device (dev=0xffff000000ebb810) at drivers/base/bus.c:532
> 21 0xffff8000808ac77c in device_add (dev=0xfffffbfffe800000) at drivers/base/core.c:3625
> 22 0xffff8000808b3428 in platform_device_add (pdev=0xffff000000ebb800) at drivers/base/platform.c:716
> 23 0xffff800081b5dc0c in init_kgdboc () at drivers/tty/serial/kgdboc.c:292
> 24 0xffff800080014db0 in do_one_initcall (fn=0xffff800081b5dba4 <init_kgdboc>) at init/main.c:1236
> 25 0xffff800081b0114c in do_initcall_level (command_line=<optimized out>, level=<optimized out>) at init/main.c:1298
> 26 do_initcalls () at init/main.c:1314
> 27 do_basic_setup () at init/main.c:1333
> 28 kernel_init_freeable () at init/main.c:1551
> 29 0xffff8000810271ec in kernel_init (unused=0xfffffbfffe800000) at init/main.c:1441
> 30 0xffff800080015e80 in ret_from_fork () at arch/arm64/kernel/entry.S:857
>
> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> ---
>  drivers/tty/serial/kgdboc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c
> index 7ce7bb164005..7f8364507f55 100644
> --- a/drivers/tty/serial/kgdboc.c
> +++ b/drivers/tty/serial/kgdboc.c
> @@ -622,7 +622,7 @@ console_initcall(kgdboc_earlycon_late_init);
>
>  #endif /* IS_BUILTIN(CONFIG_KGDB_SERIAL_CONSOLE) */
>
> -module_init(init_kgdboc);
> +late_initcall_sync(init_kgdboc);

While I'm not denying that you hit a bug, I don't think this is the
correct fix. The way it's supposed to work is:

1. init_kgdboc() runs and registers the singleton kgdb "platform driver".

2. The platform driver's probe function, kgdboc_probe(), runs and
checks to see if the console is ready by looking at the return value
of configure_kgdboc(). If it's ready then we're good to go. If it's
not ready then we defer.

So I think the bug here is that somehow the console looks "ready"
(because tty_find_polling_driver() can find it) but it isn't actually
ready yet (because it crashes). That's what you need to fix.

I'll note that, in the past, I've definitely used kgdb on 8250-based
UARTs. Is your hardware somehow special or is this a regression?

-Doug
  
Michael Nazzareno Trimarchi Dec. 11, 2023, 9:41 p.m. UTC | #2
Hi Doug

On Mon, Dec 11, 2023 at 10:39 PM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Fri, Dec 8, 2023 at 1:28 PM Michael Trimarchi
> <michael@amarulasolutions.com> wrote:
> >
> > Use late_initcall_sync insted of module init to be sure that
> > serial driver is really probed and get take handover from
> > early driver.
>
> Awesome that you used the earlycon driver to debug problems with
> registering the normal driver! :-P
>
>
> > The 8250 register the platform driver after
> > the 8250 core is initialized. As shown by kdbg
> >
> > Thread 2 received signal SIGSEGV, Segmentation fault.
> > [Switching to Thread 1]
> > _outb (addr=<optimized out>, value=<optimized out>) at ./include/asm-generic/io.h:584
> > 584             __raw_writeb(value, PCI_IOBASE + addr);
> > (gdb) bt
> >
> > This section of the code is too early because in this case
> > the omap serial is not probed
> >
> > Thread 2 received signal SIGSEGV, Segmentation fault.
> > [Switching to Thread 1]
> > _outb (addr=<optimized out>, value=<optimized out>) at ./include/asm-generic/io.h:584
> > 584             __raw_writeb(value, PCI_IOBASE + addr);
> > (gdb) bt
> >
> > Thread 2 received signal SIGSEGV, Segmentation fault.
> > [Switching to Thread 1]
> > _outb (addr=<optimized out>, value=<optimized out>) at ./include/asm-generic/io.h:584
> > 584             __raw_writeb(value, PCI_IOBASE + addr);
> > (gdb) bt
> > 0  _outb (addr=<optimized out>, value=<optimized out>) at ./include/asm-generic/io.h:584
> > 1  logic_outb (value=0 '\000', addr=18446739675637874689) at lib/logic_pio.c:299
> > 2  0xffff80008082dfcc in io_serial_out (p=0x0, offset=16760830, value=0) at drivers/tty/serial/8250/8250_port.c:416
> > 3  0xffff80008082fe34 in serial_port_out (value=<optimized out>, offset=<optimized out>, up=<optimized out>)
> >     at ./include/linux/serial_core.h:677
> > 4  serial8250_do_set_termios (port=0xffff8000828ee940 <serial8250_ports+1568>, termios=0xffff80008292b93c, old=0x0)
> >     at drivers/tty/serial/8250/8250_port.c:2860
> > 5  0xffff800080830064 in serial8250_set_termios (port=0xfffffbfffe800000, termios=0xffbffe, old=0x0)
> >     at drivers/tty/serial/8250/8250_port.c:2912
> > 6  0xffff80008082571c in uart_set_options (port=0xffff8000828ee940 <serial8250_ports+1568>, co=0x0, baud=115200, parity=110, bits=8, flow=110)
> >     at drivers/tty/serial/serial_core.c:2285
> > 7  0xffff800080828434 in uart_poll_init (driver=0xfffffbfffe800000, line=16760830, options=0xffff8000828f7506 <config+6> "115200n8")
> >     at drivers/tty/serial/serial_core.c:2656
> > 8  0xffff800080801690 in tty_find_polling_driver (name=0xffff8000828f7500 <config> "ttyS2,115200n8", line=0xffff80008292ba90)
> >     at drivers/tty/tty_io.c:410
> > 9  0xffff80008086c0b0 in configure_kgdboc () at drivers/tty/serial/kgdboc.c:194
> > 10 0xffff80008086c1ec in kgdboc_probe (pdev=0xfffffbfffe800000) at drivers/tty/serial/kgdboc.c:249
> > 11 0xffff8000808b399c in platform_probe (_dev=0xffff000000ebb810) at drivers/base/platform.c:1404
> > 12 0xffff8000808b0b44 in call_driver_probe (drv=<optimized out>, dev=<optimized out>) at drivers/base/dd.c:579
> > 13 really_probe (dev=0xffff000000ebb810, drv=0xffff80008277f138 <kgdboc_platform_driver+48>) at drivers/base/dd.c:658
> > 14 0xffff8000808b0d2c in __driver_probe_device (drv=0xffff80008277f138 <kgdboc_platform_driver+48>, dev=0xffff000000ebb810)
> >     at drivers/base/dd.c:800
> > 15 0xffff8000808b0eb8 in driver_probe_device (drv=0xfffffbfffe800000, dev=0xffff000000ebb810) at drivers/base/dd.c:830
> > 16 0xffff8000808b0ff4 in __device_attach_driver (drv=0xffff80008277f138 <kgdboc_platform_driver+48>, _data=0xffff80008292bc48)
> >     at drivers/base/dd.c:958
> > 17 0xffff8000808ae970 in bus_for_each_drv (bus=0xfffffbfffe800000, start=0x0, data=0xffff80008292bc48,
> >     fn=0xffff8000808b0f3c <__device_attach_driver>) at drivers/base/bus.c:457
> > 18 0xffff8000808b1408 in __device_attach (dev=0xffff000000ebb810, allow_async=true) at drivers/base/dd.c:1030
> > 19 0xffff8000808b16d8 in device_initial_probe (dev=0xfffffbfffe800000) at drivers/base/dd.c:1079
> > 20 0xffff8000808af9f4 in bus_probe_device (dev=0xffff000000ebb810) at drivers/base/bus.c:532
> > 21 0xffff8000808ac77c in device_add (dev=0xfffffbfffe800000) at drivers/base/core.c:3625
> > 22 0xffff8000808b3428 in platform_device_add (pdev=0xffff000000ebb800) at drivers/base/platform.c:716
> > 23 0xffff800081b5dc0c in init_kgdboc () at drivers/tty/serial/kgdboc.c:292
> > 24 0xffff800080014db0 in do_one_initcall (fn=0xffff800081b5dba4 <init_kgdboc>) at init/main.c:1236
> > 25 0xffff800081b0114c in do_initcall_level (command_line=<optimized out>, level=<optimized out>) at init/main.c:1298
> > 26 do_initcalls () at init/main.c:1314
> > 27 do_basic_setup () at init/main.c:1333
> > 28 kernel_init_freeable () at init/main.c:1551
> > 29 0xffff8000810271ec in kernel_init (unused=0xfffffbfffe800000) at init/main.c:1441
> > 30 0xffff800080015e80 in ret_from_fork () at arch/arm64/kernel/entry.S:857
> >
> > Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> > ---
> >  drivers/tty/serial/kgdboc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c
> > index 7ce7bb164005..7f8364507f55 100644
> > --- a/drivers/tty/serial/kgdboc.c
> > +++ b/drivers/tty/serial/kgdboc.c
> > @@ -622,7 +622,7 @@ console_initcall(kgdboc_earlycon_late_init);
> >
> >  #endif /* IS_BUILTIN(CONFIG_KGDB_SERIAL_CONSOLE) */
> >
> > -module_init(init_kgdboc);
> > +late_initcall_sync(init_kgdboc);
>
> While I'm not denying that you hit a bug, I don't think this is the
> correct fix. The way it's supposed to work is:
>

Yes it's a bug

> 1. init_kgdboc() runs and registers the singleton kgdb "platform driver".
>
> 2. The platform driver's probe function, kgdboc_probe(), runs and
> checks to see if the console is ready by looking at the return value
> of configure_kgdboc(). If it's ready then we're good to go. If it's
> not ready then we defer.
>
> So I think the bug here is that somehow the console looks "ready"
> (because tty_find_polling_driver() can find it) but it isn't actually
> ready yet (because it crashes). That's what you need to fix.
>

The polling driver look for uart and uart8250_core is registered and 4 fake uart
are there but there are not still replaced by platform driver that can
come later.
The try_polling find it but it's the isa-8250 driver. It means that
add_uart 8250 is
not still happen

> I'll note that, in the past, I've definitely used kgdb on 8250-based
> UARTs. Is your hardware somehow special or is this a regression?
>

Michael

> -Doug
  
Doug Anderson Dec. 11, 2023, 10 p.m. UTC | #3
Hi,

On Mon, Dec 11, 2023 at 1:42 PM Michael Nazzareno Trimarchi
<michael@amarulasolutions.com> wrote:
>
> > 1. init_kgdboc() runs and registers the singleton kgdb "platform driver".
> >
> > 2. The platform driver's probe function, kgdboc_probe(), runs and
> > checks to see if the console is ready by looking at the return value
> > of configure_kgdboc(). If it's ready then we're good to go. If it's
> > not ready then we defer.
> >
> > So I think the bug here is that somehow the console looks "ready"
> > (because tty_find_polling_driver() can find it) but it isn't actually
> > ready yet (because it crashes). That's what you need to fix.
> >
>
> The polling driver look for uart and uart8250_core is registered and 4 fake uart
> are there but there are not still replaced by platform driver that can
> come later.
> The try_polling find it but it's the isa-8250 driver. It means that
> add_uart 8250 is
> not still happen

The 8250 driver is always a maze, so you might need to do a bunch of
digging. ...but it sure sounds like the console shouldn't be
registered until the correct ops are in place. That either means
getting the ops put in place earlier or deferring when the console is
registered...

-Doug
  
Michael Nazzareno Trimarchi Dec. 12, 2023, 8:54 a.m. UTC | #4
Hi Doug

On Mon, Dec 11, 2023 at 11:00 PM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Mon, Dec 11, 2023 at 1:42 PM Michael Nazzareno Trimarchi
> <michael@amarulasolutions.com> wrote:
> >
> > > 1. init_kgdboc() runs and registers the singleton kgdb "platform driver".
> > >
> > > 2. The platform driver's probe function, kgdboc_probe(), runs and
> > > checks to see if the console is ready by looking at the return value
> > > of configure_kgdboc(). If it's ready then we're good to go. If it's
> > > not ready then we defer.
> > >
> > > So I think the bug here is that somehow the console looks "ready"
> > > (because tty_find_polling_driver() can find it) but it isn't actually
> > > ready yet (because it crashes). That's what you need to fix.
> > >
> >
> > The polling driver look for uart and uart8250_core is registered and 4 fake uart
> > are there but there are not still replaced by platform driver that can
> > come later.
> > The try_polling find it but it's the isa-8250 driver. It means that
> > add_uart 8250 is
> > not still happen
>
> The 8250 driver is always a maze, so you might need to do a bunch of
> digging. ...but it sure sounds like the console shouldn't be
> registered until the correct ops are in place. That either means
> getting the ops put in place earlier or deferring when the console is
> registered...
>

Your point is pretty clear and my initial idea was to find a real fix.
This come to avoid
breaking existing setup but anyway I will dig in it more

Michael

> -Doug
  
Michael Nazzareno Trimarchi Dec. 16, 2023, 1:45 p.m. UTC | #5
Hi Doug

On Tue, Dec 12, 2023 at 9:54 AM Michael Nazzareno Trimarchi
<michael@amarulasolutions.com> wrote:
>
> Hi Doug
>
> On Mon, Dec 11, 2023 at 11:00 PM Doug Anderson <dianders@chromium.org> wrote:
> >
> > Hi,
> >
> > On Mon, Dec 11, 2023 at 1:42 PM Michael Nazzareno Trimarchi
> > <michael@amarulasolutions.com> wrote:
> > >
> > > > 1. init_kgdboc() runs and registers the singleton kgdb "platform driver".
> > > >
> > > > 2. The platform driver's probe function, kgdboc_probe(), runs and
> > > > checks to see if the console is ready by looking at the return value
> > > > of configure_kgdboc(). If it's ready then we're good to go. If it's
> > > > not ready then we defer.
> > > >
> > > > So I think the bug here is that somehow the console looks "ready"
> > > > (because tty_find_polling_driver() can find it) but it isn't actually
> > > > ready yet (because it crashes). That's what you need to fix.
> > > >
> > >
> > > The polling driver look for uart and uart8250_core is registered and 4 fake uart
> > > are there but there are not still replaced by platform driver that can
> > > come later.
> > > The try_polling find it but it's the isa-8250 driver. It means that
> > > add_uart 8250 is
> > > not still happen
> >
> > The 8250 driver is always a maze, so you might need to do a bunch of
> > digging. ...but it sure sounds like the console shouldn't be
> > registered until the correct ops are in place. That either means
> > getting the ops put in place earlier or deferring when the console is
> > registered...
> >
>
> Your point is pretty clear and my initial idea was to find a real fix.
> This come to avoid
> breaking existing setup but anyway I will dig in it more
>
> Michael

What about this?

--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -385,6 +385,7 @@ struct tty_driver *tty_find_polling_driver(char
*name, int *line)
        int tty_line = 0;
        int len;
        char *str, *stp;
+       int index;

        for (str = name; *str; str++)
                if ((*str >= '0' && *str <= '9') || *str == ',')
@@ -406,7 +407,7 @@ struct tty_driver *tty_find_polling_driver(char
*name, int *line)
                if (*stp == '\0')
                        stp = NULL;

-               if (tty_line >= 0 && tty_line < p->num && p->ops &&
+               if (tty_line >= 0 && tty_line < p->num && p->ops &&
console_device(&index) == p &&
                    p->ops->poll_init && !p->ops->poll_init(p, tty_line, stp)) {
                        res = tty_driver_kref_get(p);
                        *line = tty_line;

I will send proper patch

[   18.885348] printk: legacy console [ttyS2] disabled
[   18.890821] 2800000.serial: ttyS2 at MMIO 0x2800000 (irq = 283,
base_baud = 3000000) is a 8250
[   18.899727] printk: legacy console [ttyS2] enabled
[   18.909440] printk: legacy bootconsole [ns16550a0] disabled
[   18.923263] omap8250_probe: register uart 2800000.serial

Michael
>
> > -Doug
>
>
>
> --
> Michael Nazzareno Trimarchi
> Co-Founder & Chief Executive Officer
> M. +39 347 913 2170
> michael@amarulasolutions.com
> __________________________________
>
> Amarula Solutions BV
> Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> T. +31 (0)85 111 9172
> info@amarulasolutions.com
> www.amarulasolutions.com
  
Michael Nazzareno Trimarchi Dec. 16, 2023, 4:20 p.m. UTC | #6
Hi

On Sat, Dec 16, 2023 at 2:45 PM Michael Nazzareno Trimarchi
<michael@amarulasolutions.com> wrote:
>
> Hi Doug
>
> On Tue, Dec 12, 2023 at 9:54 AM Michael Nazzareno Trimarchi
> <michael@amarulasolutions.com> wrote:
> >
> > Hi Doug
> >
> > On Mon, Dec 11, 2023 at 11:00 PM Doug Anderson <dianders@chromium.org> wrote:
> > >
> > > Hi,
> > >
> > > On Mon, Dec 11, 2023 at 1:42 PM Michael Nazzareno Trimarchi
> > > <michael@amarulasolutions.com> wrote:
> > > >
> > > > > 1. init_kgdboc() runs and registers the singleton kgdb "platform driver".
> > > > >
> > > > > 2. The platform driver's probe function, kgdboc_probe(), runs and
> > > > > checks to see if the console is ready by looking at the return value
> > > > > of configure_kgdboc(). If it's ready then we're good to go. If it's
> > > > > not ready then we defer.
> > > > >
> > > > > So I think the bug here is that somehow the console looks "ready"
> > > > > (because tty_find_polling_driver() can find it) but it isn't actually
> > > > > ready yet (because it crashes). That's what you need to fix.
> > > > >
> > > >
> > > > The polling driver look for uart and uart8250_core is registered and 4 fake uart
> > > > are there but there are not still replaced by platform driver that can
> > > > come later.
> > > > The try_polling find it but it's the isa-8250 driver. It means that
> > > > add_uart 8250 is
> > > > not still happen
> > >
> > > The 8250 driver is always a maze, so you might need to do a bunch of
> > > digging. ...but it sure sounds like the console shouldn't be
> > > registered until the correct ops are in place. That either means
> > > getting the ops put in place earlier or deferring when the console is
> > > registered...
> > >
> >
> > Your point is pretty clear and my initial idea was to find a real fix.
> > This come to avoid
> > breaking existing setup but anyway I will dig in it more
> >
> > Michael
>
> What about this?
>
> --- a/drivers/tty/tty_io.c
> +++ b/drivers/tty/tty_io.c
> @@ -385,6 +385,7 @@ struct tty_driver *tty_find_polling_driver(char
> *name, int *line)
>         int tty_line = 0;
>         int len;
>         char *str, *stp;
> +       int index;
>
>         for (str = name; *str; str++)
>                 if ((*str >= '0' && *str <= '9') || *str == ',')
> @@ -406,7 +407,7 @@ struct tty_driver *tty_find_polling_driver(char
> *name, int *line)
>                 if (*stp == '\0')
>                         stp = NULL;
>
> -               if (tty_line >= 0 && tty_line < p->num && p->ops &&
> +               if (tty_line >= 0 && tty_line < p->num && p->ops &&
> console_device(&index) == p &&
>                     p->ops->poll_init && !p->ops->poll_init(p, tty_line, stp)) {
>                         res = tty_driver_kref_get(p);
>                         *line = tty_line;
>
> I will send proper patch
>
> [   18.885348] printk: legacy console [ttyS2] disabled
> [   18.890821] 2800000.serial: ttyS2 at MMIO 0x2800000 (irq = 283,
> base_baud = 3000000) is a 8250
> [   18.899727] printk: legacy console [ttyS2] enabled
> [   18.909440] printk: legacy bootconsole [ns16550a0] disabled
> [   18.923263] omap8250_probe: register uart 2800000.serial

I read better the documentation is this can not work, because the
requirement can be any uart and not the console one

Micahel
>
> Michael
> >
> > > -Doug
> >
> >
> >
> > --
> > Michael Nazzareno Trimarchi
> > Co-Founder & Chief Executive Officer
> > M. +39 347 913 2170
> > michael@amarulasolutions.com
> > __________________________________
> >
> > Amarula Solutions BV
> > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> > T. +31 (0)85 111 9172
> > info@amarulasolutions.com
> > www.amarulasolutions.com
>
>
>
> --
> Michael Nazzareno Trimarchi
> Co-Founder & Chief Executive Officer
> M. +39 347 913 2170
> michael@amarulasolutions.com
> __________________________________
>
> Amarula Solutions BV
> Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> T. +31 (0)85 111 9172
> info@amarulasolutions.com
> www.amarulasolutions.com
  

Patch

diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c
index 7ce7bb164005..7f8364507f55 100644
--- a/drivers/tty/serial/kgdboc.c
+++ b/drivers/tty/serial/kgdboc.c
@@ -622,7 +622,7 @@  console_initcall(kgdboc_earlycon_late_init);
 
 #endif /* IS_BUILTIN(CONFIG_KGDB_SERIAL_CONSOLE) */
 
-module_init(init_kgdboc);
+late_initcall_sync(init_kgdboc);
 module_exit(exit_kgdboc);
 module_param_call(kgdboc, param_set_kgdboc_var, param_get_string, &kps, 0644);
 MODULE_PARM_DESC(kgdboc, "<serial_device>[,baud]");