|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] xen/arm: uart interrupts handling
Hi Tim,
On Thu, Dec 4, 2014 at 9:10 PM, Tim Deegan <tim@xxxxxxx> wrote:
> At 13:51 +0000 on 04 Dec (1417697518), Julien Grall wrote:
>> On 04/12/14 03:50, Vijay Kilari wrote:
>> > Hi Tim,
>>
>> Hi Vijay,
>>
>> > I see that on uart interrupt, ICR is written to clear the all
>> > interrupts except TX, RX and RX timeout. With this, cpu always finds
>> > TX/RX is active and never
>> > comes out of the loop.
>>
>> FWIW, the PL011 serial code has been copied from the Linux drivers.
>>
>> Linux interrupt handler also clear all interrupts except TX, RX, RX
>> timeout. So do you see the issue on Linux?
>>
>> >
>> > With the below changes, TX, RX & RTI are cleared before handling this
>> > interrupts.
>> >
>> > Is my observation is correct?. If so I wonder how it is working on
>> > platforms that
>> > are using pl011. Without this for my cpu just keeps looping here.
>> >
>> > index fba0a55..d21bce3 100644
>> > --- a/xen/drivers/char/pl011.c
>> > +++ b/xen/drivers/char/pl011.c
>> > @@ -63,7 +63,7 @@ static void pl011_interrupt(int irq, void *data,
>> > struct cpu_user_regs *regs)
>> > {
>> > do
>> > {
>> > - pl011_write(uart, ICR, status & ~(TXI|RTI|RXI));
>> > + pl011_write(uart, ICR, status & (TXI|RTI|RXI));
>>
>>
>> This changes looks wrong to me. We want to clear the bit in status we
>> don't handle. Otherwise the interrupt will be fired in loop.
>
> Yes - even if we do want to explicitly clear the TXI|RTI|RXI bits, we
> must clear the others too!
>
>> If I'm not mistaken, TXI/RTI/RXI will be cleared when data is read or
>> write into the fifo. So we should not clear automatically.
>
> I've been looking at that and I think it's OK for the RX path -- we
> ought to keep calling serial_rx_interrupt() until we've drained the
> FIFO, at which point both RTI and RXI will be cleared. Certainly we
> shouldn't unconditionally clear RXI/RTI without somehow making sure
> that we're not leaving bytes in the FIFO.
>
> The TX path looks more suspect -- if the uart raises TXI and we have
> nothing buffered to send, then TXI won't get cleared.
> Still, I wonder why you're seeing this and other people haven't.
Yes, this is the behaviour that Iam seeing. In Linux, uart driver
masks TXI interrupt
in IMSC if buffer is empty. However in xen, this scenario is not
handled. This is the reason why cpu does not come out of uart irq
routine if TX interrupt is raised but buffer is empty.
I have added below changes to fix this on top of your suggested change
diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
index dd19ce8..ad48df3 100644
--- a/xen/drivers/char/pl011.c
+++ b/xen/drivers/char/pl011.c
@@ -109,6 +109,8 @@ static void __init pl011_init_preirq(struct
serial_port *port)
panic("pl011: No Baud rate configured\n");
uart->baud = (uart->clock_hz << 2) / divisor;
}
+ /* Trigger RX interrupt at 1/2 full, TX interrupt at 7/8 empty */
+ pl011_write(uart, IFLS, (2<<3 | 0));
/* This write must follow FBRD and IBRD writes. */
pl011_write(uart, LCR_H, (uart->data_bits - 5) << 5
| FEN
@@ -197,6 +199,20 @@ static const struct vuart_info
*pl011_vuart(struct serial_port *port)
return &uart->vuart;
}
+static void pl011_tx_stop(struct serial_port *port)
+{
+ struct pl011 *uart = port->uart;
+
+ pl011_write(uart, IMSC, pl011_read(uart, IMSC) & ~(TXI));
+}
+
+static void pl011_tx_start(struct serial_port *port)
+{
+ struct pl011 *uart = port->uart;
+
+ pl011_write(uart, IMSC, pl011_read(uart, IMSC) | (TXI));
+}
+
static struct uart_driver __read_mostly pl011_driver = {
.init_preirq = pl011_init_preirq,
.init_postirq = pl011_init_postirq,
@@ -207,6 +223,8 @@ static struct uart_driver __read_mostly pl011_driver = {
.putc = pl011_putc,
.getc = pl011_getc,
.irq = pl011_irq,
+ .start_tx = pl011_tx_start,
+ .stop_tx = pl011_tx_stop,
.vuart_info = pl011_vuart,
};
diff --git a/xen/drivers/char/serial.c b/xen/drivers/char/serial.c
index 44026b1..0f26d40 100644
--- a/xen/drivers/char/serial.c
+++ b/xen/drivers/char/serial.c
@@ -76,6 +76,18 @@ void serial_tx_interrupt(struct serial_port *port,
struct cpu_user_regs *regs)
cpu_relax();
}
+ if ( port->txbufc == port->txbufp )
+ {
+ /* Disable TX. nothing to send */
+ port->driver->stop_tx(port);
+ spin_unlock(&port->tx_lock);
+ goto out;
+ }
+ else
+ {
+ if ( port->driver->tx_ready(port) )
+ port->driver->start_tx(port);
+ }
for ( i = 0, n = port->driver->tx_ready(port); i < n; i++ )
{
if ( port->txbufc == port->txbufp )
@@ -117,6 +129,8 @@ static void __serial_putc(struct serial_port *port, char c)
cpu_relax();
if ( n > 0 )
{
+ /* Enable TX before sending chars */
+ port->driver->start_tx(port);
while ( n-- )
port->driver->putc(
port,
@@ -135,6 +149,8 @@ static void __serial_putc(struct serial_port *port, char c)
if ( ((port->txbufp - port->txbufc) == 0) &&
port->driver->tx_ready(port) > 0 )
{
+ /* Enable TX before sending chars */
+ port->driver->start_tx(port);
/* Buffer and UART FIFO are both empty, and port is available. */
port->driver->putc(port, c);
}
@@ -152,11 +168,16 @@ static void __serial_putc(struct serial_port
*port, char c)
while ( !(n = port->driver->tx_ready(port)) )
cpu_relax();
if ( n > 0 )
+ {
+ /* Enable TX before sending chars */
+ port->driver->start_tx(port);
port->driver->putc(port, c);
+ }
}
else
{
/* Simple synchronous transmitter. */
+ port->driver->start_tx(port);
port->driver->putc(port, c);
}
}
@@ -404,6 +425,7 @@ void serial_start_sync(int handle)
/* port is unavailable and might not come up until reenabled by
dom0, we can't really do proper sync */
break;
+ port->driver->start_tx(port);
port->driver->putc(
port, port->txbuf[mask_serial_txbuf_idx(port->txbufc++)]);
}
diff --git a/xen/include/xen/serial.h b/xen/include/xen/serial.h
index 9f4451b..71e6ade 100644
--- a/xen/include/xen/serial.h
+++ b/xen/include/xen/serial.h
@@ -81,6 +81,10 @@ struct uart_driver {
int (*getc)(struct serial_port *, char *);
/* Get IRQ number for this port's serial line: returns -1 if none. */
int (*irq)(struct serial_port *);
+ /* Unmask TX interrupt */
+ void (*start_tx)(struct serial_port *);
+ /* Mask TX interrupt */
+ void (*stop_tx)(struct serial_port *);
/* Get serial information */
const struct vuart_info *(*vuart_info)(struct serial_port *);
};
-Vijay
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |