Ben Guthro
2013-Apr-23 19:34 UTC
[PATCH] ns16550: delay resume until dom0 ACPI has a chance to run (v2)
Check for ioport access, before fully resuming operation, to avoid
spinning in __ns16550_poll when reading the LSR register returns 0xFF
on failing ioport access.
On some systems (like Lenovo T410, and some HP machines of similar vintage)
there is a SuperIO card that provides this legacy ioport on the LPC bus.
In this case, we need to wait for dom0''s ACPI processing to run the
proper
AML to re-initialize the chip, before we can use the card again.
This may cause a small amount of garbage to be written to the serial log
while we wait patiently for that AML to be executed.
This implementation limits the number of retries, to avoid a situation
where we keep trying over and over again, in the case of some other failure
on the ioport.
v2: Formatting, comment adjustment
Signed-Off-By: Ben Guthro <benjamin.guthro@citrix.com>
---
xen/drivers/char/ns16550.c | 59 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 58 insertions(+), 1 deletion(-)
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index a91743e..7390874 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -45,6 +45,7 @@ static struct ns16550 {
struct irqaction irqaction;
/* UART with no IRQ line: periodically-polled I/O. */
struct timer timer;
+ struct timer resume_timer;
unsigned int timeout_ms;
bool_t intr_works;
/* PCI card parameters. */
@@ -123,6 +124,10 @@ static struct ns16550 {
/* Frequency of external clock source. This definition assumes PC platform. */
#define UART_CLOCK_HZ 1843200
+/* Resume retry settings */
+#define RESUME_DELAY MILLISECS(10)
+#define RESUME_RETRIES 100
+
static char ns_read_reg(struct ns16550 *uart, int reg)
{
if ( uart->remapped_io_base == NULL )
@@ -351,7 +356,7 @@ static void ns16550_suspend(struct serial_port *port)
uart->ps_bdf[2], PCI_COMMAND);
}
-static void ns16550_resume(struct serial_port *port)
+static void __ns16550_resume(struct serial_port *port)
{
struct ns16550 *uart = port->uart;
@@ -367,6 +372,58 @@ static void ns16550_resume(struct serial_port *port)
ns16550_setup_postirq(port->uart);
}
+static int ns16550_ioport_invalid(struct ns16550 *uart)
+{
+ return ((((unsigned char)ns_read_reg(uart, LSR)) == 0xff) &&
+ (((unsigned char)ns_read_reg(uart, MCR)) == 0xff) &&
+ (((unsigned char)ns_read_reg(uart, IER)) == 0xff) &&
+ (((unsigned char)ns_read_reg(uart, IIR)) == 0xff) &&
+ (((unsigned char)ns_read_reg(uart, LCR)) == 0xff));
+}
+
+static int delayed_resume_tries;
+static void ns16550_delayed_resume(void *data)
+{
+ struct serial_port *port = data;
+ struct ns16550 *uart = port->uart;
+
+ if ( ns16550_ioport_invalid(port->uart) &&
delayed_resume_tries-- )
+ {
+ set_timer(&uart->resume_timer, NOW() + RESUME_DELAY);
+ return;
+ }
+
+ __ns16550_resume(port);
+}
+
+static void ns16550_resume(struct serial_port *port)
+{
+ struct ns16550 *uart = port->uart;
+
+ /*
+ * Check for ioport access, before fully resuming operation.
+ * On some systems, there is a SuperIO card that provides
+ * this legacy ioport on the LPC bus.
+ *
+ * We need to wait for dom0''s ACPI processing to run the proper
+ * AML to re-initialize the chip, before we can use the card again.
+ *
+ * This may cause a small amount of garbage to be written
+ * to the serial log while we wait patiently for that AML to
+ * be executed. However, this is preferable to spinning in an
+ * infinite loop, as seen on a Lenovo T430, when serial was enabled.
+ */
+ if ( ns16550_ioport_invalid(uart) )
+ {
+ delayed_resume_tries = RESUME_RETRIES;
+ init_timer(&uart->resume_timer, ns16550_delayed_resume, port,
0);
+ set_timer(&uart->resume_timer, NOW() + RESUME_DELAY);
+ return;
+ }
+
+ __ns16550_resume(port);
+}
+
#ifdef CONFIG_X86
static void __init ns16550_endboot(struct serial_port *port)
{
--
1.7.9.5
Keir Fraser
2013-Apr-23 20:21 UTC
Re: [PATCH] ns16550: delay resume until dom0 ACPI has a chance to run (v2)
On 23/04/2013 20:34, "Ben Guthro" <benjamin.guthro@citrix.com> wrote:> Check for ioport access, before fully resuming operation, to avoid > spinning in __ns16550_poll when reading the LSR register returns 0xFF > on failing ioport access. > > On some systems (like Lenovo T410, and some HP machines of similar vintage) > there is a SuperIO card that provides this legacy ioport on the LPC bus. > > In this case, we need to wait for dom0''s ACPI processing to run the proper > AML to re-initialize the chip, before we can use the card again. > > This may cause a small amount of garbage to be written to the serial log > while we wait patiently for that AML to be executed. > > This implementation limits the number of retries, to avoid a situation > where we keep trying over and over again, in the case of some other failure > on the ioport. > > v2: Formatting, comment adjustment > > Signed-Off-By: Ben Guthro <benjamin.guthro@citrix.com>Acked-by: Keir Fraser <keir@xen.org>> --- > xen/drivers/char/ns16550.c | 59 > +++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 58 insertions(+), 1 deletion(-) > > diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c > index a91743e..7390874 100644 > --- a/xen/drivers/char/ns16550.c > +++ b/xen/drivers/char/ns16550.c > @@ -45,6 +45,7 @@ static struct ns16550 { > struct irqaction irqaction; > /* UART with no IRQ line: periodically-polled I/O. */ > struct timer timer; > + struct timer resume_timer; > unsigned int timeout_ms; > bool_t intr_works; > /* PCI card parameters. */ > @@ -123,6 +124,10 @@ static struct ns16550 { > /* Frequency of external clock source. This definition assumes PC platform. > */ > #define UART_CLOCK_HZ 1843200 > > +/* Resume retry settings */ > +#define RESUME_DELAY MILLISECS(10) > +#define RESUME_RETRIES 100 > + > static char ns_read_reg(struct ns16550 *uart, int reg) > { > if ( uart->remapped_io_base == NULL ) > @@ -351,7 +356,7 @@ static void ns16550_suspend(struct serial_port *port) > uart->ps_bdf[2], PCI_COMMAND); > } > > -static void ns16550_resume(struct serial_port *port) > +static void __ns16550_resume(struct serial_port *port) > { > struct ns16550 *uart = port->uart; > > @@ -367,6 +372,58 @@ static void ns16550_resume(struct serial_port *port) > ns16550_setup_postirq(port->uart); > } > > +static int ns16550_ioport_invalid(struct ns16550 *uart) > +{ > + return ((((unsigned char)ns_read_reg(uart, LSR)) == 0xff) && > + (((unsigned char)ns_read_reg(uart, MCR)) == 0xff) && > + (((unsigned char)ns_read_reg(uart, IER)) == 0xff) && > + (((unsigned char)ns_read_reg(uart, IIR)) == 0xff) && > + (((unsigned char)ns_read_reg(uart, LCR)) == 0xff)); > +} > + > +static int delayed_resume_tries; > +static void ns16550_delayed_resume(void *data) > +{ > + struct serial_port *port = data; > + struct ns16550 *uart = port->uart; > + > + if ( ns16550_ioport_invalid(port->uart) && delayed_resume_tries-- ) > + { > + set_timer(&uart->resume_timer, NOW() + RESUME_DELAY); > + return; > + } > + > + __ns16550_resume(port); > +} > + > +static void ns16550_resume(struct serial_port *port) > +{ > + struct ns16550 *uart = port->uart; > + > + /* > + * Check for ioport access, before fully resuming operation. > + * On some systems, there is a SuperIO card that provides > + * this legacy ioport on the LPC bus. > + * > + * We need to wait for dom0''s ACPI processing to run the proper > + * AML to re-initialize the chip, before we can use the card again. > + * > + * This may cause a small amount of garbage to be written > + * to the serial log while we wait patiently for that AML to > + * be executed. However, this is preferable to spinning in an > + * infinite loop, as seen on a Lenovo T430, when serial was enabled. > + */ > + if ( ns16550_ioport_invalid(uart) ) > + { > + delayed_resume_tries = RESUME_RETRIES; > + init_timer(&uart->resume_timer, ns16550_delayed_resume, port, 0); > + set_timer(&uart->resume_timer, NOW() + RESUME_DELAY); > + return; > + } > + > + __ns16550_resume(port); > +} > + > #ifdef CONFIG_X86 > static void __init ns16550_endboot(struct serial_port *port) > {