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

[Akhil R]

On Wed, 25 Feb 2026 11:23:08 +0000 Jon Hunter wrote:
> On 25/02/2026 10:27, Akhil R wrote:
>> On Tue, 24 Feb 2026 16:59:44 -0500 Frank Li wrote:
>>> 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@xxxxxxxxxx>
>>>>>> ---
>>>>>>   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.
>> 
>> I think we will not get to use chan->dev->device before
>> async_device_register() and thats why I added a separate loop to
>> configure the channels.
> 
> I assume that this needs to be done after the async_device_register()? 
> If so we should note that in the commit message to explain that we need 
> a 2nd loop. Unless we can move the 1st loop after the 
> async_device_register() and just have one loop?

dma_dev.channels is populated by vchan_init(). So, the first loop must
exist before async_device_register(). If we have to restrict to one loop,
we would need to use dma_async_device_channel_register() to register the
channels within this driver directly. I feel having a second loop is
better than trying to restrict it to one.

> 
>> I can add a comment on why we need this loop. Do you suggest
>> changing it to a for-loop itself? Let me know your thoughts.
> 
> If using the list avoids the following check, then probably good to keep 
> as is, but yes explain why we do it this way.
> 
>   /* Check for channel mask */
>   if (!(tdma->chan_mask & BIT(i)))
>           continue;

Yes. Using dma_dev.channels avoids the above check. I will describe this
better in the commit message and as comments.

Regards,
Akhil