Re: [PATCH 4/4] dt-binding:perf: Add Amlogic DDR PMU

[Jiucheng Xu]

Thanks for your time ^^.

On 7/12/2022 3:15 PM, Krzysztof Kozlowski wrote:
> [ EXTERNAL EMAIL ]
>
> On 12/07/2022 08:36, Jiucheng Xu wrote:
> > Add binding documentation for the Amlogic G12 series DDR
> > performance monitor unit.
> You need to fix subject - use a prefix matching subsystem. Space after
> each ':'.
I will make the change.
> > Signed-off-by: Jiucheng Xu <jiucheng.xu@xxxxxxxxxxx>
> > ---
> >   .../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
> Filename: aml,g12-ddr-pmu.yaml
I will make the change.
> > @@ -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@xxxxxxxxxxx>
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - enum:
> > +          - aml,g12-ddr-pmu
> > +      - items:
> > +          - enum:
> > +              - aml,g12-ddr-pmu
> > +          - const: aml,g12-ddr-pmu
> This does not make any sense. Why do you use two compatibles
> "aml,g12-ddr-pmu", "aml,g12-ddr-pmu" after each other?
Sorry, I think I have a wrong understanding. I will make the change.
> > +
> > +  reg:
> > +    maxItems: 2
> You need to describe the items.
I will make the change.
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +required:
> > +  - compatible
> > +  - model
> > +  - dmc_nr
> > +  - chann_nr
> How these ended up here?
> No underscores.
I will make the change.
> > +  - reg
> > +  - interrupts
> > +  - interrupt-names
> Also something new. No.
I will make the change.
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +          ddr_pmu: ddr_pmu {
> Wrong indentation. 4 spaces for DTS example.
>
> Generic node name, so "pmu", no underscores in node names.
Okay, I will make the change.
> > +                  compatible = "amlogic,g12-ddr-pmu";
> > +                  model = "g12a";
> > +                  dmc_nr = <1>;
> > +                  chann_nr = <4>;
> This does not pass the test. Please do not send untested bindings.

Sorry,  due to some problems, I got wrong patch sent. I will make the 
change.


Thanks & Best Regards,

Jiucheng

> Best regards,
> Krzysztof