On Fri, Nov 22, 2024, at 21:44, Guenter Roeck wrote:
> On 11/22/24 11:24, Arnd Bergmann wrote:
>>
>> serial8250_setup_port() is where it ends up using the
>> uninitialized serial8250_ports[index] contents. Since
>> port->iotype is not set to anything here, it defaults to
>> '0', which happens to be UPIO_PORT.
>>
>> The reason it doesn't immediately crash and burn is that
>> this is still only setting up the structures for later
>> use, but I assume that trying to use console=ttyS0, or
>> opening /dev/ttyS0 on the uninitialized port would
>> then cause an oops.
>>
>
> All four affected platforms use ttyS0, only it is mmio based,
> not io port based.
Right, so I think the problem is really the 8250_platform.c
file trying to handle all possible cases on x86 (acpi,
isapnp, legacy isa) that may define the same UARTs, but
also trying to handle non-legacy ports with their own
special cases on other architectures.
The patch below is a first idea of how we can skip the
legacy ISA case on architectures that don't define those.
If this works, we can try to come up with a better way of
doing that. Ideally all the pre-DT boardfile machines that
define their own "serial8250" platform_device (with platform_data)
should use a different identifier the x86-legacy case that
uses the platform_device (without platform_data). Similarly,
I think the riscv special ACPI port needs its own trivial
driver without all the ISA magic.
Arnd
diff --git a/drivers/tty/serial/8250/8250_platform.c b/drivers/tty/serial/8250/8250_platform.c
index 66fd6d5d4835..610b31517734 100644
--- a/drivers/tty/serial/8250/8250_platform.c
+++ b/drivers/tty/serial/8250/8250_platform.c
@@ -101,7 +101,8 @@ static void __init __serial8250_isa_init_ports(void)
void __init serial8250_isa_init_ports(void)
{
- DO_ONCE(__serial8250_isa_init_ports);
+ if (ARRAY_SIZE(old_serial_port))
+ DO_ONCE(__serial8250_isa_init_ports);
}
/*
@@ -283,7 +284,7 @@ static const struct acpi_device_id acpi_platform_serial_table[] = {
};
MODULE_DEVICE_TABLE(acpi, acpi_platform_serial_table);
-static struct platform_driver serial8250_isa_driver = {
+static struct platform_driver serial8250_platform_driver = {
.probe = serial8250_probe,
.remove = serial8250_remove,
.suspend = serial8250_suspend,
@@ -300,7 +301,7 @@ static struct platform_driver serial8250_isa_driver = {
*/
struct platform_device *serial8250_isa_devs;
-static int __init serial8250_init(void)
+static int __init serial8250_isa_init(void)
{
int ret;
@@ -337,11 +338,8 @@ static int __init serial8250_init(void)
serial8250_register_ports(&serial8250_reg, &serial8250_isa_devs->dev);
- ret = platform_driver_register(&serial8250_isa_driver);
- if (ret == 0)
- goto out;
+ return 0;
- platform_device_del(serial8250_isa_devs);
put_dev:
platform_device_put(serial8250_isa_devs);
unreg_pnp:
@@ -355,6 +353,17 @@ static int __init serial8250_init(void)
out:
return ret;
}
+
+static int __init serial8250_init(void)
+{
+ if (ARRAY_SIZE(old_serial_port)) {
+ int ret = serial8250_isa_init();
+ if (ret)
+ return ret;
+ }
+
+ return platform_driver_register(&serial8250_platform_driver);
+}
module_init(serial8250_init);
static void __exit serial8250_exit(void)
@@ -368,7 +377,11 @@ static void __exit serial8250_exit(void)
*/
serial8250_isa_devs = NULL;
- platform_driver_unregister(&serial8250_isa_driver);
+ platform_driver_unregister(&serial8250_platform_driver);
+
+ if (!ARRAY_SIZE(old_serial_port))
+ return;
+
platform_device_unregister(isa_dev);
serial8250_pnp_exit();
