Re: [RFC v4 01/10] dt-bindings: pwm: Document Synopsys DesignWare snps,pwm-dw-apb-timers-pwm2

[Ben Dooks]

On 17/08/2022 06:58, Krzysztof Kozlowski wrote:
> On 17/08/2022 00:14, Ben Dooks wrote:
> > Add documentation for the bindings for Synopsys' DesignWare PWM block
> > as we will be adding DT/platform support to the Linux driver soon.
> >
> > Signed-off-by: Ben Dooks <ben.dooks@xxxxxxxxxx>
> > ---
> > v4:
> >   - fixed typos, added reg
> > v3:
> >   - add description and example
> >   - merge the snps,pwm-number into this patch
> >   - rename snps,pwm to snps,dw-apb-timers-pwm2
> > v2:
> >   - fix #pwm-cells to be 3
> >   - fix indentation and ordering issues
> > ---
> >   .../bindings/pwm/snps,dw-apb-timers-pwm2.yaml | 69 +++++++++++++++++++
> >   1 file changed, 69 insertions(+)
> >   create mode 100644 Documentation/devicetree/bindings/pwm/snps,dw-apb-timers-pwm2.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/pwm/snps,dw-apb-timers-pwm2.yaml b/Documentation/devicetree/bindings/pwm/snps,dw-apb-timers-pwm2.yaml
> > new file mode 100644
> > index 000000000000..e7feae6d4404
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pwm/snps,dw-apb-timers-pwm2.yaml
> > @@ -0,0 +1,69 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +# Copyright (C) 2022 SiFive, Inc.
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pwm/snps,dw-apb-timers-pwm2.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Synopsys DW-APB timers PWM controller
> > +
> > +maintainers:
> > +  - Ben Dooks <ben.dooks@xxxxxxxxxx>
> > +
> > +description:
> > +  This describes the DesignWare APB timers module when used in the PWM
> > +  mode. The IP core can be generated with various options which can
> > +  control the functionality, the number of PWMs available and other
> > +  internal controls the designer requires.
> > +
> > +  The IP block has a version register so this can be used for detection
> > +  instead of having to encode the IP version number in the device tree
> > +  comaptible.
> > +
> > +allOf:
> > +  - $ref: pwm.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    const: snps,dw-apb-timers-pwm2
> > +
> > +  "#pwm-cells":
> > +    const: 3
> > +
> > +  clocks:
> > +    items:
> > +      - description: Interface bus clock
> > +      - description: PWM reference clock
> > +
> > +  clock-names:
> > +    items:
> > +      - const: bus
> > +      - const: timer
> > +
> > +  snps,pwm-number:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: The number of PWM channels configured for this instance
> > +    enum: [1, 2, 3, 4, 5, 6, 7, 8]
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +required:
> > +  - "#pwm-cells"
> > +  - compatible
> > +  - reg
>
> Keep the same order as list of properties.

ok, will fix.

> > +  - clocks
> > +  - clock-names
> > +
> > +additionalProperties: false
> > +
> > +
>
> Just one blank line.
>
> > +examples:
> > +  - |
> > +    pwm: pwm@180000 {
> > +      #pwm-cells = <3>;
> > +      compatible = "snps,dw-apb-timers-pwm2";
> > +      reg = <0x180000 0x200>;
>
> The convention of DTS is: compatible, then reg, then rest of properties.
>
> > +      clocks = <&bus &timer>;
>
> You put here one item, not two. This has to be <&bus>, <&timer>
>
> > +      clock-names = "bus", "timer";
> > +    };

Argh, thanks, I completely missed this as our platform only has
one clock provider for this (the bus and timer clocks are the
same)


> Best regards,
> Krzysztof

Thanks for the review.

I guess this is now too late for 6.0-rc ?