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

Groups > linux.kernel > #1535837

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

From Thierry Reding <thierry.reding@gmail.com>
Newsgroups linux.kernel
Subject Re: [PATCH v3 3/7] PWM: add pwm-stm32 DT bindings
Date 2016-12-05 08:00 +0100
Message-ID <sKVpL-14V-9@gated-at.bofh.it> (permalink)
References <sJT6F-2yH-1@gated-at.bofh.it> <sJT6G-2yH-27@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw

[Multipart message — attachments visible in raw view] - view raw

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.

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

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