Re: [PATCH 0/2] perf/smmuv3: Support devicetree

On 2021-11-16 15:42, Jean-Philippe Brucker wrote:

On Tue, Nov 16, 2021 at 12:02:47PM +0000, Robin Murphy wrote:

quoted

On 2021-11-16 11:35, Jean-Philippe Brucker wrote:

quoted

Add devicetree binding for the SMMUv3 PMU, called Performance Monitoring
Counter Group (PMCG) in the spec. Each SMMUv3 implementation can have
multiple independent PMCGs, for example one for the Translation Control
Unit (TCU) and one per Translation Buffer Unit (TBU).

I previously sent the binding as reply to Jay Chen's thread implementing
device tree support [1]. This posting addresses the comments from that
thread.

Ha, I'd also resurrected this and was planning to post it at some point this
week[0] - you should have said :)

Ah sorry about that, I just resent because there was some demand for it at
Linaro

Heh, no worries - it's not like you were even CC'ed on the thread where 
I only mentioned I *might* do it.

Can I get away with being cheeky and just saying that my review comments 
are the diff between my branch and yours, I wonder...

quoted

quoted

Patch 1 adds two compatible strings. "arm,smmu-v3-pmcg" is common to all
PMCGs. "hisilicon,smmu-v3-pmcg-hip08" allows to support the same quirk
as IORT for that implementation (see patch 2). We'll probably want to
also introduce compatible strings for each implementation that has
additional perf events. For example the MMU-600 implementation has
different events for TCU and TBU PMCGs [2], but both components have the
same device IDs. So the driver could differentiate them if they had two
distinct compatible strings such as "arm,mmu-600-pmcg-tbu" and
"arm,mmu-600-pmcg-tcu".

Actually it only needs a general MMU-600 compatible, since once you know
it's an Arm Ltd. implementation, you can assume the pattern for the IMP_DEF
ID registers to figure out the rest.

It might be an error in the MMU-600 spec specifically, both TBU and TCU
PMU registers have a 0x83 PIDR0, where I think the TBU should be 0x84 (the
revC model uses that value). It's possible that the implementation
actually has 0x84 instead.

Yup, it's a mistake in the TRM. I just checked a real MMU-600 and the 
PMU PIDRs match the main TCU/TBU PIDRs as expected. At least the MMU-700 
docs haven't repeated the same error.

Cheers,
Robin.