Re: [PATCH V4 1/3] iommu/arm-smmu-v3: Add device-tree support for CMDQV driver
From: Ashish Mhetre <hidden>
Date: 2025-12-11 06:41:26
Also in:
linux-devicetree, linux-iommu, linux-tegra, lkml, oe-kbuild-all
On 12/11/2025 11:39 AM, Nicolin Chen wrote:
On Wed, Dec 10, 2025 at 12:39:49PM +0000, Robin Murphy wrote:quoted
On 2025-12-10 5:19 am, Ashish Mhetre wrote: [...]quoted
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.
quoted hunk ↗ jump to hunk
From 7b69a638276e66b16b923b0ce1743d3efc85a04c Mon Sep 17 00:00:00 2001 From: Nicolin Chen <redacted> 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 <redacted> --- 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 queuesdiff --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;