2023-08-01 23:32:30

by Raphael Gallais-Pou

[permalink] [raw]
Subject: [PATCH] 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 <[email protected]>
---
.../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 <[email protected]>
+
+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



2023-08-02 08:38:36

by Uwe Kleine-König

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

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/ |


Attachments:
(No filename) (664.00 B)
signature.asc (499.00 B)
Download all attachments

2023-08-03 08:03:26

by Raphael Gallais-Pou

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

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
>

2023-08-03 09:15:24

by Uwe Kleine-König

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

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/ |


Attachments:
(No filename) (1.80 kB)
signature.asc (499.00 B)
Download all attachments

2023-08-03 17:00:14

by Conor Dooley

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

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


Attachments:
(No filename) (1.90 kB)
signature.asc (235.00 B)
Download all attachments

2023-08-03 19:19:59

by Raphael Gallais-Pou

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

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

2023-08-11 20:20:12

by Rob Herring

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

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