Re: [PATCH 1/2] serial: use dmaengine_get_dma_device() instead of chan->device->dev

[Andy Shevchenko]

On Fri, Sep 25, 2026 at 04:08:30PM -0400, Frank.Li@xxxxxxxxxxx wrote:
> From: Frank Li <Frank.Li@xxxxxxx>
> 
> Replace direct dma_chan::device::dev access with the proper
> dmaengine_get_dma_device() for consumer API
> 
> chan->device->dev is not always the device used for DMA mapping.
> Some DMA engines support per-channel IOMMU mappings, so different
> channels may use different DMA devices.  dmaengine_get_dma_device()
> returns the correct device for each channel.
> 
> This also prepares for making the DMA engine provider data structures
> private. DMA consumers should not access DMA engine internals directly.

...

>  	/* RX buffer */
>  	if (!dma->rx_size)
>  		dma->rx_size = PAGE_SIZE;
>  
> -	dma->rx_buf = dma_alloc_coherent(dma->rxchan->device->dev, dma->rx_size,
> +	dma->rx_buf = dma_alloc_coherent(rx_dev, dma->rx_size,
>  					&dma->rx_addr, GFP_KERNEL);

Now one parameter can be moved up and positive outcome the split becomes logical
(on a logic boundaries).

>  	if (!dma->rx_buf) {
>  		ret = -ENOMEM;

...

>  	/* TX buffer */
> -	dma->tx_addr = dma_map_single(dma->txchan->device->dev,
> +	dma->tx_addr = dma_map_single(tx_dev,
>  					p->port.state->port.xmit_buf,
>  					UART_XMIT_SIZE,
>  					DMA_TO_DEVICE);

You can fix indentation while at it.

> -	if (dma_mapping_error(dma->txchan->device->dev, dma->tx_addr)) {
> -		dma_free_coherent(dma->rxchan->device->dev, dma->rx_size,
> +	if (dma_mapping_error(tx_dev, dma->tx_addr)) {
> +		dma_free_coherent(rx_dev, dma->rx_size,
>  				  dma->rx_buf, dma->rx_addr);
>  		ret = -ENOMEM;

...

>  	/* Release RX resources */
>  	dmaengine_terminate_sync(dma->rxchan);
>  	dma->rx_running = 0;
> -	dma_free_coherent(dma->rxchan->device->dev, dma->rx_size, dma->rx_buf,
> +	dma_free_coherent(dmaengine_get_dma_device(dma->rxchan), dma->rx_size, dma->rx_buf,
>  			  dma->rx_addr);

And here the last parameter of the previous line can be moved to the next line.

...

> +	db->buf = dma_alloc_coherent(dmaengine_get_dma_device(chan), PL011_DMA_BUFFER_SIZE,
>  				     &db->dma, GFP_KERNEL);

^^^ (1)

>  	if (!db->buf)
>  		return -ENOMEM;

...

>  {
>  	if (db->buf) {
> -		dma_free_coherent(chan->device->dev,
> +		dma_free_coherent(dmaengine_get_dma_device(chan),
>  				  PL011_DMA_BUFFER_SIZE, db->buf, db->dma);

Perhaps you want both (1) and this be consistent, either (1) be rewrapped,
or this one

		dma_free_coherent(dmaengine_get_dma_device(chan), PL011_DMA_BUFFER_SIZE,
				  db->buf, db->dma);

>  	}

...

>  	struct pch_dma_slave *param = slave;
>  
>  	if ((chan->chan_id == param->chan_id) && (param->dma_dev ==
> -						  chan->device->dev)) {
> +						  dmaengine_get_dma_device(chan))) {

Even original code has broken indentation. What about rewrapping it?

	if ((chan->chan_id == param->chan_id) &&
	    (param->dma_dev == dmaengine_get_dma_device(chan))) {

>  		chan->private = param;
>  		return true;

-- 
With Best Regards,
Andy Shevchenko