Add IRQ support to the LiteX LiteUART serial interface
Changes from v5:
- picked up r/b Geert Uytterhoeven on select subset of commits
- 09/14 ("clean up rx loop variables"):
- eliminate `status` variable altogether as its value is known
and constant inside the loop;
- switch `ch` to `u8`, to match the call *producing* it
(`litex_read8()`) instead of those consuming its value.
- 12/14 ("add IRQ support for the RX path"):
- s/dev_err()/dev_warn()/ when falling back to polling in
`liteuart_startup()`
- use `spin_[lock_irqsave|unlock_irqrestore]()` in the interrupt
handler, since the same code may also be called in "serving_softirq"
context when using a poll timer instead of interrupts
> Changes from v4:
> - picked up r/b Ilpo Järvinen on 12/14 and 13/14
> - 12/14 ("add IRQ support for the RX path"): using dev_err() instead
> of combining pr_err() and pr_fmt(); also, remove superfluous comment
>
> Changes from v3:
> - picked up r/b Ilpo Järvinen on select subset of commits
> - rebased entire series on top of tty-next tree
> - 2/14 ("use bit number macros"): explicitly include <linux/bits.h>
> - 3/14 ("remove unused uart_ops stubs"): don't add gratuitous comment
> removed later on in the series
> - 12/14 ("add IRQ support for the RX path"): add shadow irq register
> to support polling mode and avoid reading hardware mmio irq register
> to learn which irq flags are enabled
> - this also simplifies both liteuart_interrupt() and liteuart_startup()
> - 13/14 ("add IRQ support for the TX path"):
> - removed superfluous curly braces from liteuart_interrupt()
> - simplified [start|stop]_tx() by using shadow irq register from 12/14
> - simplified liteuart_tx_chars() by rebasing on top of tty-next tree
>
> Changes from v2:
> - further split out "separate RX loop from poll timer" into
> dedicated patches highlighting additional changes explicitly:
> - factoring out tty_flip_buffer_push() (6/14)
> - ack only RX events in RX loop (7/14)
> - pass constant flag to uart_insert_char() directly (8/14)
> - fix variable types in rx loop (9/14)
> - separating RX loop from poll timer (10/14)
> - added patch (11/14) to move function definitions to a more
> convenient location, making subsequent changes easier to read
> in diff patch form.
> - fixes and clarifications for RX path IRQ support patch (now 12/14):
> - only return IRQ_HANDLED for RX events
> - use pr_fmt() to improve style of irq setup error message
> - remove unnecessary locking from around single-register access
> - modify TX path to support both IRQ-mode and polling (13/14)
> - move polling-only liteuart_putchar() behind same conditional
> (CONFIG_SERIAL_LITEUART_CONSOLE) as the rest of the code that's
> still using it.
>
> Changes from v1:
> - split minor cosmetic changes out into individual patches
> (1/3 became 1..5/7)
> - patches 6/7 and 7/7 unchanged (used to be 2/3 and 3/3)
Gabriel Somlo (14):
serial: liteuart: use KBUILD_MODNAME as driver name
serial: liteuart: use bit number macros
serial: liteuart: remove unused uart_ops stubs
serial: liteuart: don't set unused port fields
serial: liteuart: minor style fix in liteuart_init()
serial: liteuart: move tty_flip_buffer_push() out of rx loop
serial: liteuart: rx loop should only ack rx events
serial: liteuart: simplify passing of uart_insert_char() flag
serial: liteuart: clean up rx loop variables
serial: liteuart: separate rx loop from poll timer
serial: liteuart: move function definitions
serial: liteuart: add IRQ support for the RX path
serial: liteuart: add IRQ support for the TX path
serial: liteuart: move polling putchar() function
drivers/tty/serial/liteuart.c | 217 +++++++++++++++++++++-------------
1 file changed, 132 insertions(+), 85 deletions(-)
--
2.38.1
Signed-off-by: Gabriel Somlo <[email protected]>
Reviewed-by: Ilpo Järvinen <[email protected]>
Reviewed-by: Geert Uytterhoeven <[email protected]>
---
drivers/tty/serial/liteuart.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/liteuart.c b/drivers/tty/serial/liteuart.c
index c6eb7eba5af8..1e3429bcc2ad 100644
--- a/drivers/tty/serial/liteuart.c
+++ b/drivers/tty/serial/liteuart.c
@@ -397,12 +397,10 @@ static int __init liteuart_init(void)
return res;
res = platform_driver_register(&liteuart_platform_driver);
- if (res) {
+ if (res)
uart_unregister_driver(&liteuart_driver);
- return res;
- }
- return 0;
+ return res;
}
static void __exit liteuart_exit(void)
--
2.38.1
Add support for IRQ-driven RX. Support for the TX path will be added
in a separate commit.
Signed-off-by: Gabriel Somlo <[email protected]>
Reviewed-by: Ilpo Järvinen <[email protected]>
---
Changes from v5:
- s/dev_err/dev_warn/ when falling back to polling in `liteuart_startup()`
- use `spin_[lock_irqsave|unlock_irqrestore]()` in the interrupt handler,
since the same code may also be called in "serving_softirq" context when
using a poll timer instead of hardware interrupts
> Changes from v4:
> - using dev_err() instead of a combo of pr_err() and pr_fmt()
> - dropped "get irq" comment in probe()
>
> Changes from v3:
> - add shadow irq register to support polling mode and avoid reading
> hardware mmio irq register to learn which irq flags are enabled
> - this also simplifies both liteuart_interrupt() and liteuart_startup()
drivers/tty/serial/liteuart.c | 81 ++++++++++++++++++++++++++++++++---
1 file changed, 74 insertions(+), 7 deletions(-)
diff --git a/drivers/tty/serial/liteuart.c b/drivers/tty/serial/liteuart.c
index 0b9d96e5efcf..8685c97d391e 100644
--- a/drivers/tty/serial/liteuart.c
+++ b/drivers/tty/serial/liteuart.c
@@ -7,6 +7,7 @@
#include <linux/bits.h>
#include <linux/console.h>
+#include <linux/interrupt.h>
#include <linux/litex.h>
#include <linux/module.h>
#include <linux/of.h>
@@ -46,6 +47,7 @@ struct liteuart_port {
struct uart_port port;
struct timer_list timer;
u32 id;
+ u8 irq_reg;
};
#define to_liteuart_port(port) container_of(port, struct liteuart_port, port)
@@ -76,6 +78,19 @@ static void liteuart_putchar(struct uart_port *port, unsigned char ch)
litex_write8(port->membase + OFF_RXTX, ch);
}
+static void liteuart_update_irq_reg(struct uart_port *port, bool set, u8 mask)
+{
+ struct liteuart_port *uart = to_liteuart_port(port);
+
+ if (set)
+ uart->irq_reg |= mask;
+ else
+ uart->irq_reg &= ~mask;
+
+ if (port->irq)
+ litex_write8(port->membase + OFF_EV_ENABLE, uart->irq_reg);
+}
+
static void liteuart_stop_tx(struct uart_port *port)
{
}
@@ -129,13 +144,32 @@ static void liteuart_rx_chars(struct uart_port *port)
tty_flip_buffer_push(&port->state->port);
}
+static irqreturn_t liteuart_interrupt(int irq, void *data)
+{
+ struct liteuart_port *uart = data;
+ struct uart_port *port = &uart->port;
+ unsigned long flags;
+ u8 isr;
+
+ /*
+ * if polling, the context would be "in_serving_softirq", so use
+ * irq[save|restore] spin_lock variants to cover all possibilities
+ */
+ spin_lock_irqsave(&port->lock, flags);
+ isr = litex_read8(port->membase + OFF_EV_PENDING) & uart->irq_reg;
+ if (isr & EV_RX)
+ liteuart_rx_chars(port);
+ spin_unlock_irqrestore(&port->lock, flags);
+
+ return IRQ_RETVAL(isr);
+}
+
static void liteuart_timer(struct timer_list *t)
{
struct liteuart_port *uart = from_timer(uart, t, timer);
struct uart_port *port = &uart->port;
- liteuart_rx_chars(port);
-
+ liteuart_interrupt(0, port);
mod_timer(&uart->timer, jiffies + uart_poll_timeout(port));
}
@@ -161,19 +195,46 @@ static unsigned int liteuart_get_mctrl(struct uart_port *port)
static int liteuart_startup(struct uart_port *port)
{
struct liteuart_port *uart = to_liteuart_port(port);
+ unsigned long flags;
+ int ret;
- /* disable events */
- litex_write8(port->membase + OFF_EV_ENABLE, 0);
+ if (port->irq) {
+ ret = request_irq(port->irq, liteuart_interrupt, 0,
+ KBUILD_MODNAME, uart);
+ if (ret) {
+ dev_warn(port->dev,
+ "line %d irq %d failed: switch to polling\n",
+ port->line, port->irq);
+ port->irq = 0;
+ }
+ }
- /* prepare timer for polling */
- timer_setup(&uart->timer, liteuart_timer, 0);
- mod_timer(&uart->timer, jiffies + uart_poll_timeout(port));
+ spin_lock_irqsave(&port->lock, flags);
+ /* only enabling rx irqs during startup */
+ liteuart_update_irq_reg(port, true, EV_RX);
+ spin_unlock_irqrestore(&port->lock, flags);
+
+ if (!port->irq) {
+ timer_setup(&uart->timer, liteuart_timer, 0);
+ mod_timer(&uart->timer, jiffies + uart_poll_timeout(port));
+ }
return 0;
}
static void liteuart_shutdown(struct uart_port *port)
{
+ struct liteuart_port *uart = to_liteuart_port(port);
+ unsigned long flags;
+
+ spin_lock_irqsave(&port->lock, flags);
+ liteuart_update_irq_reg(port, false, EV_RX | EV_TX);
+ spin_unlock_irqrestore(&port->lock, flags);
+
+ if (port->irq)
+ free_irq(port->irq, port);
+ else
+ del_timer_sync(&uart->timer);
}
static void liteuart_set_termios(struct uart_port *port, struct ktermios *new,
@@ -262,6 +323,12 @@ static int liteuart_probe(struct platform_device *pdev)
goto err_erase_id;
}
+ ret = platform_get_irq_optional(pdev, 0);
+ if (ret < 0 && ret != -ENXIO)
+ return ret;
+ if (ret > 0)
+ port->irq = ret;
+
/* values not from device tree */
port->dev = &pdev->dev;
port->iotype = UPIO_MEM;
--
2.38.1
The `status` variable will always be `1` when passed into the call
to `uart_insert_char()`, so it can be eliminated altogether.
Use `u8` as the type for `ch`, as it matches the return type of
the `litex_read8()` call which produces its value.
Signed-off-by: Gabriel Somlo <[email protected]>
Reviewed-by: Ilpo Järvinen <[email protected]>
---
Changes from v5:
- removing `status` variable altogether
- using `u8` for `ch`, matching the type of the call producing it
rather than that of the calls that consume its value
drivers/tty/serial/liteuart.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/liteuart.c b/drivers/tty/serial/liteuart.c
index 81aa7c1da73c..62bfd2ed9051 100644
--- a/drivers/tty/serial/liteuart.c
+++ b/drivers/tty/serial/liteuart.c
@@ -73,10 +73,9 @@ static void liteuart_timer(struct timer_list *t)
struct liteuart_port *uart = from_timer(uart, t, timer);
struct uart_port *port = &uart->port;
unsigned char __iomem *membase = port->membase;
- int ch;
- unsigned long status;
+ u8 ch;
- while ((status = !litex_read8(membase + OFF_RXEMPTY)) == 1) {
+ while (!litex_read8(membase + OFF_RXEMPTY)) {
ch = litex_read8(membase + OFF_RXTX);
port->icount.rx++;
@@ -85,7 +84,7 @@ static void liteuart_timer(struct timer_list *t)
/* no overflow bits in status */
if (!(uart_handle_sysrq_char(port, ch)))
- uart_insert_char(port, status, 0, ch, TTY_NORMAL);
+ uart_insert_char(port, 1, 0, ch, TTY_NORMAL);
}
tty_flip_buffer_push(&port->state->port);
--
2.38.1
Remove stub uart_ops methods that are not called unconditionally
from serial_core.
Signed-off-by: Gabriel Somlo <[email protected]>
Reviewed-by: Ilpo Järvinen <[email protected]>
Reviewed-by: Geert Uytterhoeven <[email protected]>
---
drivers/tty/serial/liteuart.c | 17 -----------------
1 file changed, 17 deletions(-)
diff --git a/drivers/tty/serial/liteuart.c b/drivers/tty/serial/liteuart.c
index 18c1eb315ee9..989a4f8d5bd4 100644
--- a/drivers/tty/serial/liteuart.c
+++ b/drivers/tty/serial/liteuart.c
@@ -154,11 +154,6 @@ static void liteuart_stop_rx(struct uart_port *port)
del_timer(&uart->timer);
}
-static void liteuart_break_ctl(struct uart_port *port, int break_state)
-{
- /* LiteUART doesn't support sending break signal */
-}
-
static int liteuart_startup(struct uart_port *port)
{
struct liteuart_port *uart = to_liteuart_port(port);
@@ -197,15 +192,6 @@ static const char *liteuart_type(struct uart_port *port)
return "liteuart";
}
-static void liteuart_release_port(struct uart_port *port)
-{
-}
-
-static int liteuart_request_port(struct uart_port *port)
-{
- return 0;
-}
-
static void liteuart_config_port(struct uart_port *port, int flags)
{
/*
@@ -232,13 +218,10 @@ static const struct uart_ops liteuart_ops = {
.stop_tx = liteuart_stop_tx,
.start_tx = liteuart_start_tx,
.stop_rx = liteuart_stop_rx,
- .break_ctl = liteuart_break_ctl,
.startup = liteuart_startup,
.shutdown = liteuart_shutdown,
.set_termios = liteuart_set_termios,
.type = liteuart_type,
- .release_port = liteuart_release_port,
- .request_port = liteuart_request_port,
.config_port = liteuart_config_port,
.verify_port = liteuart_verify_port,
};
--
2.38.1
The polling liteuart_putchar() function is only called from methods
conditionally enabled by CONFIG_SERIAL_LITEUART_CONSOLE. Move its
definition closer to the console code where it is dependent on the
same config option.
Signed-off-by: Gabriel Somlo <[email protected]>
Reviewed-by: Ilpo Järvinen <[email protected]>
---
drivers/tty/serial/liteuart.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/tty/serial/liteuart.c b/drivers/tty/serial/liteuart.c
index 6e9f58d3957c..ef557d59e4c8 100644
--- a/drivers/tty/serial/liteuart.c
+++ b/drivers/tty/serial/liteuart.c
@@ -70,14 +70,6 @@ static struct uart_driver liteuart_driver = {
#endif
};
-static void liteuart_putchar(struct uart_port *port, unsigned char ch)
-{
- while (litex_read8(port->membase + OFF_TXFULL))
- cpu_relax();
-
- litex_write8(port->membase + OFF_RXTX, ch);
-}
-
static void liteuart_update_irq_reg(struct uart_port *port, bool set, u8 mask)
{
struct liteuart_port *uart = to_liteuart_port(port);
@@ -377,6 +369,14 @@ static struct platform_driver liteuart_platform_driver = {
#ifdef CONFIG_SERIAL_LITEUART_CONSOLE
+static void liteuart_putchar(struct uart_port *port, unsigned char ch)
+{
+ while (litex_read8(port->membase + OFF_TXFULL))
+ cpu_relax();
+
+ litex_write8(port->membase + OFF_RXTX, ch);
+}
+
static void liteuart_console_write(struct console *co, const char *s,
unsigned int count)
{
--
2.38.1
Switch the TX path to IRQ-driven operation, while maintaining support
for polling mode via the poll timer.
Signed-off-by: Gabriel Somlo <[email protected]>
Reviewed-by: Ilpo Järvinen <[email protected]>
---
drivers/tty/serial/liteuart.c | 30 +++++++++++++-----------------
1 file changed, 13 insertions(+), 17 deletions(-)
diff --git a/drivers/tty/serial/liteuart.c b/drivers/tty/serial/liteuart.c
index 8685c97d391e..6e9f58d3957c 100644
--- a/drivers/tty/serial/liteuart.c
+++ b/drivers/tty/serial/liteuart.c
@@ -93,27 +93,12 @@ static void liteuart_update_irq_reg(struct uart_port *port, bool set, u8 mask)
static void liteuart_stop_tx(struct uart_port *port)
{
+ liteuart_update_irq_reg(port, false, EV_TX);
}
static void liteuart_start_tx(struct uart_port *port)
{
- struct circ_buf *xmit = &port->state->xmit;
- unsigned char ch;
-
- if (unlikely(port->x_char)) {
- litex_write8(port->membase + OFF_RXTX, port->x_char);
- port->icount.tx++;
- port->x_char = 0;
- } else if (!uart_circ_empty(xmit)) {
- while (xmit->head != xmit->tail) {
- ch = xmit->buf[xmit->tail];
- uart_xmit_advance(port, 1);
- liteuart_putchar(port, ch);
- }
- }
-
- if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
- uart_write_wakeup(port);
+ liteuart_update_irq_reg(port, true, EV_TX);
}
static void liteuart_stop_rx(struct uart_port *port)
@@ -144,6 +129,15 @@ static void liteuart_rx_chars(struct uart_port *port)
tty_flip_buffer_push(&port->state->port);
}
+static void liteuart_tx_chars(struct uart_port *port)
+{
+ u8 ch;
+
+ uart_port_tx(port, ch,
+ !litex_read8(port->membase + OFF_TXFULL),
+ litex_write8(port->membase + OFF_RXTX, ch));
+}
+
static irqreturn_t liteuart_interrupt(int irq, void *data)
{
struct liteuart_port *uart = data;
@@ -159,6 +153,8 @@ static irqreturn_t liteuart_interrupt(int irq, void *data)
isr = litex_read8(port->membase + OFF_EV_PENDING) & uart->irq_reg;
if (isr & EV_RX)
liteuart_rx_chars(port);
+ if (isr & EV_TX)
+ liteuart_tx_chars(port);
spin_unlock_irqrestore(&port->lock, flags);
return IRQ_RETVAL(isr);
--
2.38.1
Replace hard-coded instances of "liteuart" with KBUILD_MODNAME.
Signed-off-by: Gabriel Somlo <[email protected]>
Reviewed-by: Geert Uytterhoeven <[email protected]>
---
drivers/tty/serial/liteuart.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/liteuart.c b/drivers/tty/serial/liteuart.c
index 062812fe1b09..db898751ffe3 100644
--- a/drivers/tty/serial/liteuart.c
+++ b/drivers/tty/serial/liteuart.c
@@ -57,7 +57,7 @@ static struct console liteuart_console;
static struct uart_driver liteuart_driver = {
.owner = THIS_MODULE,
- .driver_name = "liteuart",
+ .driver_name = KBUILD_MODNAME,
.dev_name = "ttyLXU",
.major = 0,
.minor = 0,
@@ -321,7 +321,7 @@ static struct platform_driver liteuart_platform_driver = {
.probe = liteuart_probe,
.remove = liteuart_remove,
.driver = {
- .name = "liteuart",
+ .name = KBUILD_MODNAME,
.of_match_table = liteuart_of_match,
},
};
@@ -367,7 +367,7 @@ static int liteuart_console_setup(struct console *co, char *options)
}
static struct console liteuart_console = {
- .name = "liteuart",
+ .name = KBUILD_MODNAME,
.write = liteuart_console_write,
.device = uart_console_device,
.setup = liteuart_console_setup,
--
2.38.1
Replace magic bit constants (e.g., 1, 2, 4) with BIT(x) expressions.
Signed-off-by: Gabriel Somlo <[email protected]>
Reviewed-by: Geert Uytterhoeven <[email protected]>
---
drivers/tty/serial/liteuart.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/liteuart.c b/drivers/tty/serial/liteuart.c
index db898751ffe3..18c1eb315ee9 100644
--- a/drivers/tty/serial/liteuart.c
+++ b/drivers/tty/serial/liteuart.c
@@ -5,6 +5,7 @@
* Copyright (C) 2019-2020 Antmicro <http://www.antmicro.com>
*/
+#include <linux/bits.h>
#include <linux/console.h>
#include <linux/litex.h>
#include <linux/module.h>
@@ -38,8 +39,8 @@
#define OFF_EV_ENABLE 0x14
/* events */
-#define EV_TX 0x1
-#define EV_RX 0x2
+#define EV_TX BIT(0)
+#define EV_RX BIT(1)
struct liteuart_port {
struct uart_port port;
--
2.38.1
Convert the rx loop into its own dedicated function, and (for now)
call it from the poll timer. This is in preparation for adding irq
support to the receive path.
Signed-off-by: Gabriel Somlo <[email protected]>
Reviewed-by: Ilpo Järvinen <[email protected]>
---
drivers/tty/serial/liteuart.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/liteuart.c b/drivers/tty/serial/liteuart.c
index 62bfd2ed9051..ab6837f3e40d 100644
--- a/drivers/tty/serial/liteuart.c
+++ b/drivers/tty/serial/liteuart.c
@@ -68,10 +68,8 @@ static struct uart_driver liteuart_driver = {
#endif
};
-static void liteuart_timer(struct timer_list *t)
+static void liteuart_rx_chars(struct uart_port *port)
{
- struct liteuart_port *uart = from_timer(uart, t, timer);
- struct uart_port *port = &uart->port;
unsigned char __iomem *membase = port->membase;
u8 ch;
@@ -88,6 +86,14 @@ static void liteuart_timer(struct timer_list *t)
}
tty_flip_buffer_push(&port->state->port);
+}
+
+static void liteuart_timer(struct timer_list *t)
+{
+ struct liteuart_port *uart = from_timer(uart, t, timer);
+ struct uart_port *port = &uart->port;
+
+ liteuart_rx_chars(port);
mod_timer(&uart->timer, jiffies + uart_poll_timeout(port));
}
--
2.38.1
Remove regshift and iobase port fields, since they are unused
by the driver.
Signed-off-by: Gabriel Somlo <[email protected]>
Reviewed-by: Geert Uytterhoeven <[email protected]>
---
drivers/tty/serial/liteuart.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/tty/serial/liteuart.c b/drivers/tty/serial/liteuart.c
index 989a4f8d5bd4..c6eb7eba5af8 100644
--- a/drivers/tty/serial/liteuart.c
+++ b/drivers/tty/serial/liteuart.c
@@ -263,9 +263,7 @@ static int liteuart_probe(struct platform_device *pdev)
port->iotype = UPIO_MEM;
port->flags = UPF_BOOT_AUTOCONF;
port->ops = &liteuart_ops;
- port->regshift = 2;
port->fifosize = 16;
- port->iobase = 1;
port->type = PORT_UNKNOWN;
port->line = dev_id;
spin_lock_init(&port->lock);
--
2.38.1
On 23. 11. 22, 14:04, Gabriel Somlo wrote:
> Add IRQ support to the LiteX LiteUART serial interface
LGTM
Reviewed-by: Jiri Slaby <[email protected]>
thanks,
--
js
suse labs