Re: [RFC PATCH v2 1/5] iommu/arm-smmu-v3: Add basic debugfs framework

[Nicolin Chen <nicolinc@nvidia.com>]

On Sat, Mar 28, 2026 at 06:17:02PM +0800, Qinxin Xia wrote:
> Add basic debugfs framework for ARM SMMUv3 driver.This creates the

Needs a space after "."

> +static int smmu_debugfs_capabilities_show(struct seq_file *seq, void *unused)
> +{
> +	struct arm_smmu_device *smmu = seq->private;
> +
> +	if (!smmu) {
> +		seq_puts(seq, "SMMU not available\n");
> +		return 0;
> +	}
> +
> +	seq_puts(seq, "SMMUv3 Capabilities:\n");
> +	seq_printf(seq, "  Stage1 Translation: %s\n",
> +		   smmu->features & ARM_SMMU_FEAT_TRANS_S1 ? "Yes" : "No");
> +	seq_printf(seq, "  Stage2 Translation: %s\n",
> +		   smmu->features & ARM_SMMU_FEAT_TRANS_S2 ? "Yes" : "No");
> +	seq_printf(seq, "  Coherent Walk: %s\n",
> +		   smmu->features & ARM_SMMU_FEAT_COHERENCY ? "Yes" : "No");
> +	seq_printf(seq, "  ATS Support: %s\n",
> +		   smmu->features & ARM_SMMU_FEAT_ATS ? "Yes" : "No");
> +	seq_printf(seq, "  PRI Support: %s\n",
> +		   smmu->features & ARM_SMMU_FEAT_PRI ? "Yes" : "No");
> +	seq_printf(seq, "  Stream Table Size: %d\n", 1 << smmu->sid_bits);
> +	seq_printf(seq, "  Command Queue Depth: %d\n",
> +		   1 << smmu->cmdq.q.llq.max_n_shift);
> +	seq_printf(seq, "  Event Queue Depth: %d\n",
> +		   1 << smmu->evtq.q.llq.max_n_shift);

Nit: should we do all sizes or all depths? Any good reason to mix
them here?

> +/**
> + * arm_smmu_debugfs_remove() - Clean up debugfs entries for an SMMU device
> + * @smmu: SMMU device
> + *
> + * This function removes the debugfs directories created by setup.
> + */
> +void arm_smmu_debugfs_remove(struct arm_smmu_device *smmu)
> +{
> +	struct arm_smmu_debugfs *debugfs;
> +
> +	scoped_guard(mutex, &arm_smmu_debugfs_lock) {

It could be just normal guard().

> 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 4d00d796f078..cbb3fccc501b 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -4904,6 +4904,15 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
>  	/* Check for RMRs and install bypass STEs if any */
>  	arm_smmu_rmr_install_bypass_ste(smmu);
>  
> +#ifdef CONFIG_ARM_SMMU_V3_DEBUGFS
> +	char name[32];

This could be moved to the top, as iommu_device_sysfs_add() can use
it, whether CONFIG_ARM_SMMU_V3_DEBUGFS=y or =n.

> +	snprintf(name, sizeof(name), "smmu3.%pa", &ioaddr);

And this could be moved after ioaddr gets a copy from res->start.

>  
> +#ifdef CONFIG_ARM_SMMU_V3_DEBUGFS
> +struct arm_smmu_debugfs {
> +	struct dentry			*smmu_dir;

A personal preference: for new structures, maybe drop those tabs?

>  /* An SMMUv3 instance */
>  struct arm_smmu_device {
>  	struct device			*dev;
> @@ -803,6 +813,11 @@ struct arm_smmu_device {
>  
>  	struct rb_root			streams;
>  	struct mutex			streams_mutex;
> +
> +#ifdef CONFIG_ARM_SMMU_V3_DEBUGFS
> +	/* DebugFS Info */

Doesn't seem very useful. I'd drop it.

> +	struct arm_smmu_debugfs		*debugfs;
> +#endif
 
Nicolin