Re: [PATCH v6 2/2] of: Add support for "iommu-ranges" on a device node

[Vikash Garodia <vikash.garodia@oss.qualcomm.com>]

Hi Rob,

Appreciate your reviews/suggestions on this patch.

On 11/4/2026 9:33 PM, Rob Herring wrote:
> On Tue, Nov 03, 2026 at 08:59:33PM +0530, Vikash Garodia wrote:
>> Reserving an IOVA address using "iommu-addresses" requires it to be
>> placed in the reserved-memory node. But when "iommu-addresses" is the
>> only property being described and there is no backing "reg" (i.e. no
>> actual reserved system memory), it does not really belong under
>> /reserved-memory. A device IOVA range is specific to its own address
>> space, and does not describe the system physical memory to make it
>> qualify under reserved-memory. Given this, place "iommu-addresses"
>> inside reserved-memory only when it is paired with a "reg", otherwise,
>> define it within the device own node. It was discussed here:
>> https://lore.kernel.org/all/662f7093-fb0a-4564-9ca0-98e03e68ba9a@kernel.org
>>
>> Existing "iommu-addresses" property expects a phandle, which does
>> not make sense when the property is defined within the device node.
>> "iommu-ranges" was introduced to specify the device specific IOVA ranges
>> when there is no backing "reg".
>>
>> Suggested-by: Rob Herring <robh@kernel.org>
>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
>> Co-developed-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
>> Signed-off-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
>> Signed-off-by: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
>> ---
>>   drivers/iommu/of_iommu.c | 134 +++++++++++++++++++++++++++++++++++++++++++----
>>   1 file changed, 125 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
>> index 5c266511db78910d6fd221af5437cecff4931c22..0428ceb50dec58f11872b4fb64e283fa8d9046a3 100644
>> --- a/drivers/iommu/of_iommu.c
>> +++ b/drivers/iommu/of_iommu.c
>> @@ -185,11 +185,118 @@ iommu_resv_region_get_type(struct device *dev,
>>   	if (start == phys->start && end == phys->end)
>>   		return IOMMU_RESV_DIRECT;
>>   
>> -	dev_warn(dev, "treating non-direct mapping [%pr] -> [%pap-%pap] as reservation\n", phys,
>> -		 &start, &end);
>> +	dev_err(dev, "non-direct mapping [%pr] -> [%pap-%pap] is not supported\n", phys,
>> +		&start, &end);
>>   	return IOMMU_RESV_RESERVED;
>>   }
>>   
>> +/**
>> + * of_iommu_derive_resv_regions - derive reserved regions which
>> + *                                are outside of iommu-ranges
>> + * @dev: device for which to get reserved regions
>> + * @list: reserved region list
>> + *
>> + * A device can describe its own usable IOVA ranges directly on its node
>> + * via "iommu-ranges". Everything not under those ranges is derived
>> + * as a reserved region so the IOMMU allocator won't use it. Entries may
>> + * appear in any order in the property.
>> + */
>> +#if IS_ENABLED(CONFIG_OF_ADDRESS)
> 
> Is this really needed? Assuming CONFIG_OF is enabled, this is only false
> on Sparc. I don't think Sparc needs anything in this file. There
> aren't any IOMMU drivers for Sparc unless COMPILE_TEST is enabled. So
> maybe building of_iommu.c as a whole should depend on CONFIG_OF_ADDRESS?

Agree, that's much better than adding the check, will drop from other 
places too and make it conditional compilation.

Regards,
Vikash

> 
> Rob