[PATCH] dt-bindings: pwm: st: convert sti-pwm to DT schema

[PATCH] dt-bindings: pwm: st: convert sti-pwm to DT schema

[Raphael Gallais-Pou]

Converts st,sti-pwm binding to DT schema format

Signed-off-by: Raphael Gallais-Pou <rgallaispou@gmail.com>
---
 .../devicetree/bindings/pwm/pwm-st.txt        | 43 -----------
 .../devicetree/bindings/pwm/st,sti-pwm.yaml   | 74 +++++++++++++++++++
 2 files changed, 74 insertions(+), 43 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/pwm/pwm-st.txt
 create mode 100644 Documentation/devicetree/bindings/pwm/st,sti-pwm.yaml

diff --git a/Documentation/devicetree/bindings/pwm/pwm-st.txt b/Documentation/devicetree/bindings/pwm/pwm-st.txt
deleted file mode 100644
index 19fce774cafa..000000000000
--- a/Documentation/devicetree/bindings/pwm/pwm-st.txt
+++ /dev/null
@@ -1,43 +0,0 @@
-STMicroelectronics PWM driver bindings
---------------------------------------
-
-Required parameters:
-- compatible :		"st,pwm"
-- #pwm-cells : 		Number of cells used to specify a PWM. First cell
-			specifies the per-chip index of the PWM to use and the
-			second cell is the period in nanoseconds - fixed to 2
-			for STiH41x.
-- reg :			Physical base address and length of the controller's
-			registers.
-- pinctrl-names: 	Set to "default".
-- pinctrl-0: 		List of phandles pointing to pin configuration nodes
-			for PWM module.
-			For Pinctrl properties, please refer to [1].
-- clock-names: 		Valid entries are "pwm" and/or "capture".
-- clocks: 		phandle of the clock used by the PWM module.
-			For Clk properties, please refer to [2].
-- interrupts:		IRQ for the Capture device
-
-Optional properties:
-- st,pwm-num-chan:	Number of available PWM channels.  Default is 0.
-- st,capture-num-chan:	Number of available Capture channels.  Default is 0.
-
-[1] Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
-[2] Documentation/devicetree/bindings/clock/clock-bindings.txt
-
-Example:
-
-pwm1: pwm@fe510000 {
-	compatible = "st,pwm";
-	reg = <0xfe510000 0x68>;
-	#pwm-cells = <2>;
-	pinctrl-names = "default";
-	pinctrl-0 = <&pinctrl_pwm1_chan0_default
-		     &pinctrl_pwm1_chan1_default
-		     &pinctrl_pwm1_chan2_default
-		     &pinctrl_pwm1_chan3_default>;
-	clocks = <&clk_sysin>;
-	clock-names = "pwm";
-	st,pwm-num-chan = <4>;
-	st,capture-num-chan = <2>;
-};
diff --git a/Documentation/devicetree/bindings/pwm/st,sti-pwm.yaml b/Documentation/devicetree/bindings/pwm/st,sti-pwm.yaml
new file mode 100644
index 000000000000..8a7833e9c10c
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/st,sti-pwm.yaml
@@ -0,0 +1,74 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/st,sti-pwm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: STMicroelectronics STi PWM controller
+
+maintainers:
+  - Patrice Chotard <patrice.chotard@foss.st.com>
+
+allOf:
+  - $ref: pwm.yaml#
+
+properties:
+  compatible:
+    const: st,sti-pwm
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  pinctrl-names:
+    const: default
+
+  pinctrl-0:
+    description: Configuration for the default state.
+
+  clock-names:
+    const: pwm
+
+  clocks:
+    $ref: "/schemas/types.yaml#/definitions/phandle"
+
+  st,pwm-num-chan:
+    $ref: "/schemas/types.yaml#/definitions/uint32"
+    description: Number of available PWM channels.
+
+  st,capture-num-chan:
+    $ref: "/schemas/types.yaml#/definitions/uint32"
+    description: Number of available Capture channels.
+
+  "#pwm-cells":
+    const: 2
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clock-names
+  - clocks
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    pwm1: pwm@9510000 {
+        compatible = "st,sti-pwm";
+        #pwm-cells = <2>;
+        reg = <0x9510000 0x68>;
+        interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>;
+        pinctrl-names = "default";
+        pinctrl-0 = <&pinctrl_pwm1_chan0_default
+                 &pinctrl_pwm1_chan1_default
+                 &pinctrl_pwm1_chan2_default
+                 &pinctrl_pwm1_chan3_default>;
+        clock-names = "pwm";
+        clocks = <&clk_sysin>;
+        st,pwm-num-chan = <4>;
+    };
+...
-- 
2.41.0

Re: [PATCH] dt-bindings: pwm: st: convert sti-pwm to DT schema

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 664 bytes --]

Hello,

On Wed, Aug 02, 2023 at 12:05:59AM +0200, Raphael Gallais-Pou wrote:
> +  st,capture-num-chan:
> +    $ref: "/schemas/types.yaml#/definitions/uint32"
> +    description: Number of available Capture channels.

I have the theory that nobody actually uses the capture feature and I'd
like to get rid of it. People who do use it, should better switch to the
counter driver.

I wonder if this is the opportunity to drop st,capture-num-chan. There
is no mainline user.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] dt-bindings: pwm: st: convert sti-pwm to DT schema

[Raphaël Gallais-Pou]

Hi

Le 02/08/2023 à 10:02, Uwe Kleine-König a écrit :
> Hello,
> 
> On Wed, Aug 02, 2023 at 12:05:59AM +0200, Raphael Gallais-Pou wrote:
>> +  st,capture-num-chan:
>> +    $ref: "/schemas/types.yaml#/definitions/uint32"
>> +    description: Number of available Capture channels.
> 
> I have the theory that nobody actually uses the capture feature and I'd
> like to get rid of it. People who do use it, should better switch to the
> counter driver.

TBH I only found two drivers using it, including this one.

$ grep -rinI "\.capture" drivers/pwm/ | wc -l
2

While I agree that there is not much drivers using capture feature, I 
have mixed feelings about removing it.
> 
> I wonder if this is the opportunity to drop st,capture-num-chan. There
> is no mainline user.

If there is no opposition about removing this feature I suggest to do it 
in a second time, in a serie.

Regards,
Raphaël
> 
> Best regards
> Uwe
> 

Re: [PATCH] dt-bindings: pwm: st: convert sti-pwm to DT schema

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1839 bytes --]

On Thu, Aug 03, 2023 at 09:18:14AM +0200, Raphaël Gallais-Pou wrote:
> Hi
> 
> Le 02/08/2023 à 10:02, Uwe Kleine-König a écrit :
> > Hello,
> > 
> > On Wed, Aug 02, 2023 at 12:05:59AM +0200, Raphael Gallais-Pou wrote:
> > > +  st,capture-num-chan:
> > > +    $ref: "/schemas/types.yaml#/definitions/uint32"
> > > +    description: Number of available Capture channels.
> > 
> > I have the theory that nobody actually uses the capture feature and I'd
> > like to get rid of it. People who do use it, should better switch to the
> > counter driver.
> 
> TBH I only found two drivers using it, including this one.
> 
> $ grep -rinI "\.capture" drivers/pwm/ | wc -l
> 2

Right, there is pwm-stm32 and pwm-sti that support capture.

There are a few machines that have a st,sti-pwm device:

	$ grep -rl st,sti-pwm arch/arm/boot/dts/*.dtb
	arch/arm/boot/dts/stih407-b2120.dtb
	arch/arm/boot/dts/stih410-b2120.dtb
	arch/arm/boot/dts/stih410-b2260.dtb
	arch/arm/boot/dts/stih418-b2199.dtb
	arch/arm/boot/dts/stih418-b2264.dtb

but to actually use capture the device tree must have a property
st,capture-num-chan. "st,capture-num-chan" isn't set by any of the
devices.

I think for stm32 it's not that trivial to show that it's unused.
While the capture code isn't a big maintenance burden, I still would
prefer to get rid of it if nobody uses it. Still more given that there
are better alternatives available.

> If there is no opposition about removing this feature I suggest to do it in
> a second time, in a serie.

Does that mean you will do that? I guess not, but at least this means
you're not using capture support.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] dt-bindings: pwm: st: convert sti-pwm to DT schema

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1948 bytes --]

On Thu, Aug 03, 2023 at 10:56:45AM +0200, Uwe Kleine-König wrote:
> On Thu, Aug 03, 2023 at 09:18:14AM +0200, Raphaël Gallais-Pou wrote:
> > Hi
> > 
> > Le 02/08/2023 à 10:02, Uwe Kleine-König a écrit :
> > > Hello,
> > > 
> > > On Wed, Aug 02, 2023 at 12:05:59AM +0200, Raphael Gallais-Pou wrote:
> > > > +  st,capture-num-chan:
> > > > +    $ref: "/schemas/types.yaml#/definitions/uint32"
> > > > +    description: Number of available Capture channels.
> > > 
> > > I have the theory that nobody actually uses the capture feature and I'd
> > > like to get rid of it. People who do use it, should better switch to the
> > > counter driver.
> > 
> > TBH I only found two drivers using it, including this one.
> > 
> > $ grep -rinI "\.capture" drivers/pwm/ | wc -l
> > 2
> 
> Right, there is pwm-stm32 and pwm-sti that support capture.
> 
> There are a few machines that have a st,sti-pwm device:
> 
> 	$ grep -rl st,sti-pwm arch/arm/boot/dts/*.dtb
> 	arch/arm/boot/dts/stih407-b2120.dtb
> 	arch/arm/boot/dts/stih410-b2120.dtb
> 	arch/arm/boot/dts/stih410-b2260.dtb
> 	arch/arm/boot/dts/stih418-b2199.dtb
> 	arch/arm/boot/dts/stih418-b2264.dtb
> 
> but to actually use capture the device tree must have a property
> st,capture-num-chan. "st,capture-num-chan" isn't set by any of the
> devices.
> 
> I think for stm32 it's not that trivial to show that it's unused.
> While the capture code isn't a big maintenance burden, I still would
> prefer to get rid of it if nobody uses it. Still more given that there
> are better alternatives available.
> 
> > If there is no opposition about removing this feature I suggest to do it in
> > a second time, in a serie.
> 
> Does that mean you will do that? I guess not, but at least this means
> you're not using capture support.

It seems like it should either be done as part of the conversion or as a
second patch in the series doing the conversion /shrug

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] dt-bindings: pwm: st: convert sti-pwm to DT schema

[Raphaël Gallais-Pou]

Hi,

Le 03/08/2023 à 18:09, Conor Dooley a écrit :
> On Thu, Aug 03, 2023 at 10:56:45AM +0200, Uwe Kleine-König wrote:
>> On Thu, Aug 03, 2023 at 09:18:14AM +0200, Raphaël Gallais-Pou wrote:
>>> Hi
>>>
>>> Le 02/08/2023 à 10:02, Uwe Kleine-König a écrit :
>>>> Hello,
>>>>
>>>> On Wed, Aug 02, 2023 at 12:05:59AM +0200, Raphael Gallais-Pou wrote:
>>>>> +  st,capture-num-chan:
>>>>> +    $ref: "/schemas/types.yaml#/definitions/uint32"
>>>>> +    description: Number of available Capture channels.
>>>>
>>>> I have the theory that nobody actually uses the capture feature and I'd
>>>> like to get rid of it. People who do use it, should better switch to the
>>>> counter driver.
>>>
>>> TBH I only found two drivers using it, including this one.
>>>
>>> $ grep -rinI "\.capture" drivers/pwm/ | wc -l
>>> 2
>>
>> Right, there is pwm-stm32 and pwm-sti that support capture.
>>
>> There are a few machines that have a st,sti-pwm device:
>>
>> 	$ grep -rl st,sti-pwm arch/arm/boot/dts/*.dtb
>> 	arch/arm/boot/dts/stih407-b2120.dtb
>> 	arch/arm/boot/dts/stih410-b2120.dtb
>> 	arch/arm/boot/dts/stih410-b2260.dtb
>> 	arch/arm/boot/dts/stih418-b2199.dtb
>> 	arch/arm/boot/dts/stih418-b2264.dtb
>>
>> but to actually use capture the device tree must have a property
>> st,capture-num-chan. "st,capture-num-chan" isn't set by any of the
>> devices.

This is also what I came across, this is the reason why I'm not 
reluctant to remove it.

>>
>> I think for stm32 it's not that trivial to show that it's unused.
>> While the capture code isn't a big maintenance burden, I still would
>> prefer to get rid of it if nobody uses it. Still more given that there
>> are better alternatives available.

Regarding stm32, I think the owner of the driver would prefer to handle it.

>>
>>> If there is no opposition about removing this feature I suggest to do it in
>>> a second time, in a serie.
>>
>> Does that mean you will do that? I guess not, but at least this means
>> you're not using capture support.
> 
> It seems like it should either be done as part of the conversion or as a
> second patch in the series doing the conversion /shrug

Splitting the conversion and the capture removal is clearer IMO. Mixing 
both could lead to confusion. I'll send another serie to do this.


Regards,
Raphaël

Re: [PATCH] dt-bindings: pwm: st: convert sti-pwm to DT schema

[Rob Herring]

On Wed, Aug 02, 2023 at 12:05:59AM +0200, Raphael Gallais-Pou wrote:
> Converts st,sti-pwm binding to DT schema format
> 
> Signed-off-by: Raphael Gallais-Pou <rgallaispou@gmail.com>
> ---
>  .../devicetree/bindings/pwm/pwm-st.txt        | 43 -----------
>  .../devicetree/bindings/pwm/st,sti-pwm.yaml   | 74 +++++++++++++++++++
>  2 files changed, 74 insertions(+), 43 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/pwm/pwm-st.txt
>  create mode 100644 Documentation/devicetree/bindings/pwm/st,sti-pwm.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pwm/pwm-st.txt b/Documentation/devicetree/bindings/pwm/pwm-st.txt
> deleted file mode 100644
> index 19fce774cafa..000000000000
> --- a/Documentation/devicetree/bindings/pwm/pwm-st.txt
> +++ /dev/null
> @@ -1,43 +0,0 @@
> -STMicroelectronics PWM driver bindings
> ---------------------------------------
> -
> -Required parameters:
> -- compatible :		"st,pwm"
> -- #pwm-cells : 		Number of cells used to specify a PWM. First cell
> -			specifies the per-chip index of the PWM to use and the
> -			second cell is the period in nanoseconds - fixed to 2
> -			for STiH41x.
> -- reg :			Physical base address and length of the controller's
> -			registers.
> -- pinctrl-names: 	Set to "default".
> -- pinctrl-0: 		List of phandles pointing to pin configuration nodes
> -			for PWM module.
> -			For Pinctrl properties, please refer to [1].
> -- clock-names: 		Valid entries are "pwm" and/or "capture".
> -- clocks: 		phandle of the clock used by the PWM module.
> -			For Clk properties, please refer to [2].
> -- interrupts:		IRQ for the Capture device
> -
> -Optional properties:
> -- st,pwm-num-chan:	Number of available PWM channels.  Default is 0.
> -- st,capture-num-chan:	Number of available Capture channels.  Default is 0.
> -
> -[1] Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> -[2] Documentation/devicetree/bindings/clock/clock-bindings.txt
> -
> -Example:
> -
> -pwm1: pwm@fe510000 {
> -	compatible = "st,pwm";
> -	reg = <0xfe510000 0x68>;
> -	#pwm-cells = <2>;
> -	pinctrl-names = "default";
> -	pinctrl-0 = <&pinctrl_pwm1_chan0_default
> -		     &pinctrl_pwm1_chan1_default
> -		     &pinctrl_pwm1_chan2_default
> -		     &pinctrl_pwm1_chan3_default>;
> -	clocks = <&clk_sysin>;
> -	clock-names = "pwm";
> -	st,pwm-num-chan = <4>;
> -	st,capture-num-chan = <2>;
> -};
> diff --git a/Documentation/devicetree/bindings/pwm/st,sti-pwm.yaml b/Documentation/devicetree/bindings/pwm/st,sti-pwm.yaml
> new file mode 100644
> index 000000000000..8a7833e9c10c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pwm/st,sti-pwm.yaml
> @@ -0,0 +1,74 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pwm/st,sti-pwm.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: STMicroelectronics STi PWM controller
> +
> +maintainers:
> +  - Patrice Chotard <patrice.chotard@foss.st.com>
> +
> +allOf:
> +  - $ref: pwm.yaml#
> +
> +properties:
> +  compatible:
> +    const: st,sti-pwm
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  pinctrl-names:
> +    const: default
> +
> +  pinctrl-0:
> +    description: Configuration for the default state.
> +
> +  clock-names:
> +    const: pwm
> +
> +  clocks:
> +    $ref: "/schemas/types.yaml#/definitions/phandle"

Drop quotes

> +
> +  st,pwm-num-chan:
> +    $ref: "/schemas/types.yaml#/definitions/uint32"
> +    description: Number of available PWM channels.

Constraints?

> +
> +  st,capture-num-chan:
> +    $ref: "/schemas/types.yaml#/definitions/uint32"
> +    description: Number of available Capture channels.

Constraints?

> +
> +  "#pwm-cells":
> +    const: 2
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clock-names
> +  - clocks
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    pwm1: pwm@9510000 {
> +        compatible = "st,sti-pwm";
> +        #pwm-cells = <2>;
> +        reg = <0x9510000 0x68>;
> +        interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>;
> +        pinctrl-names = "default";
> +        pinctrl-0 = <&pinctrl_pwm1_chan0_default
> +                 &pinctrl_pwm1_chan1_default
> +                 &pinctrl_pwm1_chan2_default
> +                 &pinctrl_pwm1_chan3_default>;
> +        clock-names = "pwm";
> +        clocks = <&clk_sysin>;
> +        st,pwm-num-chan = <4>;
> +    };
> +...
> -- 
> 2.41.0
>