| Message ID | 20251205065850.3841834-2-amhetre@nvidia.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add device tree support for NVIDIA Tegra CMDQV | expand |
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
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
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.
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;
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; >
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;