Re: [Xen-devel] [PATCH] ns16550: delay resume until dom0 ACPI has a chance to run

[Pasi Kärkkäinen <pasik@xxxxxx>]

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 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.
> 
> It 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.
> 

So I assume this patch fixes the ACPI S3 suspend/resume on the Lenovo T430/T530 
laptops?
It might be good to mention that aswell..

-- Pasi


> Signed-Off-By: Ben Guthro <benjamin.guthro@xxxxxxxxxx>
> ---
>  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 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 )
> @@ -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 = 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)) == 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.
> +     */
> +    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
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel