From mboxrd@z Thu Jan 1 00:00:00 1970 From: Malcolm Crossley Subject: Re: [PATCH] ns16550: delay resume until dom0 ACPI has a chance to run Date: Wed, 16 Jan 2013 21:40:40 +0000 Message-ID: <50F71E58.4000004@citrix.com> References: <1358367656-5333-1-git-send-email-benjamin.guthro@citrix.com> <20130116212414.GT8912@reaktio.net> <50F71C3F.3040105@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1"; Format="flowed" Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <50F71C3F.3040105@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 16/01/13 21:31, Ben Guthro wrote: > > 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 iopo= rt >>> on the LPC bus. >>> >>> In this case, we need to wait for dom0's ACPI processing to run the pro= per >>> 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 try= ing >>> 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 T4= 30/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. Do these laptops (T430/T530) have built in serial? Or is the hang fixed because you were using a serial Express card to debug? BTW, nearly all PC's have external SUPERIO devices, it just seems we = have managed to be lucky that most platforms seem to default to using = the BIOS to enable the serial port after resume instead of the OS. Malcolm > 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 pl= atform. */ >>> #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 *por= t) >>> 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 *por= t) >>> 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 > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel