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]>
---
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 | 76 +++++++++++++++++++++++++++++++----
1 file changed, 69 insertions(+), 7 deletions(-)
diff --git a/drivers/tty/serial/liteuart.c b/drivers/tty/serial/liteuart.c
index 8a6e176be08e..fad778578986 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,27 @@ 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;
+
+ spin_lock(&port->lock);
+ isr = litex_read8(port->membase + OFF_EV_PENDING) & uart->irq_reg;
+ 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));
}
@@ -161,19 +190,45 @@ 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) {
+ 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));
+ 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 +317,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;
--
2.38.1
On Wed, 16 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 <[email protected]>
> ---
>
> 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 | 76 +++++++++++++++++++++++++++++++----
> 1 file changed, 69 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/tty/serial/liteuart.c b/drivers/tty/serial/liteuart.c
> index 8a6e176be08e..fad778578986 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,27 @@ 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;
> +
> + spin_lock(&port->lock);
> + isr = litex_read8(port->membase + OFF_EV_PENDING) & uart->irq_reg;
> + 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));
> }
>
> @@ -161,19 +190,45 @@ 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) {
> + pr_err(pr_fmt("line %d irq %d failed: using polling\n"),
> + port->line, port->irq);
dev_err() seems more appropriate here.
> + 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 +317,13 @@ static int liteuart_probe(struct platform_device *pdev)
> goto err_erase_id;
> }
>
> + /* get irq */
This comment is unnecessary.
Other than that,
Reviewed-by: Ilpo J?rvinen <[email protected]>
--
i.
> + 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;
>
On Fri, Nov 18, 2022 at 03:58:47PM +0200, Ilpo J?rvinen wrote:
> On Wed, 16 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 <[email protected]>
> > ---
> >
> > 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 | 76 +++++++++++++++++++++++++++++++----
> > 1 file changed, 69 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/tty/serial/liteuart.c b/drivers/tty/serial/liteuart.c
> > index 8a6e176be08e..fad778578986 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,27 @@ 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;
> > +
> > + spin_lock(&port->lock);
> > + isr = litex_read8(port->membase + OFF_EV_PENDING) & uart->irq_reg;
> > + 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));
> > }
> >
> > @@ -161,19 +190,45 @@ 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) {
> > + pr_err(pr_fmt("line %d irq %d failed: using polling\n"),
> > + port->line, port->irq);
>
> dev_err() seems more appropriate here.
Makes sense, good thing `struct uart_port` has a `dev` field :)
I've made the change, and lined it up for the upcoming v5 of the
series.
> > + 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 +317,13 @@ static int liteuart_probe(struct platform_device *pdev)
> > goto err_erase_id;
> > }
> >
> > + /* get irq */
>
> This comment is unnecessary.
Removed in v5.
> Other than that,
>
> Reviewed-by: Ilpo J?rvinen <[email protected]>
Thanks much, I'll send out v5 a bit later today.
--Gabriel
> --
> i.
>
>
> > + 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;
> >
On Fri, 18 Nov 2022, Gabriel L. Somlo wrote:
> On Fri, Nov 18, 2022 at 03:58:47PM +0200, Ilpo J?rvinen wrote:
> > On Wed, 16 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 <[email protected]>
> > > ---
> > >
> > > 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 | 76 +++++++++++++++++++++++++++++++----
> > > 1 file changed, 69 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/tty/serial/liteuart.c b/drivers/tty/serial/liteuart.c
> > > index 8a6e176be08e..fad778578986 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,27 @@ 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;
> > > +
> > > + spin_lock(&port->lock);
> > > + isr = litex_read8(port->membase + OFF_EV_PENDING) & uart->irq_reg;
> > > + 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));
> > > }
> > >
> > > @@ -161,19 +190,45 @@ 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) {
> > > + pr_err(pr_fmt("line %d irq %d failed: using polling\n"),
> > > + port->line, port->irq);
> >
> > dev_err() seems more appropriate here.
>
> Makes sense, good thing `struct uart_port` has a `dev` field :)
> I've made the change, and lined it up for the upcoming v5 of the
> series.
Also that pr_fmt() should be dropped (in case you didn't already). It was
in my mind ealier but should have stated it explicitly.
--
i.
On Fri, Nov 18, 2022 at 04:30:15PM +0200, Ilpo J?rvinen wrote:
> On Fri, 18 Nov 2022, Gabriel L. Somlo wrote:
>
> > On Fri, Nov 18, 2022 at 03:58:47PM +0200, Ilpo J?rvinen wrote:
> > > On Wed, 16 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 <[email protected]>
> > > > ---
> > > >
> > > > 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 | 76 +++++++++++++++++++++++++++++++----
> > > > 1 file changed, 69 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/drivers/tty/serial/liteuart.c b/drivers/tty/serial/liteuart.c
> > > > index 8a6e176be08e..fad778578986 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,27 @@ 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;
> > > > +
> > > > + spin_lock(&port->lock);
> > > > + isr = litex_read8(port->membase + OFF_EV_PENDING) & uart->irq_reg;
> > > > + 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));
> > > > }
> > > >
> > > > @@ -161,19 +190,45 @@ 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) {
> > > > + pr_err(pr_fmt("line %d irq %d failed: using polling\n"),
> > > > + port->line, port->irq);
> > >
> > > dev_err() seems more appropriate here.
> >
> > Makes sense, good thing `struct uart_port` has a `dev` field :)
> > I've made the change, and lined it up for the upcoming v5 of the
> > series.
>
> Also that pr_fmt() should be dropped (in case you didn't already). It was
> in my mind ealier but should have stated it explicitly.
Understood. I've used dev_err() before, it just slipped my mind to
check for the availability of a `dev` field in `struct uart_port`... :)
Thanks again for the review!
Best,
--Gabriel