Re: [PATCH V2 4/6] dt-bindings: pwm: sprd: Convert to YAML

[Rob Herring]

On Thu, Jan 25, 2024 at 05:31:48PM +0000, Conor Dooley wrote:
> Hey,
> 
> On Thu, Jan 25, 2024 at 10:55:31AM +0800, Wenhua Lin wrote:
> > Convert Spreadtrum PWM controller bindings to DT schema.
> > 
> > Signed-off-by: Wenhua Lin <Wenhua.Lin@xxxxxxxxxx>
> > ---
> >  .../devicetree/bindings/pwm/pwm-sprd.txt      | 40 --------
> >  .../devicetree/bindings/pwm/pwm-sprd.yaml     | 93 +++++++++++++++++++
> >  2 files changed, 93 insertions(+), 40 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/pwm/pwm-sprd.txt
> >  create mode 100644 Documentation/devicetree/bindings/pwm/pwm-sprd.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/pwm/pwm-sprd.txt b/Documentation/devicetree/bindings/pwm/pwm-sprd.txt
> > deleted file mode 100644
> > index 87b206fd0618..000000000000
> > --- a/Documentation/devicetree/bindings/pwm/pwm-sprd.txt
> > +++ /dev/null
> > @@ -1,40 +0,0 @@
> > -Spreadtrum PWM controller
> > -
> > -Spreadtrum SoCs PWM controller provides 4 PWM channels.
> > -
> > -Required properties:
> > -- compatible : Should be "sprd,ums512-pwm".
> > -- reg: Physical base address and length of the controller's registers.
> > -- clocks: The phandle and specifier referencing the controller's clocks.
> > -- clock-names: Should contain following entries:
> > -  "pwmn": used to derive the functional clock for PWM channel n (n range: 0 ~ 3).
> > -  "enablen": for PWM channel n enable clock (n range: 0 ~ 3).
> > -- #pwm-cells: Should be 2. See pwm.yaml in this directory for a description of
> > -  the cells format.
> > -
> > -Optional properties:
> > -- assigned-clocks: Reference to the PWM clock entries.
> > -- assigned-clock-parents: The phandle of the parent clock of PWM clock.
> > -
> > -Example:
> > -	pwms: pwm@32260000 {
> > -		compatible = "sprd,ums512-pwm";
> > -		reg = <0 0x32260000 0 0x10000>;
> > -		clock-names = "pwm0", "enable0",
> > -			"pwm1", "enable1",
> > -			"pwm2", "enable2",
> > -			"pwm3", "enable3";
> > -		clocks = <&aon_clk CLK_PWM0>, <&aonapb_gate CLK_PWM0_EB>,
> > -		       <&aon_clk CLK_PWM1>, <&aonapb_gate CLK_PWM1_EB>,
> > -		       <&aon_clk CLK_PWM2>, <&aonapb_gate CLK_PWM2_EB>,
> > -		       <&aon_clk CLK_PWM3>, <&aonapb_gate CLK_PWM3_EB>;
> > -		assigned-clocks = <&aon_clk CLK_PWM0>,
> > -			<&aon_clk CLK_PWM1>,
> > -			<&aon_clk CLK_PWM2>,
> > -			<&aon_clk CLK_PWM3>;
> > -		assigned-clock-parents = <&ext_26m>,
> > -			<&ext_26m>,
> > -			<&ext_26m>,
> > -			<&ext_26m>;
> > -		#pwm-cells = <2>;
> > -	};
> > diff --git a/Documentation/devicetree/bindings/pwm/pwm-sprd.yaml b/Documentation/devicetree/bindings/pwm/pwm-sprd.yaml
> > new file mode 100644
> > index 000000000000..81c5fd688c3c
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pwm/pwm-sprd.yaml
> > @@ -0,0 +1,93 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +# Copyright 2023 Unisoc Inc.
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pwm/pwm-sprd.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Spreadtrum PWM controller
> > +
> > +maintainers:
> > +  - Orson Zhai <orsonzhai@xxxxxxxxx>
> > +  - Baolin Wang <baolin.wang7@xxxxxxxxx>
> > +  - Chunyan Zhang <zhang.lyra@xxxxxxxxx>
> > +
> > +description: |
> 
> The | here is not need, you have no formatting to preserve.
> 
> > +  Spreadtrum SoCs PWM controller provides 4 PWM channels.
> > +
> > +allOf:
> > +  - $ref: pwm.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +          - sprd,ums512-pwm
> 
> this is just
> 
> compatible:
>   const: sprd,ums512-pwm
> 
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    minItems: 8
> > +    maxItems: 8
> > +
> > +  clock-names:
> > +    items:
> > +      - const: pwm0
> > +      - const: enable0
> > +      - const: pwm1
> > +      - const: enable1
> > +      - const: pwm2
> > +      - const: enable2
> > +      - const: pwm3
> > +      - const: enable3
> 
> > +    description: |
> > +      Should contain following entries:
> > +      "pwmn": used to derive the functional clock for PWM channel n (n range: 0 ~ 3).
> > +      "enablen": for PWM channel n enable clock (n range: 0 ~ 3).
> 
> I would drop this description from here, and create an items list under
> clocks, describing each clock. You can then drop the "minItems: 8" &
> "maxItems: 8" from there.
> 
> > +
> > +  assigned-clocks:
> > +    minItems: 4
> > +    maxItems: 4
> > +
> > +  assigned-clock-parents:
> > +    minItems: 4
> > +    maxItems: 4

Also, drop assigned-clock* here and from the example. It's outside the 
scope of bindings.

Rob