diff mbox series

[v3,3/9] tty: serial: fsl_lpuart: handle EPROBE_DEFER for DMA

Message ID 20200303174306.6015-4-michael@walle.cc (mailing list archive)
State New, archived
Headers show
Series tty: serial: fsl_lpuart various fixes and LS1028A support | expand

Commit Message

Michael Walle March 3, 2020, 5:43 p.m. UTC
The DMA channel might not be available at probe time. This is esp. the
case if the DMA controller has an IOMMU mapping.

There is also another caveat. If there is no DMA controller at all,
dma_request_chan() will also return -EPROBE_DEFER. Thus we cannot test
for -EPROBE_DEFER in probe(). Otherwise the lpuart driver will fail to
probe if, for example, the DMA driver is not enabled in the kernel
configuration.

To workaround this, we request the DMA channel in _startup(). Other
serial drivers do it the same way.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 drivers/tty/serial/fsl_lpuart.c | 84 +++++++++++++++++++++------------
 1 file changed, 53 insertions(+), 31 deletions(-)

Comments

Rob Herring (Arm) March 3, 2020, 6:47 p.m. UTC | #1
On Tue, Mar 03, 2020 at 06:43:00PM +0100, Michael Walle wrote:
> The DMA channel might not be available at probe time. This is esp. the
> case if the DMA controller has an IOMMU mapping.

The subject should be updated as this doesn't involve deferred probe any 
more. 

> There is also another caveat. If there is no DMA controller at all,
> dma_request_chan() will also return -EPROBE_DEFER. Thus we cannot test
> for -EPROBE_DEFER in probe(). Otherwise the lpuart driver will fail to
> probe if, for example, the DMA driver is not enabled in the kernel
> configuration.
> 
> To workaround this, we request the DMA channel in _startup(). Other
> serial drivers do it the same way.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
>  drivers/tty/serial/fsl_lpuart.c | 84 +++++++++++++++++++++------------
>  1 file changed, 53 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
> index c31b8f3db6bf..0b8c477b32a3 100644
> --- a/drivers/tty/serial/fsl_lpuart.c
> +++ b/drivers/tty/serial/fsl_lpuart.c
> @@ -1493,36 +1493,63 @@ 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_slave_channel(sport->port.dev, "tx");
> +	if (!sport->dma_tx_chan) {
> +		dev_info_once(sport->port.dev,
> +			      "DMA tx channel request failed, operating without tx DMA\n");

Might be useful to print the errno too.

Rob
Michael Walle March 3, 2020, 8:57 p.m. UTC | #2
Am 3. März 2020 19:47:37 MEZ schrieb Rob Herring <robh@kernel.org>:
>On Tue, Mar 03, 2020 at 06:43:00PM +0100, Michael Walle wrote:
>> The DMA channel might not be available at probe time. This is esp.
>the
>> case if the DMA controller has an IOMMU mapping.
>
>The subject should be updated as this doesn't involve deferred probe
>any more. 

ok

>
>> There is also another caveat. If there is no DMA controller at all,
>> dma_request_chan() will also return -EPROBE_DEFER. Thus we cannot
>test
>> for -EPROBE_DEFER in probe(). Otherwise the lpuart driver will fail
>to
>> probe if, for example, the DMA driver is not enabled in the kernel
>> configuration.
>> 
>> To workaround this, we request the DMA channel in _startup(). Other
>> serial drivers do it the same way.
>> 
>> Signed-off-by: Michael Walle <michael@walle.cc>
>> ---
>>  drivers/tty/serial/fsl_lpuart.c | 84
>+++++++++++++++++++++------------
>>  1 file changed, 53 insertions(+), 31 deletions(-)
>> 
>> diff --git a/drivers/tty/serial/fsl_lpuart.c
>b/drivers/tty/serial/fsl_lpuart.c
>> index c31b8f3db6bf..0b8c477b32a3 100644
>> --- a/drivers/tty/serial/fsl_lpuart.c
>> +++ b/drivers/tty/serial/fsl_lpuart.c
>> @@ -1493,36 +1493,63 @@ 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_slave_channel(sport->port.dev,
>"tx");
>> +	if (!sport->dma_tx_chan) {
>> +		dev_info_once(sport->port.dev,
>> +			      "DMA tx channel request failed, operating without tx
>DMA\n");
>
>Might be useful to print the errno too.

I didn't want to change the original error message. But I can change that when I update the subject. 

-michael
diff mbox series

Patch

diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index c31b8f3db6bf..0b8c477b32a3 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -1493,36 +1493,63 @@  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_slave_channel(sport->port.dev, "tx");
+	if (!sport->dma_tx_chan) {
+		dev_info_once(sport->port.dev,
+			      "DMA tx channel request failed, operating without tx DMA\n");
+		goto err;
+	}
+
+	ret = lpuart_dma_tx_request(&sport->port);
+	if (!ret)
+		goto err;
+
+	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_slave_channel(sport->port.dev, "rx");
+	if (!sport->dma_rx_chan) {
+		dev_info_once(sport->port.dev,
+			      "DMA rx channel request failed, operating without rx DMA\n");
+		goto err;
 	}
+
+	ret = lpuart_start_rx_dma(sport);
+	if (ret)
+		goto err;
+
+	/* 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 +1642,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 +2552,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: