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

[Frank Li <Frank.li@nxp.com>]

On Tue, Feb 24, 2026 at 11:55:43AM +0530, Akhil R wrote:
> 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?

not big issue, just strange, previous code use
for (i = 0; i < cdata->nr_channels; i++) {
}

why need enumerate it again and use difference method.

Frank
>
> >> +
> >> +		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