[RFC,4/5] serial: amba-pl011: Enable NMI aware uart port

Message ID 1595333413-30052-5-git-send-email-sumit.garg@linaro.org
State New
Headers show
Series
  • Introduce NMI aware serial drivers
Related show

Commit Message

Sumit Garg July 21, 2020, 12:10 p.m.
Allow serial device interrupt to be requested as an NMI during
initialization in polling mode. If the irqchip doesn't support serial
device interrupt as an NMI then fallback to it being as a normal IRQ.

Currently this NMI aware uart port only supports NMI driven programmed
IO operation whereas DMA operation isn't supported.

And while operating in NMI mode, RX always remains active irrespective
of whether corresponding TTY port is active or not. So we directly bail
out of startup, shutdown and rx_stop APIs if NMI mode is active.

Also, get rid of modification to interrupts enable mask in pl011_hwinit()
as now we have a proper way to enable interrupts for NMI entry using
pl011_enable_interrupts().

Signed-off-by: Sumit Garg <sumit.garg@linaro.org>

---
 drivers/tty/serial/amba-pl011.c | 124 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 113 insertions(+), 11 deletions(-)

-- 
2.7.4

Comments

Doug Anderson Aug. 12, 2020, 11:59 p.m. | #1
Hi,

On Tue, Jul 21, 2020 at 5:11 AM Sumit Garg <sumit.garg@linaro.org> wrote:
>

> Allow serial device interrupt to be requested as an NMI during

> initialization in polling mode. If the irqchip doesn't support serial

> device interrupt as an NMI then fallback to it being as a normal IRQ.

>

> Currently this NMI aware uart port only supports NMI driven programmed

> IO operation whereas DMA operation isn't supported.

>

> And while operating in NMI mode, RX always remains active irrespective

> of whether corresponding TTY port is active or not. So we directly bail

> out of startup, shutdown and rx_stop APIs if NMI mode is active.

>

> Also, get rid of modification to interrupts enable mask in pl011_hwinit()

> as now we have a proper way to enable interrupts for NMI entry using

> pl011_enable_interrupts().

>

> Signed-off-by: Sumit Garg <sumit.garg@linaro.org>

> ---

>  drivers/tty/serial/amba-pl011.c | 124 ++++++++++++++++++++++++++++++++++++----

>  1 file changed, 113 insertions(+), 11 deletions(-)


Overall: I ran out of time to do a super full review, but presumably
you're going to spin this series anyway and I'll look at it again
then.  For now a few things I noticed below...


> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c

> index 0983c5e..5df1c07 100644

> --- a/drivers/tty/serial/amba-pl011.c

> +++ b/drivers/tty/serial/amba-pl011.c

> @@ -41,6 +41,8 @@

>  #include <linux/sizes.h>

>  #include <linux/io.h>

>  #include <linux/acpi.h>

> +#include <linux/irq.h>

> +#include <linux/irqdesc.h>

>

>  #include "amba-pl011.h"

>

> @@ -347,6 +349,10 @@ static int pl011_fifo_to_tty(struct uart_amba_port *uap)

>                 if (uart_handle_sysrq_char(&uap->port, ch & 255))

>                         continue;

>

> +               if (uart_nmi_handle_char(&uap->port, ch, UART011_DR_OE, ch,

> +                                        flag))

> +                       continue;

> +

>                 uart_insert_char(&uap->port, ch, UART011_DR_OE, ch, flag);

>         }

>

> @@ -1316,6 +1322,9 @@ static void pl011_stop_rx(struct uart_port *port)

>         struct uart_amba_port *uap =

>             container_of(port, struct uart_amba_port, port);

>

> +       if (uart_nmi_active(port))

> +               return;

> +

>         uap->im &= ~(UART011_RXIM|UART011_RTIM|UART011_FEIM|

>                      UART011_PEIM|UART011_BEIM|UART011_OEIM);

>         pl011_write(uap->im, uap, REG_IMSC);

> @@ -1604,13 +1613,6 @@ static int pl011_hwinit(struct uart_port *port)

>                     UART011_FEIS | UART011_RTIS | UART011_RXIS,

>                     uap, REG_ICR);

>

> -       /*

> -        * Save interrupts enable mask, and enable RX interrupts in case if

> -        * the interrupt is used for NMI entry.

> -        */

> -       uap->im = pl011_read(uap, REG_IMSC);

> -       pl011_write(UART011_RTIM | UART011_RXIM, uap, REG_IMSC);

> -

>         if (dev_get_platdata(uap->port.dev)) {

>                 struct amba_pl011_data *plat;

>

> @@ -1711,6 +1713,96 @@ static void pl011_put_poll_char(struct uart_port *port,

>         pl011_write(ch, uap, REG_DR);

>  }

>

> +static irqreturn_t pl011_nmi_int(int irq, void *dev_id)

> +{


I wish there was a better way to share code between this and
pl011_int(), but I guess it'd be too ugly?  If nothing else it feels
like you should do something to make it more obvious to anyone looking
at them that they are sister functions and any change to one of them
should be reflected in the other.  Maybe they should be logically next
to each other?


> +       struct uart_amba_port *uap = dev_id;

> +       unsigned int status, pass_counter = AMBA_ISR_PASS_LIMIT;

> +       int handled = 0;

> +

> +       status = pl011_read(uap, REG_MIS);

> +       if (status) {

> +               do {

> +                       check_apply_cts_event_workaround(uap);

> +

> +                       pl011_write(status, uap, REG_ICR);

> +

> +                       if (status & (UART011_RTIS|UART011_RXIS)) {

> +                               pl011_fifo_to_tty(uap);

> +                               irq_work_queue(&uap->port.nmi_state.rx_work);


It feels like it might be beneficial to not call irq_work_queue() in a
loop.  It doesn't hurt but it feels like, at least, it's going to keep
doing a bunch of atomic operations.  It's not like it'll cause the
work to run any sooner because it has to run on the same CPU, right?


> +                       }

> +

> +                       if (status & UART011_TXIS)

> +                               irq_work_queue(&uap->port.nmi_state.tx_work);


Here too...


> +

> +                       if (pass_counter-- == 0)

> +                               break;

> +

> +                       status = pl011_read(uap, REG_MIS);

> +               } while (status != 0);

> +               handled = 1;

> +       }

> +

> +       return IRQ_RETVAL(handled);

> +}

> +

> +static int pl011_allocate_nmi(struct uart_amba_port *uap)

> +{

> +       int ret;

> +

> +       irq_set_status_flags(uap->port.irq, IRQ_NOAUTOEN);

> +       ret = request_nmi(uap->port.irq, pl011_nmi_int, IRQF_PERCPU,

> +                         "uart-pl011", uap);

> +       if (ret) {

> +               irq_clear_status_flags(uap->port.irq, IRQ_NOAUTOEN);

> +               return ret;

> +       }

> +

> +       enable_irq(uap->port.irq);

> +

> +       return ret;

> +}

> +

> +static void pl011_tx_irq_callback(struct uart_port *port)

> +{

> +       struct uart_amba_port *uap =

> +           container_of(port, struct uart_amba_port, port);

> +

> +       spin_lock(&port->lock);

> +       pl011_tx_chars(uap, true);

> +       spin_unlock(&port->lock);

> +}

> +

> +static int pl011_poll_init(struct uart_port *port)

> +{

> +       struct uart_amba_port *uap =

> +           container_of(port, struct uart_amba_port, port);

> +       int retval;

> +

> +       retval = pl011_hwinit(port);

> +       if (retval)

> +               goto clk_dis;


I don't think you want "goto clk_dis" here.


> +

> +       /* In case NMI isn't supported, fallback to normal interrupt mode */

> +       retval = pl011_allocate_nmi(uap);

> +       if (retval)

> +               return 0;

> +

> +       retval = uart_nmi_state_init(port);

> +       if (retval)

> +               goto clk_dis;


Wouldn't you also need to to somehow call free_nmi() in the error case?


> +       port->nmi_state.tx_irq_callback = pl011_tx_irq_callback;

> +       uart_set_nmi_active(port, true);

> +

> +       pl011_enable_interrupts(uap);

> +

> +       return 0;

> +

> + clk_dis:

> +       clk_disable_unprepare(uap->clk);

> +       return retval;

> +}

> +

>  #endif /* CONFIG_CONSOLE_POLL */

>

>  static bool pl011_split_lcrh(const struct uart_amba_port *uap)

> @@ -1736,8 +1828,6 @@ static void pl011_write_lcr_h(struct uart_amba_port *uap, unsigned int lcr_h)

>

>  static int pl011_allocate_irq(struct uart_amba_port *uap)

>  {

> -       pl011_write(uap->im, uap, REG_IMSC);

> -

>         return request_irq(uap->port.irq, pl011_int, IRQF_SHARED, "uart-pl011", uap);

>  }

>

> @@ -1748,6 +1838,9 @@ static int pl011_startup(struct uart_port *port)

>         unsigned int cr;

>         int retval;

>

> +       if (uart_nmi_active(port))

> +               return 0;

> +

>         retval = pl011_hwinit(port);

>         if (retval)

>                 goto clk_dis;

> @@ -1790,6 +1883,9 @@ static int sbsa_uart_startup(struct uart_port *port)

>                 container_of(port, struct uart_amba_port, port);

>         int retval;

>

> +       if (uart_nmi_active(port))

> +               return 0;

> +

>         retval = pl011_hwinit(port);

>         if (retval)

>                 return retval;

> @@ -1859,6 +1955,9 @@ static void pl011_shutdown(struct uart_port *port)

>         struct uart_amba_port *uap =

>                 container_of(port, struct uart_amba_port, port);

>

> +       if (uart_nmi_active(port))

> +               return;

> +

>         pl011_disable_interrupts(uap);

>

>         pl011_dma_shutdown(uap);

> @@ -1891,6 +1990,9 @@ static void sbsa_uart_shutdown(struct uart_port *port)

>         struct uart_amba_port *uap =

>                 container_of(port, struct uart_amba_port, port);

>

> +       if (uart_nmi_active(port))

> +               return;

> +

>         pl011_disable_interrupts(uap);

>

>         free_irq(uap->port.irq, uap);

> @@ -2142,7 +2244,7 @@ static const struct uart_ops amba_pl011_pops = {

>         .config_port    = pl011_config_port,

>         .verify_port    = pl011_verify_port,

>  #ifdef CONFIG_CONSOLE_POLL

> -       .poll_init     = pl011_hwinit,

> +       .poll_init     = pl011_poll_init,


Do we need to add a "free" at this point?



>         .poll_get_char = pl011_get_poll_char,

>         .poll_put_char = pl011_put_poll_char,

>  #endif

> @@ -2173,7 +2275,7 @@ static const struct uart_ops sbsa_uart_pops = {

>         .config_port    = pl011_config_port,

>         .verify_port    = pl011_verify_port,

>  #ifdef CONFIG_CONSOLE_POLL

> -       .poll_init     = pl011_hwinit,

> +       .poll_init     = pl011_poll_init,

>         .poll_get_char = pl011_get_poll_char,

>         .poll_put_char = pl011_put_poll_char,

>  #endif

> --

> 2.7.4

>
Sumit Garg Aug. 13, 2020, 10:34 a.m. | #2
On Thu, 13 Aug 2020 at 05:29, Doug Anderson <dianders@chromium.org> wrote:
>

> Hi,

>

> On Tue, Jul 21, 2020 at 5:11 AM Sumit Garg <sumit.garg@linaro.org> wrote:

> >

> > Allow serial device interrupt to be requested as an NMI during

> > initialization in polling mode. If the irqchip doesn't support serial

> > device interrupt as an NMI then fallback to it being as a normal IRQ.

> >

> > Currently this NMI aware uart port only supports NMI driven programmed

> > IO operation whereas DMA operation isn't supported.

> >

> > And while operating in NMI mode, RX always remains active irrespective

> > of whether corresponding TTY port is active or not. So we directly bail

> > out of startup, shutdown and rx_stop APIs if NMI mode is active.

> >

> > Also, get rid of modification to interrupts enable mask in pl011_hwinit()

> > as now we have a proper way to enable interrupts for NMI entry using

> > pl011_enable_interrupts().

> >

> > Signed-off-by: Sumit Garg <sumit.garg@linaro.org>

> > ---

> >  drivers/tty/serial/amba-pl011.c | 124 ++++++++++++++++++++++++++++++++++++----

> >  1 file changed, 113 insertions(+), 11 deletions(-)

>

> Overall: I ran out of time to do a super full review, but presumably

> you're going to spin this series anyway and I'll look at it again

> then.  For now a few things I noticed below...

>


Sure and thanks for your review.

>

> > diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c

> > index 0983c5e..5df1c07 100644

> > --- a/drivers/tty/serial/amba-pl011.c

> > +++ b/drivers/tty/serial/amba-pl011.c

> > @@ -41,6 +41,8 @@

> >  #include <linux/sizes.h>

> >  #include <linux/io.h>

> >  #include <linux/acpi.h>

> > +#include <linux/irq.h>

> > +#include <linux/irqdesc.h>

> >

> >  #include "amba-pl011.h"

> >

> > @@ -347,6 +349,10 @@ static int pl011_fifo_to_tty(struct uart_amba_port *uap)

> >                 if (uart_handle_sysrq_char(&uap->port, ch & 255))

> >                         continue;

> >

> > +               if (uart_nmi_handle_char(&uap->port, ch, UART011_DR_OE, ch,

> > +                                        flag))

> > +                       continue;

> > +

> >                 uart_insert_char(&uap->port, ch, UART011_DR_OE, ch, flag);

> >         }

> >

> > @@ -1316,6 +1322,9 @@ static void pl011_stop_rx(struct uart_port *port)

> >         struct uart_amba_port *uap =

> >             container_of(port, struct uart_amba_port, port);

> >

> > +       if (uart_nmi_active(port))

> > +               return;

> > +

> >         uap->im &= ~(UART011_RXIM|UART011_RTIM|UART011_FEIM|

> >                      UART011_PEIM|UART011_BEIM|UART011_OEIM);

> >         pl011_write(uap->im, uap, REG_IMSC);

> > @@ -1604,13 +1613,6 @@ static int pl011_hwinit(struct uart_port *port)

> >                     UART011_FEIS | UART011_RTIS | UART011_RXIS,

> >                     uap, REG_ICR);

> >

> > -       /*

> > -        * Save interrupts enable mask, and enable RX interrupts in case if

> > -        * the interrupt is used for NMI entry.

> > -        */

> > -       uap->im = pl011_read(uap, REG_IMSC);

> > -       pl011_write(UART011_RTIM | UART011_RXIM, uap, REG_IMSC);

> > -

> >         if (dev_get_platdata(uap->port.dev)) {

> >                 struct amba_pl011_data *plat;

> >

> > @@ -1711,6 +1713,96 @@ static void pl011_put_poll_char(struct uart_port *port,

> >         pl011_write(ch, uap, REG_DR);

> >  }

> >

> > +static irqreturn_t pl011_nmi_int(int irq, void *dev_id)

> > +{

>

> I wish there was a better way to share code between this and

> pl011_int(), but I guess it'd be too ugly?  If nothing else it feels

> like you should do something to make it more obvious to anyone looking

> at them that they are sister functions and any change to one of them

> should be reflected in the other.  Maybe they should be logically next

> to each other?

>


Yes, I can make them sit logically next to each other.

>

> > +       struct uart_amba_port *uap = dev_id;

> > +       unsigned int status, pass_counter = AMBA_ISR_PASS_LIMIT;

> > +       int handled = 0;

> > +

> > +       status = pl011_read(uap, REG_MIS);

> > +       if (status) {

> > +               do {

> > +                       check_apply_cts_event_workaround(uap);

> > +

> > +                       pl011_write(status, uap, REG_ICR);

> > +

> > +                       if (status & (UART011_RTIS|UART011_RXIS)) {

> > +                               pl011_fifo_to_tty(uap);

> > +                               irq_work_queue(&uap->port.nmi_state.rx_work);

>

> It feels like it might be beneficial to not call irq_work_queue() in a

> loop.  It doesn't hurt but it feels like, at least, it's going to keep

> doing a bunch of atomic operations.  It's not like it'll cause the

> work to run any sooner because it has to run on the same CPU, right?

>


AFAIK, the loop here is about checking interrupt status if another
interrupt is raised while we are servicing the prior one. But I think
it would be an unlikely case here as we defer actual work and given
the slow serial transfer rate.

>

> > +                       }

> > +

> > +                       if (status & UART011_TXIS)

> > +                               irq_work_queue(&uap->port.nmi_state.tx_work);

>

> Here too...

>


Ditto.

>

> > +

> > +                       if (pass_counter-- == 0)

> > +                               break;

> > +

> > +                       status = pl011_read(uap, REG_MIS);

> > +               } while (status != 0);

> > +               handled = 1;

> > +       }

> > +

> > +       return IRQ_RETVAL(handled);

> > +}

> > +

> > +static int pl011_allocate_nmi(struct uart_amba_port *uap)

> > +{

> > +       int ret;

> > +

> > +       irq_set_status_flags(uap->port.irq, IRQ_NOAUTOEN);

> > +       ret = request_nmi(uap->port.irq, pl011_nmi_int, IRQF_PERCPU,

> > +                         "uart-pl011", uap);

> > +       if (ret) {

> > +               irq_clear_status_flags(uap->port.irq, IRQ_NOAUTOEN);

> > +               return ret;

> > +       }

> > +

> > +       enable_irq(uap->port.irq);

> > +

> > +       return ret;

> > +}

> > +

> > +static void pl011_tx_irq_callback(struct uart_port *port)

> > +{

> > +       struct uart_amba_port *uap =

> > +           container_of(port, struct uart_amba_port, port);

> > +

> > +       spin_lock(&port->lock);

> > +       pl011_tx_chars(uap, true);

> > +       spin_unlock(&port->lock);

> > +}

> > +

> > +static int pl011_poll_init(struct uart_port *port)

> > +{

> > +       struct uart_amba_port *uap =

> > +           container_of(port, struct uart_amba_port, port);

> > +       int retval;

> > +

> > +       retval = pl011_hwinit(port);

> > +       if (retval)

> > +               goto clk_dis;

>

> I don't think you want "goto clk_dis" here.

>


Yeah, will fix it to return here instead.

>

> > +

> > +       /* In case NMI isn't supported, fallback to normal interrupt mode */

> > +       retval = pl011_allocate_nmi(uap);

> > +       if (retval)

> > +               return 0;

> > +

> > +       retval = uart_nmi_state_init(port);

> > +       if (retval)

> > +               goto clk_dis;

>

> Wouldn't you also need to to somehow call free_nmi() in the error case?

>


Yes, will fix it.

>

> > +       port->nmi_state.tx_irq_callback = pl011_tx_irq_callback;

> > +       uart_set_nmi_active(port, true);

> > +

> > +       pl011_enable_interrupts(uap);

> > +

> > +       return 0;

> > +

> > + clk_dis:

> > +       clk_disable_unprepare(uap->clk);

> > +       return retval;

> > +}

> > +

> >  #endif /* CONFIG_CONSOLE_POLL */

> >

> >  static bool pl011_split_lcrh(const struct uart_amba_port *uap)

> > @@ -1736,8 +1828,6 @@ static void pl011_write_lcr_h(struct uart_amba_port *uap, unsigned int lcr_h)

> >

> >  static int pl011_allocate_irq(struct uart_amba_port *uap)

> >  {

> > -       pl011_write(uap->im, uap, REG_IMSC);

> > -

> >         return request_irq(uap->port.irq, pl011_int, IRQF_SHARED, "uart-pl011", uap);

> >  }

> >

> > @@ -1748,6 +1838,9 @@ static int pl011_startup(struct uart_port *port)

> >         unsigned int cr;

> >         int retval;

> >

> > +       if (uart_nmi_active(port))

> > +               return 0;

> > +

> >         retval = pl011_hwinit(port);

> >         if (retval)

> >                 goto clk_dis;

> > @@ -1790,6 +1883,9 @@ static int sbsa_uart_startup(struct uart_port *port)

> >                 container_of(port, struct uart_amba_port, port);

> >         int retval;

> >

> > +       if (uart_nmi_active(port))

> > +               return 0;

> > +

> >         retval = pl011_hwinit(port);

> >         if (retval)

> >                 return retval;

> > @@ -1859,6 +1955,9 @@ static void pl011_shutdown(struct uart_port *port)

> >         struct uart_amba_port *uap =

> >                 container_of(port, struct uart_amba_port, port);

> >

> > +       if (uart_nmi_active(port))

> > +               return;

> > +

> >         pl011_disable_interrupts(uap);

> >

> >         pl011_dma_shutdown(uap);

> > @@ -1891,6 +1990,9 @@ static void sbsa_uart_shutdown(struct uart_port *port)

> >         struct uart_amba_port *uap =

> >                 container_of(port, struct uart_amba_port, port);

> >

> > +       if (uart_nmi_active(port))

> > +               return;

> > +

> >         pl011_disable_interrupts(uap);

> >

> >         free_irq(uap->port.irq, uap);

> > @@ -2142,7 +2244,7 @@ static const struct uart_ops amba_pl011_pops = {

> >         .config_port    = pl011_config_port,

> >         .verify_port    = pl011_verify_port,

> >  #ifdef CONFIG_CONSOLE_POLL

> > -       .poll_init     = pl011_hwinit,

> > +       .poll_init     = pl011_poll_init,

>

> Do we need to add a "free" at this point?

>


Where do you envision its usage? As currently if we enable NMI once,
we would like it to be active throughout a boot cycle.

-Sumit

>

>

> >         .poll_get_char = pl011_get_poll_char,

> >         .poll_put_char = pl011_put_poll_char,

> >  #endif

> > @@ -2173,7 +2275,7 @@ static const struct uart_ops sbsa_uart_pops = {

> >         .config_port    = pl011_config_port,

> >         .verify_port    = pl011_verify_port,

> >  #ifdef CONFIG_CONSOLE_POLL

> > -       .poll_init     = pl011_hwinit,

> > +       .poll_init     = pl011_poll_init,

> >         .poll_get_char = pl011_get_poll_char,

> >         .poll_put_char = pl011_put_poll_char,

> >  #endif

> > --

> > 2.7.4

> >

Patch

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 0983c5e..5df1c07 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -41,6 +41,8 @@ 
 #include <linux/sizes.h>
 #include <linux/io.h>
 #include <linux/acpi.h>
+#include <linux/irq.h>
+#include <linux/irqdesc.h>
 
 #include "amba-pl011.h"
 
@@ -347,6 +349,10 @@  static int pl011_fifo_to_tty(struct uart_amba_port *uap)
 		if (uart_handle_sysrq_char(&uap->port, ch & 255))
 			continue;
 
+		if (uart_nmi_handle_char(&uap->port, ch, UART011_DR_OE, ch,
+					 flag))
+			continue;
+
 		uart_insert_char(&uap->port, ch, UART011_DR_OE, ch, flag);
 	}
 
@@ -1316,6 +1322,9 @@  static void pl011_stop_rx(struct uart_port *port)
 	struct uart_amba_port *uap =
 	    container_of(port, struct uart_amba_port, port);
 
+	if (uart_nmi_active(port))
+		return;
+
 	uap->im &= ~(UART011_RXIM|UART011_RTIM|UART011_FEIM|
 		     UART011_PEIM|UART011_BEIM|UART011_OEIM);
 	pl011_write(uap->im, uap, REG_IMSC);
@@ -1604,13 +1613,6 @@  static int pl011_hwinit(struct uart_port *port)
 		    UART011_FEIS | UART011_RTIS | UART011_RXIS,
 		    uap, REG_ICR);
 
-	/*
-	 * Save interrupts enable mask, and enable RX interrupts in case if
-	 * the interrupt is used for NMI entry.
-	 */
-	uap->im = pl011_read(uap, REG_IMSC);
-	pl011_write(UART011_RTIM | UART011_RXIM, uap, REG_IMSC);
-
 	if (dev_get_platdata(uap->port.dev)) {
 		struct amba_pl011_data *plat;
 
@@ -1711,6 +1713,96 @@  static void pl011_put_poll_char(struct uart_port *port,
 	pl011_write(ch, uap, REG_DR);
 }
 
+static irqreturn_t pl011_nmi_int(int irq, void *dev_id)
+{
+	struct uart_amba_port *uap = dev_id;
+	unsigned int status, pass_counter = AMBA_ISR_PASS_LIMIT;
+	int handled = 0;
+
+	status = pl011_read(uap, REG_MIS);
+	if (status) {
+		do {
+			check_apply_cts_event_workaround(uap);
+
+			pl011_write(status, uap, REG_ICR);
+
+			if (status & (UART011_RTIS|UART011_RXIS)) {
+				pl011_fifo_to_tty(uap);
+				irq_work_queue(&uap->port.nmi_state.rx_work);
+			}
+
+			if (status & UART011_TXIS)
+				irq_work_queue(&uap->port.nmi_state.tx_work);
+
+			if (pass_counter-- == 0)
+				break;
+
+			status = pl011_read(uap, REG_MIS);
+		} while (status != 0);
+		handled = 1;
+	}
+
+	return IRQ_RETVAL(handled);
+}
+
+static int pl011_allocate_nmi(struct uart_amba_port *uap)
+{
+	int ret;
+
+	irq_set_status_flags(uap->port.irq, IRQ_NOAUTOEN);
+	ret = request_nmi(uap->port.irq, pl011_nmi_int, IRQF_PERCPU,
+			  "uart-pl011", uap);
+	if (ret) {
+		irq_clear_status_flags(uap->port.irq, IRQ_NOAUTOEN);
+		return ret;
+	}
+
+	enable_irq(uap->port.irq);
+
+	return ret;
+}
+
+static void pl011_tx_irq_callback(struct uart_port *port)
+{
+	struct uart_amba_port *uap =
+	    container_of(port, struct uart_amba_port, port);
+
+	spin_lock(&port->lock);
+	pl011_tx_chars(uap, true);
+	spin_unlock(&port->lock);
+}
+
+static int pl011_poll_init(struct uart_port *port)
+{
+	struct uart_amba_port *uap =
+	    container_of(port, struct uart_amba_port, port);
+	int retval;
+
+	retval = pl011_hwinit(port);
+	if (retval)
+		goto clk_dis;
+
+	/* In case NMI isn't supported, fallback to normal interrupt mode */
+	retval = pl011_allocate_nmi(uap);
+	if (retval)
+		return 0;
+
+	retval = uart_nmi_state_init(port);
+	if (retval)
+		goto clk_dis;
+
+	port->nmi_state.tx_irq_callback = pl011_tx_irq_callback;
+	uart_set_nmi_active(port, true);
+
+	pl011_enable_interrupts(uap);
+
+	return 0;
+
+ clk_dis:
+	clk_disable_unprepare(uap->clk);
+	return retval;
+}
+
 #endif /* CONFIG_CONSOLE_POLL */
 
 static bool pl011_split_lcrh(const struct uart_amba_port *uap)
@@ -1736,8 +1828,6 @@  static void pl011_write_lcr_h(struct uart_amba_port *uap, unsigned int lcr_h)
 
 static int pl011_allocate_irq(struct uart_amba_port *uap)
 {
-	pl011_write(uap->im, uap, REG_IMSC);
-
 	return request_irq(uap->port.irq, pl011_int, IRQF_SHARED, "uart-pl011", uap);
 }
 
@@ -1748,6 +1838,9 @@  static int pl011_startup(struct uart_port *port)
 	unsigned int cr;
 	int retval;
 
+	if (uart_nmi_active(port))
+		return 0;
+
 	retval = pl011_hwinit(port);
 	if (retval)
 		goto clk_dis;
@@ -1790,6 +1883,9 @@  static int sbsa_uart_startup(struct uart_port *port)
 		container_of(port, struct uart_amba_port, port);
 	int retval;
 
+	if (uart_nmi_active(port))
+		return 0;
+
 	retval = pl011_hwinit(port);
 	if (retval)
 		return retval;
@@ -1859,6 +1955,9 @@  static void pl011_shutdown(struct uart_port *port)
 	struct uart_amba_port *uap =
 		container_of(port, struct uart_amba_port, port);
 
+	if (uart_nmi_active(port))
+		return;
+
 	pl011_disable_interrupts(uap);
 
 	pl011_dma_shutdown(uap);
@@ -1891,6 +1990,9 @@  static void sbsa_uart_shutdown(struct uart_port *port)
 	struct uart_amba_port *uap =
 		container_of(port, struct uart_amba_port, port);
 
+	if (uart_nmi_active(port))
+		return;
+
 	pl011_disable_interrupts(uap);
 
 	free_irq(uap->port.irq, uap);
@@ -2142,7 +2244,7 @@  static const struct uart_ops amba_pl011_pops = {
 	.config_port	= pl011_config_port,
 	.verify_port	= pl011_verify_port,
 #ifdef CONFIG_CONSOLE_POLL
-	.poll_init     = pl011_hwinit,
+	.poll_init     = pl011_poll_init,
 	.poll_get_char = pl011_get_poll_char,
 	.poll_put_char = pl011_put_poll_char,
 #endif
@@ -2173,7 +2275,7 @@  static const struct uart_ops sbsa_uart_pops = {
 	.config_port	= pl011_config_port,
 	.verify_port	= pl011_verify_port,
 #ifdef CONFIG_CONSOLE_POLL
-	.poll_init     = pl011_hwinit,
+	.poll_init     = pl011_poll_init,
 	.poll_get_char = pl011_get_poll_char,
 	.poll_put_char = pl011_put_poll_char,
 #endif