From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Guthro Subject: Re: [PATCH] ns16550: delay resume until dom0 ACPI has a chance to run Date: Wed, 16 Jan 2013 16:31:43 -0500 Message-ID: <50F71C3F.3040105@citrix.com> References: <1358367656-5333-1-git-send-email-benjamin.guthro@citrix.com> <20130116212414.GT8912@reaktio.net> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1"; Format="flowed" Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <20130116212414.GT8912@reaktio.net> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: =?ISO-8859-1?Q?Pasi_K=E4rkk=E4inen?= Cc: "xen-devel@lists.xen.org" , Ben Guthro List-Id: xen-devel@lists.xenproject.org On 01/16/2013 04:24 PM, Pasi K=E4rkk=E4inen wrote: > On Wed, Jan 16, 2013 at 03:20:56PM -0500, Ben Guthro 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, 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 prop= er >> 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. >> >> It limits the number of retries, to avoid a situation where we keep tryi= ng >> over and over again, in the case of some other failure on the ioport. >> > > So I assume this patch fixes the ACPI S3 suspend/resume on the Lenovo T43= 0/T530 laptops? > It might be good to mention that aswell.. Yes, it seems to resolve the issues seen on T430, T530, and Intel = Ivybridge mobile SDP. It likely fixes others, as the Intel mobile SDP is = the reference platform for all the OEMs. If I need to re-submit this patch for any other reason, I will be sure = to mention this in the comment. That said - there seems to still be an issue on some older Sandy Bridge = machines, like a Lenovo T520, and X220 - but I don't yet know if it is = related. Ben > > -- Pasi > > >> Signed-Off-By: Ben Guthro >> --- >> xen/drivers/char/ns16550.c | 56 ++++++++++++++++++++++++++++++++++++= +++++++- >> 1 file changed, 55 insertions(+), 1 deletion(-) >> >> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c >> index d77042e..27555c7 100644 >> --- a/xen/drivers/char/ns16550.c >> +++ b/xen/drivers/char/ns16550.c >> @@ -42,6 +42,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. */ >> @@ -120,6 +121,10 @@ static struct ns16550 { >> /* Frequency of external clock source. This definition assumes PC plat= form. */ >> #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 =3D=3D NULL ) >> @@ -330,7 +335,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 =3D port->uart; >> >> @@ -346,6 +351,55 @@ 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)) =3D=3D 0xff) && >> + (((unsigned char)ns_read_reg(uart, MCR)) =3D=3D 0xff) && >> + (((unsigned char)ns_read_reg(uart, IER)) =3D=3D 0xff) && >> + (((unsigned char)ns_read_reg(uart, IIR)) =3D=3D 0xff) && >> + (((unsigned char)ns_read_reg(uart, LCR)) =3D=3D 0xff)); >> +} >> + >> +static int delayed_resume_tries; >> +static void ns16550_delayed_resume(void *data) >> +{ >> + struct serial_port *port =3D data; >> + struct ns16550 *uart =3D 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 =3D 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. >> + */ >> + if (ns16550_ioport_invalid(uart)) { >> + delayed_resume_tries =3D 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 >> >> >> _______________________________________________ >> Xen-devel mailing list >> Xen-devel@lists.xen.org >> http://lists.xen.org/xen-devel