On Wed, 17 Mar 2010 13:01:59 +0000
Alan Cox <alan@xxxxxxxxxxxxxxx> wrote:
> On Wed, 17 Mar 2010 13:30:50 +0100
> Simon Kagstrom <simon.kagstrom@xxxxxxxxxxxxxx> wrote:
>
> > Port 0x80 is not safe to use on all x86 boards (see
> > arch/x86/kernel/io_delay.c), so use native_io_delay instead.
> >
> > Signed-off-by: Simon Kagstrom <simon.kagstrom@xxxxxxxxxxxxxx>
>
> native_io_delay() won't work if the system is being run with no delays.
> The I/O cycle isn't for the delay but to force the bus signals. So in
> various modes (paravirt, udelay, no delay) the native_io_delay won't
> actually do what is required.
You are right, I should have seen that.
Would something similar to the other patch be acceptable, i.e., like
the diff below?
> I'm actually surprised you hit this path and if anything the right fix
> is to avoid hitting this kind of probing in the first place.
But isn't this code path pretty much always being executed? If I read
the code correct, unless we have a buggy UART it will be executed if
UPF_BOOT_AUTOCONF is set.
// Simon
diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
index 524f6ab..c5e3f9a 100644
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -38,6 +38,7 @@
#include <linux/serial_8250.h>
#include <linux/nmi.h>
#include <linux/mutex.h>
+#include <linux/io.h>
#include <asm/io.h>
#include <asm/irq.h>
@@ -1071,6 +1072,19 @@ static void autoconfig_16550a(struct uart_8250_port *up)
serial_outp(up, UART_IER, iersave);
}
+static void bus_delay(u8 val)
+{
+#ifdef __i386__
+# ifdef CONFIG_IO_DELAY_TYPE_0XED
+ const u16 io_port = 0xed;
+# else
+ const u16 io_port = 0x80;
+#endif
+
+ outb(0xff, io_port);
+#endif
+}
+
/*
* This routine is called by rs_init() to initialize a specific serial
* port. It determines what type of UART chip this serial port is
@@ -1104,29 +1118,24 @@ static void autoconfig(struct uart_8250_port *up, unsigned int probeflags)
* Do a simple existence test first; if we fail this,
* there's no point trying anything else.
*
- * 0x80 is used as a nonsense port to prevent against
- * false positives due to ISA bus float. The
- * assumption is that 0x80 is a non-existent port;
- * which should be safe since include/asm/io.h also
- * makes this assumption.
+ * The IO delay is used to prevent against false positives
+ * due to ISA bus float.
*
* Note: this is safe as long as MCR bit 4 is clear
* and the device is in "PC" mode.
*/
scratch = serial_inp(up, UART_IER);
serial_outp(up, UART_IER, 0);
-#ifdef __i386__
- outb(0xff, 0x080);
-#endif
+ bus_delay(0xff);
+
/*
* Mask out IER[7:4] bits for test as some UARTs (e.g. TL
* 16C754B) allow only to modify them if an EFR bit is set.
*/
scratch2 = serial_inp(up, UART_IER) & 0x0f;
serial_outp(up, UART_IER, 0x0F);
-#ifdef __i386__
- outb(0, 0x080);
-#endif
+ bus_delay(0x0);
+
scratch3 = serial_inp(up, UART_IER) & 0x0f;
serial_outp(up, UART_IER, scratch);
if (scratch2 != 0 || scratch3 != 0x0F) {
[simkag@marrow kernel]$
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
