Re: [PATCH v5] serial: samsung: use dma_ops of DMA if attached

[Marek Szyprowski]

On 29.06.2021 06:59, Tamseel Shams wrote:
> When DMA is used for TX and RX by serial driver, it should
> pass the DMA device pointer to DMA API instead of UART device
> pointer. DMA device should be used for DMA API because only
> the DMA device is aware of how the device connects to the memory.
> There might be an extra level of address translation due to a
> SMMU attached to the DMA device. When serial device is used for
> DMA API, the DMA API will have no clue of the SMMU attached to
> the DMA device.
>
> This patch is necessary to fix the SMMU page faults
> which is observed when a DMA(with SMMU enabled) is attached
> to UART for transfer.
>
> Signed-off-by: Tamseel Shams <m.shams@xxxxxxxxxxx>
> Signed-off-by: Ajay Kumar <ajaykumar.rs@xxxxxxxxxxx>

Acked-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>

> ---
> Changes since v1:
> 1. Rebased the patch on "tty-next" branch of TTY driver tree
>
> Changes since v2:
> 1. Updated the commit message.
> 2. Changed the comment description
>
> Changes since v3:
> 1. Removed the null pointer check for "dma", "dma->tx_chan" and
> "dma->rx_chan" and instead sending DMA device pointer while calling
> DMA API.
>
> Changes since v4:
> 1. Fixed the alignments of arguments.
>
>   drivers/tty/serial/samsung_tty.c | 46 +++++++++++++++++---------------
>   1 file changed, 25 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
> index 9fbc61151c2e..0cf4dfe77c32 100644
> --- a/drivers/tty/serial/samsung_tty.c
> +++ b/drivers/tty/serial/samsung_tty.c
> @@ -305,8 +305,9 @@ static void s3c24xx_serial_stop_tx(struct uart_port *port)
>   		dmaengine_pause(dma->tx_chan);
>   		dmaengine_tx_status(dma->tx_chan, dma->tx_cookie, &state);
>   		dmaengine_terminate_all(dma->tx_chan);
> -		dma_sync_single_for_cpu(ourport->port.dev,
> -			dma->tx_transfer_addr, dma->tx_size, DMA_TO_DEVICE);
> +		dma_sync_single_for_cpu(dma->tx_chan->device->dev,
> +					dma->tx_transfer_addr, dma->tx_size,
> +					DMA_TO_DEVICE);
>   		async_tx_ack(dma->tx_desc);
>   		count = dma->tx_bytes_requested - state.residue;
>   		xmit->tail = (xmit->tail + count) & (UART_XMIT_SIZE - 1);
> @@ -338,8 +339,9 @@ static void s3c24xx_serial_tx_dma_complete(void *args)
>   	count = dma->tx_bytes_requested - state.residue;
>   	async_tx_ack(dma->tx_desc);
>   
> -	dma_sync_single_for_cpu(ourport->port.dev, dma->tx_transfer_addr,
> -				dma->tx_size, DMA_TO_DEVICE);
> +	dma_sync_single_for_cpu(dma->tx_chan->device->dev,
> +				dma->tx_transfer_addr, dma->tx_size,
> +				DMA_TO_DEVICE);
>   
>   	spin_lock_irqsave(&port->lock, flags);
>   
> @@ -443,8 +445,9 @@ static int s3c24xx_serial_start_tx_dma(struct s3c24xx_uart_port *ourport,
>   	dma->tx_size = count & ~(dma_get_cache_alignment() - 1);
>   	dma->tx_transfer_addr = dma->tx_addr + xmit->tail;
>   
> -	dma_sync_single_for_device(ourport->port.dev, dma->tx_transfer_addr,
> -				dma->tx_size, DMA_TO_DEVICE);
> +	dma_sync_single_for_device(dma->tx_chan->device->dev,
> +				   dma->tx_transfer_addr, dma->tx_size,
> +				   DMA_TO_DEVICE);
>   
>   	dma->tx_desc = dmaengine_prep_slave_single(dma->tx_chan,
>   				dma->tx_transfer_addr, dma->tx_size,
> @@ -515,7 +518,7 @@ static void s3c24xx_uart_copy_rx_to_tty(struct s3c24xx_uart_port *ourport,
>   	if (!count)
>   		return;
>   
> -	dma_sync_single_for_cpu(ourport->port.dev, dma->rx_addr,
> +	dma_sync_single_for_cpu(dma->rx_chan->device->dev, dma->rx_addr,
>   				dma->rx_size, DMA_FROM_DEVICE);
>   
>   	ourport->port.icount.rx += count;
> @@ -636,8 +639,8 @@ static void s3c64xx_start_rx_dma(struct s3c24xx_uart_port *ourport)
>   {
>   	struct s3c24xx_uart_dma *dma = ourport->dma;
>   
> -	dma_sync_single_for_device(ourport->port.dev, dma->rx_addr,
> -				dma->rx_size, DMA_FROM_DEVICE);
> +	dma_sync_single_for_device(dma->rx_chan->device->dev, dma->rx_addr,
> +				   dma->rx_size, DMA_FROM_DEVICE);
>   
>   	dma->rx_desc = dmaengine_prep_slave_single(dma->rx_chan,
>   				dma->rx_addr, dma->rx_size, DMA_DEV_TO_MEM,
> @@ -1102,18 +1105,19 @@ static int s3c24xx_serial_request_dma(struct s3c24xx_uart_port *p)
>   		goto err_release_tx;
>   	}
>   
> -	dma->rx_addr = dma_map_single(p->port.dev, dma->rx_buf,
> -				dma->rx_size, DMA_FROM_DEVICE);
> -	if (dma_mapping_error(p->port.dev, dma->rx_addr)) {
> +	dma->rx_addr = dma_map_single(dma->rx_chan->device->dev, dma->rx_buf,
> +				      dma->rx_size, DMA_FROM_DEVICE);
> +	if (dma_mapping_error(dma->rx_chan->device->dev, dma->rx_addr)) {
>   		reason = "DMA mapping error for RX buffer";
>   		ret = -EIO;
>   		goto err_free_rx;
>   	}
>   
>   	/* TX buffer */
> -	dma->tx_addr = dma_map_single(p->port.dev, p->port.state->xmit.buf,
> -				UART_XMIT_SIZE, DMA_TO_DEVICE);
> -	if (dma_mapping_error(p->port.dev, dma->tx_addr)) {
> +	dma->tx_addr = dma_map_single(dma->tx_chan->device->dev,
> +				      p->port.state->xmit.buf, UART_XMIT_SIZE,
> +				      DMA_TO_DEVICE);
> +	if (dma_mapping_error(dma->tx_chan->device->dev, dma->tx_addr)) {
>   		reason = "DMA mapping error for TX buffer";
>   		ret = -EIO;
>   		goto err_unmap_rx;
> @@ -1122,8 +1126,8 @@ static int s3c24xx_serial_request_dma(struct s3c24xx_uart_port *p)
>   	return 0;
>   
>   err_unmap_rx:
> -	dma_unmap_single(p->port.dev, dma->rx_addr, dma->rx_size,
> -			 DMA_FROM_DEVICE);
> +	dma_unmap_single(dma->rx_chan->device->dev, dma->rx_addr,
> +			 dma->rx_size, DMA_FROM_DEVICE);
>   err_free_rx:
>   	kfree(dma->rx_buf);
>   err_release_tx:
> @@ -1142,8 +1146,8 @@ static void s3c24xx_serial_release_dma(struct s3c24xx_uart_port *p)
>   
>   	if (dma->rx_chan) {
>   		dmaengine_terminate_all(dma->rx_chan);
> -		dma_unmap_single(p->port.dev, dma->rx_addr,
> -				dma->rx_size, DMA_FROM_DEVICE);
> +		dma_unmap_single(dma->rx_chan->device->dev, dma->rx_addr,
> +				 dma->rx_size, DMA_FROM_DEVICE);
>   		kfree(dma->rx_buf);
>   		dma_release_channel(dma->rx_chan);
>   		dma->rx_chan = NULL;
> @@ -1151,8 +1155,8 @@ static void s3c24xx_serial_release_dma(struct s3c24xx_uart_port *p)
>   
>   	if (dma->tx_chan) {
>   		dmaengine_terminate_all(dma->tx_chan);
> -		dma_unmap_single(p->port.dev, dma->tx_addr,
> -				UART_XMIT_SIZE, DMA_TO_DEVICE);
> +		dma_unmap_single(dma->tx_chan->device->dev, dma->tx_addr,
> +				 UART_XMIT_SIZE, DMA_TO_DEVICE);
>   		dma_release_channel(dma->tx_chan);
>   		dma->tx_chan = NULL;
>   	}

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland