* [PATCH v1] xen/arm: Manage pl011 uart TX interrupt correctly
@ 2014-12-09 4:39 vijay.kilari
2014-12-11 12:15 ` Tim Deegan
2014-12-11 12:24 ` Ian Campbell
0 siblings, 2 replies; 6+ messages in thread
From: vijay.kilari @ 2014-12-09 4:39 UTC (permalink / raw)
To: Ian.Campbell, julien.grall, stefano.stabellini,
stefano.stabellini, tim, xen-devel
Cc: Prasun.Kapoor, manish.jaggi, Vijaya Kumar K, vijay.kilari
From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
In pl011.c, when TX interrupt is received
serial_tx_interrupt() is called to push next
characters. If TX buffer is empty, serial_tx_interrupt()
does not disable TX interrupt and hence pl011 UART
irq handler pl011_interrupt() always sees TX interrupt
status set in MIS register and cpu does not come out of
UART irq handler.
With this patch, mask TX interrupt by writing 0 to
IMSC register when TX buffer is empty and unmask by
writing 1 to IMSC register before sending characters.
Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
---
xen/drivers/char/pl011.c | 16 ++++++++++++++++
xen/drivers/char/serial.c | 34 ++++++++++++++++++++++++++++++++++
xen/include/xen/serial.h | 4 ++++
3 files changed, 54 insertions(+)
diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
index dd19ce8..57274d9 100644
--- a/xen/drivers/char/pl011.c
+++ b/xen/drivers/char/pl011.c
@@ -197,6 +197,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 +221,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..c583a48 100644
--- a/xen/drivers/char/serial.c
+++ b/xen/drivers/char/serial.c
@@ -31,6 +31,18 @@ static struct serial_port com[SERHND_IDX + 1] = {
static bool_t __read_mostly post_irq;
+static inline void serial_start_tx(struct serial_port *port)
+{
+ if ( port->driver->start_tx != NULL )
+ port->driver->start_tx(port);
+}
+
+static inline void serial_stop_tx(struct serial_port *port)
+{
+ if ( port->driver->stop_tx != NULL )
+ port->driver->stop_tx(port);
+}
+
void serial_rx_interrupt(struct serial_port *port, struct cpu_user_regs *regs)
{
char c;
@@ -76,6 +88,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 */
+ serial_stop_tx(port);
+ spin_unlock(&port->tx_lock);
+ goto out;
+ }
+ else
+ {
+ if ( port->driver->tx_ready(port) )
+ serial_start_tx(port);
+ }
for ( i = 0, n = port->driver->tx_ready(port); i < n; i++ )
{
if ( port->txbufc == port->txbufp )
@@ -117,6 +141,8 @@ static void __serial_putc(struct serial_port *port, char c)
cpu_relax();
if ( n > 0 )
{
+ /* Enable TX before sending chars */
+ serial_start_tx(port);
while ( n-- )
port->driver->putc(
port,
@@ -135,6 +161,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 */
+ serial_start_tx(port);
/* Buffer and UART FIFO are both empty, and port is available. */
port->driver->putc(port, c);
}
@@ -152,11 +180,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 */
+ serial_start_tx(port);
port->driver->putc(port, c);
+ }
}
else
{
/* Simple synchronous transmitter. */
+ serial_start_tx(port);
port->driver->putc(port, c);
}
}
@@ -404,6 +437,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;
+ serial_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 *);
};
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v1] xen/arm: Manage pl011 uart TX interrupt correctly
2014-12-09 4:39 [PATCH v1] xen/arm: Manage pl011 uart TX interrupt correctly vijay.kilari
@ 2014-12-11 12:15 ` Tim Deegan
2015-01-08 14:59 ` Ian Campbell
2014-12-11 12:24 ` Ian Campbell
1 sibling, 1 reply; 6+ messages in thread
From: Tim Deegan @ 2014-12-11 12:15 UTC (permalink / raw)
To: vijay.kilari
Cc: Ian.Campbell, stefano.stabellini, Prasun.Kapoor, manish.jaggi,
julien.grall, xen-devel, stefano.stabellini, Vijaya Kumar K
At 10:09 +0530 on 09 Dec (1418116195), vijay.kilari@gmail.com wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
>
> In pl011.c, when TX interrupt is received
> serial_tx_interrupt() is called to push next
> characters. If TX buffer is empty, serial_tx_interrupt()
> does not disable TX interrupt and hence pl011 UART
> irq handler pl011_interrupt() always sees TX interrupt
> status set in MIS register and cpu does not come out of
> UART irq handler.
>
> With this patch, mask TX interrupt by writing 0 to
> IMSC register when TX buffer is empty and unmask by
> writing 1 to IMSC register before sending characters.
>
> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
Reviewed-by: Tim Deegan <tim@xen.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] xen/arm: Manage pl011 uart TX interrupt correctly
2014-12-09 4:39 [PATCH v1] xen/arm: Manage pl011 uart TX interrupt correctly vijay.kilari
2014-12-11 12:15 ` Tim Deegan
@ 2014-12-11 12:24 ` Ian Campbell
2014-12-15 12:09 ` Julien Grall
1 sibling, 1 reply; 6+ messages in thread
From: Ian Campbell @ 2014-12-11 12:24 UTC (permalink / raw)
To: vijay.kilari
Cc: Keir Fraser, stefano.stabellini, Prasun.Kapoor, manish.jaggi,
julien.grall, tim, xen-devel, stefano.stabellini, Vijaya Kumar K
On Tue, 2014-12-09 at 10:09 +0530, vijay.kilari@gmail.com wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
>
> In pl011.c, when TX interrupt is received
> serial_tx_interrupt() is called to push next
> characters. If TX buffer is empty, serial_tx_interrupt()
> does not disable TX interrupt and hence pl011 UART
> irq handler pl011_interrupt() always sees TX interrupt
> status set in MIS register and cpu does not come out of
> UART irq handler.
>
> With this patch, mask TX interrupt by writing 0 to
> IMSC register when TX buffer is empty and unmask by
> writing 1 to IMSC register before sending characters.
>
> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
> ---
> xen/drivers/char/pl011.c | 16 ++++++++++++++++
> xen/drivers/char/serial.c | 34 ++++++++++++++++++++++++++++++++++
> xen/include/xen/serial.h | 4 ++++
These last two changes require that you cc the common serial maintainer,
not just the ARM maintainers. In this case that means Keir, who I have
CCd.
./scripts/get_maintainers.pl can help automate this.
> 3 files changed, 54 insertions(+)
>
> diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
> index dd19ce8..57274d9 100644
> --- a/xen/drivers/char/pl011.c
> +++ b/xen/drivers/char/pl011.c
> @@ -197,6 +197,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 +221,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..c583a48 100644
> --- a/xen/drivers/char/serial.c
> +++ b/xen/drivers/char/serial.c
> @@ -31,6 +31,18 @@ static struct serial_port com[SERHND_IDX + 1] = {
>
> static bool_t __read_mostly post_irq;
>
> +static inline void serial_start_tx(struct serial_port *port)
> +{
> + if ( port->driver->start_tx != NULL )
> + port->driver->start_tx(port);
> +}
> +
> +static inline void serial_stop_tx(struct serial_port *port)
> +{
> + if ( port->driver->stop_tx != NULL )
> + port->driver->stop_tx(port);
> +}
> +
> void serial_rx_interrupt(struct serial_port *port, struct cpu_user_regs *regs)
> {
> char c;
> @@ -76,6 +88,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 */
> + serial_stop_tx(port);
> + spin_unlock(&port->tx_lock);
> + goto out;
> + }
> + else
> + {
> + if ( port->driver->tx_ready(port) )
> + serial_start_tx(port);
> + }
> for ( i = 0, n = port->driver->tx_ready(port); i < n; i++ )
> {
> if ( port->txbufc == port->txbufp )
> @@ -117,6 +141,8 @@ static void __serial_putc(struct serial_port *port, char c)
> cpu_relax();
> if ( n > 0 )
> {
> + /* Enable TX before sending chars */
> + serial_start_tx(port);
> while ( n-- )
> port->driver->putc(
> port,
> @@ -135,6 +161,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 */
> + serial_start_tx(port);
> /* Buffer and UART FIFO are both empty, and port is available. */
> port->driver->putc(port, c);
> }
> @@ -152,11 +180,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 */
> + serial_start_tx(port);
> port->driver->putc(port, c);
> + }
> }
> else
> {
> /* Simple synchronous transmitter. */
> + serial_start_tx(port);
> port->driver->putc(port, c);
> }
> }
> @@ -404,6 +437,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;
> + serial_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 *);
> };
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] xen/arm: Manage pl011 uart TX interrupt correctly
2014-12-11 12:24 ` Ian Campbell
@ 2014-12-15 12:09 ` Julien Grall
0 siblings, 0 replies; 6+ messages in thread
From: Julien Grall @ 2014-12-15 12:09 UTC (permalink / raw)
To: Ian Campbell, vijay.kilari
Cc: Keir Fraser, stefano.stabellini, Prasun.Kapoor, manish.jaggi, tim,
xen-devel, stefano.stabellini, Vijaya Kumar K
On 11/12/14 12:24, Ian Campbell wrote:
> These last two changes require that you cc the common serial maintainer,
> not just the ARM maintainers. In this case that means Keir, who I have
> CCd.
> ./scripts/get_maintainers.pl can help automate this.
The serial code lives in the "THE REST" category. So I don't think it's
mandatory to have Keir's ack on this patch.
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] xen/arm: Manage pl011 uart TX interrupt correctly
2014-12-11 12:15 ` Tim Deegan
@ 2015-01-08 14:59 ` Ian Campbell
2015-01-20 14:46 ` Ian Campbell
0 siblings, 1 reply; 6+ messages in thread
From: Ian Campbell @ 2015-01-08 14:59 UTC (permalink / raw)
To: Tim Deegan
Cc: vijay.kilari, stefano.stabellini, Prasun.Kapoor, manish.jaggi,
julien.grall, xen-devel, stefano.stabellini, Vijaya Kumar K
On Thu, 2014-12-11 at 13:15 +0100, Tim Deegan wrote:
> At 10:09 +0530 on 09 Dec (1418116195), vijay.kilari@gmail.com wrote:
> > From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
> >
> > In pl011.c, when TX interrupt is received
> > serial_tx_interrupt() is called to push next
> > characters. If TX buffer is empty, serial_tx_interrupt()
> > does not disable TX interrupt and hence pl011 UART
> > irq handler pl011_interrupt() always sees TX interrupt
> > status set in MIS register and cpu does not come out of
> > UART irq handler.
> >
> > With this patch, mask TX interrupt by writing 0 to
> > IMSC register when TX buffer is empty and unmask by
> > writing 1 to IMSC register before sending characters.
> >
> > Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
>
> Reviewed-by: Tim Deegan <tim@xen.org>
Since Keir hasn't objected I've now applied, thanks.
Should this be a candidate for backporting (to 4.4 and/or 4.5)?
Ian.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] xen/arm: Manage pl011 uart TX interrupt correctly
2015-01-08 14:59 ` Ian Campbell
@ 2015-01-20 14:46 ` Ian Campbell
0 siblings, 0 replies; 6+ messages in thread
From: Ian Campbell @ 2015-01-20 14:46 UTC (permalink / raw)
To: Tim Deegan
Cc: vijay.kilari, stefano.stabellini, Prasun.Kapoor, manish.jaggi,
julien.grall, xen-devel, stefano.stabellini, Vijaya Kumar K
On Thu, 2015-01-08 at 14:59 +0000, Ian Campbell wrote:
> On Thu, 2014-12-11 at 13:15 +0100, Tim Deegan wrote:
> > At 10:09 +0530 on 09 Dec (1418116195), vijay.kilari@gmail.com wrote:
> > > From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
> > >
> > > In pl011.c, when TX interrupt is received
> > > serial_tx_interrupt() is called to push next
> > > characters. If TX buffer is empty, serial_tx_interrupt()
> > > does not disable TX interrupt and hence pl011 UART
> > > irq handler pl011_interrupt() always sees TX interrupt
> > > status set in MIS register and cpu does not come out of
> > > UART irq handler.
> > >
> > > With this patch, mask TX interrupt by writing 0 to
> > > IMSC register when TX buffer is empty and unmask by
> > > writing 1 to IMSC register before sending characters.
> > >
> > > Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
> >
> > Reviewed-by: Tim Deegan <tim@xen.org>
>
> Since Keir hasn't objected I've now applied, thanks.
>
> Should this be a candidate for backporting (to 4.4 and/or 4.5)?
It didn't cherry-pick cleanly and since none of the platforms supported
by 4.4 seemed affected I think we should skip it in that tree.
I'll consider it for 4.5 once that opens for backports.
Ian.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-01-20 14:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-09 4:39 [PATCH v1] xen/arm: Manage pl011 uart TX interrupt correctly vijay.kilari
2014-12-11 12:15 ` Tim Deegan
2015-01-08 14:59 ` Ian Campbell
2015-01-20 14:46 ` Ian Campbell
2014-12-11 12:24 ` Ian Campbell
2014-12-15 12:09 ` Julien Grall
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.