[PATCH 4/4] dt-binding:perf: Add Amlogic DDR PMU
Jiucheng Xu
jiucheng.xu at amlogic.com
Thu Jul 14 02:13:06 PDT 2022
On 7/12/2022 8:54 PM, Robin Murphy wrote:
> [ EXTERNAL EMAIL ]
>
> On 2022-07-12 07:36, Jiucheng Xu wrote:
>> Add binding documentation for the Amlogic G12 series DDR
>> performance monitor unit.
>>
>> Signed-off-by: Jiucheng Xu <jiucheng.xu at amlogic.com>
>> ---
>> .../devicetree/bindings/perf/aml-ddr-pmu.yaml | 51 +++++++++++++++++++
>> MAINTAINERS | 1 +
>> 2 files changed, 52 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/perf/aml-ddr-pmu.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/perf/aml-ddr-pmu.yaml
>> b/Documentation/devicetree/bindings/perf/aml-ddr-pmu.yaml
>> new file mode 100644
>> index 000000000000..c586b4ab4009
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/perf/aml-ddr-pmu.yaml
>> @@ -0,0 +1,51 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/perf/aml-ddr-pmu.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Amlogic G12 DDR performance monitor
>> +
>> +maintainers:
>> + - Jiucheng Xu <jiucheng.xu at amlogic.com>
>> +
>> +properties:
>> + compatible:
>> + oneOf:
>> + - enum:
>> + - aml,g12-ddr-pmu
>> + - items:
>> + - enum:
>> + - aml,g12-ddr-pmu
>> + - const: aml,g12-ddr-pmu
>
> Judging by what the driver actually implements, this should probably be:
>
> compatible:
> items:
> - enum:
> - amlogic,g12a-ddr-pmu
> - amlogic,g12b-ddr-pmu
> - amlogic,sm1-ddr-pmu
> - const: amlogic,g12-ddr-pmu
>
> There doesn't seem much point in allowing only the common compatible
> without a SoC-specific identifier. Note also that "aml," is not the
> documented vendor prefix.
Okay, I finally know what you mean.
>
>> +
>> + reg:
>> + maxItems: 2
>> +
>> + interrupts:
>> + maxItems: 1
>> +
>> +required:
>> + - compatible
>> + - model
>
> Remove this, and use the compatible strings properly as above.
Okay. I will make the change.
>
>> + - dmc_nr
>> + - chann_nr
>
> I suspect those could probably be inferred from the correct compatible
> string, but if not (i.e. within one SoC you have multiple PMUs
> supporting the same events but with different numbers of usable
> channels), then document what exactly they mean.
>
Yes, as you mentioned, these could be inferred from the compatible
string. I will make the change.
>> + - reg
>> + - interrupts
>> + - interrupt-names
>
> As mentioned in the driver review, if you really want to use a named
> interrupt (which should usually be unnecessary when there's only one),
> it has to be a defined name. DT is not a mechanism for overriding what
> Linux shows in /proc/interrupts.
>
> Thanks,
> Robin.
>
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + ddr_pmu: ddr_pmu {
>> + compatible = "amlogic,g12-ddr-pmu";
>> + model = "g12a";
>> + dmc_nr = <1>;
>> + chann_nr = <4>;
>> + reg = <0x0 0xff638000 0x0 0x100
>> + 0x0 0xff638c00 0x0 0x100>;
>> + interrupts = <GIC_SPI 52 IRQ_TYPE_EDGE_RISING>;
>> + interrupt-names = "ddr_pmu";
>> + };
>> + };
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index fd2a56a339b4..ac0a1df4622d 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -1055,6 +1055,7 @@ M: Jiucheng Xu <jiucheng.xu at amlogic.com>
>> S: Supported
>> W: http://www.amlogic.com
>> F: Documentation/admin-guide/perf/aml-ddr-pmu.rst
>> +F: Documentation/devicetree/bindings/perf/aml-ddr-pmu.yaml
>> F: drivers/perf/amlogic/
>> F: include/soc/amlogic/
>
More information about the linux-amlogic
mailing list