Xen devel - Oct 2011 - [PATCH] ns16550: fix poll handling regression

Prior to c/s 23811:f1349a968a5a LSR_THRE was checked only once, while
there it got promoted into the surrounding loop''s condition. Since that
bit may not clear for an extended period of time (i.e. when no new
output is generated), it must not be used in this way indefinitely.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -157,15 +157,18 @@ static void __ns16550_poll(struct cpu_us
 {
     struct serial_port *port = this_cpu(poll_port);
     struct ns16550 *uart = port->uart;
-    char lsr;
+    unsigned char lsr, mask = LSR_DR | LSR_THRE;
 
     if ( uart->intr_works )
         return; /* Interrupts work - no more polling */
 
-    while ( (lsr = ns_read_reg(uart, LSR)) & (LSR_DR|LSR_THRE) )
+    while ( (lsr = ns_read_reg(uart, LSR)) & mask )
     {
         if ( lsr & LSR_THRE )
+        {
             serial_tx_interrupt(port, regs);
+            mask &= ~LSR_THRE;
+        }
         if ( lsr & LSR_DR )
             serial_rx_interrupt(port, regs);
     }




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 12/10/2011 16:39, "Jan Beulich" <JBeulich@suse.com> wrote:
> Prior to c/s 23811:f1349a968a5a LSR_THRE was checked only once, while
> there it got promoted into the surrounding loop''s condition. Since
that
> bit may not clear for an extended period of time (i.e. when no new
> output is generated), it must not be used in this way indefinitely.
That particular part of 23811 was gratuitous on my part, as well as being
wrong. I''ve simply reverted it, as c/s 23948. Thanks for spotting it or
tracking it down!

 -- Keir
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -157,15 +157,18 @@ static void __ns16550_poll(struct cpu_us
>  {
>      struct serial_port *port = this_cpu(poll_port);
>      struct ns16550 *uart = port->uart;
> -    char lsr;
> +    unsigned char lsr, mask = LSR_DR | LSR_THRE;
>  
>      if ( uart->intr_works )
>          return; /* Interrupts work - no more polling */
>  
> -    while ( (lsr = ns_read_reg(uart, LSR)) & (LSR_DR|LSR_THRE) )
> +    while ( (lsr = ns_read_reg(uart, LSR)) & mask )
>      {
>          if ( lsr & LSR_THRE )
> +        {
>              serial_tx_interrupt(port, regs);
> +            mask &= ~LSR_THRE;
> +        }
>          if ( lsr & LSR_DR )
>              serial_rx_interrupt(port, regs);
>      }
> 
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel