>
> So we cannot support wrong usage cases, that is my thought.
>
> Thanks,
> Andy
>
>>
>> >
>> > Andy
>> >> >
>> >> > To workaround this, we request the DMA channel in _startup().
>> >> > Other serial drivers do it the same way.
>> >> >
>> >> > Signed-off-by: Michael Walle <michael@xxxxxxxx>
>> >>
>> >> This appears to cause boot hangs on imx8qxp-mek (boards boots fine
>> >> on
>> >> next-20200324 if this patch is reverted)
>> >>
>> >> > ---
>> >> > drivers/tty/serial/fsl_lpuart.c | 88
>> +++++++++++++++++++++------------
>> >> > 1 file changed, 57 insertions(+), 31 deletions(-)
>> >> >
>> >> > diff --git a/drivers/tty/serial/fsl_lpuart.c
>> >> > b/drivers/tty/serial/fsl_lpuart.c index
>> >> > c31b8f3db6bf..33798df4d727
>> >> > 100644
>> >> > --- a/drivers/tty/serial/fsl_lpuart.c
>> >> > +++ b/drivers/tty/serial/fsl_lpuart.c
>> >> > @@ -1493,36 +1493,67 @@ static void rx_dma_timer_init(struct
>> >> lpuart_port *sport)
>> >> > static void lpuart_tx_dma_startup(struct lpuart_port *sport)
>> >> > {
>> >> > u32 uartbaud;
>> >> > + int ret;
>> >> >
>> >> > - if (sport->dma_tx_chan
&& !lpuart_dma_tx_request(&sport->port)) {
>> >> > - init_waitqueue_head(&sport->dma_wait);
>> >> > - sport->lpuart_dma_tx_use = true;
>> >> > - if (lpuart_is_32(sport)) {
>> >> > - uartbaud = lpuart32_read(&sport->port,
>> UARTBAUD);
>> >> > - lpuart32_write(&sport->port,
>> >> > - uartbaud |
>> UARTBAUD_TDMAE, UARTBAUD);
>> >> > - } else {
>> >> > - writeb(readb(sport->port.membase +
UARTCR5)
>> |
>> >> > - UARTCR5_TDMAS,
>> sport->port.membase + UARTCR5);
>> >> > - }
>> >> > + 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));
>> >>
>> >> It seems that this since this is called from lpuart32_startup with
>> >> &sport->port.lock held and lpuart32_console_write takes the same
>> >> lock it can and hang.
>> >>
>> >> As a workaround I can just remove this print but there are other
>> >> possible error conditions in dmaengine code which can cause a printk.
>> >>
>> >> Maybe the port lock should only be held around register manipulation?
>> >>
>> >> > + sport->dma_tx_chan = NULL;
>> >> > + goto err;
>> >> > + }
>> >> > +
>> >> > + ret = lpuart_dma_tx_request(&sport->port);
>> >> > + if (!ret)
>> >> > + goto err;
>> >>
>> >> This is backwards: lpuart_dma_tx_request returns negative errno on
>> >> failure.
>> >>
>> >> > +
>> >> > + init_waitqueue_head(&sport->dma_wait);
>> >> > + sport->lpuart_dma_tx_use = true; if (lpuart_is_32(sport)) {
>> >> > + uartbaud = lpuart32_read(&sport->port, UARTBAUD);
>> >> > + lpuart32_write(&sport->port,
>> >> > + uartbaud | UARTBAUD_TDMAE,
>> UARTBAUD);
>> >> > } else {
>> >> > - sport->lpuart_dma_tx_use = false;
>> >> > + writeb(readb(sport->port.membase + UARTCR5) |
>> >> > + UARTCR5_TDMAS, sport->port.membase +
>> UARTCR5);
>> >> > }
>> >> > +
>> >> > + return;
>> >> > +
>> >> > +err:
>> >> > + sport->lpuart_dma_tx_use = false;
>> >> > }
>> >> >
>> >> > static void lpuart_rx_dma_startup(struct lpuart_port *sport)
>> >> > {
>> >> > - if (sport->dma_rx_chan && !lpuart_start_rx_dma(sport)) {
>> >> > - /* set Rx DMA timeout */
>> >> > - sport->dma_rx_timeout =
>> msecs_to_jiffies(DMA_RX_TIMEOUT);
>> >> > - if (!sport->dma_rx_timeout)
>> >> > - sport->dma_rx_timeout = 1;
>> >> > + int ret;
>> >> >
>> >> > - sport->lpuart_dma_rx_use = true;
>> >> > - rx_dma_timer_init(sport);
>> >> > - } else {
>> >> > - sport->lpuart_dma_rx_use = false;
>> >> > + 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;
>> >> > + goto err;
>> >> > }
>> >> > +
>> >> > + ret = lpuart_start_rx_dma(sport); if (ret)
>> >> > + goto err;
>> >>
>> >> This is not backwards.
>> >>
>> >> > +
>> >> > + /* set Rx DMA timeout */
>> >> > + sport->dma_rx_timeout = msecs_to_jiffies(DMA_RX_TIMEOUT);
if
>> >> > + (!sport->dma_rx_timeout)
>> >> > + sport->dma_rx_timeout = 1;
>> >> > +
>> >> > + sport->lpuart_dma_rx_use = true; rx_dma_timer_init(sport);
>> >> > +
>> >> > + return;
>> >> > +
>> >> > +err:
>> >> > + sport->lpuart_dma_rx_use = false;
>> >> > }
>> >> >
>> >> > static int lpuart_startup(struct uart_port *port) @@ -1615,6
>> >> > +1646,11 @@ static void lpuart_dma_shutdown(struct lpuart_port
>> >> > +*sport)
>> >> >
>> dmaengine_terminate_all(sport->dma_tx_chan);
>> >> > }
>> >> > }
>> >> > +
>> >> > + if (sport->dma_tx_chan)
>> >> > + dma_release_channel(sport->dma_tx_chan);
>> >> > + if (sport->dma_rx_chan)
>> >> > + dma_release_channel(sport->dma_rx_chan);
>> >> > }
>> >> >
>> >> > static void lpuart_shutdown(struct uart_port *port) @@
>> >> > -2520,16
>> >> > +2556,6 @@ static int lpuart_probe(struct platform_device *pdev)
>> >> >
>> >> > sport->port.rs485_config(&sport->port, &sport->port.rs485);
>> >> >
>> >> > - sport->dma_tx_chan =
>> >> > dma_request_slave_channel(sport->port.dev,
>> >> "tx");
>> >> > - if (!sport->dma_tx_chan)
>> >> > - dev_info(sport->port.dev, "DMA tx channel request
failed, "
>> >> > - "operating without tx DMA\n");
>> >> > -
>> >> > - sport->dma_rx_chan =
>> >> > dma_request_slave_channel(sport->port.dev,
>> >> "rx");
>> >> > - if (!sport->dma_rx_chan)
>> >> > - dev_info(sport->port.dev, "DMA rx channel request
failed, "
>> >> > - "operating without rx DMA\n");
>> >> > -
>> >> > return 0;
>> >> >
>> >> > failed_attach_port:
>> >> >
