2022-08-16 21:41:40

by Ben Dooks

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

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 <[email protected]>
---
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 <[email protected]>
+
+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
+ - clocks
+ - clock-names
+
+additionalProperties: false
+
+
+examples:
+ - |
+ pwm: pwm@180000 {
+ #pwm-cells = <3>;
+ compatible = "snps,dw-apb-timers-pwm2";
+ reg = <0x180000 0x200>;
+ clocks = <&bus &timer>;
+ clock-names = "bus", "timer";
+ };
--
2.35.1


2022-08-17 06:41:17

by Krzysztof Kozlowski

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

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 <[email protected]>
> ---
> 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 <[email protected]>
> +
> +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.

> + - 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";
> + };


Best regards,
Krzysztof

2022-08-18 14:08:41

by Ben Dooks

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

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 <[email protected]>
>> ---
>> 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 <[email protected]>
>> +
>> +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 ?

2022-08-18 14:13:49

by Krzysztof Kozlowski

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

On 18/08/2022 16:43, Ben Dooks wrote:
>
>> Best regards,
>> Krzysztof
>
> Thanks for the review.
>
> I guess this is now too late for 6.0-rc ?

Code for 6.0-rc would have to be accepted by subsystem maintainer
between 3 to 5 weeks ago (depending on maintainer), so it's late by 3-5
weeks.

Best regards,
Krzysztof