On 2023-12-08 7:33 pm, Rob Herring wrote:
On Tue, Dec 05, 2023 at 04:51:57PM +0000, Robin Murphy wrote:
Add a binding for implementations of the Arm CoreSight Performance
Monitoring Unit Architecture. Not to be confused with CoreSight debug
and trace, the PMU architecture defines a standard MMIO interface for
event counters similar to the CPU PMU architecture, where the
implementation and most of its features are discoverable through ID
registers.
The implementation is separate from the CPU PMU rather than an MMIO view
of it. Not really clear in my quick read of the spec.
Yeah, the architecture seems to have aspirations of being able to
describe the CPU PMU, but the main intent of this binding is to
accommodate all of the arbitrary MMIO non-CPU things. However that's not
to say it *couldn't* ever be used for the memory-mapped view of a CPU's
PMU via its external debug interface, if it is suitably compatible. I
concur that I'm rather light on description here, but that's mostly
because the architecture itself isn't prescriptive - it really is pretty
much just an interface to whatever PMU functionality can be made to fit
it (and with plenty of imp-def leeway).
CC: Rob Herring <robh+dt@xxxxxxxxxx>
CC: Krzysztof Kozlowski <krzysztof.kozlowski+dt@xxxxxxxxxx>
CC: Conor Dooley <conor+dt@xxxxxxxxxx>
Signed-off-by: Robin Murphy <robin.murphy@xxxxxxx>
---
.../bindings/perf/arm,coresight-pmu.yaml | 39 +++++++++++++++++++
1 file changed, 39 insertions(+)
create mode 100644 Documentation/devicetree/bindings/perf/arm,coresight-pmu.yaml
diff --git a/Documentation/devicetree/bindings/perf/arm,coresight-pmu.yaml b/Documentation/devicetree/bindings/perf/arm,coresight-pmu.yaml
new file mode 100644
index 000000000000..12c7b28eee35
--- /dev/null
+++ b/Documentation/devicetree/bindings/perf/arm,coresight-pmu.yaml
@@ -0,0 +1,39 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/perf/arm,coresight-pmu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Arm Coresight Performance Monitoring Unit Architecture
+
+maintainers:
+ - Robin Murphy <robin.murphy@xxxxxxx>
+
+properties:
+ compatible:
+ const: arm,coresight-pmu
+
+ reg:
+ items:
+ - description: Register page 0
+ - description: Register page 1 (if dual-page extension implemented)
+ minItems: 1
+
+ interrupts:
+ items:
+ - description: Overflow interrupt
+
+ cpus:
+ $ref: /schemas/types.yaml#/definitions/phandle-array
Don't need a type. Already defined.
Ah, I hadn't noticed this was a common property now. Good to know, thanks.
+ minItems: 1
1 is always the minimum.
+ description: List of CPUs with which the PMU is associated, if applicable
When is it applicable? Presumably when it is associated with only a
subset of CPUs?
Affinity to one CPU or some subset, for things like tightly coupled
accelerators or cluster-level things like DSU, would want explicitly
describing, but for interconnects, memory controllers and random other
standalone devices usually no meaningful association will exist (either
they can be considered affine to no CPUs, or to all of them in an
implicit manner).
+
+ arm,64-bit-atomic:
+ type: boolean
+ description: Register accesses are single-copy atomic at doubleword granularity
As this is recommended, shouldn't the property be the inverse.
It may be recommended, but in practice I'm convinced it's going to
remain the exception. It's mandatory (and thus assumable) for the 64-bit
programmers model extension, and effectively moot if only 32-bit or
smaller counters are implemented, so it's really only relevant to the
in-between case of the standard "32-bit" programmers model with 64-bit
(or at least >32-bit) counters, but even then I'd bet most folks are
still going to implement those behind an APB interface that ends up with
larger accesses split into 32-bit bursts if they're even accepted at
all. I'm aware of 3 implementations so far; one I'm not sure how wide
the counters are, while the other two have proven to be 64-bit *without*
atomic access ;)
Maybe the standard 'reg-io-width = <4>' would be sufficient here?
Oh, indeed I'd forgotten that was a thing - IIRC in common cases like
UARTs it's used to represent a *minimum* access size, whereas here it
would represent a maximum, but I imagine that ambiguity may well already
exist via other bindings, so as long as that's OK I'm happy to go with
it. As above I would still be inclined to make it default to 4 if
absent, but permit either 4 or 8 to be specified.
Thanks,
Robin.
