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

[Jon Hunter <jonathanh@nvidia.com>]

On 01/12/2025 16:32, Ashish Mhetre wrote:
> 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    | 43 ++++++++++++++++++-
>   3 files changed, 74 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 a33fbd12a0dd..206dffabc9c0 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..2608bf6518b4 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;
> @@ -1346,4 +1371,20 @@ tegra241_cmdqv_init_vintf_user(struct arm_vsmmu *vsmmu,
>   	return ret;
>   }
>   
> +static const struct of_device_id tegra241_cmdqv_of_match[] = {
> +	{ .compatible = "nvidia,tegra264-cmdqv" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, tegra241_cmdqv_of_match);
> +
> +static struct platform_driver tegra241_cmdqv_driver = {
> +	.driver = {
> +		.name = "tegra241-cmdqv",
> +		.of_match_table = tegra241_cmdqv_of_match,
> +	},
> +};
> +module_platform_driver(tegra241_cmdqv_driver);

This part looks a bit weird, because it appears to do nothing. The 
compatible string is not used at all in the driver AFAICT. Have you 
tried this bit out because this is not a proper driver?

Jon

-- 
nvpublic