Re: [PATCH 1/1] tty: serial: handle HAS_IOPORT dependencies

["Arnd Bergmann" <arnd@kernel.org>]

On Sat, Nov 23, 2024, at 00:34, Niklas Schnelle wrote:
>
> Am I seeing it right that despite the warning and the code setting
> no_serial_in / no_serial_out the console=ttyS0 in the above qemu boots
> still worked? Also for example in the nios2 case I see the warning 4
> times. So this makes me wonder since the warning is new is it possible
> that set_io_from_upio() has been called with an invalid / all
> zero port before but it was invisible.

Yes, that is certainly the case, sorry if I wasn't clear about this.

The warning shows that there is something wrong with the code,
but that problem has likely existed for a long time. We can
obviously just hide that warning again and ignore the underlying
problem without causing any regressions to the previous state,
but I hope we can improve it in some way that makes it less
broken on non-x86 architectures.

> The way I'm reading __serial8250_isa_init_ports() and in particular the
> first loop if nr_uarts is e.g. 5 in the nios case but only the first
> entry in serial8250_ports[] has the IOMEM 8250 it will still call
> serial8250_setup_port() on the 4 other unintalized/zero elements which
> would explain the iotype being 0. And as far as I can see nr_uarts is
> just set to the value of CONFIG_SERIAL_8250_RUNTIME_UARTS in
> 8250_platform.c.

The way I was reading the code, all five would be uninitialized
at the time we call __serial8250_isa_init_ports(), and the first
port only gets set later on.

Another thing I see is that the 8250 driver ("serial") is the
only one that ends up registering a lot of ports at boot time,
while the other ones only register the ones they actually detect.

E.g. on my Apple workstation, I get

# head /proc/tty/driver/*
==> /proc/tty/driver/IMX-uart <==
serinfo:1.0 driver revision:

==> /proc/tty/driver/fsl-linflexuart <==
serinfo:1.0 driver revision:

==> /proc/tty/driver/fsl-lpuart <==
serinfo:1.0 driver revision:

==> /proc/tty/driver/msm_serial <==
serinfo:1.0 driver revision:

==> /proc/tty/driver/mvebu_serial <==
serinfo:1.0 driver revision:

==> /proc/tty/driver/qcom_geni_console <==
serinfo:1.0 driver revision:

==> /proc/tty/driver/qcom_geni_uart <==
serinfo:1.0 driver revision:

==> /proc/tty/driver/s3c2410_serial <==
serinfo:1.0 driver revision:
0: uart:APPLE S5L mmio:0x39B200000 irq:42 tx:0 rx:0 CTS|DSR|CD

==> /proc/tty/driver/serial <==
serinfo:1.0 driver revision:
0: uart:unknown port:00000000 irq:0
1: uart:unknown port:00000000 irq:0
2: uart:unknown port:00000000 irq:0
3: uart:unknown port:00000000 irq:0
4: uart:unknown port:00000000 irq:0
5: uart:unknown port:00000000 irq:0
6: uart:unknown port:00000000 irq:0
7: uart:unknown port:00000000 irq:0
8: uart:unknown port:00000000 irq:0

==> /proc/tty/driver/tegra_hsuart <==
serinfo:1.0 driver revision:

==> /proc/tty/driver/usbserial <==
usbserinfo:1.0 driver:2.0

The way that this driver was meant to handle that originally
is that /sbin/setserial can be used at runtime to configure
any UART that is attached to an ISA bus even if it did not
get detected at boot time, by setting the correct port and
IRQ numbers from userspace. This may be required on i486
systems with ISA cards providing additional non-PNP 8250
on a nonstandard port number, but is likely not useful on
anything other than x86-32.

      Arnd