Skip to contentSkip to conversation

add polling support for ns16550

PATCH In review Alex Williamson 2 participants 3 messages Nov 23, 2005 – Nov 24, 2005

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 add polling support for ns16550

The patch below adds ac_timer based polling to the ns16550 UART
driver. This is useful when the interrupt line is not connected in hardware or the mechanism to enable it is not readily available in the hypervisor. Polling is only enabled when the UART IRQ is set to zero. Thanks,

Alex

---

xen/drivers/char/ns16550.c +54 -1
diff -r 378e1c58bcd2 xen/drivers/char/ns16550.c
@@ -30,6 +30,8 @@
     unsigned long io_base;   /* I/O port or memory-mapped I/O address. */
     char *remapped_io_base;  /* Remapped virtual address of mmap I/O.  */ 
     struct irqaction irqaction;
+    struct ac_timer timer;
+    unsigned int timeout;
 } ns16550_com[2] = { { 0 } };
 
 /* Register offsets */
@@ -121,6 +123,34 @@
     }
 }
 
+static void ns16550_timeout(void *data)
+{
+    struct serial_port *port = (struct serial_port *)data;
+    struct ns16550 *uart = port->uart;
+    struct cpu_user_regs *regs = guest_cpu_user_regs();
+    char lsr, ier;
+    int rx_cnt = port->tx_fifo_size;
+
+    /* interrupts must be disabled, this is likely to generate some */
+    ier = ns_read_reg(uart, IER);
+    if ( ier )
+        ns_write_reg(uart, IER, 0);
+
+    lsr = ns_read_reg(uart, LSR);
+
+    while ( (lsr & LSR_DR) && rx_cnt-- ) {
+        serial_rx_interrupt(port, regs);
+        lsr = ns_read_reg(uart, LSR);
+    }
+    if ( lsr & LSR_THRE )
+        serial_tx_interrupt(port, regs);
+
+    if ( ier )
+        ns_write_reg(uart, IER, ier);
+
+    set_ac_timer(&uart->timer, NOW() + MILLISECS(uart->timeout));
+}
+
 static int ns16550_tx_empty(struct serial_port *port)
 {
     struct ns16550 *uart = port->uart;
@@ -178,15 +208,38 @@
         port->tx_fifo_size = 16;
 }
 
+static void ns16550_set_timeout(struct serial_port *port)
+{
+    struct ns16550 *uart = port->uart;
+    unsigned int bits;
+
+    bits = uart->data_bits + uart->stop_bits;
+
+    if ( uart->parity != PARITY_NONE )
+        bits++;
+
+    bits = bits * port->tx_fifo_size;
+
+    /* uart timeout in ms */
+    uart->timeout = (1000 * bits) / uart->baud;
+}
+
 static void ns16550_init_postirq(struct serial_port *port)
 {
     struct ns16550 *uart = port->uart;
     int rc;
 
-    if ( uart->irq <= 0 )
+    if ( uart->irq < 0 )
         return;
 
     serial_async_transmit(port);
+
+    if ( uart->irq == 0 ) {
+        ns16550_set_timeout(port);
+        init_ac_timer(&uart->timer, ns16550_timeout, port, smp_processor_id());
+        set_ac_timer(&uart->timer, NOW() + MILLISECS(uart->timeout));
+        return;
+    }
 
     uart->irqaction.handler = ns16550_interrupt;
     uart->irqaction.name    = "ns16550";
#1 lore raw
Quoted 5 lines · Alex Williamson

The patch below adds ac_timer based polling to the ns16550 UART
driver. This is useful when the interrupt line is not connected in hardware or the mechanism to enable it is not readily available in the hypervisor. Polling is only enabled when the UART IRQ is set to zero. Thanks,

I reworked the patch a little and checked it in, thanks.

One thing I removed was the clear-and-reset of IER in the timeout handler. Given we write zero to IER in the preirq steup function, and also we do not set the master-enable bit in the MCR, I cannot see how you could end up accidentally fielding interrupts in the timeout handler. Did I miss something?

-- Keir

#2 lore raw
Quoted 9 lines · Keir Fraser
Quoted 5 lines · Alex Williamson

The patch below adds ac_timer based polling to the ns16550 UART
driver. This is useful when the interrupt line is not connected in hardware or the mechanism to enable it is not readily available in the hypervisor. Polling is only enabled when the UART IRQ is set to zero. Thanks,

I reworked the patch a little and checked it in, thanks.

Thanks!

Quoted 5 lines

One thing I removed was the clear-and-reset of IER in the timeout handler. Given we write zero to IER in the preirq steup function, and also we do not set the master-enable bit in the MCR, I cannot see how you could end up accidentally fielding interrupts in the timeout handler. Did I miss something?

It was mainly paranoia, but on ia64 we don't have a good way to hide
UARTs from the guest domains, so it's possible that a dom0 with 8250 support will enable those bits on the UART. All sorts of strange things happen when the hypervisor and a guest compete for access to a UART, but I thought I would at least avoid the hypervisor polling triggering the real interrupt. I'm ok with dropping it, the UART isn't really functional no matter what paranoia checks are added when multiple drivers are competing to use it. Thanks,

Alex