Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1540554 > unrolled thread

Re: [PATCH v6 3/8] PWM: add pwm-stm32 DT bindings

Started byRob Herring <robh@kernel.org>
First post2016-12-12 20:10 +0100
Last post2016-12-19 14:00 +0100
Articles 6 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v6 3/8] PWM: add pwm-stm32 DT bindings Rob Herring <robh@kernel.org> - 2016-12-12 20:10 +0100
    Re: [PATCH v6 3/8] PWM: add pwm-stm32 DT bindings Lee Jones <lee.jones@linaro.org> - 2016-12-13 12:10 +0100
      Re: [PATCH v6 3/8] PWM: add pwm-stm32 DT bindings Rob Herring <robh@kernel.org> - 2016-12-13 17:10 +0100
        Re: [PATCH v6 3/8] PWM: add pwm-stm32 DT bindings Benjamin Gaignard <benjamin.gaignard@linaro.org> - 2016-12-13 18:20 +0100
          Re: [PATCH v6 3/8] PWM: add pwm-stm32 DT bindings Rob Herring <robh@kernel.org> - 2016-12-13 18:20 +0100
        Re: [PATCH v6 3/8] PWM: add pwm-stm32 DT bindings Lee Jones <lee.jones@linaro.org> - 2016-12-19 14:00 +0100

#1540554 — Re: [PATCH v6 3/8] PWM: add pwm-stm32 DT bindings

FromRob Herring <robh@kernel.org>
Date2016-12-12 20:10 +0100
SubjectRe: [PATCH v6 3/8] PWM: add pwm-stm32 DT bindings
Message-ID<sNE93-8b3-19@gated-at.bofh.it>
On Fri, Dec 09, 2016 at 03:15:14PM +0100, Benjamin Gaignard wrote:
> Define bindings for pwm-stm32
> 
> version 6:
> - change st,breakinput parameter format to make it usuable on stm32f7 too.
> 
> version 2:
> - use parameters instead of compatible of handle the hardware configuration
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> ---
>  .../devicetree/bindings/pwm/pwm-stm32.txt          | 33 ++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pwm/pwm-stm32.txt
> 
> diff --git a/Documentation/devicetree/bindings/pwm/pwm-stm32.txt b/Documentation/devicetree/bindings/pwm/pwm-stm32.txt
> new file mode 100644
> index 0000000..866f222
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pwm/pwm-stm32.txt
> @@ -0,0 +1,33 @@
> +STMicroelectronics STM32 Timers PWM bindings
> +
> +Must be a sub-node of an STM32 Timers device tree node.
> +See ../mfd/stm32-timers.txt for details about the parent node.
> +
> +Required parameters:
> +- compatible:		Must be "st,stm32-pwm".
> +- pinctrl-names: 	Set to "default".
> +- pinctrl-0: 		List of phandles pointing to pin configuration nodes for PWM module.
> +			For Pinctrl properties see ../pinctrl/pinctrl-bindings.txt
> +
> +Optional parameters:
> +- st,breakinput:	Arrays of three u32 <index level filter> to describe break input configurations.
> +			"index" indicates on which break input the configuration should be applied.
> +			"level" gives the active level (0=low or 1=high) for this configuration.
> +			"filter" gives the filtering value to be applied.
> +
> +Example:
> +	timers@40010000 {

timer@...

With that,

Acked-by: Rob Herring <robh@kernel.org>


> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		compatible = "st,stm32-timers";
> +		reg = <0x40010000 0x400>;
> +		clocks = <&rcc 0 160>;
> +		clock-names = "clk_int";
> +
> +		pwm {
> +			compatible = "st,stm32-pwm";
> +			pinctrl-0	= <&pwm1_pins>;
> +			pinctrl-names	= "default";
> +			st,breakinput = <0 1 5>;
> +		};
> +	};
> -- 
> 1.9.1
>

[toc] | [next] | [standalone]

#1541000

FromLee Jones <lee.jones@linaro.org>
Date2016-12-13 12:10 +0100
Message-ID<sNT85-tf-7@gated-at.bofh.it>
In reply to#1540554
On Mon, 12 Dec 2016, Rob Herring wrote:

> On Fri, Dec 09, 2016 at 03:15:14PM +0100, Benjamin Gaignard wrote:
> > Define bindings for pwm-stm32
> > 
> > version 6:
> > - change st,breakinput parameter format to make it usuable on stm32f7 too.
> > 
> > version 2:
> > - use parameters instead of compatible of handle the hardware configuration
> > 
> > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> > ---
> >  .../devicetree/bindings/pwm/pwm-stm32.txt          | 33 ++++++++++++++++++++++
> >  1 file changed, 33 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/pwm/pwm-stm32.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/pwm/pwm-stm32.txt b/Documentation/devicetree/bindings/pwm/pwm-stm32.txt
> > new file mode 100644
> > index 0000000..866f222
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pwm/pwm-stm32.txt
> > @@ -0,0 +1,33 @@
> > +STMicroelectronics STM32 Timers PWM bindings
> > +
> > +Must be a sub-node of an STM32 Timers device tree node.
> > +See ../mfd/stm32-timers.txt for details about the parent node.
> > +
> > +Required parameters:
> > +- compatible:		Must be "st,stm32-pwm".
> > +- pinctrl-names: 	Set to "default".
> > +- pinctrl-0: 		List of phandles pointing to pin configuration nodes for PWM module.
> > +			For Pinctrl properties see ../pinctrl/pinctrl-bindings.txt
> > +
> > +Optional parameters:
> > +- st,breakinput:	Arrays of three u32 <index level filter> to describe break input configurations.
> > +			"index" indicates on which break input the configuration should be applied.
> > +			"level" gives the active level (0=low or 1=high) for this configuration.
> > +			"filter" gives the filtering value to be applied.
> > +
> > +Example:
> > +	timers@40010000 {
> 
> timer@...

No, it should be timers.

The 's' is intentional, since this parent (MFD) device houses 3
different types of timers.  The "timer" node is a child of this one.

> With that,
> 
> Acked-by: Rob Herring <robh@kernel.org>
> 
> 
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +		compatible = "st,stm32-timers";
> > +		reg = <0x40010000 0x400>;
> > +		clocks = <&rcc 0 160>;
> > +		clock-names = "clk_int";
> > +
> > +		pwm {
> > +			compatible = "st,stm32-pwm";
> > +			pinctrl-0	= <&pwm1_pins>;
> > +			pinctrl-names	= "default";
> > +			st,breakinput = <0 1 5>;
> > +		};
> > +	};

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[toc] | [prev] | [next] | [standalone]

#1541212

FromRob Herring <robh@kernel.org>
Date2016-12-13 17:10 +0100
Message-ID<sNXOp-3i2-1@gated-at.bofh.it>
In reply to#1541000
On Tue, Dec 13, 2016 at 5:11 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Mon, 12 Dec 2016, Rob Herring wrote:
>
>> On Fri, Dec 09, 2016 at 03:15:14PM +0100, Benjamin Gaignard wrote:
>> > Define bindings for pwm-stm32
>> >
>> > version 6:
>> > - change st,breakinput parameter format to make it usuable on stm32f7 too.
>> >
>> > version 2:
>> > - use parameters instead of compatible of handle the hardware configuration
>> >
>> > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>> > ---
>> >  .../devicetree/bindings/pwm/pwm-stm32.txt          | 33 ++++++++++++++++++++++
>> >  1 file changed, 33 insertions(+)
>> >  create mode 100644 Documentation/devicetree/bindings/pwm/pwm-stm32.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/pwm/pwm-stm32.txt b/Documentation/devicetree/bindings/pwm/pwm-stm32.txt
>> > new file mode 100644
>> > index 0000000..866f222
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/pwm/pwm-stm32.txt
>> > @@ -0,0 +1,33 @@
>> > +STMicroelectronics STM32 Timers PWM bindings
>> > +
>> > +Must be a sub-node of an STM32 Timers device tree node.
>> > +See ../mfd/stm32-timers.txt for details about the parent node.
>> > +
>> > +Required parameters:
>> > +- compatible:              Must be "st,stm32-pwm".
>> > +- pinctrl-names:   Set to "default".
>> > +- pinctrl-0:               List of phandles pointing to pin configuration nodes for PWM module.
>> > +                   For Pinctrl properties see ../pinctrl/pinctrl-bindings.txt
>> > +
>> > +Optional parameters:
>> > +- st,breakinput:   Arrays of three u32 <index level filter> to describe break input configurations.
>> > +                   "index" indicates on which break input the configuration should be applied.
>> > +                   "level" gives the active level (0=low or 1=high) for this configuration.
>> > +                   "filter" gives the filtering value to be applied.
>> > +
>> > +Example:
>> > +   timers@40010000 {
>>
>> timer@...
>
> No, it should be timers.

Read the spec. "timer" is a generic node name. "timers" is not. How
many is not relevant.

> The 's' is intentional, since this parent (MFD) device houses 3
> different types of timers.  The "timer" node is a child of this one.

[toc] | [prev] | [next] | [standalone]

#1541287

FromBenjamin Gaignard <benjamin.gaignard@linaro.org>
Date2016-12-13 18:20 +0100
Message-ID<sNYUb-3UP-51@gated-at.bofh.it>
In reply to#1541212
2016-12-13 16:57 GMT+01:00 Rob Herring <robh@kernel.org>:
> On Tue, Dec 13, 2016 at 5:11 AM, Lee Jones <lee.jones@linaro.org> wrote:
>> On Mon, 12 Dec 2016, Rob Herring wrote:
>>
>>> On Fri, Dec 09, 2016 at 03:15:14PM +0100, Benjamin Gaignard wrote:
>>> > Define bindings for pwm-stm32
>>> >
>>> > version 6:
>>> > - change st,breakinput parameter format to make it usuable on stm32f7 too.
>>> >
>>> > version 2:
>>> > - use parameters instead of compatible of handle the hardware configuration
>>> >
>>> > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>>> > ---
>>> >  .../devicetree/bindings/pwm/pwm-stm32.txt          | 33 ++++++++++++++++++++++
>>> >  1 file changed, 33 insertions(+)
>>> >  create mode 100644 Documentation/devicetree/bindings/pwm/pwm-stm32.txt
>>> >
>>> > diff --git a/Documentation/devicetree/bindings/pwm/pwm-stm32.txt b/Documentation/devicetree/bindings/pwm/pwm-stm32.txt
>>> > new file mode 100644
>>> > index 0000000..866f222
>>> > --- /dev/null
>>> > +++ b/Documentation/devicetree/bindings/pwm/pwm-stm32.txt
>>> > @@ -0,0 +1,33 @@
>>> > +STMicroelectronics STM32 Timers PWM bindings
>>> > +
>>> > +Must be a sub-node of an STM32 Timers device tree node.
>>> > +See ../mfd/stm32-timers.txt for details about the parent node.
>>> > +
>>> > +Required parameters:
>>> > +- compatible:              Must be "st,stm32-pwm".
>>> > +- pinctrl-names:   Set to "default".
>>> > +- pinctrl-0:               List of phandles pointing to pin configuration nodes for PWM module.
>>> > +                   For Pinctrl properties see ../pinctrl/pinctrl-bindings.txt
>>> > +
>>> > +Optional parameters:
>>> > +- st,breakinput:   Arrays of three u32 <index level filter> to describe break input configurations.
>>> > +                   "index" indicates on which break input the configuration should be applied.
>>> > +                   "level" gives the active level (0=low or 1=high) for this configuration.
>>> > +                   "filter" gives the filtering value to be applied.
>>> > +
>>> > +Example:
>>> > +   timers@40010000 {
>>>
>>> timer@...
>>
>> No, it should be timers.
>
> Read the spec. "timer" is a generic node name. "timers" is not. How
> many is not relevant.

"timer" is already used in stm32 DT for clocksource node... It is also why
I use "timers" for this.

>
>> The 's' is intentional, since this parent (MFD) device houses 3
>> different types of timers.  The "timer" node is a child of this one.



-- 
Benjamin Gaignard

Graphic Study Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog

[toc] | [prev] | [next] | [standalone]

#1541289

FromRob Herring <robh@kernel.org>
Date2016-12-13 18:20 +0100
Message-ID<sNYUb-3UP-53@gated-at.bofh.it>
In reply to#1541287
On Tue, Dec 13, 2016 at 10:28 AM, Benjamin Gaignard
<benjamin.gaignard@linaro.org> wrote:
> 2016-12-13 16:57 GMT+01:00 Rob Herring <robh@kernel.org>:
>> On Tue, Dec 13, 2016 at 5:11 AM, Lee Jones <lee.jones@linaro.org> wrote:
>>> On Mon, 12 Dec 2016, Rob Herring wrote:
>>>
>>>> On Fri, Dec 09, 2016 at 03:15:14PM +0100, Benjamin Gaignard wrote:
>>>> > Define bindings for pwm-stm32
>>>> >
>>>> > version 6:
>>>> > - change st,breakinput parameter format to make it usuable on stm32f7 too.
>>>> >
>>>> > version 2:
>>>> > - use parameters instead of compatible of handle the hardware configuration
>>>> >
>>>> > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>>>> > ---
>>>> >  .../devicetree/bindings/pwm/pwm-stm32.txt          | 33 ++++++++++++++++++++++
>>>> >  1 file changed, 33 insertions(+)
>>>> >  create mode 100644 Documentation/devicetree/bindings/pwm/pwm-stm32.txt
>>>> >
>>>> > diff --git a/Documentation/devicetree/bindings/pwm/pwm-stm32.txt b/Documentation/devicetree/bindings/pwm/pwm-stm32.txt
>>>> > new file mode 100644
>>>> > index 0000000..866f222
>>>> > --- /dev/null
>>>> > +++ b/Documentation/devicetree/bindings/pwm/pwm-stm32.txt
>>>> > @@ -0,0 +1,33 @@
>>>> > +STMicroelectronics STM32 Timers PWM bindings
>>>> > +
>>>> > +Must be a sub-node of an STM32 Timers device tree node.
>>>> > +See ../mfd/stm32-timers.txt for details about the parent node.
>>>> > +
>>>> > +Required parameters:
>>>> > +- compatible:              Must be "st,stm32-pwm".
>>>> > +- pinctrl-names:   Set to "default".
>>>> > +- pinctrl-0:               List of phandles pointing to pin configuration nodes for PWM module.
>>>> > +                   For Pinctrl properties see ../pinctrl/pinctrl-bindings.txt
>>>> > +
>>>> > +Optional parameters:
>>>> > +- st,breakinput:   Arrays of three u32 <index level filter> to describe break input configurations.
>>>> > +                   "index" indicates on which break input the configuration should be applied.
>>>> > +                   "level" gives the active level (0=low or 1=high) for this configuration.
>>>> > +                   "filter" gives the filtering value to be applied.
>>>> > +
>>>> > +Example:
>>>> > +   timers@40010000 {
>>>>
>>>> timer@...
>>>
>>> No, it should be timers.
>>
>> Read the spec. "timer" is a generic node name. "timers" is not. How
>> many is not relevant.
>
> "timer" is already used in stm32 DT for clocksource node... It is also why
> I use "timers" for this.

That doesn't matter. They are at different levels in the hierarchy.

Rob

[toc] | [prev] | [next] | [standalone]

#1544508

FromLee Jones <lee.jones@linaro.org>
Date2016-12-19 14:00 +0100
Message-ID<sQ5HP-7R7-1@gated-at.bofh.it>
In reply to#1541212
On Tue, 13 Dec 2016, Rob Herring wrote:
> On Tue, Dec 13, 2016 at 5:11 AM, Lee Jones <lee.jones@linaro.org> wrote:
> > On Mon, 12 Dec 2016, Rob Herring wrote:
> >
> >> On Fri, Dec 09, 2016 at 03:15:14PM +0100, Benjamin Gaignard wrote:
> >> > Define bindings for pwm-stm32
> >> >
> >> > version 6:
> >> > - change st,breakinput parameter format to make it usuable on stm32f7 too.
> >> >
> >> > version 2:
> >> > - use parameters instead of compatible of handle the hardware configuration
> >> >
> >> > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> >> > ---
> >> >  .../devicetree/bindings/pwm/pwm-stm32.txt          | 33 ++++++++++++++++++++++
> >> >  1 file changed, 33 insertions(+)
> >> >  create mode 100644 Documentation/devicetree/bindings/pwm/pwm-stm32.txt
> >> >
> >> > diff --git a/Documentation/devicetree/bindings/pwm/pwm-stm32.txt b/Documentation/devicetree/bindings/pwm/pwm-stm32.txt
> >> > new file mode 100644
> >> > index 0000000..866f222
> >> > --- /dev/null
> >> > +++ b/Documentation/devicetree/bindings/pwm/pwm-stm32.txt
> >> > @@ -0,0 +1,33 @@
> >> > +STMicroelectronics STM32 Timers PWM bindings
> >> > +
> >> > +Must be a sub-node of an STM32 Timers device tree node.
> >> > +See ../mfd/stm32-timers.txt for details about the parent node.
> >> > +
> >> > +Required parameters:
> >> > +- compatible:              Must be "st,stm32-pwm".
> >> > +- pinctrl-names:   Set to "default".
> >> > +- pinctrl-0:               List of phandles pointing to pin configuration nodes for PWM module.
> >> > +                   For Pinctrl properties see ../pinctrl/pinctrl-bindings.txt
> >> > +
> >> > +Optional parameters:
> >> > +- st,breakinput:   Arrays of three u32 <index level filter> to describe break input configurations.
> >> > +                   "index" indicates on which break input the configuration should be applied.
> >> > +                   "level" gives the active level (0=low or 1=high) for this configuration.
> >> > +                   "filter" gives the filtering value to be applied.
> >> > +
> >> > +Example:
> >> > +   timers@40010000 {
> >>
> >> timer@...
> >
> > No, it should be timers.
> 
> Read the spec. "timer" is a generic node name. "timers" is not. How
> many is not relevant.

It's not the amount of timers that there are, it's the different types
of timers which this one IP contains.

In MFD we usually list the part number or IP name, however in this
case its difficult because the same IP doesn't have a specific name
listed, and provides; advanced, general purpose and basic timers, as
well as PWM functionality.

This IP is not a timer (although one of its children is one).  It's
the parent device of many different types of timer.

timer {
      timer {
      };

      pwm {
      };
};

... looks weird.

"timer" is not right for the parent IP.  Happy for you to provide an
alternative to "timers" though?

> > The 's' is intentional, since this parent (MFD) device houses 3
> > different types of timers.  The "timer" node is a child of this one.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web