Re: [RFC PATCH v1 2/2] perf/smmuv3: To support the dts to get options

On 2020-07-14 00:25, Rob Herring wrote:

On Tue, Jul 07, 2020 at 05:01:14PM +0200, Jean-Philippe Brucker wrote:

quoted

Hi,

On Mon, Jul 06, 2020 at 04:03:34PM +0100, Robin Murphy wrote:

quoted

On 2020-07-06 12:22, Jay Chen wrote:

quoted

For the smmuv3 pmu for support the dts to get the
options

Signed-off-by: Jay Chen <redacted>

[...]

quoted

quoted

+static const struct of_device_id smmu_pmu_of_match[] = {
+	{ .compatible = "arm-smmu-v3-pmcg", },

Please define the DT binding first. IIRC Jean-Philippe wrote some patches a
while back that never got posted, but I suppose it should be YAML now...

Yes, I've never followed through with that because it only supported the
RevC FastModel with non-default model parameters. I attached the binding I
currently have, converted to YAML.

Thanks,
Jean

quoted

quoted

From b117e5b4ce96a5a8327333ab408cf61200850d4f Mon Sep 17 00:00:00 2001

From: Jean-Philippe Brucker <redacted>
Date: Tue, 7 Jul 2020 16:55:16 +0200
Subject: [PATCH] dt-bindings: Add SMMUv3 PMCG binding

Add binding for the SMMUv3 PMU. Each node represents a PMCG, and is placed
as a sibling node of the SMMU. As PMCGs are mainly implementation
defined there is no 1-1 relation between SMMU and PMCG. The SMMU could
have PMU counters for the TCU and each TBU, or a single PMCG.

TODO: although the Linux implementation doesn't need them, it'd be nice
to have links from the PMCG node to its associated SMMU. IORT does offer
this (Node reference) and perhaps it could later help users figure out
which PMCG is which on systems with dozens of SMMU.

Is the PMCG really a separate block or a new node is just convenient to
instantiate a driver?

Yes, PMCGs are their own thing with their own little programming 
interfaces and interrupts, there are typically multiple PMCG instances 
per SMMU, and they may even belong to "non-SMMU" components which 
participate in translation, like PCIe root complexes or devices with 
their own embedded TLBs.

quoted

Signed-off-by: Jean-Philippe Brucker <redacted>
---
  .../bindings/iommu/arm,smmu-v3-pmcg.yaml      | 58 +++++++++++++++++++
  1 file changed, 58 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.yaml

diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.yaml
new file mode 100644
index 000000000000..23190a617e7e
--- /dev/null
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.yaml

@@ -0,0 +1,58 @@
+# SPDX-License-Identifier: GPL-2.0-only

Dual license new bindings.

quoted

+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iommu/arm,smmu-v3-pmcg.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ARM SMMUv3 Performance Monitor Counter Group
+
+maintainers:
+  - Will Deacon [off-list ref]
+  - Robin Murphy [off-list ref]
+
+description: |+
+  An SMMUv3 may have several Performance Monitor Counter Group (PMCG).
+  They are standalone performance monitoring units that support both
+  architected and IMPLEMENTATION DEFINED event counters.
+
+properties:
+  $nodename:
+    pattern: "^smmu-pmcg@[0-9a-f]*"

Should be generic:

pmu@...

(or whatever we've used for PMUs).

quoted

+  compatible:
+    const: arm,smmu-v3-pmcg

This is correct, but doesn't match the driver.

To be fair, this binding wasn't originally written for this particular 
driver patch ;)

quoted

+
+  reg:
+    minItems: 1
+    maxItems: 2

More than 1 entry needs a description of what each one is.

A variable number of 'reg' entries generally implies more than 1
compatible unless the 2nd entry is optional.

The second is "optional" in terms of the architecture (and thus the 
generic compatible), but fixed for any specific implementation - it's a 
choice of whether the counter registers are in the same page as the 
control registers or in a separate page, but that can be architecturally 
discovered from an ID register in the first page (see the handling of 
SMMU_PMCG_CFGR_RELOC_CTRS if you're interested).
Robin.

quoted

+
+  interrupts:
+    maxItems: 1
+
+  msi-parent: true
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |+
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    tcu: smmu-pmcg@2b420000 {

Drop unused labels.

quoted

+            compatible = "arm,smmu-v3-pmcg";
+            reg = <0 0x2b420000 0 0x1000>,
+                  <0 0x2b430000 0 0x1000>;
+            interrupts = <GIC_SPI 80 IRQ_TYPE_EDGE_RISING>;
+            msi-parent = <&its 0xff0000>;
+    };
+
+    tbu0: smmu-pmcg@2b440000 {
+            compatible = "arm,smmu-v3-pmcg";
+            reg = <0 0x2b440000 0 0x1000>,
+                  <0 0x2b450000 0 0x1000>;
+            interrupts = <GIC_SPI 81 IRQ_TYPE_EDGE_RISING>;
+            msi-parent = <&its 0xff0000>;
+    };
-- 
2.27.0

quoted

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel