* [PATCH v2 1/2] tty: serial: fsl_lpuart: move dma_request_chan()
@ 2020-03-25 9:06 Michael Walle
2020-03-25 9:06 ` [PATCH v2 2/2] tty: serial: fsl_lpuart: fix return value checking Michael Walle
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Michael Walle @ 2020-03-25 9:06 UTC (permalink / raw)
To: linux-serial, linux-kernel
Cc: Jiri Slaby, Greg Kroah-Hartman, Andy Duan, Michael Walle,
Leonard Crestez
Move dma_request_chan() out of the atomic context. First this call
should not be in the atomic context at all and second the
dev_info_once() may cause a hang because because the console takes this
spinlock, too.
Fixes: 159381df1442f ("tty: serial: fsl_lpuart: fix DMA operation when using IOMMU")
Reported-by: Leonard Crestez <leonard.crestez@nxp.com>
Signed-off-by: Michael Walle <michael@walle.cc>
---
changes since v1:
- instead of just moving the dev_info_once() out of the spinlock protected
section, move the whole dma_request_chan(). Thanks Andy!
I've tested this on my board. Andy, Leonard, can you double check it? For
all which are not aware, this deadlock happens only if you have the kernel
console output on the lpuart, so if someone wants to test it, make sure you
have something like console=ttyLP0,115200.
drivers/tty/serial/fsl_lpuart.c | 36 +++++++++++++++++++++------------
1 file changed, 23 insertions(+), 13 deletions(-)
diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index 9c6a018b1390..131018979b77 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -1510,20 +1510,33 @@ static void rx_dma_timer_init(struct lpuart_port *sport)
add_timer(&sport->lpuart_timer);
}
-static void lpuart_tx_dma_startup(struct lpuart_port *sport)
+static void lpuart_request_dma(struct lpuart_port *sport)
{
- u32 uartbaud;
- int ret;
-
sport->dma_tx_chan = dma_request_chan(sport->port.dev, "tx");
if (IS_ERR(sport->dma_tx_chan)) {
dev_info_once(sport->port.dev,
"DMA tx channel request failed, operating without tx DMA (%ld)\n",
PTR_ERR(sport->dma_tx_chan));
sport->dma_tx_chan = NULL;
- goto err;
}
+ sport->dma_rx_chan = dma_request_chan(sport->port.dev, "rx");
+ if (IS_ERR(sport->dma_rx_chan)) {
+ dev_info_once(sport->port.dev,
+ "DMA rx channel request failed, operating without rx DMA (%ld)\n",
+ PTR_ERR(sport->dma_rx_chan));
+ sport->dma_rx_chan = NULL;
+ }
+}
+
+static void lpuart_tx_dma_startup(struct lpuart_port *sport)
+{
+ u32 uartbaud;
+ int ret;
+
+ if (!sport->dma_tx_chan)
+ goto err;
+
ret = lpuart_dma_tx_request(&sport->port);
if (!ret)
goto err;
@@ -1549,14 +1562,8 @@ static void lpuart_rx_dma_startup(struct lpuart_port *sport)
{
int ret;
- sport->dma_rx_chan = dma_request_chan(sport->port.dev, "rx");
- if (IS_ERR(sport->dma_rx_chan)) {
- dev_info_once(sport->port.dev,
- "DMA rx channel request failed, operating without rx DMA (%ld)\n",
- PTR_ERR(sport->dma_rx_chan));
- sport->dma_rx_chan = NULL;
+ if (!sport->dma_rx_chan)
goto err;
- }
ret = lpuart_start_rx_dma(sport);
if (ret)
@@ -1592,6 +1599,8 @@ static int lpuart_startup(struct uart_port *port)
sport->rxfifo_size = UARTFIFO_DEPTH((temp >> UARTPFIFO_RXSIZE_OFF) &
UARTPFIFO_FIFOSIZE_MASK);
+ lpuart_request_dma(sport);
+
spin_lock_irqsave(&sport->port.lock, flags);
lpuart_setup_watermark_enable(sport);
@@ -1649,11 +1658,12 @@ static int lpuart32_startup(struct uart_port *port)
sport->port.fifosize = sport->txfifo_size;
}
+ lpuart_request_dma(sport);
+
spin_lock_irqsave(&sport->port.lock, flags);
lpuart32_setup_watermark_enable(sport);
-
lpuart_rx_dma_startup(sport);
lpuart_tx_dma_startup(sport);
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 2/2] tty: serial: fsl_lpuart: fix return value checking
2020-03-25 9:06 [PATCH v2 1/2] tty: serial: fsl_lpuart: move dma_request_chan() Michael Walle
@ 2020-03-25 9:06 ` Michael Walle
2020-03-25 10:07 ` [EXT] " Andy Duan
2020-03-25 10:03 ` [EXT] [PATCH v2 1/2] tty: serial: fsl_lpuart: move dma_request_chan() Andy Duan
2020-03-25 18:05 ` Leonard Crestez
2 siblings, 1 reply; 6+ messages in thread
From: Michael Walle @ 2020-03-25 9:06 UTC (permalink / raw)
To: linux-serial, linux-kernel
Cc: Jiri Slaby, Greg Kroah-Hartman, Andy Duan, Michael Walle,
Leonard Crestez
The return value of lpuart_dma_tx_request() is an negative errno on
failure and zero on success.
Fixes: 159381df1442f ("tty: serial: fsl_lpuart: fix DMA operation when using IOMMU")
Reported-by: Leonard Crestez <leonard.crestez@nxp.com>
Signed-off-by: Michael Walle <michael@walle.cc>
---
changes since v1:
- none
drivers/tty/serial/fsl_lpuart.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index 131018979b77..5d41075964f2 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -1538,7 +1538,7 @@ static void lpuart_tx_dma_startup(struct lpuart_port *sport)
goto err;
ret = lpuart_dma_tx_request(&sport->port);
- if (!ret)
+ if (ret)
goto err;
init_waitqueue_head(&sport->dma_wait);
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* RE: [EXT] [PATCH v2 2/2] tty: serial: fsl_lpuart: fix return value checking
2020-03-25 9:06 ` [PATCH v2 2/2] tty: serial: fsl_lpuart: fix return value checking Michael Walle
@ 2020-03-25 10:07 ` Andy Duan
0 siblings, 0 replies; 6+ messages in thread
From: Andy Duan @ 2020-03-25 10:07 UTC (permalink / raw)
To: Michael Walle, linux-serial, linux-kernel
Cc: Jiri Slaby, Greg Kroah-Hartman, Leonard Crestez
From: Michael Walle <michael@walle.cc> Sent: Wednesday, March 25, 2020 5:07 PM
> The return value of lpuart_dma_tx_request() is an negative errno on failure
> and zero on success.
>
> Fixes: 159381df1442f ("tty: serial: fsl_lpuart: fix DMA operation when using
> IOMMU")
> Reported-by: Leonard Crestez <leonard.crestez@nxp.com>
> Signed-off-by: Michael Walle <michael@walle.cc>
Reviewed-by: Fugang Duan <fugang.duan@nxp.com>
> ---
> changes since v1:
> - none
>
> drivers/tty/serial/fsl_lpuart.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c index
> 131018979b77..5d41075964f2 100644
> --- a/drivers/tty/serial/fsl_lpuart.c
> +++ b/drivers/tty/serial/fsl_lpuart.c
> @@ -1538,7 +1538,7 @@ static void lpuart_tx_dma_startup(struct
> lpuart_port *sport)
> goto err;
>
> ret = lpuart_dma_tx_request(&sport->port);
> - if (!ret)
> + if (ret)
> goto err;
>
> init_waitqueue_head(&sport->dma_wait);
> --
> 2.20.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [EXT] [PATCH v2 1/2] tty: serial: fsl_lpuart: move dma_request_chan()
2020-03-25 9:06 [PATCH v2 1/2] tty: serial: fsl_lpuart: move dma_request_chan() Michael Walle
2020-03-25 9:06 ` [PATCH v2 2/2] tty: serial: fsl_lpuart: fix return value checking Michael Walle
@ 2020-03-25 10:03 ` Andy Duan
2020-03-25 18:05 ` Leonard Crestez
2 siblings, 0 replies; 6+ messages in thread
From: Andy Duan @ 2020-03-25 10:03 UTC (permalink / raw)
To: Michael Walle, linux-serial, linux-kernel
Cc: Jiri Slaby, Greg Kroah-Hartman, Leonard Crestez
From: Michael Walle <michael@walle.cc> Sent: Wednesday, March 25, 2020 5:07 PM
> Move dma_request_chan() out of the atomic context. First this call should not
> be in the atomic context at all and second the
> dev_info_once() may cause a hang because because the console takes this
> spinlock, too.
>
> Fixes: 159381df1442f ("tty: serial: fsl_lpuart: fix DMA operation when using
> IOMMU")
> Reported-by: Leonard Crestez <leonard.crestez@nxp.com>
> Signed-off-by: Michael Walle <michael@walle.cc>
Reviewed-by: Fugang Duan <fugang.duan@nxp.com>
> ---
> changes since v1:
> - instead of just moving the dev_info_once() out of the spinlock protected
> section, move the whole dma_request_chan(). Thanks Andy!
>
> I've tested this on my board. Andy, Leonard, can you double check it? For all
> which are not aware, this deadlock happens only if you have the kernel
> console output on the lpuart, so if someone wants to test it, make sure you
> have something like console=ttyLP0,115200.
>
> drivers/tty/serial/fsl_lpuart.c | 36 +++++++++++++++++++++------------
> 1 file changed, 23 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c index
> 9c6a018b1390..131018979b77 100644
> --- a/drivers/tty/serial/fsl_lpuart.c
> +++ b/drivers/tty/serial/fsl_lpuart.c
> @@ -1510,20 +1510,33 @@ static void rx_dma_timer_init(struct lpuart_port
> *sport)
> add_timer(&sport->lpuart_timer); }
>
> -static void lpuart_tx_dma_startup(struct lpuart_port *sport)
> +static void lpuart_request_dma(struct lpuart_port *sport)
> {
> - u32 uartbaud;
> - int ret;
> -
> sport->dma_tx_chan = dma_request_chan(sport->port.dev, "tx");
> if (IS_ERR(sport->dma_tx_chan)) {
> dev_info_once(sport->port.dev,
> "DMA tx channel request failed,
> operating without tx DMA (%ld)\n",
> PTR_ERR(sport->dma_tx_chan));
> sport->dma_tx_chan = NULL;
> - goto err;
> }
>
> + sport->dma_rx_chan = dma_request_chan(sport->port.dev, "rx");
> + if (IS_ERR(sport->dma_rx_chan)) {
> + dev_info_once(sport->port.dev,
> + "DMA rx channel request failed,
> operating without rx DMA (%ld)\n",
> + PTR_ERR(sport->dma_rx_chan));
> + sport->dma_rx_chan = NULL;
> + }
> +}
> +
> +static void lpuart_tx_dma_startup(struct lpuart_port *sport) {
> + u32 uartbaud;
> + int ret;
> +
> + if (!sport->dma_tx_chan)
> + goto err;
> +
> ret = lpuart_dma_tx_request(&sport->port);
> if (!ret)
> goto err;
> @@ -1549,14 +1562,8 @@ static void lpuart_rx_dma_startup(struct
> lpuart_port *sport) {
> int ret;
>
> - sport->dma_rx_chan = dma_request_chan(sport->port.dev, "rx");
> - if (IS_ERR(sport->dma_rx_chan)) {
> - dev_info_once(sport->port.dev,
> - "DMA rx channel request failed,
> operating without rx DMA (%ld)\n",
> - PTR_ERR(sport->dma_rx_chan));
> - sport->dma_rx_chan = NULL;
> + if (!sport->dma_rx_chan)
> goto err;
> - }
>
> ret = lpuart_start_rx_dma(sport);
> if (ret)
> @@ -1592,6 +1599,8 @@ static int lpuart_startup(struct uart_port *port)
> sport->rxfifo_size = UARTFIFO_DEPTH((temp >>
> UARTPFIFO_RXSIZE_OFF) &
>
> UARTPFIFO_FIFOSIZE_MASK);
>
> + lpuart_request_dma(sport);
> +
> spin_lock_irqsave(&sport->port.lock, flags);
>
> lpuart_setup_watermark_enable(sport);
> @@ -1649,11 +1658,12 @@ static int lpuart32_startup(struct uart_port
> *port)
> sport->port.fifosize = sport->txfifo_size;
> }
>
> + lpuart_request_dma(sport);
> +
> spin_lock_irqsave(&sport->port.lock, flags);
>
> lpuart32_setup_watermark_enable(sport);
>
> -
> lpuart_rx_dma_startup(sport);
> lpuart_tx_dma_startup(sport);
>
> --
> 2.20.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2 1/2] tty: serial: fsl_lpuart: move dma_request_chan()
2020-03-25 9:06 [PATCH v2 1/2] tty: serial: fsl_lpuart: move dma_request_chan() Michael Walle
2020-03-25 9:06 ` [PATCH v2 2/2] tty: serial: fsl_lpuart: fix return value checking Michael Walle
2020-03-25 10:03 ` [EXT] [PATCH v2 1/2] tty: serial: fsl_lpuart: move dma_request_chan() Andy Duan
@ 2020-03-25 18:05 ` Leonard Crestez
2020-03-26 14:32 ` Greg Kroah-Hartman
2 siblings, 1 reply; 6+ messages in thread
From: Leonard Crestez @ 2020-03-25 18:05 UTC (permalink / raw)
To: Michael Walle, Greg Kroah-Hartman
Cc: linux-serial, linux-kernel, Jiri Slaby, Andy Duan
On 2020-03-25 11:07 AM, Michael Walle wrote:
> Move dma_request_chan() out of the atomic context. First this call
> should not be in the atomic context at all and second the
> dev_info_once() may cause a hang because because the console takes this
> spinlock, too.
>
> Fixes: 159381df1442f ("tty: serial: fsl_lpuart: fix DMA operation when using IOMMU")
> Reported-by: Leonard Crestez <leonard.crestez@nxp.com>
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
> changes since v1:
> - instead of just moving the dev_info_once() out of the spinlock protected
> section, move the whole dma_request_chan(). Thanks Andy!
>
> I've tested this on my board. Andy, Leonard, can you double check it? For
> all which are not aware, this deadlock happens only if you have the kernel
> console output on the lpuart, so if someone wants to test it, make sure you
> have something like console=ttyLP0,115200.
>
> drivers/tty/serial/fsl_lpuart.c | 36 +++++++++++++++++++++------------
> 1 file changed, 23 insertions(+), 13 deletions(-)
Tested-by: Leonard Crestez <leonard.crestez@nxp.com>
Since the original commit only made it into next it might make sense to
squash the commits in the tty tree.
This way future bisections won't get stuck on a boot failure.
>
> diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
> index 9c6a018b1390..131018979b77 100644
> --- a/drivers/tty/serial/fsl_lpuart.c
> +++ b/drivers/tty/serial/fsl_lpuart.c
> @@ -1510,20 +1510,33 @@ static void rx_dma_timer_init(struct lpuart_port *sport)
> add_timer(&sport->lpuart_timer);
> }
>
> -static void lpuart_tx_dma_startup(struct lpuart_port *sport)
> +static void lpuart_request_dma(struct lpuart_port *sport)
> {
> - u32 uartbaud;
> - int ret;
> -
> sport->dma_tx_chan = dma_request_chan(sport->port.dev, "tx");
> if (IS_ERR(sport->dma_tx_chan)) {
> dev_info_once(sport->port.dev,
> "DMA tx channel request failed, operating without tx DMA (%ld)\n",
> PTR_ERR(sport->dma_tx_chan));
> sport->dma_tx_chan = NULL;
> - goto err;
> }
>
> + sport->dma_rx_chan = dma_request_chan(sport->port.dev, "rx");
> + if (IS_ERR(sport->dma_rx_chan)) {
> + dev_info_once(sport->port.dev,
> + "DMA rx channel request failed, operating without rx DMA (%ld)\n",
> + PTR_ERR(sport->dma_rx_chan));
> + sport->dma_rx_chan = NULL;
> + }
> +}
> +
> +static void lpuart_tx_dma_startup(struct lpuart_port *sport)
> +{
> + u32 uartbaud;
> + int ret;
> +
> + if (!sport->dma_tx_chan)
> + goto err;
> +
> ret = lpuart_dma_tx_request(&sport->port);
> if (!ret)
> goto err;
> @@ -1549,14 +1562,8 @@ static void lpuart_rx_dma_startup(struct lpuart_port *sport)
> {
> int ret;
>
> - sport->dma_rx_chan = dma_request_chan(sport->port.dev, "rx");
> - if (IS_ERR(sport->dma_rx_chan)) {
> - dev_info_once(sport->port.dev,
> - "DMA rx channel request failed, operating without rx DMA (%ld)\n",
> - PTR_ERR(sport->dma_rx_chan));
> - sport->dma_rx_chan = NULL;
> + if (!sport->dma_rx_chan)
> goto err;
> - }
>
> ret = lpuart_start_rx_dma(sport);
> if (ret)
> @@ -1592,6 +1599,8 @@ static int lpuart_startup(struct uart_port *port)
> sport->rxfifo_size = UARTFIFO_DEPTH((temp >> UARTPFIFO_RXSIZE_OFF) &
> UARTPFIFO_FIFOSIZE_MASK);
>
> + lpuart_request_dma(sport);
> +
> spin_lock_irqsave(&sport->port.lock, flags);
>
> lpuart_setup_watermark_enable(sport);
> @@ -1649,11 +1658,12 @@ static int lpuart32_startup(struct uart_port *port)
> sport->port.fifosize = sport->txfifo_size;
> }
>
> + lpuart_request_dma(sport);
> +
> spin_lock_irqsave(&sport->port.lock, flags);
>
> lpuart32_setup_watermark_enable(sport);
>
> -
> lpuart_rx_dma_startup(sport);
> lpuart_tx_dma_startup(sport);
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2 1/2] tty: serial: fsl_lpuart: move dma_request_chan()
2020-03-25 18:05 ` Leonard Crestez
@ 2020-03-26 14:32 ` Greg Kroah-Hartman
0 siblings, 0 replies; 6+ messages in thread
From: Greg Kroah-Hartman @ 2020-03-26 14:32 UTC (permalink / raw)
To: Leonard Crestez
Cc: Michael Walle, linux-serial, linux-kernel, Jiri Slaby, Andy Duan
On Wed, Mar 25, 2020 at 06:05:16PM +0000, Leonard Crestez wrote:
> On 2020-03-25 11:07 AM, Michael Walle wrote:
> > Move dma_request_chan() out of the atomic context. First this call
> > should not be in the atomic context at all and second the
> > dev_info_once() may cause a hang because because the console takes this
> > spinlock, too.
> >
> > Fixes: 159381df1442f ("tty: serial: fsl_lpuart: fix DMA operation when using IOMMU")
> > Reported-by: Leonard Crestez <leonard.crestez@nxp.com>
> > Signed-off-by: Michael Walle <michael@walle.cc>
> > ---
> > changes since v1:
> > - instead of just moving the dev_info_once() out of the spinlock protected
> > section, move the whole dma_request_chan(). Thanks Andy!
> >
> > I've tested this on my board. Andy, Leonard, can you double check it? For
> > all which are not aware, this deadlock happens only if you have the kernel
> > console output on the lpuart, so if someone wants to test it, make sure you
> > have something like console=ttyLP0,115200.
> >
> > drivers/tty/serial/fsl_lpuart.c | 36 +++++++++++++++++++++------------
> > 1 file changed, 23 insertions(+), 13 deletions(-)
>
> Tested-by: Leonard Crestez <leonard.crestez@nxp.com>
>
> Since the original commit only made it into next it might make sense to
> squash the commits in the tty tree.
>
> This way future bisections won't get stuck on a boot failure.
My tree does not rebase, sorry. And neither should any other public git
tree without really really good reasons.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-03-26 14:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-25 9:06 [PATCH v2 1/2] tty: serial: fsl_lpuart: move dma_request_chan() Michael Walle
2020-03-25 9:06 ` [PATCH v2 2/2] tty: serial: fsl_lpuart: fix return value checking Michael Walle
2020-03-25 10:07 ` [EXT] " Andy Duan
2020-03-25 10:03 ` [EXT] [PATCH v2 1/2] tty: serial: fsl_lpuart: move dma_request_chan() Andy Duan
2020-03-25 18:05 ` Leonard Crestez
2020-03-26 14:32 ` Greg Kroah-Hartman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).