serial legacy ports and ACPI

Hi,

I am trying to come up with a solution to tell the serial driver to not try and probe for serial ports on legacy IO addresses. While the current code seems to work on SN2 it's a bit of a pain on systems which do not come with a stack of 16550's glued to the motherboard.

Looking at the ACPI spec, it seems reasonable to me to disable this probe if the FADT doesn't have the BAF_LEGACY_DEVICES bit set. Anyone having oppinions on this?

I am proposing the below patch to solve the problem (relative to 2.4.20). It works for me on SN2, but I don't know what other platforms set in their FADT.

Cheers,
Jes

@@ -230,6 +230,9 @@
 #ifdef CONFIG_MAGIC_SYSRQ
 #include <linux/sysrq.h>
 #endif
+#ifdef CONFIG_ACPI
+#include <linux/acpi.h>
+#endif
 
 /*
  * All of the compatibilty code so we can compile serial.c against

@@ -5524,6 +5527,10 @@
 {
 	int i;
 	struct serial_state * state;
+#ifdef CONFIG_ACPI
+	acpi_buffer acpi_buf;
+	unsigned int acpi_status;
+#endif
 
 	init_bh(SERIAL_BH, do_serial_bh);
 	init_timer(&serial_timer);

@@ -5624,7 +5631,24 @@
 		panic("Couldn't register serial driver\n");
 	if (tty_register_driver(&callout_driver))
 		panic("Couldn't register callout driver\n");
-	
+
+#ifdef CONFIG_ACPI
+	acpi_buf.pointer = NULL;
+	acpi_buf.length = ACPI_ALLOCATE_BUFFER;
+
+	acpi_status = acpi_get_table(ACPI_TABLE_FADT, 1, &acpi_buf);
+	if (!acpi_status) {
+		fadt_descriptor_rev2 *fadt;
+		u16 iapc_boot_arch;
+
+		fadt = acpi_buf.pointer;
+		iapc_boot_arch = fadt->iapc_boot_arch;
+		kfree(acpi_buf.pointer);
+
+		if (!iapc_boot_arch & BAF_LEGACY_DEVICES)
+			goto skip_legacy_probe;
+	}
+#endif
 	for (i = 0, state = rs_table; i < NR_PORTS; i++,state++) {
 		state->magic = SSTATE_MAGIC;
 		state->line = i;

@@ -5677,6 +5701,9 @@
 		tty_register_devfs(&callout_driver, 0,
 				   callout_driver.minor_start + state->line);
 	}
+#ifdef CONFIG_ACPI
+ skip_legacy_probe:
+#endif
 #ifdef CONFIG_SERIAL_GSC
 	probe_serial_gsc();
 #endif

I like the idea of this, but
1) You might pass it by the ACPI folks to make sure this is the intent of the spec (sure looks reasonable to me, though).
2) I don't really like the implementation in the sense that
a) The spec suggests that LEGACY_DEVICES might also cover
things other than serial ports, i.e., a legacy LPT port,
so:
b) I think the check should go in arch/.../acpi.c. We did something similar for keyboard detection (see acpi_kbd_controller_present), and I think it worked out pretty well.

Bjorn

Bjorn> I like the idea of this, but 1) You might pass it by the ACPI Bjorn> folks to make sure this is the intent of the spec (sure looks Bjorn> reasonable to me, though). 2) I don't really like the Bjorn> implementation in the sense that a) The spec suggests that Bjorn> LEGACY_DEVICES might also cover things other than serial ports, Bjorn> i.e., a legacy LPT port, so: b) I think the check should go in Bjorn> arch/.../acpi.c. We did something similar for keyboard Bjorn> detection (see acpi_kbd_controller_present), and I think it Bjorn> worked out pretty well.

Hi Bjorn,

I had a chat with Grant and Matthew about is a little while ago (and possibly a few other people whom I can't remember right now) and we basically came to a similar conclusion. Once I find a bit more time, I'd like to introduce an interface along the lines of

legacy_device_present(LEGACY_{IDE,SERIAL,PARALLEL})

This could be quite useful for other architectures as well I believe.

Cheers,
Jes

Jes,

Digging up an old topic. I started looking at this from a serial
port numbering perspective and came up with a fairly simple way for the architecure code to handle it on it's own. Here's my idea vs 2.6:

@@ -41,6 +41,9 @@
 #include <linux/irq.h>
 #include <linux/acpi.h>
 #include <linux/efi.h>
+#include <linux/tty.h>
+#include <linux/serial.h>
+#include <linux/serial_core.h>
 #include <asm/io.h>
 #include <asm/iosapic.h>
 #include <asm/machvec.h>

@@ -530,6 +533,20 @@
        if (!(fadt->iapc_boot_arch & BAF_8042_KEYBOARD_CONTROLLER))
                acpi_kbd_controller_present = 0;
 
+#ifdef CONFIG_SERIAL_8250
+       if (!(fadt->iapc_boot_arch & BAF_LEGACY_DEVICES)) {
+               int i;
+               struct uart_port port;
+
+               memset(&port, 0, sizeof(port));
+
+               for (i = 0 ; i < 4 ; i++) {
+                       port.line = i;
+                       early_serial_setup(&port);
+               }
+       }
+#endif
+
        acpi_register_irq(fadt->sci_int, ACPI_ACTIVE_LOW, ACPI_LEVEL_SENSITIVE);
        return 0;
 }

This example only hits the first 4 ports, but we could clear the whole
table if we decide that's the right thing to do. Does anybody really use EXTRA_SERIAL_PORT_DEFNS and HUB6_SERIAL_PORT_DFNS or should those be considered "legacy" as well? The serial port part of this problem kind of feels like an architecture problem since we start out passing in a table of serial ports to probe. Thoughts?

Alex

Alex Williamson                             HP Linux & Open Source Lab