dt-bindings: pwm: st: convert sti-pwm to DT schema

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

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

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
>

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

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

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

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
>