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

Groups > linux.kernel > #1535894

Re: [PATCH v3 3/7] PWM: add pwm-stm32 DT bindings

From Lee Jones <lee.jones@linaro.org>
Newsgroups linux.kernel
Subject Re: [PATCH v3 3/7] PWM: add pwm-stm32 DT bindings
Date 2016-12-05 09:40 +0100
Message-ID <sKWYx-2e2-7@gated-at.bofh.it> (permalink)
References <sJT6F-2yH-1@gated-at.bofh.it> <sJT6G-2yH-27@gated-at.bofh.it> <sKVpL-14V-9@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw

On Mon, 05 Dec 2016, Thierry Reding wrote:

> On Fri, Dec 02, 2016 at 11:17:18AM +0100, Benjamin Gaignard wrote:
> > Define bindings for pwm-stm32
> > 
> > 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          | 38 ++++++++++++++++++++++
> >  1 file changed, 38 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..575b9fb
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pwm/pwm-stm32.txt
> > @@ -0,0 +1,38 @@
> > +STMicroelectronics PWM driver bindings for STM32
> 
> Technically this bindings describe devices, so "driver binding" is a
> somewhat odd wording. Perhaps:
> 
> 	STMicroelectronics STM32 General Purpose Timer PWM bindings
> 
> ?
> 
> > +
> > +Must be a sub-node of STM32 general purpose timer driver
> > +Parent node properties are describe in ../mfd/stm32-general-purpose-timer.txt
> 
> Again, "driver parent node" is odd. Perhaps:
> 
> 	Must be a sub-node of an STM32 General Purpose Timer device tree
> 	node. See ../mfd/stm32-general-purpose-timer.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, please refer to [1].
> 
> Your indentation and capitalization are inconsistent. Also, please refer
> to the pinctrl bindings by relative path and inline, rather than as a
> footnote reference.
> 
> > +
> > +Optional parameters:
> > +- st,breakinput:	Set if the hardware have break input capabilities
> > +- st,breakinput-polarity: Set break input polarity. Default is 0
> > +			 The value define the active polarity:
> > +			  - 0 (active LOW)
> > +			  - 1 (active HIGH)
> 
> Could we fold these into a single property? If st,breakinput-polarity is
> not present it could simply mean that there is no break input, and if it
> is present you don't have to rely on a default.
> 
> > +- st,pwm-num-chan:	Number of available PWM channels.  Default is 0.
> 
> The pwm- prefix is rather redundant since the node is already named pwm.
> Why not simply st,channels? Or simply channels, since it's not really
> anything specific to this hardware.
> 
> Come to think of it, might be worth having a discussion with our DT
> gurus about what their stance is on using the # as prefix for numbers
> (such as in #address-cells or #size-cells). This could be #channels to
> mark it more explicitly as representing a count.

Unfortunately that ship has sailed.

st,pwm-num-chan already exists (with your blessing).  It's usually
suggested to reuse exiting properties when writing new bindings.

> > +- st,32bits-counter:	Set if the hardware have a 32 bits counter
> > +- st,complementary:	Set if the hardware have complementary output channels
> 
> "hardware has" and also maybe mention explicitly that this is a boolean
> property. Otherwise people might be left wondering what it should be set
> to. Or maybe word this differently to imply that it's boolean:
> 
> 	- st,32bits-counter: if present, the hardware has a 32 bit counter
> 	- st,complementary: if present, the hardware has a complementary
> 	                    output channel
> 
> Thierry



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

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v3 0/7] Add pwm and IIO timer drivers for stm32 Benjamin Gaignard <benjamin.gaignard@linaro.org> - 2016-12-02 11:20 +0100
  [PATCH v3 6/7] IIO: add STM32 timer trigger driver Benjamin Gaignard <benjamin.gaignard@linaro.org> - 2016-12-02 11:20 +0100
    Re: [PATCH v3 6/7] IIO: add STM32 timer trigger driver Lee Jones <lee.jones@linaro.org> - 2016-12-02 15:00 +0100
  [PATCH v3 7/7] ARM: dts: stm32: add stm32 general purpose timer driver in DT Benjamin Gaignard <benjamin.gaignard@linaro.org> - 2016-12-02 11:20 +0100
    Re: [PATCH v3 7/7] ARM: dts: stm32: add stm32 general purpose timer  driver in DT Lee Jones <lee.jones@linaro.org> - 2016-12-02 14:20 +0100
      Re: [PATCH v3 7/7] ARM: dts: stm32: add stm32 general purpose timer  driver in DT Jonathan Cameron <jic23@kernel.org> - 2016-12-03 15:50 +0100
        Re: [PATCH v3 7/7] ARM: dts: stm32: add stm32 general purpose timer  driver in DT Lee Jones <lee.jones@linaro.org> - 2016-12-05 10:20 +0100
      Re: [PATCH v3 7/7] ARM: dts: stm32: add stm32 general purpose timer  driver in DT Alexandre Torgue <alexandre.torgue@st.com> - 2016-12-05 17:20 +0100
        Re: [PATCH v3 7/7] ARM: dts: stm32: add stm32 general purpose timer  driver in DT Lee Jones <lee.jones@linaro.org> - 2016-12-06 10:50 +0100
          Re: [PATCH v3 7/7] ARM: dts: stm32: add stm32 general purpose timer  driver in DT Alexandre Torgue <alexandre.torgue@st.com> - 2016-12-06 11:00 +0100
            Re: [PATCH v3 7/7] ARM: dts: stm32: add stm32 general purpose timer  driver in DT Lee Jones <lee.jones@linaro.org> - 2016-12-06 14:00 +0100
    Re: [PATCH v3 7/7] ARM: dts: stm32: add stm32 general purpose timer  driver in DT Alexandre Torgue <alexandre.torgue@st.com> - 2016-12-02 14:50 +0100
    Re: [PATCH v3 7/7] ARM: dts: stm32: add stm32 general purpose timer  driver in DT Jonathan Cameron <jic23@kernel.org> - 2016-12-03 15:50 +0100
  [PATCH v3 3/7] PWM: add pwm-stm32 DT bindings Benjamin Gaignard <benjamin.gaignard@linaro.org> - 2016-12-02 11:20 +0100
    Re: [PATCH v3 3/7] PWM: add pwm-stm32 DT bindings Lee Jones <lee.jones@linaro.org> - 2016-12-02 15:30 +0100
    Re: [PATCH v3 3/7] PWM: add pwm-stm32 DT bindings Thierry Reding <thierry.reding@gmail.com> - 2016-12-05 08:00 +0100
      Re: [PATCH v3 3/7] PWM: add pwm-stm32 DT bindings Lee Jones <lee.jones@linaro.org> - 2016-12-05 09:40 +0100
        Re: [PATCH v3 3/7] PWM: add pwm-stm32 DT bindings Thierry Reding <thierry.reding@gmail.com> - 2016-12-05 12:00 +0100
      Re: [PATCH v3 3/7] PWM: add pwm-stm32 DT bindings Benjamin Gaignard <benjamin.gaignard@linaro.org> - 2016-12-05 12:10 +0100
        Re: [PATCH v3 3/7] PWM: add pwm-stm32 DT bindings Thierry Reding <thierry.reding@gmail.com> - 2016-12-05 12:30 +0100
          Re: [PATCH v3 3/7] PWM: add pwm-stm32 DT bindings Benjamin Gaignard <benjamin.gaignard@linaro.org> - 2016-12-05 13:20 +0100
            Re: [PATCH v3 3/7] PWM: add pwm-stm32 DT bindings Thierry Reding <thierry.reding@gmail.com> - 2016-12-05 13:30 +0100
  [PATCH v3 5/7] IIO: add bindings for stm32 timer trigger driver Benjamin Gaignard <benjamin.gaignard@linaro.org> - 2016-12-02 11:20 +0100
    Re: [PATCH v3 5/7] IIO: add bindings for stm32 timer trigger driver Lee Jones <lee.jones@linaro.org> - 2016-12-02 15:00 +0100
      Re: [PATCH v3 5/7] IIO: add bindings for stm32 timer trigger driver Benjamin Gaignard <benjamin.gaignard@linaro.org> - 2016-12-02 15:30 +0100
        Re: [PATCH v3 5/7] IIO: add bindings for stm32 timer trigger driver Jonathan Cameron <jic23@kernel.org> - 2016-12-03 15:50 +0100
  [PATCH v3 4/7] PWM: add pwm driver for stm32 plaftorm Benjamin Gaignard <benjamin.gaignard@linaro.org> - 2016-12-02 11:20 +0100
    Re: [PATCH v3 4/7] PWM: add pwm driver for stm32 plaftorm Thierry Reding <thierry.reding@gmail.com> - 2016-12-05 08:30 +0100
      Re: [PATCH v3 4/7] PWM: add pwm driver for stm32 plaftorm Lee Jones <lee.jones@linaro.org> - 2016-12-05 09:30 +0100
      Re: [PATCH v3 4/7] PWM: add pwm driver for stm32 plaftorm Benjamin Gaignard <benjamin.gaignard@linaro.org> - 2016-12-05 12:10 +0100
        Re: [PATCH v3 4/7] PWM: add pwm driver for stm32 plaftorm Thierry Reding <thierry.reding@gmail.com> - 2016-12-05 12:40 +0100
  [PATCH v3 2/7] MFD: add stm32 general purpose timer driver Benjamin Gaignard <benjamin.gaignard@linaro.org> - 2016-12-02 11:20 +0100
    Re: [PATCH v3 2/7] MFD: add stm32 general purpose timer driver Lee Jones <lee.jones@linaro.org> - 2016-12-02 15:30 +0100
  [PATCH v3 1/7] MFD: add bindings for stm32 general purpose timer driver Benjamin Gaignard <benjamin.gaignard@linaro.org> - 2016-12-02 11:20 +0100
  Re: [PATCH v3 0/7] Add pwm and IIO timer drivers for stm32 Jonathan Cameron <jic23@kernel.org> - 2016-12-03 15:50 +0100

csiph-web