[PATCH V4 1/3] iommu/arm-smmu-v3: Add device-tree support for CMDQV driver

Ashish Mhetre posted 3 patches 2 weeks ago
There is a newer version of this series
[PATCH V4 1/3] iommu/arm-smmu-v3: Add device-tree support for CMDQV driver
Posted by Ashish Mhetre 2 weeks ago
Add device tree support to the CMDQV driver to enable usage on Tegra264
SoCs. The implementation parses the nvidia,cmdqv phandle from the SMMU
device tree node to associate each SMMU with its corresponding CMDQV
instance based on compatible string.

Remove the dependency from Kconfig as the driver now supports both ACPI
and device tree initialization through conditional compilation and
ARM_SMMU_V3 depends on ARM64 which implies at least OF.

Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Ashish Mhetre <amhetre@nvidia.com>
---
 drivers/iommu/arm/Kconfig                     |  1 -
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   | 32 +++++++++++++++++++
 .../iommu/arm/arm-smmu-v3/tegra241-cmdqv.c    | 27 +++++++++++++++-
 3 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/arm/Kconfig b/drivers/iommu/arm/Kconfig
index ef42bbe07dbe..5fac08b89dee 100644
--- a/drivers/iommu/arm/Kconfig
+++ b/drivers/iommu/arm/Kconfig
@@ -121,7 +121,6 @@ config ARM_SMMU_V3_KUNIT_TEST
 
 config TEGRA241_CMDQV
 	bool "NVIDIA Tegra241 CMDQ-V extension support for ARM SMMUv3"
-	depends on ACPI
 	help
 	  Support for NVIDIA CMDQ-Virtualization extension for ARM SMMUv3. The
 	  CMDQ-V extension is similar to v3.3 ECMDQ for multi command queues
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index d16d35c78c06..9433cd91c68f 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -4530,6 +4530,35 @@ static int arm_smmu_device_hw_probe(struct arm_smmu_device *smmu)
 	return 0;
 }
 
+#ifdef CONFIG_TEGRA241_CMDQV
+static void tegra_cmdqv_dt_probe(struct device_node *smmu_node,
+				 struct arm_smmu_device *smmu)
+{
+	struct platform_device *pdev;
+	struct device_node *np;
+
+	np = of_parse_phandle(smmu_node, "nvidia,cmdqv", 0);
+	if (!np)
+		return;
+
+	pdev = of_find_device_by_node(np);
+	of_node_put(np);
+	if (!pdev)
+		return;
+
+	smmu->impl_dev = &pdev->dev;
+	smmu->options |= ARM_SMMU_OPT_TEGRA241_CMDQV;
+	dev_info(smmu->dev, "found companion CMDQV device: %s\n",
+		 dev_name(smmu->impl_dev));
+	put_device(&pdev->dev);
+}
+#else
+static void tegra_cmdqv_dt_probe(struct device_node *smmu_node,
+				 struct arm_smmu_device *smmu)
+{
+}
+#endif
+
 #ifdef CONFIG_ACPI
 #ifdef CONFIG_TEGRA241_CMDQV
 static void acpi_smmu_dsdt_probe_tegra241_cmdqv(struct acpi_iort_node *node,
@@ -4634,6 +4663,9 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev,
 	if (of_dma_is_coherent(dev->of_node))
 		smmu->features |= ARM_SMMU_FEAT_COHERENCY;
 
+	if (of_device_is_compatible(dev->of_node, "nvidia,tegra264-smmu"))
+		tegra_cmdqv_dt_probe(dev->of_node, smmu);
+
 	return ret;
 }
 
diff --git a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
index 378104cd395e..c096c8229c5d 100644
--- a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
+++ b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
@@ -11,6 +11,8 @@
 #include <linux/iommufd.h>
 #include <linux/iopoll.h>
 #include <uapi/linux/iommufd.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
 
 #include <acpi/acpixf.h>
 
@@ -917,6 +919,26 @@ tegra241_cmdqv_find_acpi_resource(struct device *dev, int *irq)
 	return res;
 }
 
+static struct resource *
+tegra241_cmdqv_find_dt_resource(struct device *dev, int *irq)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct resource *res;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(dev, "no memory resource found for CMDQV\n");
+		return NULL;
+	}
+
+	if (irq)
+		*irq = platform_get_irq_optional(pdev, 0);
+	if (!irq || *irq <= 0)
+		dev_warn(dev, "no interrupt. errors will not be reported\n");
+
+	return res;
+}
+
 static int tegra241_cmdqv_init_structures(struct arm_smmu_device *smmu)
 {
 	struct tegra241_cmdqv *cmdqv =
@@ -1048,11 +1070,14 @@ struct arm_smmu_device *tegra241_cmdqv_probe(struct arm_smmu_device *smmu)
 
 	if (!smmu->dev->of_node)
 		res = tegra241_cmdqv_find_acpi_resource(smmu->impl_dev, &irq);
+	else
+		res = tegra241_cmdqv_find_dt_resource(smmu->impl_dev, &irq);
 	if (!res)
 		goto out_fallback;
 
 	new_smmu = __tegra241_cmdqv_probe(smmu, res, irq);
-	kfree(res);
+	if (!smmu->dev->of_node)
+		kfree(res);
 
 	if (new_smmu)
 		return new_smmu;
-- 
2.25.1
Re: [PATCH V4 1/3] iommu/arm-smmu-v3: Add device-tree support for CMDQV driver
Posted by kernel test robot 1 week, 3 days ago
Hi Ashish,

kernel test robot noticed the following build warnings:

[auto build test WARNING on next-20251204]
[also build test WARNING on v6.18]
[cannot apply to robh/for-next linus/master v6.18 v6.18-rc7 v6.18-rc6]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ashish-Mhetre/iommu-arm-smmu-v3-Add-device-tree-support-for-CMDQV-driver/20251205-151258
base:   next-20251204
patch link:    https://lore.kernel.org/r/20251205065850.3841834-2-amhetre%40nvidia.com
patch subject: [PATCH V4 1/3] iommu/arm-smmu-v3: Add device-tree support for CMDQV driver
config: arm64-randconfig-004-20251209 (https://download.01.org/0day-ci/archive/20251209/202512090331.QAFgb6vQ-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 11.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251209/202512090331.QAFgb6vQ-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202512090331.QAFgb6vQ-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c: In function 'tegra241_cmdqv_acpi_is_memory':
   drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c:863:17: error: implicit declaration of function 'acpi_dev_resource_address_space' [-Werror=implicit-function-declaration]
     863 |         return !acpi_dev_resource_address_space(res, &win);
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c: In function 'tegra241_cmdqv_acpi_get_irqs':
   drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c:871:26: error: implicit declaration of function 'acpi_dev_resource_interrupt' [-Werror=implicit-function-declaration]
     871 |         if (*irq <= 0 && acpi_dev_resource_interrupt(ares, 0, &r))
         |                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c: In function 'tegra241_cmdqv_find_acpi_resource':
   drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c:879:36: error: implicit declaration of function 'to_acpi_device'; did you mean 'to_acpi_device_node'? [-Werror=implicit-function-declaration]
     879 |         struct acpi_device *adev = to_acpi_device(dev);
         |                                    ^~~~~~~~~~~~~~
         |                                    to_acpi_device_node
>> drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c:879:36: warning: initialization of 'struct acpi_device *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
   drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c:886:15: error: implicit declaration of function 'acpi_dev_get_resources'; did you mean 'acpi_get_event_resources'? [-Werror=implicit-function-declaration]
     886 |         ret = acpi_dev_get_resources(adev, &resource_list,
         |               ^~~~~~~~~~~~~~~~~~~~~~
         |               acpi_get_event_resources
   drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c:907:9: error: implicit declaration of function 'acpi_dev_free_resource_list' [-Werror=implicit-function-declaration]
     907 |         acpi_dev_free_resource_list(&resource_list);
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +879 drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c

918eb5c856f6ce Nate Watterson 2024-08-29  875  
918eb5c856f6ce Nate Watterson 2024-08-29  876  static struct resource *
918eb5c856f6ce Nate Watterson 2024-08-29  877  tegra241_cmdqv_find_acpi_resource(struct device *dev, int *irq)
918eb5c856f6ce Nate Watterson 2024-08-29  878  {
918eb5c856f6ce Nate Watterson 2024-08-29 @879  	struct acpi_device *adev = to_acpi_device(dev);
918eb5c856f6ce Nate Watterson 2024-08-29  880  	struct list_head resource_list;
918eb5c856f6ce Nate Watterson 2024-08-29  881  	struct resource_entry *rentry;
918eb5c856f6ce Nate Watterson 2024-08-29  882  	struct resource *res = NULL;
918eb5c856f6ce Nate Watterson 2024-08-29  883  	int ret;
918eb5c856f6ce Nate Watterson 2024-08-29  884  
918eb5c856f6ce Nate Watterson 2024-08-29  885  	INIT_LIST_HEAD(&resource_list);
918eb5c856f6ce Nate Watterson 2024-08-29  886  	ret = acpi_dev_get_resources(adev, &resource_list,
918eb5c856f6ce Nate Watterson 2024-08-29  887  				     tegra241_cmdqv_acpi_is_memory, NULL);
918eb5c856f6ce Nate Watterson 2024-08-29  888  	if (ret < 0) {
918eb5c856f6ce Nate Watterson 2024-08-29  889  		dev_err(dev, "failed to get memory resource: %d\n", ret);
918eb5c856f6ce Nate Watterson 2024-08-29  890  		return NULL;
918eb5c856f6ce Nate Watterson 2024-08-29  891  	}
918eb5c856f6ce Nate Watterson 2024-08-29  892  
918eb5c856f6ce Nate Watterson 2024-08-29  893  	rentry = list_first_entry_or_null(&resource_list,
918eb5c856f6ce Nate Watterson 2024-08-29  894  					  struct resource_entry, node);
918eb5c856f6ce Nate Watterson 2024-08-29  895  	if (!rentry) {
918eb5c856f6ce Nate Watterson 2024-08-29  896  		dev_err(dev, "failed to get memory resource entry\n");
918eb5c856f6ce Nate Watterson 2024-08-29  897  		goto free_list;
918eb5c856f6ce Nate Watterson 2024-08-29  898  	}
918eb5c856f6ce Nate Watterson 2024-08-29  899  
918eb5c856f6ce Nate Watterson 2024-08-29  900  	/* Caller must free the res */
918eb5c856f6ce Nate Watterson 2024-08-29  901  	res = kzalloc(sizeof(*res), GFP_KERNEL);
918eb5c856f6ce Nate Watterson 2024-08-29  902  	if (!res)
918eb5c856f6ce Nate Watterson 2024-08-29  903  		goto free_list;
918eb5c856f6ce Nate Watterson 2024-08-29  904  
918eb5c856f6ce Nate Watterson 2024-08-29  905  	*res = *rentry->res;
918eb5c856f6ce Nate Watterson 2024-08-29  906  
918eb5c856f6ce Nate Watterson 2024-08-29  907  	acpi_dev_free_resource_list(&resource_list);
918eb5c856f6ce Nate Watterson 2024-08-29  908  
918eb5c856f6ce Nate Watterson 2024-08-29  909  	INIT_LIST_HEAD(&resource_list);
918eb5c856f6ce Nate Watterson 2024-08-29  910  
918eb5c856f6ce Nate Watterson 2024-08-29  911  	if (irq)
918eb5c856f6ce Nate Watterson 2024-08-29  912  		ret = acpi_dev_get_resources(adev, &resource_list,
918eb5c856f6ce Nate Watterson 2024-08-29  913  					     tegra241_cmdqv_acpi_get_irqs, irq);
918eb5c856f6ce Nate Watterson 2024-08-29  914  	if (ret < 0 || !irq || *irq <= 0)
918eb5c856f6ce Nate Watterson 2024-08-29  915  		dev_warn(dev, "no interrupt. errors will not be reported\n");
918eb5c856f6ce Nate Watterson 2024-08-29  916  
918eb5c856f6ce Nate Watterson 2024-08-29  917  free_list:
918eb5c856f6ce Nate Watterson 2024-08-29  918  	acpi_dev_free_resource_list(&resource_list);
918eb5c856f6ce Nate Watterson 2024-08-29  919  	return res;
918eb5c856f6ce Nate Watterson 2024-08-29  920  }
918eb5c856f6ce Nate Watterson 2024-08-29  921  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH V4 1/3] iommu/arm-smmu-v3: Add device-tree support for CMDQV driver
Posted by Ashish Mhetre 1 week, 2 days ago
On 12/9/2025 1:13 AM, kernel test robot wrote:
> External email: Use caution opening links or attachments
>
>
> Hi Ashish,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on next-20251204]
> [also build test WARNING on v6.18]
> [cannot apply to robh/for-next linus/master v6.18 v6.18-rc7 v6.18-rc6]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url:    https://github.com/intel-lab-lkp/linux/commits/Ashish-Mhetre/iommu-arm-smmu-v3-Add-device-tree-support-for-CMDQV-driver/20251205-151258
> base:   next-20251204
> patch link:    https://lore.kernel.org/r/20251205065850.3841834-2-amhetre%40nvidia.com
> patch subject: [PATCH V4 1/3] iommu/arm-smmu-v3: Add device-tree support for CMDQV driver
> config: arm64-randconfig-004-20251209 (https://download.01.org/0day-ci/archive/20251209/202512090331.QAFgb6vQ-lkp@intel.com/config)
> compiler: aarch64-linux-gcc (GCC) 11.5.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251209/202512090331.QAFgb6vQ-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202512090331.QAFgb6vQ-lkp@intel.com/
>
> All warnings (new ones prefixed by >>):
>
>     drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c: In function 'tegra241_cmdqv_acpi_is_memory':
>     drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c:863:17: error: implicit declaration of function 'acpi_dev_resource_address_space' [-Werror=implicit-function-declaration]
>       863 |         return !acpi_dev_resource_address_space(res, &win);
>           |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>     drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c: In function 'tegra241_cmdqv_acpi_get_irqs':
>     drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c:871:26: error: implicit declaration of function 'acpi_dev_resource_interrupt' [-Werror=implicit-function-declaration]
>       871 |         if (*irq <= 0 && acpi_dev_resource_interrupt(ares, 0, &r))
>           |                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>     drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c: In function 'tegra241_cmdqv_find_acpi_resource':
>     drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c:879:36: error: implicit declaration of function 'to_acpi_device'; did you mean 'to_acpi_device_node'? [-Werror=implicit-function-declaration]
>       879 |         struct acpi_device *adev = to_acpi_device(dev);
>           |                                    ^~~~~~~~~~~~~~
>           |                                    to_acpi_device_node
>>> drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c:879:36: warning: initialization of 'struct acpi_device *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
>     drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c:886:15: error: implicit declaration of function 'acpi_dev_get_resources'; did you mean 'acpi_get_event_resources'? [-Werror=implicit-function-declaration]
>       886 |         ret = acpi_dev_get_resources(adev, &resource_list,
>           |               ^~~~~~~~~~~~~~~~~~~~~~
>           |               acpi_get_event_resources
>     drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c:907:9: error: implicit declaration of function 'acpi_dev_free_resource_list' [-Werror=implicit-function-declaration]
>       907 |         acpi_dev_free_resource_list(&resource_list);
>           |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>     cc1: some warnings being treated as errors

Hi Robin, Nic,
We removed ACPI dependency in Kconfig but driver still depends
on ACPI for these functions. I will be protecting ACPIspecific
tegra241-cmdqv code under CONFIG_ACPI similar to what is done
in arm-smmu-v3 driver. Is this the correct thing to do or do you
have any other suggestions?

> vim +879 drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
>
> 918eb5c856f6ce Nate Watterson 2024-08-29  875
> 918eb5c856f6ce Nate Watterson 2024-08-29  876  static struct resource *
> 918eb5c856f6ce Nate Watterson 2024-08-29  877  tegra241_cmdqv_find_acpi_resource(struct device *dev, int *irq)
> 918eb5c856f6ce Nate Watterson 2024-08-29  878  {
> 918eb5c856f6ce Nate Watterson 2024-08-29 @879   struct acpi_device *adev = to_acpi_device(dev);
> 918eb5c856f6ce Nate Watterson 2024-08-29  880   struct list_head resource_list;
> 918eb5c856f6ce Nate Watterson 2024-08-29  881   struct resource_entry *rentry;
> 918eb5c856f6ce Nate Watterson 2024-08-29  882   struct resource *res = NULL;
> 918eb5c856f6ce Nate Watterson 2024-08-29  883   int ret;
> 918eb5c856f6ce Nate Watterson 2024-08-29  884
> 918eb5c856f6ce Nate Watterson 2024-08-29  885   INIT_LIST_HEAD(&resource_list);
> 918eb5c856f6ce Nate Watterson 2024-08-29  886   ret = acpi_dev_get_resources(adev, &resource_list,
> 918eb5c856f6ce Nate Watterson 2024-08-29  887                                tegra241_cmdqv_acpi_is_memory, NULL);
> 918eb5c856f6ce Nate Watterson 2024-08-29  888   if (ret < 0) {
> 918eb5c856f6ce Nate Watterson 2024-08-29  889           dev_err(dev, "failed to get memory resource: %d\n", ret);
> 918eb5c856f6ce Nate Watterson 2024-08-29  890           return NULL;
> 918eb5c856f6ce Nate Watterson 2024-08-29  891   }
> 918eb5c856f6ce Nate Watterson 2024-08-29  892
> 918eb5c856f6ce Nate Watterson 2024-08-29  893   rentry = list_first_entry_or_null(&resource_list,
> 918eb5c856f6ce Nate Watterson 2024-08-29  894                                     struct resource_entry, node);
> 918eb5c856f6ce Nate Watterson 2024-08-29  895   if (!rentry) {
> 918eb5c856f6ce Nate Watterson 2024-08-29  896           dev_err(dev, "failed to get memory resource entry\n");
> 918eb5c856f6ce Nate Watterson 2024-08-29  897           goto free_list;
> 918eb5c856f6ce Nate Watterson 2024-08-29  898   }
> 918eb5c856f6ce Nate Watterson 2024-08-29  899
> 918eb5c856f6ce Nate Watterson 2024-08-29  900   /* Caller must free the res */
> 918eb5c856f6ce Nate Watterson 2024-08-29  901   res = kzalloc(sizeof(*res), GFP_KERNEL);
> 918eb5c856f6ce Nate Watterson 2024-08-29  902   if (!res)
> 918eb5c856f6ce Nate Watterson 2024-08-29  903           goto free_list;
> 918eb5c856f6ce Nate Watterson 2024-08-29  904
> 918eb5c856f6ce Nate Watterson 2024-08-29  905   *res = *rentry->res;
> 918eb5c856f6ce Nate Watterson 2024-08-29  906
> 918eb5c856f6ce Nate Watterson 2024-08-29  907   acpi_dev_free_resource_list(&resource_list);
> 918eb5c856f6ce Nate Watterson 2024-08-29  908
> 918eb5c856f6ce Nate Watterson 2024-08-29  909   INIT_LIST_HEAD(&resource_list);
> 918eb5c856f6ce Nate Watterson 2024-08-29  910
> 918eb5c856f6ce Nate Watterson 2024-08-29  911   if (irq)
> 918eb5c856f6ce Nate Watterson 2024-08-29  912           ret = acpi_dev_get_resources(adev, &resource_list,
> 918eb5c856f6ce Nate Watterson 2024-08-29  913                                        tegra241_cmdqv_acpi_get_irqs, irq);
> 918eb5c856f6ce Nate Watterson 2024-08-29  914   if (ret < 0 || !irq || *irq <= 0)
> 918eb5c856f6ce Nate Watterson 2024-08-29  915           dev_warn(dev, "no interrupt. errors will not be reported\n");
> 918eb5c856f6ce Nate Watterson 2024-08-29  916
> 918eb5c856f6ce Nate Watterson 2024-08-29  917  free_list:
> 918eb5c856f6ce Nate Watterson 2024-08-29  918   acpi_dev_free_resource_list(&resource_list);
> 918eb5c856f6ce Nate Watterson 2024-08-29  919   return res;
> 918eb5c856f6ce Nate Watterson 2024-08-29  920  }
> 918eb5c856f6ce Nate Watterson 2024-08-29  921
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
Re: [PATCH V4 1/3] iommu/arm-smmu-v3: Add device-tree support for CMDQV driver
Posted by Robin Murphy 1 week, 2 days ago
On 2025-12-10 5:19 am, Ashish Mhetre wrote:
[...]
> Hi Robin, Nic,
> We removed ACPI dependency in Kconfig but driver still depends
> on ACPI for these functions. I will be protecting ACPIspecific
> tegra241-cmdqv code under CONFIG_ACPI similar to what is done
> in arm-smmu-v3 driver. Is this the correct thing to do or do you
> have any other suggestions?

Yes, when I commented that "depends on ACPI || OF" was functionally the 
same as just removing "depends on ACPI", I didn't mean to suggest that 
wasn't necessarily a genuine dependency still.

I guess if you can wrap the ACPI-specific functions in a single #ifdef 
block that's reasonable, however I do now wonder whether things couldn't 
be factored out a bit more - if it's a standard DSDT/SSDT namespace 
device, shouldn't there also be a corresponding platform device created 
for it, which we could look up instead of delving directly into the _CRS 
of the ACPI node itself? (not sure off-hand if there's a straightforward 
inverse of ACPI_COMPANION()...)

Thanks,
Robin.
Re: [PATCH V4 1/3] iommu/arm-smmu-v3: Add device-tree support for CMDQV driver
Posted by Nicolin Chen 1 week, 1 day ago
On Wed, Dec 10, 2025 at 12:39:49PM +0000, Robin Murphy wrote:
> On 2025-12-10 5:19 am, Ashish Mhetre wrote:
> [...]
> > Hi Robin, Nic,
> > We removed ACPI dependency in Kconfig but driver still depends
> > on ACPI for these functions. I will be protecting ACPIspecific
> > tegra241-cmdqv code under CONFIG_ACPI similar to what is done
> > in arm-smmu-v3 driver. Is this the correct thing to do or do you
> > have any other suggestions?
> 
> Yes, when I commented that "depends on ACPI || OF" was functionally the same
> as just removing "depends on ACPI", I didn't mean to suggest that wasn't
> necessarily a genuine dependency still.
> 
> I guess if you can wrap the ACPI-specific functions in a single #ifdef block
> that's reasonable, however I do now wonder whether things couldn't be
> factored out a bit more - if it's a standard DSDT/SSDT namespace device,
> shouldn't there also be a corresponding platform device created for it,
> which we could look up instead of delving directly into the _CRS of the ACPI
> node itself? (not sure off-hand if there's a straightforward inverse of
> ACPI_COMPANION()...)

Ah, I did a quick tracer at acpi_create_platform_device(). And I
do see platform devices being created. So, we could have made it
ACPI-independent from the beginning, as you expected :)

Ashish, let's attach the following patch in your series:

From 7b69a638276e66b16b923b0ce1743d3efc85a04c Mon Sep 17 00:00:00 2001
From: Nicolin Chen <nicolinc@nvidia.com>
Date: Thu, 11 Dec 2025 05:47:05 +0000
Subject: [PATCH] iommu/tegra241-cmdqv: Decouple driver from ACPI

A platform device is created by acpi_create_platform_device() per CMDQV's
adev. That means there is no point in going through _CRS of ACPI.

Replace all the ACPI functions with standard platform functions. And drop
all ACPI dependencies. This will make the driver compatible with DT also.

Suggested-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/arm/Kconfig                     |  1 -
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   |  2 +-
 .../iommu/arm/arm-smmu-v3/tegra241-cmdqv.c    | 84 +++----------------
 3 files changed, 13 insertions(+), 74 deletions(-)

diff --git a/drivers/iommu/arm/Kconfig b/drivers/iommu/arm/Kconfig
index ef42bbe07dbef..5fac08b89deea 100644
--- a/drivers/iommu/arm/Kconfig
+++ b/drivers/iommu/arm/Kconfig
@@ -121,7 +121,6 @@ config ARM_SMMU_V3_KUNIT_TEST
 
 config TEGRA241_CMDQV
 	bool "NVIDIA Tegra241 CMDQ-V extension support for ARM SMMUv3"
-	depends on ACPI
 	help
 	  Support for NVIDIA CMDQ-Virtualization extension for ARM SMMUv3. The
 	  CMDQ-V extension is similar to v3.3 ECMDQ for multi command queues
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 38028e4a52f7f..0c98be3135c63 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -5280,7 +5280,7 @@ static void acpi_smmu_dsdt_probe_tegra241_cmdqv(struct acpi_iort_node *node,
 	adev = acpi_dev_get_first_match_dev("NVDA200C", uid, -1);
 	if (adev) {
 		/* Tegra241 CMDQV driver is responsible for put_device() */
-		smmu->impl_dev = &adev->dev;
+		smmu->impl_dev = acpi_get_first_physical_node(adev);
 		smmu->options |= ARM_SMMU_OPT_TEGRA241_CMDQV;
 		dev_info(smmu->dev, "found companion CMDQV device: %s\n",
 			 dev_name(smmu->impl_dev));
diff --git a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
index 378104cd395e5..1fc03b72beb88 100644
--- a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
+++ b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
@@ -3,17 +3,15 @@
 
 #define dev_fmt(fmt) "tegra241_cmdqv: " fmt
 
-#include <linux/acpi.h>
 #include <linux/debugfs.h>
 #include <linux/dma-mapping.h>
 #include <linux/interrupt.h>
 #include <linux/iommu.h>
 #include <linux/iommufd.h>
 #include <linux/iopoll.h>
+#include <linux/platform_device.h>
 #include <uapi/linux/iommufd.h>
 
-#include <acpi/acpixf.h>
-
 #include "arm-smmu-v3.h"
 
 /* CMDQV register page base and size defines */
@@ -854,69 +852,6 @@ static struct arm_smmu_impl_ops tegra241_cmdqv_impl_ops = {
 
 /* Probe Functions */
 
-static int tegra241_cmdqv_acpi_is_memory(struct acpi_resource *res, void *data)
-{
-	struct resource_win win;
-
-	return !acpi_dev_resource_address_space(res, &win);
-}
-
-static int tegra241_cmdqv_acpi_get_irqs(struct acpi_resource *ares, void *data)
-{
-	struct resource r;
-	int *irq = data;
-
-	if (*irq <= 0 && acpi_dev_resource_interrupt(ares, 0, &r))
-		*irq = r.start;
-	return 1; /* No need to add resource to the list */
-}
-
-static struct resource *
-tegra241_cmdqv_find_acpi_resource(struct device *dev, int *irq)
-{
-	struct acpi_device *adev = to_acpi_device(dev);
-	struct list_head resource_list;
-	struct resource_entry *rentry;
-	struct resource *res = NULL;
-	int ret;
-
-	INIT_LIST_HEAD(&resource_list);
-	ret = acpi_dev_get_resources(adev, &resource_list,
-				     tegra241_cmdqv_acpi_is_memory, NULL);
-	if (ret < 0) {
-		dev_err(dev, "failed to get memory resource: %d\n", ret);
-		return NULL;
-	}
-
-	rentry = list_first_entry_or_null(&resource_list,
-					  struct resource_entry, node);
-	if (!rentry) {
-		dev_err(dev, "failed to get memory resource entry\n");
-		goto free_list;
-	}
-
-	/* Caller must free the res */
-	res = kzalloc(sizeof(*res), GFP_KERNEL);
-	if (!res)
-		goto free_list;
-
-	*res = *rentry->res;
-
-	acpi_dev_free_resource_list(&resource_list);
-
-	INIT_LIST_HEAD(&resource_list);
-
-	if (irq)
-		ret = acpi_dev_get_resources(adev, &resource_list,
-					     tegra241_cmdqv_acpi_get_irqs, irq);
-	if (ret < 0 || !irq || *irq <= 0)
-		dev_warn(dev, "no interrupt. errors will not be reported\n");
-
-free_list:
-	acpi_dev_free_resource_list(&resource_list);
-	return res;
-}
-
 static int tegra241_cmdqv_init_structures(struct arm_smmu_device *smmu)
 {
 	struct tegra241_cmdqv *cmdqv =
@@ -1042,18 +977,23 @@ __tegra241_cmdqv_probe(struct arm_smmu_device *smmu, struct resource *res,
 
 struct arm_smmu_device *tegra241_cmdqv_probe(struct arm_smmu_device *smmu)
 {
+	struct platform_device *pdev = to_platform_device(smmu->impl_dev);
 	struct arm_smmu_device *new_smmu;
-	struct resource *res = NULL;
+	struct resource *res;
 	int irq;
 
-	if (!smmu->dev->of_node)
-		res = tegra241_cmdqv_find_acpi_resource(smmu->impl_dev, &irq);
-	if (!res)
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "no memory resource found for CMDQV\n");
 		goto out_fallback;
+	}
 
-	new_smmu = __tegra241_cmdqv_probe(smmu, res, irq);
-	kfree(res);
+	irq = platform_get_irq_optional(pdev, 0);
+	if (irq <= 0)
+		dev_warn(&pdev->dev,
+			 "no interrupt. errors will not be reported\n");
 
+	new_smmu = __tegra241_cmdqv_probe(smmu, res, irq);
 	if (new_smmu)
 		return new_smmu;
 
-- 
2.43.0
Re: [PATCH V4 1/3] iommu/arm-smmu-v3: Add device-tree support for CMDQV driver
Posted by Ashish Mhetre 1 week, 1 day ago
On 12/11/2025 11:39 AM, Nicolin Chen wrote:
> On Wed, Dec 10, 2025 at 12:39:49PM +0000, Robin Murphy wrote:
>> On 2025-12-10 5:19 am, Ashish Mhetre wrote:
>> [...]
>>> Hi Robin, Nic,
>>> We removed ACPI dependency in Kconfig but driver still depends
>>> on ACPI for these functions. I will be protecting ACPIspecific
>>> tegra241-cmdqv code under CONFIG_ACPI similar to what is done
>>> in arm-smmu-v3 driver. Is this the correct thing to do or do you
>>> have any other suggestions?
>> Yes, when I commented that "depends on ACPI || OF" was functionally the same
>> as just removing "depends on ACPI", I didn't mean to suggest that wasn't
>> necessarily a genuine dependency still.
>>
>> I guess if you can wrap the ACPI-specific functions in a single #ifdef block
>> that's reasonable, however I do now wonder whether things couldn't be
>> factored out a bit more - if it's a standard DSDT/SSDT namespace device,
>> shouldn't there also be a corresponding platform device created for it,
>> which we could look up instead of delving directly into the _CRS of the ACPI
>> node itself? (not sure off-hand if there's a straightforward inverse of
>> ACPI_COMPANION()...)
> Ah, I did a quick tracer at acpi_create_platform_device(). And I
> do see platform devices being created. So, we could have made it
> ACPI-independent from the beginning, as you expected :)
>
> Ashish, let's attach the following patch in your series:

Ack, thanks Robin and Nic. I'll update my patches on top of Nic's patch
and re-post.

>  From 7b69a638276e66b16b923b0ce1743d3efc85a04c Mon Sep 17 00:00:00 2001
> From: Nicolin Chen <nicolinc@nvidia.com>
> Date: Thu, 11 Dec 2025 05:47:05 +0000
> Subject: [PATCH] iommu/tegra241-cmdqv: Decouple driver from ACPI
>
> A platform device is created by acpi_create_platform_device() per CMDQV's
> adev. That means there is no point in going through _CRS of ACPI.
>
> Replace all the ACPI functions with standard platform functions. And drop
> all ACPI dependencies. This will make the driver compatible with DT also.
>
> Suggested-by: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> ---
>   drivers/iommu/arm/Kconfig                     |  1 -
>   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   |  2 +-
>   .../iommu/arm/arm-smmu-v3/tegra241-cmdqv.c    | 84 +++----------------
>   3 files changed, 13 insertions(+), 74 deletions(-)
>
> diff --git a/drivers/iommu/arm/Kconfig b/drivers/iommu/arm/Kconfig
> index ef42bbe07dbef..5fac08b89deea 100644
> --- a/drivers/iommu/arm/Kconfig
> +++ b/drivers/iommu/arm/Kconfig
> @@ -121,7 +121,6 @@ config ARM_SMMU_V3_KUNIT_TEST
>   
>   config TEGRA241_CMDQV
>   	bool "NVIDIA Tegra241 CMDQ-V extension support for ARM SMMUv3"
> -	depends on ACPI
>   	help
>   	  Support for NVIDIA CMDQ-Virtualization extension for ARM SMMUv3. The
>   	  CMDQ-V extension is similar to v3.3 ECMDQ for multi command queues
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index 38028e4a52f7f..0c98be3135c63 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -5280,7 +5280,7 @@ static void acpi_smmu_dsdt_probe_tegra241_cmdqv(struct acpi_iort_node *node,
>   	adev = acpi_dev_get_first_match_dev("NVDA200C", uid, -1);
>   	if (adev) {
>   		/* Tegra241 CMDQV driver is responsible for put_device() */
> -		smmu->impl_dev = &adev->dev;
> +		smmu->impl_dev = acpi_get_first_physical_node(adev);
>   		smmu->options |= ARM_SMMU_OPT_TEGRA241_CMDQV;
>   		dev_info(smmu->dev, "found companion CMDQV device: %s\n",
>   			 dev_name(smmu->impl_dev));
> diff --git a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
> index 378104cd395e5..1fc03b72beb88 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
> @@ -3,17 +3,15 @@
>   
>   #define dev_fmt(fmt) "tegra241_cmdqv: " fmt
>   
> -#include <linux/acpi.h>
>   #include <linux/debugfs.h>
>   #include <linux/dma-mapping.h>
>   #include <linux/interrupt.h>
>   #include <linux/iommu.h>
>   #include <linux/iommufd.h>
>   #include <linux/iopoll.h>
> +#include <linux/platform_device.h>
>   #include <uapi/linux/iommufd.h>
>   
> -#include <acpi/acpixf.h>
> -
>   #include "arm-smmu-v3.h"
>   
>   /* CMDQV register page base and size defines */
> @@ -854,69 +852,6 @@ static struct arm_smmu_impl_ops tegra241_cmdqv_impl_ops = {
>   
>   /* Probe Functions */
>   
> -static int tegra241_cmdqv_acpi_is_memory(struct acpi_resource *res, void *data)
> -{
> -	struct resource_win win;
> -
> -	return !acpi_dev_resource_address_space(res, &win);
> -}
> -
> -static int tegra241_cmdqv_acpi_get_irqs(struct acpi_resource *ares, void *data)
> -{
> -	struct resource r;
> -	int *irq = data;
> -
> -	if (*irq <= 0 && acpi_dev_resource_interrupt(ares, 0, &r))
> -		*irq = r.start;
> -	return 1; /* No need to add resource to the list */
> -}
> -
> -static struct resource *
> -tegra241_cmdqv_find_acpi_resource(struct device *dev, int *irq)
> -{
> -	struct acpi_device *adev = to_acpi_device(dev);
> -	struct list_head resource_list;
> -	struct resource_entry *rentry;
> -	struct resource *res = NULL;
> -	int ret;
> -
> -	INIT_LIST_HEAD(&resource_list);
> -	ret = acpi_dev_get_resources(adev, &resource_list,
> -				     tegra241_cmdqv_acpi_is_memory, NULL);
> -	if (ret < 0) {
> -		dev_err(dev, "failed to get memory resource: %d\n", ret);
> -		return NULL;
> -	}
> -
> -	rentry = list_first_entry_or_null(&resource_list,
> -					  struct resource_entry, node);
> -	if (!rentry) {
> -		dev_err(dev, "failed to get memory resource entry\n");
> -		goto free_list;
> -	}
> -
> -	/* Caller must free the res */
> -	res = kzalloc(sizeof(*res), GFP_KERNEL);
> -	if (!res)
> -		goto free_list;
> -
> -	*res = *rentry->res;
> -
> -	acpi_dev_free_resource_list(&resource_list);
> -
> -	INIT_LIST_HEAD(&resource_list);
> -
> -	if (irq)
> -		ret = acpi_dev_get_resources(adev, &resource_list,
> -					     tegra241_cmdqv_acpi_get_irqs, irq);
> -	if (ret < 0 || !irq || *irq <= 0)
> -		dev_warn(dev, "no interrupt. errors will not be reported\n");
> -
> -free_list:
> -	acpi_dev_free_resource_list(&resource_list);
> -	return res;
> -}
> -
>   static int tegra241_cmdqv_init_structures(struct arm_smmu_device *smmu)
>   {
>   	struct tegra241_cmdqv *cmdqv =
> @@ -1042,18 +977,23 @@ __tegra241_cmdqv_probe(struct arm_smmu_device *smmu, struct resource *res,
>   
>   struct arm_smmu_device *tegra241_cmdqv_probe(struct arm_smmu_device *smmu)
>   {
> +	struct platform_device *pdev = to_platform_device(smmu->impl_dev);
>   	struct arm_smmu_device *new_smmu;
> -	struct resource *res = NULL;
> +	struct resource *res;
>   	int irq;
>   
> -	if (!smmu->dev->of_node)
> -		res = tegra241_cmdqv_find_acpi_resource(smmu->impl_dev, &irq);
> -	if (!res)
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res) {
> +		dev_err(&pdev->dev, "no memory resource found for CMDQV\n");
>   		goto out_fallback;
> +	}
>   
> -	new_smmu = __tegra241_cmdqv_probe(smmu, res, irq);
> -	kfree(res);
> +	irq = platform_get_irq_optional(pdev, 0);
> +	if (irq <= 0)
> +		dev_warn(&pdev->dev,
> +			 "no interrupt. errors will not be reported\n");
>   
> +	new_smmu = __tegra241_cmdqv_probe(smmu, res, irq);
>   	if (new_smmu)
>   		return new_smmu;
>