[v5,3/4] dt-binding: perf: Add Amlogic DDR PMU

Commit Message

Jiucheng Xu Aug. 17, 2022, 11:34 a.m. UTC

Add binding documentation for the Amlogic G12 series DDR
performance monitor unit.

Signed-off-by: Jiucheng Xu <[e-mail]>
---
Changes v4 -> v5:
  - Remove "items" in compatible since have only one item
  - Condense description of reg
  - Rename node
  - Split one reg into two reg items.
  - Binding go first
 
Changes v3 -> v4:
  - Fix "$id: relative path/filename doesn't match actual path or
    filename" warning

Changes v2 -> v3:
  - Remove oneOf
  - Add descriptions
  - Fix compiling warning

Changes v1 -> v2:
  - Rename file, from aml_ddr_pmu.yaml to amlogic,g12_ddr_pmu.yaml
  - Delete "model", "dmc_nr", "chann_nr" new properties
  - Fix compiling error
---
 .../bindings/perf/amlogic,g12-ddr-pmu.yaml    | 55 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 56 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/perf/amlogic,g12-ddr-pmu.yaml

Comments

Krzysztof Kozlowski Aug. 18, 2022, 8:25 a.m. UTC | #1

On 17/08/2022 14:34, Jiucheng Xu wrote:
> Add binding documentation for the Amlogic G12 series DDR
> performance monitor unit.
> 
> Signed-off-by: Jiucheng Xu <[e-mail]>

(...)

> +
> +  interrupts:
> +    items:
> +      - description: The IRQ of the inside timer timeout.
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    pmu {
> +        #address-cells=<2>;
> +        #size-cells=<2>;
> +
> +        pmu@ff638000 {
> +

No need for blank line.

> +            compatible = "amlogic,g12a-ddr-pmu";
> +            reg = <0x0 0xff638000 0x0 0x100>,
> +                  <0x0 0xff638c00 0x0 0x100>;
> +            interrupts = <GIC_SPI 52 IRQ_TYPE_EDGE_RISING>;
> +        };
> +    };

With above fixed:

Reviewed-by: Krzysztof Kozlowski <[e-mail]>


Best regards,
Krzysztof

Jiucheng Xu Aug. 19, 2022, 1:44 p.m. UTC | #2

On 2022/8/18 16:25, Krzysztof Kozlowski wrote:
> [ EXTERNAL EMAIL ]
>
> On 17/08/2022 14:34, Jiucheng Xu wrote:
>> Add binding documentation for the Amlogic G12 series DDR
>> performance monitor unit.
>>
>> Signed-off-by: Jiucheng Xu <[e-mail]>
> (...)
>
>> +
>> +  interrupts:
>> +    items:
>> +      - description: The IRQ of the inside timer timeout.
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    pmu {
>> +        #address-cells=<2>;
>> +        #size-cells=<2>;
>> +
>> +        pmu@ff638000 {
>> +
> No need for blank line.
>
>> +            compatible = "amlogic,g12a-ddr-pmu";
>> +            reg = <0x0 0xff638000 0x0 0x100>,
>> +                  <0x0 0xff638c00 0x0 0x100>;
>> +            interrupts = <GIC_SPI 52 IRQ_TYPE_EDGE_RISING>;
>> +        };
>> +    };
> With above fixed:
>
> Reviewed-by: Krzysztof Kozlowski <[e-mail]>

Thanks for your valuable comments and I learn a lot from them.

One more question, do you mean after modify the above two places

I could resubmit V6 as the final patch which include your reviewed tag?

>
>
> Best regards,
> Krzysztof
>

Krzysztof Kozlowski Aug. 19, 2022, 1:56 p.m. UTC | #3

On 19/08/2022 16:44, Jiucheng Xu wrote:
> 
> On 2022/8/18 16:25, Krzysztof Kozlowski wrote:
>> [ EXTERNAL EMAIL ]
>>
>> On 17/08/2022 14:34, Jiucheng Xu wrote:
>>> Add binding documentation for the Amlogic G12 series DDR
>>> performance monitor unit.
>>>
>>> Signed-off-by: Jiucheng Xu <[e-mail]>
>> (...)
>>
>>> +
>>> +  interrupts:
>>> +    items:
>>> +      - description: The IRQ of the inside timer timeout.
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +  - interrupts
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>>> +    pmu {
>>> +        #address-cells=<2>;
>>> +        #size-cells=<2>;
>>> +
>>> +        pmu@ff638000 {
>>> +
>> No need for blank line.
>>
>>> +            compatible = "amlogic,g12a-ddr-pmu";
>>> +            reg = <0x0 0xff638000 0x0 0x100>,
>>> +                  <0x0 0xff638c00 0x0 0x100>;
>>> +            interrupts = <GIC_SPI 52 IRQ_TYPE_EDGE_RISING>;
>>> +        };
>>> +    };
>> With above fixed:
>>
>> Reviewed-by: Krzysztof Kozlowski <[e-mail]>
> 
> Thanks for your valuable comments and I learn a lot from them.
> 
> One more question, do you mean after modify the above two places
> 
> I could resubmit V6 as the final patch which include your reviewed tag?

Yes.

https://elixir.bootlin.com/linux/v5.17/source/Documentation/process/submitting-patches.rst#L540


Best regards,
Krzysztof

Patch

diff --git a/Documentation/devicetree/bindings/perf/amlogic,g12-ddr-pmu.yaml b/Documentation/devicetree/bindings/perf/amlogic,g12-ddr-pmu.yaml
new file mode 100644
index 000000000000..e662a60fac4e
--- /dev/null
+++ b/Documentation/devicetree/bindings/perf/amlogic,g12-ddr-pmu.yaml
@@ -0,0 +1,55 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/perf/amlogic,g12-ddr-pmu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Amlogic G12 DDR performance monitor
+
+maintainers:
+  - Jiucheng Xu <[e-mail]>
+
+description: |
+  Amlogic G12 series SoC integrate DDR bandwidth monitor.
+  A timer is inside and can generate interrupt when timeout.
+  The bandwidth is counted in the timer ISR. Different platform
+  has different subset of event format attribute.
+
+properties:
+  compatible:
+    enum:
+      - amlogic,g12a-ddr-pmu
+      - amlogic,g12b-ddr-pmu
+      - amlogic,sm1-ddr-pmu
+
+  reg:
+    items:
+      - description: DMC bandwidth register space.
+      - description: DMC PLL register space.
+
+  interrupts:
+    items:
+      - description: The IRQ of the inside timer timeout.
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    pmu {
+        #address-cells=<2>;
+        #size-cells=<2>;
+
+        pmu@ff638000 {
+
+            compatible = "amlogic,g12a-ddr-pmu";
+            reg = <0x0 0xff638000 0x0 0x100>,
+                  <0x0 0xff638c00 0x0 0x100>;
+            interrupts = <GIC_SPI 52 IRQ_TYPE_EDGE_RISING>;
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 8ee68e699e6d..67c2c9e8c4ea 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1055,6 +1055,7 @@  M:	Jiucheng Xu <[e-mail]>
 S:	Supported
 W:	http://www.amlogic.com
 F:	Documentation/admin-guide/perf/meson-ddr-pmu.rst
+F:	Documentation/devicetree/bindings/perf/amlogic,g12-ddr-pmu.yaml
 F:	drivers/perf/amlogic/
 F:	include/soc/amlogic/