Re: [RFC PATCH 2/5] iommu/arm-smmu-v3: Add register display to debugfs

[Robin Murphy <robin.murphy@arm.com>]

On 2026-03-16 3:35 pm, Qinxin Xia wrote:
>>> +    /* 32-bit control registers */
>>> +    seq_printf(seq, "CR0: 0x%08x [%s%s%s]\n",
>>> +           readl_relaxed(base + ARM_SMMU_CR0),
>>> +           readl_relaxed(base + ARM_SMMU_CR0) & CR0_SMMUEN ?
>>> +           "Enabled " : "Disabled ",
>>> +           readl_relaxed(base + ARM_SMMU_CR0) & CR0_EVTQEN ?
>>> +           "EventQ " : "",
>>> +           readl_relaxed(base + ARM_SMMU_CR0) & CR0_CMDQEN ?
>>> +           "CmdQ " : "");
>>
>> There's really no point printing these extra strings, since if any of 
>> those were *not* enabled then we'd have already failed probe and never 
>> created the debugfs entry. And if anyone ever were to be trying to 
>> change the driver behaviour at that level, I'd very much expect them 
>> to be able to be able to read the bottom 4 bits of a CR0 value in hex 
>> anyway ;)
>>
>> Thanks,
>> Robin.
>>
> 
> Since Kunpeng supports ECMDQ(have not been submitted upstream), the
> intention was to check whether the SMMU supports ECMDQ, and the logging
> for other queues was added incidentally. The prints for the event queue
> and command queue indeed seem unnecessary. I will remove them in the
> next version.
OK, I would make a similar argument that even then, it wouldn't make 
sense to expose debugfs entries for ECMDQ_PROD registers that don't 
exist or we aren't using. Thus their enabled state should similarly be 
inherent in the files being present at all (but again either way, if a 
developer couldn't tell the significance of 0x80xxxxxx vs. 0x00xxxxxx 
already then I'd have to question their capability to usefully debug any 
more subtle ECMDQ behaviour...)

Thanks,
Robin.