On Sat, 12 Nov 2022, Gabriel Somlo wrote:
> Add support for IRQ-driven RX. Support for the TX path will be added
> in a separate commit.
>
> Signed-off-by: Gabriel Somlo <gsomlo@xxxxxxxxx>
> ---
> drivers/tty/serial/liteuart.c | 61 +++++++++++++++++++++++++++++++----
> 1 file changed, 54 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/tty/serial/liteuart.c b/drivers/tty/serial/liteuart.c
> index cf1ce597b45e..e30adb30277f 100644
> --- a/drivers/tty/serial/liteuart.c
> +++ b/drivers/tty/serial/liteuart.c
> @@ -6,6 +6,7 @@
> */
>
> #include <linux/console.h>
> +#include <linux/interrupt.h>
> #include <linux/litex.h>
> #include <linux/module.h>
> #include <linux/of.h>
> @@ -130,13 +131,29 @@ 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;
> + u8 isr = litex_read8(port->membase + OFF_EV_PENDING);
> +
> + /* for now, only rx path triggers interrupts */
Please don't add comment like this at all when your series removes it in a
later patch.
> + isr &= EV_RX;
> +
> + spin_lock(&port->lock);
> + if (isr & EV_RX)
> + liteuart_rx_chars(port);
> + spin_unlock(&port->lock);
> +
> + 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));
> }
>
> @@ -162,19 +179,42 @@ 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);
> + int ret;
> + u8 irq_mask = 0;
>
> - /* 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 == 0) {
> + /* only enable rx interrupts at this time */
This comment seems pretty useless. Your code says very much the same.
--
i.
> + irq_mask = EV_RX;
> + } else {
> + pr_err(pr_fmt("line %d irq %d failed: using 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));
> + if (!port->irq) {
> + timer_setup(&uart->timer, liteuart_timer, 0);
> + mod_timer(&uart->timer, jiffies + uart_poll_timeout(port));
> + }
> +
> + litex_write8(port->membase + OFF_EV_ENABLE, irq_mask);
>
> return 0;
> }
>
> static void liteuart_shutdown(struct uart_port *port)
> {
> + struct liteuart_port *uart = to_liteuart_port(port);
> +
> + litex_write8(port->membase + OFF_EV_ENABLE, 0);
> +
> + 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,
> @@ -263,6 +303,13 @@ static int liteuart_probe(struct platform_device *pdev)
> goto err_erase_id;
> }
>
> + /* get irq */
> + 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;
>
