Re: [PATCH 6/8] dmaengine: tegra: Use iommu-map for stream ID

[Akhil R <akhilrajeev@nvidia.com>]

On Tue, 17 Feb 2026 14:52:17 -0500, Frank Li wrote:
> On Tue, Feb 17, 2026 at 11:04:55PM +0530, Akhil R wrote:
>> Use iommu-map, when provided, to get the stream ID to be programmed
>> for each channel. Register each channel separately for allowing it
>> to use a separate IOMMU domain for the transfer.
>>
>> Channels will continue to use the same global stream ID if iommu-map
>> property is not present in the device tree.
>>
>> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
>> ---
>>  drivers/dma/tegra186-gpc-dma.c | 62 +++++++++++++++++++++++++++-------
>>  1 file changed, 49 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/dma/tegra186-gpc-dma.c b/drivers/dma/tegra186-gpc-dma.c
>> index ce3b1dd52bb3..b8ca269fa3ba 100644
>> --- a/drivers/dma/tegra186-gpc-dma.c
>> +++ b/drivers/dma/tegra186-gpc-dma.c
>> @@ -15,6 +15,7 @@
>>  #include <linux/module.h>
>>  #include <linux/of.h>
>>  #include <linux/of_dma.h>
>> +#include <linux/of_device.h>
>>  #include <linux/platform_device.h>
>>  #include <linux/reset.h>
>>  #include <linux/slab.h>
>> @@ -1403,9 +1404,12 @@ static int tegra_dma_program_sid(struct tegra_dma_channel *tdc, int stream_id)
>>  static int tegra_dma_probe(struct platform_device *pdev)
>>  {
>>  	const struct tegra_dma_chip_data *cdata = NULL;
>> +	struct tegra_dma_channel *tdc;
>> +	struct tegra_dma *tdma;
>> +	struct dma_chan *chan;
>> +	bool use_iommu_map = false;
>>  	unsigned int i;
>>  	u32 stream_id;
>> -	struct tegra_dma *tdma;
>>  	int ret;
>>
>>  	cdata = of_device_get_match_data(&pdev->dev);
>> @@ -1433,9 +1437,12 @@ static int tegra_dma_probe(struct platform_device *pdev)
>>
>>  	tdma->dma_dev.dev = &pdev->dev;
>>
>> -	if (!tegra_dev_iommu_get_stream_id(&pdev->dev, &stream_id)) {
>> -		dev_err(&pdev->dev, "Missing iommu stream-id\n");
>> -		return -EINVAL;
>> +	use_iommu_map = of_property_present(pdev->dev.of_node, "iommu-map");
>> +	if (!use_iommu_map) {
>> +		if (!tegra_dev_iommu_get_stream_id(&pdev->dev, &stream_id)) {
>> +			dev_err(&pdev->dev, "Missing iommu stream-id\n");
>> +			return -EINVAL;
>> +		}
>>  	}
>>
>>  	ret = device_property_read_u32(&pdev->dev, "dma-channel-mask",
>> @@ -1449,7 +1456,7 @@ static int tegra_dma_probe(struct platform_device *pdev)
>>
>>  	INIT_LIST_HEAD(&tdma->dma_dev.channels);
>>  	for (i = 0; i < cdata->nr_channels; i++) {
>> -		struct tegra_dma_channel *tdc = &tdma->channels[i];
>> +		tdc = &tdma->channels[i];
>>
>>  		/* Check for channel mask */
>>  		if (!(tdma->chan_mask & BIT(i)))
>> @@ -1469,10 +1476,6 @@ static int tegra_dma_probe(struct platform_device *pdev)
>>
>>  		vchan_init(&tdc->vc, &tdma->dma_dev);
>>  		tdc->vc.desc_free = tegra_dma_desc_free;
>> -
>> -		/* program stream-id for this channel */
>> -		tegra_dma_program_sid(tdc, stream_id);
>> -		tdc->stream_id = stream_id;
>>  	}
>>
>>  	ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(cdata->addr_bits));
>> @@ -1517,20 +1520,53 @@ static int tegra_dma_probe(struct platform_device *pdev)
>>  		return ret;
>>  	}
>>
>> +	list_for_each_entry(chan, &tdma->dma_dev.channels, device_node) {
>> +		struct device *chdev = &chan->dev->device;
>>
> why no use
> 	for (i = 0; i < cdata->nr_channels; i++) {
> 		struct tegra_dma_channel *tdc = &tdma->channels[i];

I thought this would ensure that we try to configure only the channels
where the chan_dev and vchan are initialized. I understand that it is
not probable in the current implementation that we will have channels
which are uninitialized, but I felt this a better approach.
Do you see any disadvantage in using the channels list?

>> +
>> +		tdc = to_tegra_dma_chan(chan);
>> +		if (use_iommu_map) {
>> +			chdev->coherent_dma_mask = pdev->dev.coherent_dma_mask;
>> +			chdev->dma_mask = &chdev->coherent_dma_mask;
>> +			chdev->bus = pdev->dev.bus;
>> +
>> +			ret = of_dma_configure_id(chdev, pdev->dev.of_node,
>> +						  true, &tdc->id);
>> +			if (ret) {
>> +				dev_err(chdev, "Failed to configure IOMMU for channel %d: %d\n",
>> +					tdc->id, ret);
>> +				goto err_unregister;
>> +			}
>> +
>> +			if (!tegra_dev_iommu_get_stream_id(chdev, &stream_id)) {
>> +				dev_err(chdev, "Failed to get stream ID for channel %d\n",
>> +					tdc->id);
>> +				goto err_unregister;
>> +			}
>> +
>> +			chan->dev->chan_dma_dev = true;
>> +		}
>> +
>> +		/* program stream-id for this channel */
>> +		tegra_dma_program_sid(tdc, stream_id);
>> +		tdc->stream_id = stream_id;
>> +	}
>> +
>>  	ret = of_dma_controller_register(pdev->dev.of_node,
>>  					 tegra_dma_of_xlate, tdma);
>>  	if (ret < 0) {
>>  		dev_err_probe(&pdev->dev, ret,
>>  			      "GPC DMA OF registration failed\n");
>> -
>> -		dma_async_device_unregister(&tdma->dma_dev);
>> -		return ret;
>> +		goto err_unregister;
>>  	}
>>
>> -	dev_info(&pdev->dev, "GPC DMA driver register %lu channels\n",
>> +	dev_info(&pdev->dev, "GPC DMA driver registered %lu channels\n",
>>  		 hweight_long(tdma->chan_mask));
>>
>>  	return 0;
>> +
>> +err_unregister:
>> +	dma_async_device_unregister(&tdma->dma_dev);
>
> Can you use dmaenginem_async_device_register() to simple error path?
>
> Frank

Agree. I will update it to use this function instead.

>> +	return ret;
>>  }
>>
>>  static void tegra_dma_remove(struct platform_device *pdev)
>> --

Regards,
Akhil