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

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

[Jean-Philippe Brucker]

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.

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".

The series doesn't deal with this because for testing I use a software
model which only implements architected events. I do not include DTS
change for that platform because enabling PMCGs requires an additional
model option. See my branch smmu/pmu-dt [3] for details.

[1] https://lore.kernel.org/all/20200707150114.GC159413@myrica/
[2] https://developer.arm.com/documentation/100310/0202/Functional-description/Operation/Performance-Monitoring-Unit
[3] https://jpbrucker.net/git/linux/log/?h=smmu/pmu-dt

Jean-Philippe Brucker (2):
  dt-bindings: Add Arm SMMUv3 PMCG binding
  perf/smmuv3: Add devicetree support

 .../bindings/iommu/arm,smmu-v3-pmcg.yaml      | 67 +++++++++++++++++++
 drivers/perf/arm_smmuv3_pmu.c                 | 25 ++++++-
 2 files changed, 90 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.yaml

-- 
2.33.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

[Jean-Philippe Brucker]

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.

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".

The series doesn't deal with this because for testing I use a software
model which only implements architected events. I do not include DTS
change for that platform because enabling PMCGs requires an additional
model option. See my branch smmu/pmu-dt [3] for details.

[1] https://lore.kernel.org/all/20200707150114.GC159413@myrica/
[2] https://developer.arm.com/documentation/100310/0202/Functional-description/Operation/Performance-Monitoring-Unit
[3] https://jpbrucker.net/git/linux/log/?h=smmu/pmu-dt

Jean-Philippe Brucker (2):
  dt-bindings: Add Arm SMMUv3 PMCG binding
  perf/smmuv3: Add devicetree support

 .../bindings/iommu/arm,smmu-v3-pmcg.yaml      | 67 +++++++++++++++++++
 drivers/perf/arm_smmuv3_pmu.c                 | 25 ++++++-
 2 files changed, 90 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.yaml

-- 
2.33.1


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

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

[Jean-Philippe Brucker]

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.

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".

The series doesn't deal with this because for testing I use a software
model which only implements architected events. I do not include DTS
change for that platform because enabling PMCGs requires an additional
model option. See my branch smmu/pmu-dt [3] for details.

[1] https://lore.kernel.org/all/20200707150114.GC159413@myrica/
[2] https://developer.arm.com/documentation/100310/0202/Functional-description/Operation/Performance-Monitoring-Unit
[3] https://jpbrucker.net/git/linux/log/?h=smmu/pmu-dt

Jean-Philippe Brucker (2):
  dt-bindings: Add Arm SMMUv3 PMCG binding
  perf/smmuv3: Add devicetree support

 .../bindings/iommu/arm,smmu-v3-pmcg.yaml      | 67 +++++++++++++++++++
 drivers/perf/arm_smmuv3_pmu.c                 | 25 ++++++-
 2 files changed, 90 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.yaml

-- 
2.33.1

[PATCH 1/2] dt-bindings: Add Arm SMMUv3 PMCG binding

[Jean-Philippe Brucker]

Add binding for the Arm SMMUv3 PMU. Each node represents a PMCG, and is
placed as a sibling node of the SMMU. Although the PMCGs registers may
be within the SMMU MMIO region, they are separate devices, and there can
be multiple PMCG devices for each SMMU (for example one for the TCU and
one for each TBU).

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 .../bindings/iommu/arm,smmu-v3-pmcg.yaml      | 67 +++++++++++++++++++
 1 file changed, 67 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..a893e071fdb4
--- /dev/null
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.yaml
@@ -0,0 +1,67 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%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 <will@kernel.org>
+  - Robin Murphy <Robin.Murphy@arm.com>
+
+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: "^pmu@[0-9a-f]*"
+  compatible:
+    oneOf:
+      - items:
+        - enum:
+          - hisilicon,smmu-v3-pmcg-hip08
+        - const: arm,smmu-v3-pmcg
+      - const: arm,smmu-v3-pmcg
+
+  reg:
+    description: |
+      Base addresses of the PMCG registers. Either a single address for Page 0
+      or an additional address for Page 1, where some registers can be
+      relocated with SMMU_PMCG_CFGR.RELOC_CTRS.
+    minItems: 1
+    maxItems: 2
+
+  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>
+
+    pmu@2b420000 {
+            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>;
+    };
+
+    pmu@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.33.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH 1/2] dt-bindings: Add Arm SMMUv3 PMCG binding

[Jean-Philippe Brucker]

Add binding for the Arm SMMUv3 PMU. Each node represents a PMCG, and is
placed as a sibling node of the SMMU. Although the PMCGs registers may
be within the SMMU MMIO region, they are separate devices, and there can
be multiple PMCG devices for each SMMU (for example one for the TCU and
one for each TBU).

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 .../bindings/iommu/arm,smmu-v3-pmcg.yaml      | 67 +++++++++++++++++++
 1 file changed, 67 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..a893e071fdb4
--- /dev/null
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.yaml
@@ -0,0 +1,67 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%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 <will@kernel.org>
+  - Robin Murphy <Robin.Murphy@arm.com>
+
+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: "^pmu@[0-9a-f]*"
+  compatible:
+    oneOf:
+      - items:
+        - enum:
+          - hisilicon,smmu-v3-pmcg-hip08
+        - const: arm,smmu-v3-pmcg
+      - const: arm,smmu-v3-pmcg
+
+  reg:
+    description: |
+      Base addresses of the PMCG registers. Either a single address for Page 0
+      or an additional address for Page 1, where some registers can be
+      relocated with SMMU_PMCG_CFGR.RELOC_CTRS.
+    minItems: 1
+    maxItems: 2
+
+  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>
+
+    pmu@2b420000 {
+            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>;
+    };
+
+    pmu@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.33.1


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

[PATCH 1/2] dt-bindings: Add Arm SMMUv3 PMCG binding

[Jean-Philippe Brucker]

Add binding for the Arm SMMUv3 PMU. Each node represents a PMCG, and is
placed as a sibling node of the SMMU. Although the PMCGs registers may
be within the SMMU MMIO region, they are separate devices, and there can
be multiple PMCG devices for each SMMU (for example one for the TCU and
one for each TBU).

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 .../bindings/iommu/arm,smmu-v3-pmcg.yaml      | 67 +++++++++++++++++++
 1 file changed, 67 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..a893e071fdb4
--- /dev/null
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.yaml
@@ -0,0 +1,67 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%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 <will@kernel.org>
+  - Robin Murphy <Robin.Murphy@arm.com>
+
+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: "^pmu@[0-9a-f]*"
+  compatible:
+    oneOf:
+      - items:
+        - enum:
+          - hisilicon,smmu-v3-pmcg-hip08
+        - const: arm,smmu-v3-pmcg
+      - const: arm,smmu-v3-pmcg
+
+  reg:
+    description: |
+      Base addresses of the PMCG registers. Either a single address for Page 0
+      or an additional address for Page 1, where some registers can be
+      relocated with SMMU_PMCG_CFGR.RELOC_CTRS.
+    minItems: 1
+    maxItems: 2
+
+  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>
+
+    pmu@2b420000 {
+            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>;
+    };
+
+    pmu@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.33.1

Re: [PATCH 1/2] dt-bindings: Add Arm SMMUv3 PMCG binding

[Rob Herring]

On Tue, 16 Nov 2021 11:35:36 +0000, Jean-Philippe Brucker wrote:
> Add binding for the Arm SMMUv3 PMU. Each node represents a PMCG, and is
> placed as a sibling node of the SMMU. Although the PMCGs registers may
> be within the SMMU MMIO region, they are separate devices, and there can
> be multiple PMCG devices for each SMMU (for example one for the TCU and
> one for each TBU).
> 
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
>  .../bindings/iommu/arm,smmu-v3-pmcg.yaml      | 67 +++++++++++++++++++
>  1 file changed, 67 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.yaml:24:9: [warning] wrong indentation: expected 10 but found 8 (indentation)
./Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.yaml:25:11: [warning] wrong indentation: expected 12 but found 10 (indentation)

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.example.dt.yaml: example-0: pmu@2b420000:reg:0: [0, 725745664, 0, 4096] is too long
	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.example.dt.yaml: example-0: pmu@2b420000:reg:1: [0, 725811200, 0, 4096] is too long
	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.example.dt.yaml: example-0: pmu@2b440000:reg:0: [0, 725876736, 0, 4096] is too long
	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.example.dt.yaml: example-0: pmu@2b440000:reg:1: [0, 725942272, 0, 4096] is too long
	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1555758

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH 1/2] dt-bindings: Add Arm SMMUv3 PMCG binding

[Rob Herring]

On Tue, 16 Nov 2021 11:35:36 +0000, Jean-Philippe Brucker wrote:
> Add binding for the Arm SMMUv3 PMU. Each node represents a PMCG, and is
> placed as a sibling node of the SMMU. Although the PMCGs registers may
> be within the SMMU MMIO region, they are separate devices, and there can
> be multiple PMCG devices for each SMMU (for example one for the TCU and
> one for each TBU).
> 
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
>  .../bindings/iommu/arm,smmu-v3-pmcg.yaml      | 67 +++++++++++++++++++
>  1 file changed, 67 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.yaml:24:9: [warning] wrong indentation: expected 10 but found 8 (indentation)
./Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.yaml:25:11: [warning] wrong indentation: expected 12 but found 10 (indentation)

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.example.dt.yaml: example-0: pmu@2b420000:reg:0: [0, 725745664, 0, 4096] is too long
	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.example.dt.yaml: example-0: pmu@2b420000:reg:1: [0, 725811200, 0, 4096] is too long
	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.example.dt.yaml: example-0: pmu@2b440000:reg:0: [0, 725876736, 0, 4096] is too long
	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.example.dt.yaml: example-0: pmu@2b440000:reg:1: [0, 725942272, 0, 4096] is too long
	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1555758

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.


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

Re: [PATCH 1/2] dt-bindings: Add Arm SMMUv3 PMCG binding

[Rob Herring]

On Tue, 16 Nov 2021 11:35:36 +0000, Jean-Philippe Brucker wrote:
> Add binding for the Arm SMMUv3 PMU. Each node represents a PMCG, and is
> placed as a sibling node of the SMMU. Although the PMCGs registers may
> be within the SMMU MMIO region, they are separate devices, and there can
> be multiple PMCG devices for each SMMU (for example one for the TCU and
> one for each TBU).
> 
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
>  .../bindings/iommu/arm,smmu-v3-pmcg.yaml      | 67 +++++++++++++++++++
>  1 file changed, 67 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.yaml:24:9: [warning] wrong indentation: expected 10 but found 8 (indentation)
./Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.yaml:25:11: [warning] wrong indentation: expected 12 but found 10 (indentation)

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.example.dt.yaml: example-0: pmu@2b420000:reg:0: [0, 725745664, 0, 4096] is too long
	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.example.dt.yaml: example-0: pmu@2b420000:reg:1: [0, 725811200, 0, 4096] is too long
	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.example.dt.yaml: example-0: pmu@2b440000:reg:0: [0, 725876736, 0, 4096] is too long
	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.example.dt.yaml: example-0: pmu@2b440000:reg:1: [0, 725942272, 0, 4096] is too long
	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1555758

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 1/2] dt-bindings: Add Arm SMMUv3 PMCG binding

[Jean-Philippe Brucker]

On Tue, Nov 16, 2021 at 08:02:53AM -0600, Rob Herring wrote:
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> 
> yamllint warnings/errors:
> ./Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.yaml:24:9: [warning] wrong indentation: expected 10 but found 8 (indentation)
> ./Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.yaml:25:11: [warning] wrong indentation: expected 12 but found 10 (indentation)
> 
> dtschema/dtc warnings/errors:
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.example.dt.yaml: example-0: pmu@2b420000:reg:0: [0, 725745664, 0, 4096] is too long
> 	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.example.dt.yaml: example-0: pmu@2b420000:reg:1: [0, 725811200, 0, 4096] is too long
> 	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.example.dt.yaml: example-0: pmu@2b440000:reg:0: [0, 725876736, 0, 4096] is too long
> 	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.example.dt.yaml: example-0: pmu@2b440000:reg:1: [0, 725942272, 0, 4096] is too long
> 	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
> 
> doc reference errors (make refcheckdocs):
> 
> See https://patchwork.ozlabs.org/patch/1555758
> 
> This check can fail if there are any dependencies. The base for a patch
> series is generally the most recent rc1.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit.

Right I'll fix those, I had only run dtbs_check

Thanks,
Jean

Re: [PATCH 1/2] dt-bindings: Add Arm SMMUv3 PMCG binding

[Jean-Philippe Brucker]

On Tue, Nov 16, 2021 at 08:02:53AM -0600, Rob Herring wrote:
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> 
> yamllint warnings/errors:
> ./Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.yaml:24:9: [warning] wrong indentation: expected 10 but found 8 (indentation)
> ./Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.yaml:25:11: [warning] wrong indentation: expected 12 but found 10 (indentation)
> 
> dtschema/dtc warnings/errors:
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.example.dt.yaml: example-0: pmu@2b420000:reg:0: [0, 725745664, 0, 4096] is too long
> 	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.example.dt.yaml: example-0: pmu@2b420000:reg:1: [0, 725811200, 0, 4096] is too long
> 	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.example.dt.yaml: example-0: pmu@2b440000:reg:0: [0, 725876736, 0, 4096] is too long
> 	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.example.dt.yaml: example-0: pmu@2b440000:reg:1: [0, 725942272, 0, 4096] is too long
> 	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
> 
> doc reference errors (make refcheckdocs):
> 
> See https://patchwork.ozlabs.org/patch/1555758
> 
> This check can fail if there are any dependencies. The base for a patch
> series is generally the most recent rc1.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit.

Right I'll fix those, I had only run dtbs_check

Thanks,
Jean


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

Re: [PATCH 1/2] dt-bindings: Add Arm SMMUv3 PMCG binding

[Jean-Philippe Brucker]

On Tue, Nov 16, 2021 at 08:02:53AM -0600, Rob Herring wrote:
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> 
> yamllint warnings/errors:
> ./Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.yaml:24:9: [warning] wrong indentation: expected 10 but found 8 (indentation)
> ./Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.yaml:25:11: [warning] wrong indentation: expected 12 but found 10 (indentation)
> 
> dtschema/dtc warnings/errors:
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.example.dt.yaml: example-0: pmu@2b420000:reg:0: [0, 725745664, 0, 4096] is too long
> 	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.example.dt.yaml: example-0: pmu@2b420000:reg:1: [0, 725811200, 0, 4096] is too long
> 	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.example.dt.yaml: example-0: pmu@2b440000:reg:0: [0, 725876736, 0, 4096] is too long
> 	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.example.dt.yaml: example-0: pmu@2b440000:reg:1: [0, 725942272, 0, 4096] is too long
> 	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
> 
> doc reference errors (make refcheckdocs):
> 
> See https://patchwork.ozlabs.org/patch/1555758
> 
> This check can fail if there are any dependencies. The base for a patch
> series is generally the most recent rc1.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit.

Right I'll fix those, I had only run dtbs_check

Thanks,
Jean

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 1/2] dt-bindings: Add Arm SMMUv3 PMCG binding

[Rob Herring]

On Tue, Nov 16, 2021 at 5:52 AM Jean-Philippe Brucker
<jean-philippe@linaro.org> wrote:
>
> Add binding for the Arm SMMUv3 PMU. Each node represents a PMCG, and is
> placed as a sibling node of the SMMU. Although the PMCGs registers may
> be within the SMMU MMIO region, they are separate devices, and there can
> be multiple PMCG devices for each SMMU (for example one for the TCU and
> one for each TBU).
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
>  .../bindings/iommu/arm,smmu-v3-pmcg.yaml      | 67 +++++++++++++++++++
>  1 file changed, 67 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..a893e071fdb4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.yaml
> @@ -0,0 +1,67 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%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 <will@kernel.org>
> +  - Robin Murphy <Robin.Murphy@arm.com>
> +
> +description: |+

Don't need '|+' if no formatting to preserve.

> +  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.

Humm, I don't know that I agree they are standalone. They could be I
guess, but looking at the MMU-600 spec the PMCG looks like it's just a
subset of registers in a larger block. This seems similar to MPAM
(which I'm working on a binding for) where it's just a register map
and interrupts, but every other possible resource is unspecified by
the architecture.

The simplest change from this would be just specifying that the PMCG
is child node(s) of whatever it is part of. The extreme would be this
is all part of the SMMU binding (i.e. reg entry X is PMCG registers,
interrupts entry Y is pmu irq).

> +
> +properties:
> +  $nodename:
> +    pattern: "^pmu@[0-9a-f]*"

s/*/+/

Need at least 1 digit.

> +  compatible:
> +    oneOf:
> +      - items:
> +        - enum:
> +          - hisilicon,smmu-v3-pmcg-hip08
> +        - const: arm,smmu-v3-pmcg
> +      - const: arm,smmu-v3-pmcg
> +
> +  reg:
> +    description: |
> +      Base addresses of the PMCG registers. Either a single address for Page 0
> +      or an additional address for Page 1, where some registers can be
> +      relocated with SMMU_PMCG_CFGR.RELOC_CTRS.
> +    minItems: 1
> +    maxItems: 2
> +
> +  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>
> +
> +    pmu@2b420000 {
> +            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>;
> +    };
> +
> +    pmu@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.33.1
>

Re: [PATCH 1/2] dt-bindings: Add Arm SMMUv3 PMCG binding

[Rob Herring]

On Tue, Nov 16, 2021 at 5:52 AM Jean-Philippe Brucker
<jean-philippe@linaro.org> wrote:
>
> Add binding for the Arm SMMUv3 PMU. Each node represents a PMCG, and is
> placed as a sibling node of the SMMU. Although the PMCGs registers may
> be within the SMMU MMIO region, they are separate devices, and there can
> be multiple PMCG devices for each SMMU (for example one for the TCU and
> one for each TBU).
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
>  .../bindings/iommu/arm,smmu-v3-pmcg.yaml      | 67 +++++++++++++++++++
>  1 file changed, 67 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..a893e071fdb4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.yaml
> @@ -0,0 +1,67 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%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 <will@kernel.org>
> +  - Robin Murphy <Robin.Murphy@arm.com>
> +
> +description: |+

Don't need '|+' if no formatting to preserve.

> +  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.

Humm, I don't know that I agree they are standalone. They could be I
guess, but looking at the MMU-600 spec the PMCG looks like it's just a
subset of registers in a larger block. This seems similar to MPAM
(which I'm working on a binding for) where it's just a register map
and interrupts, but every other possible resource is unspecified by
the architecture.

The simplest change from this would be just specifying that the PMCG
is child node(s) of whatever it is part of. The extreme would be this
is all part of the SMMU binding (i.e. reg entry X is PMCG registers,
interrupts entry Y is pmu irq).

> +
> +properties:
> +  $nodename:
> +    pattern: "^pmu@[0-9a-f]*"

s/*/+/

Need at least 1 digit.

> +  compatible:
> +    oneOf:
> +      - items:
> +        - enum:
> +          - hisilicon,smmu-v3-pmcg-hip08
> +        - const: arm,smmu-v3-pmcg
> +      - const: arm,smmu-v3-pmcg
> +
> +  reg:
> +    description: |
> +      Base addresses of the PMCG registers. Either a single address for Page 0
> +      or an additional address for Page 1, where some registers can be
> +      relocated with SMMU_PMCG_CFGR.RELOC_CTRS.
> +    minItems: 1
> +    maxItems: 2
> +
> +  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>
> +
> +    pmu@2b420000 {
> +            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>;
> +    };
> +
> +    pmu@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.33.1
>

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

Re: [PATCH 1/2] dt-bindings: Add Arm SMMUv3 PMCG binding

[Rob Herring]

On Tue, Nov 16, 2021 at 5:52 AM Jean-Philippe Brucker
<jean-philippe@linaro.org> wrote:
>
> Add binding for the Arm SMMUv3 PMU. Each node represents a PMCG, and is
> placed as a sibling node of the SMMU. Although the PMCGs registers may
> be within the SMMU MMIO region, they are separate devices, and there can
> be multiple PMCG devices for each SMMU (for example one for the TCU and
> one for each TBU).
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
>  .../bindings/iommu/arm,smmu-v3-pmcg.yaml      | 67 +++++++++++++++++++
>  1 file changed, 67 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..a893e071fdb4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.yaml
> @@ -0,0 +1,67 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%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 <will@kernel.org>
> +  - Robin Murphy <Robin.Murphy@arm.com>
> +
> +description: |+

Don't need '|+' if no formatting to preserve.

> +  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.

Humm, I don't know that I agree they are standalone. They could be I
guess, but looking at the MMU-600 spec the PMCG looks like it's just a
subset of registers in a larger block. This seems similar to MPAM
(which I'm working on a binding for) where it's just a register map
and interrupts, but every other possible resource is unspecified by
the architecture.

The simplest change from this would be just specifying that the PMCG
is child node(s) of whatever it is part of. The extreme would be this
is all part of the SMMU binding (i.e. reg entry X is PMCG registers,
interrupts entry Y is pmu irq).

> +
> +properties:
> +  $nodename:
> +    pattern: "^pmu@[0-9a-f]*"

s/*/+/

Need at least 1 digit.

> +  compatible:
> +    oneOf:
> +      - items:
> +        - enum:
> +          - hisilicon,smmu-v3-pmcg-hip08
> +        - const: arm,smmu-v3-pmcg
> +      - const: arm,smmu-v3-pmcg
> +
> +  reg:
> +    description: |
> +      Base addresses of the PMCG registers. Either a single address for Page 0
> +      or an additional address for Page 1, where some registers can be
> +      relocated with SMMU_PMCG_CFGR.RELOC_CTRS.
> +    minItems: 1
> +    maxItems: 2
> +
> +  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>
> +
> +    pmu@2b420000 {
> +            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>;
> +    };
> +
> +    pmu@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.33.1
>
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 1/2] dt-bindings: Add Arm SMMUv3 PMCG binding

[Robin Murphy]

On 2021-11-17 23:19, Rob Herring wrote:
> On Tue, Nov 16, 2021 at 5:52 AM Jean-Philippe Brucker
> <jean-philippe@linaro.org> wrote:
>>
>> Add binding for the Arm SMMUv3 PMU. Each node represents a PMCG, and is
>> placed as a sibling node of the SMMU. Although the PMCGs registers may
>> be within the SMMU MMIO region, they are separate devices, and there can
>> be multiple PMCG devices for each SMMU (for example one for the TCU and
>> one for each TBU).
>>
>> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
>> ---
>>   .../bindings/iommu/arm,smmu-v3-pmcg.yaml      | 67 +++++++++++++++++++
>>   1 file changed, 67 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..a893e071fdb4
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.yaml
>> @@ -0,0 +1,67 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%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 <will@kernel.org>
>> +  - Robin Murphy <Robin.Murphy@arm.com>
>> +
>> +description: |+
> 
> Don't need '|+' if no formatting to preserve.
> 
>> +  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.
> 
> Humm, I don't know that I agree they are standalone. They could be I
> guess, but looking at the MMU-600 spec the PMCG looks like it's just a
> subset of registers in a larger block. This seems similar to MPAM
> (which I'm working on a binding for) where it's just a register map
> and interrupts, but every other possible resource is unspecified by
> the architecture.

They're "standalone" in the sense that they don't have to be part of an 
SMMU, they could be part of a PCIe root complex or other SoC device that 
couples to an SMMU (e.g. anything that can speak AMBA DTI, in the case 
of our SMMU implementations).

In fact our SMMU TBUs are pretty much separate devices themselves, they 
just *only* speak DTI, so access to their registers is proxied through 
the TCU programming interface.

> The simplest change from this would be just specifying that the PMCG
> is child node(s) of whatever it is part of. The extreme would be this
> is all part of the SMMU binding (i.e. reg entry X is PMCG registers,
> interrupts entry Y is pmu irq).

Being a child of its associated device doesn't seem too bad 
semantically, however how would we describe a PMCG as a child of a PCIe 
node when its "reg" property still exists in the parent address space 
and not PCI config/memory space like any of its siblings? Also in 
practical terms, consuming that binding in Linux and getting the things 
to probe when it may want to be independent of whether we even 
understand the parent node at all could be... unpleasant.

Robin.

>> +
>> +properties:
>> +  $nodename:
>> +    pattern: "^pmu@[0-9a-f]*"
> 
> s/*/+/
> 
> Need at least 1 digit.
> 
>> +  compatible:
>> +    oneOf:
>> +      - items:
>> +        - enum:
>> +          - hisilicon,smmu-v3-pmcg-hip08
>> +        - const: arm,smmu-v3-pmcg
>> +      - const: arm,smmu-v3-pmcg
>> +
>> +  reg:
>> +    description: |
>> +      Base addresses of the PMCG registers. Either a single address for Page 0
>> +      or an additional address for Page 1, where some registers can be
>> +      relocated with SMMU_PMCG_CFGR.RELOC_CTRS.
>> +    minItems: 1
>> +    maxItems: 2
>> +
>> +  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>
>> +
>> +    pmu@2b420000 {
>> +            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>;
>> +    };
>> +
>> +    pmu@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.33.1
>>

Re: [PATCH 1/2] dt-bindings: Add Arm SMMUv3 PMCG binding

[Robin Murphy]

On 2021-11-17 23:19, Rob Herring wrote:
> On Tue, Nov 16, 2021 at 5:52 AM Jean-Philippe Brucker
> <jean-philippe@linaro.org> wrote:
>>
>> Add binding for the Arm SMMUv3 PMU. Each node represents a PMCG, and is
>> placed as a sibling node of the SMMU. Although the PMCGs registers may
>> be within the SMMU MMIO region, they are separate devices, and there can
>> be multiple PMCG devices for each SMMU (for example one for the TCU and
>> one for each TBU).
>>
>> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
>> ---
>>   .../bindings/iommu/arm,smmu-v3-pmcg.yaml      | 67 +++++++++++++++++++
>>   1 file changed, 67 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..a893e071fdb4
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.yaml
>> @@ -0,0 +1,67 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%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 <will@kernel.org>
>> +  - Robin Murphy <Robin.Murphy@arm.com>
>> +
>> +description: |+
> 
> Don't need '|+' if no formatting to preserve.
> 
>> +  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.
> 
> Humm, I don't know that I agree they are standalone. They could be I
> guess, but looking at the MMU-600 spec the PMCG looks like it's just a
> subset of registers in a larger block. This seems similar to MPAM
> (which I'm working on a binding for) where it's just a register map
> and interrupts, but every other possible resource is unspecified by
> the architecture.

They're "standalone" in the sense that they don't have to be part of an 
SMMU, they could be part of a PCIe root complex or other SoC device that 
couples to an SMMU (e.g. anything that can speak AMBA DTI, in the case 
of our SMMU implementations).

In fact our SMMU TBUs are pretty much separate devices themselves, they 
just *only* speak DTI, so access to their registers is proxied through 
the TCU programming interface.

> The simplest change from this would be just specifying that the PMCG
> is child node(s) of whatever it is part of. The extreme would be this
> is all part of the SMMU binding (i.e. reg entry X is PMCG registers,
> interrupts entry Y is pmu irq).

Being a child of its associated device doesn't seem too bad 
semantically, however how would we describe a PMCG as a child of a PCIe 
node when its "reg" property still exists in the parent address space 
and not PCI config/memory space like any of its siblings? Also in 
practical terms, consuming that binding in Linux and getting the things 
to probe when it may want to be independent of whether we even 
understand the parent node at all could be... unpleasant.

Robin.

>> +
>> +properties:
>> +  $nodename:
>> +    pattern: "^pmu@[0-9a-f]*"
> 
> s/*/+/
> 
> Need at least 1 digit.
> 
>> +  compatible:
>> +    oneOf:
>> +      - items:
>> +        - enum:
>> +          - hisilicon,smmu-v3-pmcg-hip08
>> +        - const: arm,smmu-v3-pmcg
>> +      - const: arm,smmu-v3-pmcg
>> +
>> +  reg:
>> +    description: |
>> +      Base addresses of the PMCG registers. Either a single address for Page 0
>> +      or an additional address for Page 1, where some registers can be
>> +      relocated with SMMU_PMCG_CFGR.RELOC_CTRS.
>> +    minItems: 1
>> +    maxItems: 2
>> +
>> +  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>
>> +
>> +    pmu@2b420000 {
>> +            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>;
>> +    };
>> +
>> +    pmu@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.33.1
>>

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

Re: [PATCH 1/2] dt-bindings: Add Arm SMMUv3 PMCG binding

[Robin Murphy]

On 2021-11-17 23:19, Rob Herring wrote:
> On Tue, Nov 16, 2021 at 5:52 AM Jean-Philippe Brucker
> <jean-philippe@linaro.org> wrote:
>>
>> Add binding for the Arm SMMUv3 PMU. Each node represents a PMCG, and is
>> placed as a sibling node of the SMMU. Although the PMCGs registers may
>> be within the SMMU MMIO region, they are separate devices, and there can
>> be multiple PMCG devices for each SMMU (for example one for the TCU and
>> one for each TBU).
>>
>> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
>> ---
>>   .../bindings/iommu/arm,smmu-v3-pmcg.yaml      | 67 +++++++++++++++++++
>>   1 file changed, 67 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..a893e071fdb4
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.yaml
>> @@ -0,0 +1,67 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%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 <will@kernel.org>
>> +  - Robin Murphy <Robin.Murphy@arm.com>
>> +
>> +description: |+
> 
> Don't need '|+' if no formatting to preserve.
> 
>> +  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.
> 
> Humm, I don't know that I agree they are standalone. They could be I
> guess, but looking at the MMU-600 spec the PMCG looks like it's just a
> subset of registers in a larger block. This seems similar to MPAM
> (which I'm working on a binding for) where it's just a register map
> and interrupts, but every other possible resource is unspecified by
> the architecture.

They're "standalone" in the sense that they don't have to be part of an 
SMMU, they could be part of a PCIe root complex or other SoC device that 
couples to an SMMU (e.g. anything that can speak AMBA DTI, in the case 
of our SMMU implementations).

In fact our SMMU TBUs are pretty much separate devices themselves, they 
just *only* speak DTI, so access to their registers is proxied through 
the TCU programming interface.

> The simplest change from this would be just specifying that the PMCG
> is child node(s) of whatever it is part of. The extreme would be this
> is all part of the SMMU binding (i.e. reg entry X is PMCG registers,
> interrupts entry Y is pmu irq).

Being a child of its associated device doesn't seem too bad 
semantically, however how would we describe a PMCG as a child of a PCIe 
node when its "reg" property still exists in the parent address space 
and not PCI config/memory space like any of its siblings? Also in 
practical terms, consuming that binding in Linux and getting the things 
to probe when it may want to be independent of whether we even 
understand the parent node at all could be... unpleasant.

Robin.

>> +
>> +properties:
>> +  $nodename:
>> +    pattern: "^pmu@[0-9a-f]*"
> 
> s/*/+/
> 
> Need at least 1 digit.
> 
>> +  compatible:
>> +    oneOf:
>> +      - items:
>> +        - enum:
>> +          - hisilicon,smmu-v3-pmcg-hip08
>> +        - const: arm,smmu-v3-pmcg
>> +      - const: arm,smmu-v3-pmcg
>> +
>> +  reg:
>> +    description: |
>> +      Base addresses of the PMCG registers. Either a single address for Page 0
>> +      or an additional address for Page 1, where some registers can be
>> +      relocated with SMMU_PMCG_CFGR.RELOC_CTRS.
>> +    minItems: 1
>> +    maxItems: 2
>> +
>> +  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>
>> +
>> +    pmu@2b420000 {
>> +            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>;
>> +    };
>> +
>> +    pmu@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.33.1
>>
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 1/2] dt-bindings: Add Arm SMMUv3 PMCG binding

[Jean-Philippe Brucker]

On Thu, Nov 18, 2021 at 03:50:54PM +0000, Robin Murphy wrote:
> > > +  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.
> > 
> > Humm, I don't know that I agree they are standalone. They could be I
> > guess, but looking at the MMU-600 spec the PMCG looks like it's just a
> > subset of registers in a larger block. This seems similar to MPAM
> > (which I'm working on a binding for) where it's just a register map
> > and interrupts, but every other possible resource is unspecified by
> > the architecture.
> 
> They're "standalone" in the sense that they don't have to be part of an
> SMMU, they could be part of a PCIe root complex or other SoC device that
> couples to an SMMU (e.g. anything that can speak AMBA DTI, in the case of
> our SMMU implementations).

The "standalone" word came from the SMMUv3 spec (IHI0070D.b 10.1):

  The Performance Monitor Counter Groups are standalone monitoring
  facilities and, as such, can be implemented in separate components that
  are all associated with (but not necessarily part of) an SMMU.

> 
> In fact our SMMU TBUs are pretty much separate devices themselves, they just
> *only* speak DTI, so access to their registers is proxied through the TCU
> programming interface.
> 
> > The simplest change from this would be just specifying that the PMCG
> > is child node(s) of whatever it is part of. The extreme would be this
> > is all part of the SMMU binding (i.e. reg entry X is PMCG registers,
> > interrupts entry Y is pmu irq).
> 
> Being a child of its associated device doesn't seem too bad semantically,
> however how would we describe a PMCG as a child of a PCIe node when its
> "reg" property still exists in the parent address space and not PCI
> config/memory space like any of its siblings? Also in practical terms,
> consuming that binding in Linux and getting the things to probe when it may
> want to be independent of whether we even understand the parent node at all
> could be... unpleasant.

So there are multiple options for what "the PMCG is part of".

(a) The SMMU: the spec guarantees that a PMCG is associated with an SMMU.

(b) The MMIO region: may be within the SMMU (as with MMU-600), outside of
    it (as does another implementation, two 64k pages after the SMMU base)
    or, theoretically, within a separate device (e.g. PCIe controller).

(c) The thing being measured: does not necessarily match the MMIO region.
    For example a TBU attached to the PCIe RC but the PMCG MMIO is within
    the SMMU region.

(d) None: the PMCG can be probed and driven separately from the SMMU and
    other components, as demonstrated by Linux.

Which one is normally picked to decide where to insert a devicetree node?
I guess (b)?  I picked (d) so far as the easiest choice.

(a) is also a reasonable choice, being based on the spec, but it might be
confusing to have a PMCG node inside the SMMU node when the MMIO region is
external, possibly belonging to another device. For the same reason we
could discard (c).

(b) feels more natural, although it's not clear what to do when the PMCG
MMIO region is external or adjacent to the SMMU region. Does the node go
inside the SMMU node or one level up?

Thanks,
Jean

> 
> Robin.
> 
> > > +
> > > +properties:
> > > +  $nodename:
> > > +    pattern: "^pmu@[0-9a-f]*"
> > 
> > s/*/+/
> > 
> > Need at least 1 digit.
> > 
> > > +  compatible:
> > > +    oneOf:
> > > +      - items:
> > > +        - enum:
> > > +          - hisilicon,smmu-v3-pmcg-hip08
> > > +        - const: arm,smmu-v3-pmcg
> > > +      - const: arm,smmu-v3-pmcg
> > > +
> > > +  reg:
> > > +    description: |
> > > +      Base addresses of the PMCG registers. Either a single address for Page 0
> > > +      or an additional address for Page 1, where some registers can be
> > > +      relocated with SMMU_PMCG_CFGR.RELOC_CTRS.
> > > +    minItems: 1
> > > +    maxItems: 2
> > > +
> > > +  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>
> > > +
> > > +    pmu@2b420000 {
> > > +            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>;
> > > +    };
> > > +
> > > +    pmu@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.33.1
> > > 

Re: [PATCH 1/2] dt-bindings: Add Arm SMMUv3 PMCG binding

[Jean-Philippe Brucker]

On Thu, Nov 18, 2021 at 03:50:54PM +0000, Robin Murphy wrote:
> > > +  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.
> > 
> > Humm, I don't know that I agree they are standalone. They could be I
> > guess, but looking at the MMU-600 spec the PMCG looks like it's just a
> > subset of registers in a larger block. This seems similar to MPAM
> > (which I'm working on a binding for) where it's just a register map
> > and interrupts, but every other possible resource is unspecified by
> > the architecture.
> 
> They're "standalone" in the sense that they don't have to be part of an
> SMMU, they could be part of a PCIe root complex or other SoC device that
> couples to an SMMU (e.g. anything that can speak AMBA DTI, in the case of
> our SMMU implementations).

The "standalone" word came from the SMMUv3 spec (IHI0070D.b 10.1):

  The Performance Monitor Counter Groups are standalone monitoring
  facilities and, as such, can be implemented in separate components that
  are all associated with (but not necessarily part of) an SMMU.

> 
> In fact our SMMU TBUs are pretty much separate devices themselves, they just
> *only* speak DTI, so access to their registers is proxied through the TCU
> programming interface.
> 
> > The simplest change from this would be just specifying that the PMCG
> > is child node(s) of whatever it is part of. The extreme would be this
> > is all part of the SMMU binding (i.e. reg entry X is PMCG registers,
> > interrupts entry Y is pmu irq).
> 
> Being a child of its associated device doesn't seem too bad semantically,
> however how would we describe a PMCG as a child of a PCIe node when its
> "reg" property still exists in the parent address space and not PCI
> config/memory space like any of its siblings? Also in practical terms,
> consuming that binding in Linux and getting the things to probe when it may
> want to be independent of whether we even understand the parent node at all
> could be... unpleasant.

So there are multiple options for what "the PMCG is part of".

(a) The SMMU: the spec guarantees that a PMCG is associated with an SMMU.

(b) The MMIO region: may be within the SMMU (as with MMU-600), outside of
    it (as does another implementation, two 64k pages after the SMMU base)
    or, theoretically, within a separate device (e.g. PCIe controller).

(c) The thing being measured: does not necessarily match the MMIO region.
    For example a TBU attached to the PCIe RC but the PMCG MMIO is within
    the SMMU region.

(d) None: the PMCG can be probed and driven separately from the SMMU and
    other components, as demonstrated by Linux.

Which one is normally picked to decide where to insert a devicetree node?
I guess (b)?  I picked (d) so far as the easiest choice.

(a) is also a reasonable choice, being based on the spec, but it might be
confusing to have a PMCG node inside the SMMU node when the MMIO region is
external, possibly belonging to another device. For the same reason we
could discard (c).

(b) feels more natural, although it's not clear what to do when the PMCG
MMIO region is external or adjacent to the SMMU region. Does the node go
inside the SMMU node or one level up?

Thanks,
Jean

> 
> Robin.
> 
> > > +
> > > +properties:
> > > +  $nodename:
> > > +    pattern: "^pmu@[0-9a-f]*"
> > 
> > s/*/+/
> > 
> > Need at least 1 digit.
> > 
> > > +  compatible:
> > > +    oneOf:
> > > +      - items:
> > > +        - enum:
> > > +          - hisilicon,smmu-v3-pmcg-hip08
> > > +        - const: arm,smmu-v3-pmcg
> > > +      - const: arm,smmu-v3-pmcg
> > > +
> > > +  reg:
> > > +    description: |
> > > +      Base addresses of the PMCG registers. Either a single address for Page 0
> > > +      or an additional address for Page 1, where some registers can be
> > > +      relocated with SMMU_PMCG_CFGR.RELOC_CTRS.
> > > +    minItems: 1
> > > +    maxItems: 2
> > > +
> > > +  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>
> > > +
> > > +    pmu@2b420000 {
> > > +            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>;
> > > +    };
> > > +
> > > +    pmu@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.33.1
> > > 

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

Re: [PATCH 1/2] dt-bindings: Add Arm SMMUv3 PMCG binding

[Jean-Philippe Brucker]

On Thu, Nov 18, 2021 at 03:50:54PM +0000, Robin Murphy wrote:
> > > +  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.
> > 
> > Humm, I don't know that I agree they are standalone. They could be I
> > guess, but looking at the MMU-600 spec the PMCG looks like it's just a
> > subset of registers in a larger block. This seems similar to MPAM
> > (which I'm working on a binding for) where it's just a register map
> > and interrupts, but every other possible resource is unspecified by
> > the architecture.
> 
> They're "standalone" in the sense that they don't have to be part of an
> SMMU, they could be part of a PCIe root complex or other SoC device that
> couples to an SMMU (e.g. anything that can speak AMBA DTI, in the case of
> our SMMU implementations).

The "standalone" word came from the SMMUv3 spec (IHI0070D.b 10.1):

  The Performance Monitor Counter Groups are standalone monitoring
  facilities and, as such, can be implemented in separate components that
  are all associated with (but not necessarily part of) an SMMU.

> 
> In fact our SMMU TBUs are pretty much separate devices themselves, they just
> *only* speak DTI, so access to their registers is proxied through the TCU
> programming interface.
> 
> > The simplest change from this would be just specifying that the PMCG
> > is child node(s) of whatever it is part of. The extreme would be this
> > is all part of the SMMU binding (i.e. reg entry X is PMCG registers,
> > interrupts entry Y is pmu irq).
> 
> Being a child of its associated device doesn't seem too bad semantically,
> however how would we describe a PMCG as a child of a PCIe node when its
> "reg" property still exists in the parent address space and not PCI
> config/memory space like any of its siblings? Also in practical terms,
> consuming that binding in Linux and getting the things to probe when it may
> want to be independent of whether we even understand the parent node at all
> could be... unpleasant.

So there are multiple options for what "the PMCG is part of".

(a) The SMMU: the spec guarantees that a PMCG is associated with an SMMU.

(b) The MMIO region: may be within the SMMU (as with MMU-600), outside of
    it (as does another implementation, two 64k pages after the SMMU base)
    or, theoretically, within a separate device (e.g. PCIe controller).

(c) The thing being measured: does not necessarily match the MMIO region.
    For example a TBU attached to the PCIe RC but the PMCG MMIO is within
    the SMMU region.

(d) None: the PMCG can be probed and driven separately from the SMMU and
    other components, as demonstrated by Linux.

Which one is normally picked to decide where to insert a devicetree node?
I guess (b)?  I picked (d) so far as the easiest choice.

(a) is also a reasonable choice, being based on the spec, but it might be
confusing to have a PMCG node inside the SMMU node when the MMIO region is
external, possibly belonging to another device. For the same reason we
could discard (c).

(b) feels more natural, although it's not clear what to do when the PMCG
MMIO region is external or adjacent to the SMMU region. Does the node go
inside the SMMU node or one level up?

Thanks,
Jean

> 
> Robin.
> 
> > > +
> > > +properties:
> > > +  $nodename:
> > > +    pattern: "^pmu@[0-9a-f]*"
> > 
> > s/*/+/
> > 
> > Need at least 1 digit.
> > 
> > > +  compatible:
> > > +    oneOf:
> > > +      - items:
> > > +        - enum:
> > > +          - hisilicon,smmu-v3-pmcg-hip08
> > > +        - const: arm,smmu-v3-pmcg
> > > +      - const: arm,smmu-v3-pmcg
> > > +
> > > +  reg:
> > > +    description: |
> > > +      Base addresses of the PMCG registers. Either a single address for Page 0
> > > +      or an additional address for Page 1, where some registers can be
> > > +      relocated with SMMU_PMCG_CFGR.RELOC_CTRS.
> > > +    minItems: 1
> > > +    maxItems: 2
> > > +
> > > +  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>
> > > +
> > > +    pmu@2b420000 {
> > > +            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>;
> > > +    };
> > > +
> > > +    pmu@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.33.1
> > > 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH 2/2] perf/smmuv3: Add devicetree support

[Jean-Philippe Brucker]

Add device-tree support to the SMMUv3 PMCG.  One small cosmetic change
while factoring the option mask printout: don't display it when zero, it
only contains one erratum at the moment.

Signed-off-by: Jay Chen <jkchen@linux.alibaba.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 drivers/perf/arm_smmuv3_pmu.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c
index 226348822ab3..958325ac103a 100644
--- a/drivers/perf/arm_smmuv3_pmu.c
+++ b/drivers/perf/arm_smmuv3_pmu.c
@@ -47,6 +47,7 @@
 #include <linux/kernel.h>
 #include <linux/list.h>
 #include <linux/msi.h>
+#include <linux/of.h>
 #include <linux/perf_event.h>
 #include <linux/platform_device.h>
 #include <linux/smp.h>
@@ -750,8 +751,15 @@ static void smmu_pmu_get_acpi_options(struct smmu_pmu *smmu_pmu)
 		smmu_pmu->options |= SMMU_PMCG_EVCNTR_RDONLY;
 		break;
 	}
+}
+
+static void smmu_pmu_get_of_options(struct smmu_pmu *smmu_pmu)
+{
+	struct device_node *node = smmu_pmu->dev->of_node;
 
-	dev_notice(smmu_pmu->dev, "option mask 0x%x\n", smmu_pmu->options);
+	if (of_device_is_compatible(node, "hisilicon,smmu-v3-pmcg-hip08"))
+		/* HiSilicon Erratum 162001800 */
+		smmu_pmu->options |= SMMU_PMCG_EVCNTR_RDONLY;
 }
 
 static int smmu_pmu_probe(struct platform_device *pdev)
@@ -834,7 +842,13 @@ static int smmu_pmu_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	smmu_pmu_get_acpi_options(smmu_pmu);
+	if (dev->of_node)
+		smmu_pmu_get_of_options(smmu_pmu);
+	else
+		smmu_pmu_get_acpi_options(smmu_pmu);
+
+	if (smmu_pmu->options)
+		dev_notice(dev, "option mask 0x%x\n", smmu_pmu->options);
 
 	/* Pick one CPU to be the preferred one to use */
 	smmu_pmu->on_cpu = raw_smp_processor_id();
@@ -884,10 +898,17 @@ static void smmu_pmu_shutdown(struct platform_device *pdev)
 	smmu_pmu_disable(&smmu_pmu->pmu);
 }
 
+static const struct of_device_id arm_smmu_pmu_match[] = {
+	{ .compatible = "arm,smmu-v3-pmcg" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, arm_smmu_pmu_match);
+
 static struct platform_driver smmu_pmu_driver = {
 	.driver = {
 		.name = "arm-smmu-v3-pmcg",
 		.suppress_bind_attrs = true,
+		.of_match_table = of_match_ptr(arm_smmu_pmu_match),
 	},
 	.probe = smmu_pmu_probe,
 	.remove = smmu_pmu_remove,
-- 
2.33.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH 2/2] perf/smmuv3: Add devicetree support

[Jean-Philippe Brucker]

Add device-tree support to the SMMUv3 PMCG.  One small cosmetic change
while factoring the option mask printout: don't display it when zero, it
only contains one erratum at the moment.

Signed-off-by: Jay Chen <jkchen@linux.alibaba.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 drivers/perf/arm_smmuv3_pmu.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c
index 226348822ab3..958325ac103a 100644
--- a/drivers/perf/arm_smmuv3_pmu.c
+++ b/drivers/perf/arm_smmuv3_pmu.c
@@ -47,6 +47,7 @@
 #include <linux/kernel.h>
 #include <linux/list.h>
 #include <linux/msi.h>
+#include <linux/of.h>
 #include <linux/perf_event.h>
 #include <linux/platform_device.h>
 #include <linux/smp.h>
@@ -750,8 +751,15 @@ static void smmu_pmu_get_acpi_options(struct smmu_pmu *smmu_pmu)
 		smmu_pmu->options |= SMMU_PMCG_EVCNTR_RDONLY;
 		break;
 	}
+}
+
+static void smmu_pmu_get_of_options(struct smmu_pmu *smmu_pmu)
+{
+	struct device_node *node = smmu_pmu->dev->of_node;
 
-	dev_notice(smmu_pmu->dev, "option mask 0x%x\n", smmu_pmu->options);
+	if (of_device_is_compatible(node, "hisilicon,smmu-v3-pmcg-hip08"))
+		/* HiSilicon Erratum 162001800 */
+		smmu_pmu->options |= SMMU_PMCG_EVCNTR_RDONLY;
 }
 
 static int smmu_pmu_probe(struct platform_device *pdev)
@@ -834,7 +842,13 @@ static int smmu_pmu_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	smmu_pmu_get_acpi_options(smmu_pmu);
+	if (dev->of_node)
+		smmu_pmu_get_of_options(smmu_pmu);
+	else
+		smmu_pmu_get_acpi_options(smmu_pmu);
+
+	if (smmu_pmu->options)
+		dev_notice(dev, "option mask 0x%x\n", smmu_pmu->options);
 
 	/* Pick one CPU to be the preferred one to use */
 	smmu_pmu->on_cpu = raw_smp_processor_id();
@@ -884,10 +898,17 @@ static void smmu_pmu_shutdown(struct platform_device *pdev)
 	smmu_pmu_disable(&smmu_pmu->pmu);
 }
 
+static const struct of_device_id arm_smmu_pmu_match[] = {
+	{ .compatible = "arm,smmu-v3-pmcg" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, arm_smmu_pmu_match);
+
 static struct platform_driver smmu_pmu_driver = {
 	.driver = {
 		.name = "arm-smmu-v3-pmcg",
 		.suppress_bind_attrs = true,
+		.of_match_table = of_match_ptr(arm_smmu_pmu_match),
 	},
 	.probe = smmu_pmu_probe,
 	.remove = smmu_pmu_remove,
-- 
2.33.1


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

[PATCH 2/2] perf/smmuv3: Add devicetree support

[Jean-Philippe Brucker]

Add device-tree support to the SMMUv3 PMCG.  One small cosmetic change
while factoring the option mask printout: don't display it when zero, it
only contains one erratum at the moment.

Signed-off-by: Jay Chen <jkchen@linux.alibaba.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 drivers/perf/arm_smmuv3_pmu.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c
index 226348822ab3..958325ac103a 100644
--- a/drivers/perf/arm_smmuv3_pmu.c
+++ b/drivers/perf/arm_smmuv3_pmu.c
@@ -47,6 +47,7 @@
 #include <linux/kernel.h>
 #include <linux/list.h>
 #include <linux/msi.h>
+#include <linux/of.h>
 #include <linux/perf_event.h>
 #include <linux/platform_device.h>
 #include <linux/smp.h>
@@ -750,8 +751,15 @@ static void smmu_pmu_get_acpi_options(struct smmu_pmu *smmu_pmu)
 		smmu_pmu->options |= SMMU_PMCG_EVCNTR_RDONLY;
 		break;
 	}
+}
+
+static void smmu_pmu_get_of_options(struct smmu_pmu *smmu_pmu)
+{
+	struct device_node *node = smmu_pmu->dev->of_node;
 
-	dev_notice(smmu_pmu->dev, "option mask 0x%x\n", smmu_pmu->options);
+	if (of_device_is_compatible(node, "hisilicon,smmu-v3-pmcg-hip08"))
+		/* HiSilicon Erratum 162001800 */
+		smmu_pmu->options |= SMMU_PMCG_EVCNTR_RDONLY;
 }
 
 static int smmu_pmu_probe(struct platform_device *pdev)
@@ -834,7 +842,13 @@ static int smmu_pmu_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	smmu_pmu_get_acpi_options(smmu_pmu);
+	if (dev->of_node)
+		smmu_pmu_get_of_options(smmu_pmu);
+	else
+		smmu_pmu_get_acpi_options(smmu_pmu);
+
+	if (smmu_pmu->options)
+		dev_notice(dev, "option mask 0x%x\n", smmu_pmu->options);
 
 	/* Pick one CPU to be the preferred one to use */
 	smmu_pmu->on_cpu = raw_smp_processor_id();
@@ -884,10 +898,17 @@ static void smmu_pmu_shutdown(struct platform_device *pdev)
 	smmu_pmu_disable(&smmu_pmu->pmu);
 }
 
+static const struct of_device_id arm_smmu_pmu_match[] = {
+	{ .compatible = "arm,smmu-v3-pmcg" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, arm_smmu_pmu_match);
+
 static struct platform_driver smmu_pmu_driver = {
 	.driver = {
 		.name = "arm-smmu-v3-pmcg",
 		.suppress_bind_attrs = true,
+		.of_match_table = of_match_ptr(arm_smmu_pmu_match),
 	},
 	.probe = smmu_pmu_probe,
 	.remove = smmu_pmu_remove,
-- 
2.33.1

Re: [PATCH 2/2] perf/smmuv3: Add devicetree support

[John Garry]

On 16/11/2021 11:35, Jean-Philippe Brucker wrote:
> Add device-tree support to the SMMUv3 PMCG.  One small cosmetic change
> while factoring the option mask printout: don't display it when zero, it
> only contains one erratum at the moment.
> 
> Signed-off-by: Jay Chen <jkchen@linux.alibaba.com>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
>   drivers/perf/arm_smmuv3_pmu.c | 25 +++++++++++++++++++++++--
>   1 file changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c
> index 226348822ab3..958325ac103a 100644
> --- a/drivers/perf/arm_smmuv3_pmu.c
> +++ b/drivers/perf/arm_smmuv3_pmu.c
> @@ -47,6 +47,7 @@
>   #include <linux/kernel.h>
>   #include <linux/list.h>
>   #include <linux/msi.h>
> +#include <linux/of.h>
>   #include <linux/perf_event.h>
>   #include <linux/platform_device.h>
>   #include <linux/smp.h>
> @@ -750,8 +751,15 @@ static void smmu_pmu_get_acpi_options(struct smmu_pmu *smmu_pmu)
>   		smmu_pmu->options |= SMMU_PMCG_EVCNTR_RDONLY;
>   		break;
>   	}
> +}
> +
> +static void smmu_pmu_get_of_options(struct smmu_pmu *smmu_pmu)
> +{
> +	struct device_node *node = smmu_pmu->dev->of_node;
>   
> -	dev_notice(smmu_pmu->dev, "option mask 0x%x\n", smmu_pmu->options);
> +	if (of_device_is_compatible(node, "hisilicon,smmu-v3-pmcg-hip08"))

I don't think that this is necessary. We don't support DT for hip08, nor 
have any plans to. Incidentally, was this binding missing in your series?

Thanks,
John

> +		/* HiSilicon Erratum 162001800 */
> +		smmu_pmu->options |= SMMU_PMCG_EVCNTR_RDONLY;
>   }
>   
>   static int smmu_pmu_probe(struct platform_device *pdev)
> @@ -834,7 +842,13 @@ static int smmu_pmu_probe(struct platform_device *pdev)
>   		return -EINVAL;
>   	}
>   
> -	smmu_pmu_get_acpi_options(smmu_pmu);
> +	if (dev->of_node)
> +		smmu_pmu_get_of_options(smmu_pmu);
> +	else
> +		smmu_pmu_get_acpi_options(smmu_pmu);
> +
> +	if (smmu_pmu->options)
> +		dev_notice(dev, "option mask 0x%x\n", smmu_pmu->options);
>   
>   	/* Pick one CPU to be the preferred one to use */
>   	smmu_pmu->on_cpu = raw_smp_processor_id();
> @@ -884,10 +898,17 @@ static void smmu_pmu_shutdown(struct platform_device *pdev)
>   	smmu_pmu_disable(&smmu_pmu->pmu);
>   }
>   
> +static const struct of_device_id arm_smmu_pmu_match[] = {
> +	{ .compatible = "arm,smmu-v3-pmcg" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, arm_smmu_pmu_match);
> +
>   static struct platform_driver smmu_pmu_driver = {
>   	.driver = {
>   		.name = "arm-smmu-v3-pmcg",
>   		.suppress_bind_attrs = true,
> +		.of_match_table = of_match_ptr(arm_smmu_pmu_match),
>   	},
>   	.probe = smmu_pmu_probe,
>   	.remove = smmu_pmu_remove,
> 

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 2/2] perf/smmuv3: Add devicetree support

[John Garry]

On 16/11/2021 11:35, Jean-Philippe Brucker wrote:
> Add device-tree support to the SMMUv3 PMCG.  One small cosmetic change
> while factoring the option mask printout: don't display it when zero, it
> only contains one erratum at the moment.
> 
> Signed-off-by: Jay Chen <jkchen@linux.alibaba.com>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
>   drivers/perf/arm_smmuv3_pmu.c | 25 +++++++++++++++++++++++--
>   1 file changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c
> index 226348822ab3..958325ac103a 100644
> --- a/drivers/perf/arm_smmuv3_pmu.c
> +++ b/drivers/perf/arm_smmuv3_pmu.c
> @@ -47,6 +47,7 @@
>   #include <linux/kernel.h>
>   #include <linux/list.h>
>   #include <linux/msi.h>
> +#include <linux/of.h>
>   #include <linux/perf_event.h>
>   #include <linux/platform_device.h>
>   #include <linux/smp.h>
> @@ -750,8 +751,15 @@ static void smmu_pmu_get_acpi_options(struct smmu_pmu *smmu_pmu)
>   		smmu_pmu->options |= SMMU_PMCG_EVCNTR_RDONLY;
>   		break;
>   	}
> +}
> +
> +static void smmu_pmu_get_of_options(struct smmu_pmu *smmu_pmu)
> +{
> +	struct device_node *node = smmu_pmu->dev->of_node;
>   
> -	dev_notice(smmu_pmu->dev, "option mask 0x%x\n", smmu_pmu->options);
> +	if (of_device_is_compatible(node, "hisilicon,smmu-v3-pmcg-hip08"))

I don't think that this is necessary. We don't support DT for hip08, nor 
have any plans to. Incidentally, was this binding missing in your series?

Thanks,
John

> +		/* HiSilicon Erratum 162001800 */
> +		smmu_pmu->options |= SMMU_PMCG_EVCNTR_RDONLY;
>   }
>   
>   static int smmu_pmu_probe(struct platform_device *pdev)
> @@ -834,7 +842,13 @@ static int smmu_pmu_probe(struct platform_device *pdev)
>   		return -EINVAL;
>   	}
>   
> -	smmu_pmu_get_acpi_options(smmu_pmu);
> +	if (dev->of_node)
> +		smmu_pmu_get_of_options(smmu_pmu);
> +	else
> +		smmu_pmu_get_acpi_options(smmu_pmu);
> +
> +	if (smmu_pmu->options)
> +		dev_notice(dev, "option mask 0x%x\n", smmu_pmu->options);
>   
>   	/* Pick one CPU to be the preferred one to use */
>   	smmu_pmu->on_cpu = raw_smp_processor_id();
> @@ -884,10 +898,17 @@ static void smmu_pmu_shutdown(struct platform_device *pdev)
>   	smmu_pmu_disable(&smmu_pmu->pmu);
>   }
>   
> +static const struct of_device_id arm_smmu_pmu_match[] = {
> +	{ .compatible = "arm,smmu-v3-pmcg" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, arm_smmu_pmu_match);
> +
>   static struct platform_driver smmu_pmu_driver = {
>   	.driver = {
>   		.name = "arm-smmu-v3-pmcg",
>   		.suppress_bind_attrs = true,
> +		.of_match_table = of_match_ptr(arm_smmu_pmu_match),
>   	},
>   	.probe = smmu_pmu_probe,
>   	.remove = smmu_pmu_remove,
> 


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

Re: [PATCH 2/2] perf/smmuv3: Add devicetree support

[John Garry]

On 16/11/2021 11:35, Jean-Philippe Brucker wrote:
> Add device-tree support to the SMMUv3 PMCG.  One small cosmetic change
> while factoring the option mask printout: don't display it when zero, it
> only contains one erratum at the moment.
> 
> Signed-off-by: Jay Chen <jkchen@linux.alibaba.com>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
>   drivers/perf/arm_smmuv3_pmu.c | 25 +++++++++++++++++++++++--
>   1 file changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c
> index 226348822ab3..958325ac103a 100644
> --- a/drivers/perf/arm_smmuv3_pmu.c
> +++ b/drivers/perf/arm_smmuv3_pmu.c
> @@ -47,6 +47,7 @@
>   #include <linux/kernel.h>
>   #include <linux/list.h>
>   #include <linux/msi.h>
> +#include <linux/of.h>
>   #include <linux/perf_event.h>
>   #include <linux/platform_device.h>
>   #include <linux/smp.h>
> @@ -750,8 +751,15 @@ static void smmu_pmu_get_acpi_options(struct smmu_pmu *smmu_pmu)
>   		smmu_pmu->options |= SMMU_PMCG_EVCNTR_RDONLY;
>   		break;
>   	}
> +}
> +
> +static void smmu_pmu_get_of_options(struct smmu_pmu *smmu_pmu)
> +{
> +	struct device_node *node = smmu_pmu->dev->of_node;
>   
> -	dev_notice(smmu_pmu->dev, "option mask 0x%x\n", smmu_pmu->options);
> +	if (of_device_is_compatible(node, "hisilicon,smmu-v3-pmcg-hip08"))

I don't think that this is necessary. We don't support DT for hip08, nor 
have any plans to. Incidentally, was this binding missing in your series?

Thanks,
John

> +		/* HiSilicon Erratum 162001800 */
> +		smmu_pmu->options |= SMMU_PMCG_EVCNTR_RDONLY;
>   }
>   
>   static int smmu_pmu_probe(struct platform_device *pdev)
> @@ -834,7 +842,13 @@ static int smmu_pmu_probe(struct platform_device *pdev)
>   		return -EINVAL;
>   	}
>   
> -	smmu_pmu_get_acpi_options(smmu_pmu);
> +	if (dev->of_node)
> +		smmu_pmu_get_of_options(smmu_pmu);
> +	else
> +		smmu_pmu_get_acpi_options(smmu_pmu);
> +
> +	if (smmu_pmu->options)
> +		dev_notice(dev, "option mask 0x%x\n", smmu_pmu->options);
>   
>   	/* Pick one CPU to be the preferred one to use */
>   	smmu_pmu->on_cpu = raw_smp_processor_id();
> @@ -884,10 +898,17 @@ static void smmu_pmu_shutdown(struct platform_device *pdev)
>   	smmu_pmu_disable(&smmu_pmu->pmu);
>   }
>   
> +static const struct of_device_id arm_smmu_pmu_match[] = {
> +	{ .compatible = "arm,smmu-v3-pmcg" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, arm_smmu_pmu_match);
> +
>   static struct platform_driver smmu_pmu_driver = {
>   	.driver = {
>   		.name = "arm-smmu-v3-pmcg",
>   		.suppress_bind_attrs = true,
> +		.of_match_table = of_match_ptr(arm_smmu_pmu_match),
>   	},
>   	.probe = smmu_pmu_probe,
>   	.remove = smmu_pmu_remove,
> 

Re: [PATCH 2/2] perf/smmuv3: Add devicetree support

[Jean-Philippe Brucker]

On Tue, Nov 16, 2021 at 12:06:36PM +0000, John Garry wrote:
> On 16/11/2021 11:35, Jean-Philippe Brucker wrote:
> > Add device-tree support to the SMMUv3 PMCG.  One small cosmetic change
> > while factoring the option mask printout: don't display it when zero, it
> > only contains one erratum at the moment.
> > 
> > Signed-off-by: Jay Chen <jkchen@linux.alibaba.com>
> > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > ---
> >   drivers/perf/arm_smmuv3_pmu.c | 25 +++++++++++++++++++++++--
> >   1 file changed, 23 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c
> > index 226348822ab3..958325ac103a 100644
> > --- a/drivers/perf/arm_smmuv3_pmu.c
> > +++ b/drivers/perf/arm_smmuv3_pmu.c
> > @@ -47,6 +47,7 @@
> >   #include <linux/kernel.h>
> >   #include <linux/list.h>
> >   #include <linux/msi.h>
> > +#include <linux/of.h>
> >   #include <linux/perf_event.h>
> >   #include <linux/platform_device.h>
> >   #include <linux/smp.h>
> > @@ -750,8 +751,15 @@ static void smmu_pmu_get_acpi_options(struct smmu_pmu *smmu_pmu)
> >   		smmu_pmu->options |= SMMU_PMCG_EVCNTR_RDONLY;
> >   		break;
> >   	}
> > +}
> > +
> > +static void smmu_pmu_get_of_options(struct smmu_pmu *smmu_pmu)
> > +{
> > +	struct device_node *node = smmu_pmu->dev->of_node;
> > -	dev_notice(smmu_pmu->dev, "option mask 0x%x\n", smmu_pmu->options);
> > +	if (of_device_is_compatible(node, "hisilicon,smmu-v3-pmcg-hip08"))
> 
> I don't think that this is necessary. We don't support DT for hip08, nor
> have any plans to. Incidentally, was this binding missing in your series?

Ok I'll drop this (and the compatible value from patch 1)

Thanks,
Jean

Re: [PATCH 2/2] perf/smmuv3: Add devicetree support

[Jean-Philippe Brucker]

On Tue, Nov 16, 2021 at 12:06:36PM +0000, John Garry wrote:
> On 16/11/2021 11:35, Jean-Philippe Brucker wrote:
> > Add device-tree support to the SMMUv3 PMCG.  One small cosmetic change
> > while factoring the option mask printout: don't display it when zero, it
> > only contains one erratum at the moment.
> > 
> > Signed-off-by: Jay Chen <jkchen@linux.alibaba.com>
> > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > ---
> >   drivers/perf/arm_smmuv3_pmu.c | 25 +++++++++++++++++++++++--
> >   1 file changed, 23 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c
> > index 226348822ab3..958325ac103a 100644
> > --- a/drivers/perf/arm_smmuv3_pmu.c
> > +++ b/drivers/perf/arm_smmuv3_pmu.c
> > @@ -47,6 +47,7 @@
> >   #include <linux/kernel.h>
> >   #include <linux/list.h>
> >   #include <linux/msi.h>
> > +#include <linux/of.h>
> >   #include <linux/perf_event.h>
> >   #include <linux/platform_device.h>
> >   #include <linux/smp.h>
> > @@ -750,8 +751,15 @@ static void smmu_pmu_get_acpi_options(struct smmu_pmu *smmu_pmu)
> >   		smmu_pmu->options |= SMMU_PMCG_EVCNTR_RDONLY;
> >   		break;
> >   	}
> > +}
> > +
> > +static void smmu_pmu_get_of_options(struct smmu_pmu *smmu_pmu)
> > +{
> > +	struct device_node *node = smmu_pmu->dev->of_node;
> > -	dev_notice(smmu_pmu->dev, "option mask 0x%x\n", smmu_pmu->options);
> > +	if (of_device_is_compatible(node, "hisilicon,smmu-v3-pmcg-hip08"))
> 
> I don't think that this is necessary. We don't support DT for hip08, nor
> have any plans to. Incidentally, was this binding missing in your series?

Ok I'll drop this (and the compatible value from patch 1)

Thanks,
Jean


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

Re: [PATCH 2/2] perf/smmuv3: Add devicetree support

[Jean-Philippe Brucker]

On Tue, Nov 16, 2021 at 12:06:36PM +0000, John Garry wrote:
> On 16/11/2021 11:35, Jean-Philippe Brucker wrote:
> > Add device-tree support to the SMMUv3 PMCG.  One small cosmetic change
> > while factoring the option mask printout: don't display it when zero, it
> > only contains one erratum at the moment.
> > 
> > Signed-off-by: Jay Chen <jkchen@linux.alibaba.com>
> > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > ---
> >   drivers/perf/arm_smmuv3_pmu.c | 25 +++++++++++++++++++++++--
> >   1 file changed, 23 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c
> > index 226348822ab3..958325ac103a 100644
> > --- a/drivers/perf/arm_smmuv3_pmu.c
> > +++ b/drivers/perf/arm_smmuv3_pmu.c
> > @@ -47,6 +47,7 @@
> >   #include <linux/kernel.h>
> >   #include <linux/list.h>
> >   #include <linux/msi.h>
> > +#include <linux/of.h>
> >   #include <linux/perf_event.h>
> >   #include <linux/platform_device.h>
> >   #include <linux/smp.h>
> > @@ -750,8 +751,15 @@ static void smmu_pmu_get_acpi_options(struct smmu_pmu *smmu_pmu)
> >   		smmu_pmu->options |= SMMU_PMCG_EVCNTR_RDONLY;
> >   		break;
> >   	}
> > +}
> > +
> > +static void smmu_pmu_get_of_options(struct smmu_pmu *smmu_pmu)
> > +{
> > +	struct device_node *node = smmu_pmu->dev->of_node;
> > -	dev_notice(smmu_pmu->dev, "option mask 0x%x\n", smmu_pmu->options);
> > +	if (of_device_is_compatible(node, "hisilicon,smmu-v3-pmcg-hip08"))
> 
> I don't think that this is necessary. We don't support DT for hip08, nor
> have any plans to. Incidentally, was this binding missing in your series?

Ok I'll drop this (and the compatible value from patch 1)

Thanks,
Jean

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

[Robin Murphy]

On 2021-11-16 11:35, Jean-Philippe Brucker wrote:
> 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 :)

> 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.

Robin.

[0] https://gitlab.arm.com/linux-arm/linux-rm/-/commits/smmu-pmcg

> The series doesn't deal with this because for testing I use a software
> model which only implements architected events. I do not include DTS
> change for that platform because enabling PMCGs requires an additional
> model option. See my branch smmu/pmu-dt [3] for details.
> 
> [1] https://lore.kernel.org/all/20200707150114.GC159413@myrica/
> [2] https://developer.arm.com/documentation/100310/0202/Functional-description/Operation/Performance-Monitoring-Unit
> [3] https://jpbrucker.net/git/linux/log/?h=smmu/pmu-dt
> 
> Jean-Philippe Brucker (2):
>    dt-bindings: Add Arm SMMUv3 PMCG binding
>    perf/smmuv3: Add devicetree support
> 
>   .../bindings/iommu/arm,smmu-v3-pmcg.yaml      | 67 +++++++++++++++++++
>   drivers/perf/arm_smmuv3_pmu.c                 | 25 ++++++-
>   2 files changed, 90 insertions(+), 2 deletions(-)
>   create mode 100644 Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.yaml
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

[Robin Murphy]

On 2021-11-16 11:35, Jean-Philippe Brucker wrote:
> 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 :)

> 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.

Robin.

[0] https://gitlab.arm.com/linux-arm/linux-rm/-/commits/smmu-pmcg

> The series doesn't deal with this because for testing I use a software
> model which only implements architected events. I do not include DTS
> change for that platform because enabling PMCGs requires an additional
> model option. See my branch smmu/pmu-dt [3] for details.
> 
> [1] https://lore.kernel.org/all/20200707150114.GC159413@myrica/
> [2] https://developer.arm.com/documentation/100310/0202/Functional-description/Operation/Performance-Monitoring-Unit
> [3] https://jpbrucker.net/git/linux/log/?h=smmu/pmu-dt
> 
> Jean-Philippe Brucker (2):
>    dt-bindings: Add Arm SMMUv3 PMCG binding
>    perf/smmuv3: Add devicetree support
> 
>   .../bindings/iommu/arm,smmu-v3-pmcg.yaml      | 67 +++++++++++++++++++
>   drivers/perf/arm_smmuv3_pmu.c                 | 25 ++++++-
>   2 files changed, 90 insertions(+), 2 deletions(-)
>   create mode 100644 Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.yaml
> 

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

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

[Robin Murphy]

On 2021-11-16 11:35, Jean-Philippe Brucker wrote:
> 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 :)

> 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.

Robin.

[0] https://gitlab.arm.com/linux-arm/linux-rm/-/commits/smmu-pmcg

> The series doesn't deal with this because for testing I use a software
> model which only implements architected events. I do not include DTS
> change for that platform because enabling PMCGs requires an additional
> model option. See my branch smmu/pmu-dt [3] for details.
> 
> [1] https://lore.kernel.org/all/20200707150114.GC159413@myrica/
> [2] https://developer.arm.com/documentation/100310/0202/Functional-description/Operation/Performance-Monitoring-Unit
> [3] https://jpbrucker.net/git/linux/log/?h=smmu/pmu-dt
> 
> Jean-Philippe Brucker (2):
>    dt-bindings: Add Arm SMMUv3 PMCG binding
>    perf/smmuv3: Add devicetree support
> 
>   .../bindings/iommu/arm,smmu-v3-pmcg.yaml      | 67 +++++++++++++++++++
>   drivers/perf/arm_smmuv3_pmu.c                 | 25 ++++++-
>   2 files changed, 90 insertions(+), 2 deletions(-)
>   create mode 100644 Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.yaml
> 

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

[Jean-Philippe Brucker]

On Tue, Nov 16, 2021 at 12:02:47PM +0000, Robin Murphy wrote:
> On 2021-11-16 11:35, Jean-Philippe Brucker wrote:
> > 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

> > 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.

Thanks,
Jean

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

[Jean-Philippe Brucker]

On Tue, Nov 16, 2021 at 12:02:47PM +0000, Robin Murphy wrote:
> On 2021-11-16 11:35, Jean-Philippe Brucker wrote:
> > 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

> > 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.

Thanks,
Jean


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

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

[Jean-Philippe Brucker]

On Tue, Nov 16, 2021 at 12:02:47PM +0000, Robin Murphy wrote:
> On 2021-11-16 11:35, Jean-Philippe Brucker wrote:
> > 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

> > 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.

Thanks,
Jean

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

[Robin Murphy]

On 2021-11-16 15:42, Jean-Philippe Brucker wrote:
> On Tue, Nov 16, 2021 at 12:02:47PM +0000, Robin Murphy wrote:
>> On 2021-11-16 11:35, Jean-Philippe Brucker wrote:
>>> 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...

>>> 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.

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

[Robin Murphy]

On 2021-11-16 15:42, Jean-Philippe Brucker wrote:
> On Tue, Nov 16, 2021 at 12:02:47PM +0000, Robin Murphy wrote:
>> On 2021-11-16 11:35, Jean-Philippe Brucker wrote:
>>> 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...

>>> 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.

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

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

[Robin Murphy]

On 2021-11-16 15:42, Jean-Philippe Brucker wrote:
> On Tue, Nov 16, 2021 at 12:02:47PM +0000, Robin Murphy wrote:
>> On 2021-11-16 11:35, Jean-Philippe Brucker wrote:
>>> 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...

>>> 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.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

[Jean-Philippe Brucker]

On Tue, Nov 16, 2021 at 05:00:14PM +0000, Robin Murphy wrote:
> On 2021-11-16 15:42, Jean-Philippe Brucker wrote:
> > On Tue, Nov 16, 2021 at 12:02:47PM +0000, Robin Murphy wrote:
> > > On 2021-11-16 11:35, Jean-Philippe Brucker wrote:
> > > > 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...

Sure, that works for me, I'll send a v2 this week or so

Thanks,
Jean

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

[Jean-Philippe Brucker]

On Tue, Nov 16, 2021 at 05:00:14PM +0000, Robin Murphy wrote:
> On 2021-11-16 15:42, Jean-Philippe Brucker wrote:
> > On Tue, Nov 16, 2021 at 12:02:47PM +0000, Robin Murphy wrote:
> > > On 2021-11-16 11:35, Jean-Philippe Brucker wrote:
> > > > 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...

Sure, that works for me, I'll send a v2 this week or so

Thanks,
Jean

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

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

[Jean-Philippe Brucker]

On Tue, Nov 16, 2021 at 05:00:14PM +0000, Robin Murphy wrote:
> On 2021-11-16 15:42, Jean-Philippe Brucker wrote:
> > On Tue, Nov 16, 2021 at 12:02:47PM +0000, Robin Murphy wrote:
> > > On 2021-11-16 11:35, Jean-Philippe Brucker wrote:
> > > > 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...

Sure, that works for me, I'll send a v2 this week or so

Thanks,
Jean
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu