diff mbox

[v3,7/7] ARM: dts: stm32: add stm32 general purpose timer driver in DT

Message ID 1480673842-20804-8-git-send-email-benjamin.gaignard@st.com
State Changes Requested
Headers show

Commit Message

Benjamin Gaignard Dec. 2, 2016, 10:17 a.m. UTC
Add general purpose timers and it sub-nodes into DT for stm32f4.
Define and enable pwm1 and pwm3 for stm32f469 discovery board

version 3:
- use "st,stm32-timer-trigger" in DT

version 2:
- use parameters to describe hardware capabilities
- do not use references for pwm and iio timer subnodes

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
 arch/arm/boot/dts/stm32f429.dtsi      | 333 +++++++++++++++++++++++++++++++++-
 arch/arm/boot/dts/stm32f469-disco.dts |  28 +++
 2 files changed, 360 insertions(+), 1 deletion(-)

Comments

Lee Jones Dec. 2, 2016, 1:22 p.m. UTC | #1
On Fri, 02 Dec 2016, Benjamin Gaignard wrote:

> Add general purpose timers and it sub-nodes into DT for stm32f4.
> Define and enable pwm1 and pwm3 for stm32f469 discovery board
> 
> version 3:
> - use "st,stm32-timer-trigger" in DT
> 
> version 2:
> - use parameters to describe hardware capabilities
> - do not use references for pwm and iio timer subnodes
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> ---
>  arch/arm/boot/dts/stm32f429.dtsi      | 333 +++++++++++++++++++++++++++++++++-
>  arch/arm/boot/dts/stm32f469-disco.dts |  28 +++
>  2 files changed, 360 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
> index bca491d..8c50d03 100644
> --- a/arch/arm/boot/dts/stm32f429.dtsi
> +++ b/arch/arm/boot/dts/stm32f429.dtsi
> @@ -48,7 +48,7 @@
>  #include "skeleton.dtsi"
>  #include "armv7-m.dtsi"
>  #include <dt-bindings/pinctrl/stm32f429-pinfunc.h>
> -
> +#include <dt-bindings/iio/timer/st,stm32-timer-triggers.h>
>  / {
>  	clocks {
>  		clk_hse: clk-hse {
> @@ -355,6 +355,21 @@
>  					slew-rate = <2>;
>  				};
>  			};
> +
> +			pwm1_pins: pwm@1 {
> +				pins {
> +					pinmux = <STM32F429_PA8_FUNC_TIM1_CH1>,
> +						 <STM32F429_PB13_FUNC_TIM1_CH1N>,
> +						 <STM32F429_PB12_FUNC_TIM1_BKIN>;
> +				};
> +			};
> +
> +			pwm3_pins: pwm@3 {
> +				pins {
> +					pinmux = <STM32F429_PB4_FUNC_TIM3_CH1>,
> +						 <STM32F429_PB5_FUNC_TIM3_CH2>;
> +				};
> +			};
>  		};
>  
>  		rcc: rcc@40023810 {
> @@ -426,6 +441,322 @@
>  			interrupts = <80>;
>  			clocks = <&rcc 0 38>;
>  		};
> +
> +		gptimer1: gptimer1@40010000 {

timer@xxxxxxx

Node names should be generic and not numbered.

I suggest that this isn't actually a timer either.  Is contains a
timer (and a PWM), but in it's completeness it is not a timer per
say.

> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40010000 0x400>;
> +			clocks = <&rcc 0 160>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			pwm1@0 {
> +				compatible = "st,stm32-pwm";
> +				st,pwm-num-chan = <4>;
> +				st,breakinput;
> +				st,complementary;
> +				status = "disabled";
> +			};
> +
> +			timer1@0 {
> +				compatible = "st,stm32-timer-trigger";
> +				interrupts = <27>;
> +				st,input-triggers-names = TIM5_TRGO,
> +							  TIM2_TRGO,
> +							  TIM4_TRGO,
> +							  TIM3_TRGO;

I'm still dubious with matching by strings.

I'll take a look at the C code to see what the alternatives could be.

> +				st,output-triggers-names = TIM1_TRGO,
> +							   TIM1_CH1,
> +							   TIM1_CH2,
> +							   TIM1_CH3,
> +							   TIM1_CH4;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer2: gptimer2@40000000 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40000000 0x400>;
> +			clocks = <&rcc 0 128>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			pwm2@0 {
> +				compatible = "st,stm32-pwm";
> +				st,pwm-num-chan = <4>;
> +				st,32bits-counter;
> +				status = "disabled";
> +			};
> +
> +			timer2@0 {
> +				compatible = "st,stm32-timer-trigger";
> +				interrupts = <28>;
> +				st,input-triggers-names = TIM1_TRGO,
> +							  TIM8_TRGO,
> +							  TIM3_TRGO,
> +							  TIM4_TRGO;
> +				st,output-triggers-names = TIM2_TRGO,
> +							   TIM2_CH1,
> +							   TIM2_CH2,
> +							   TIM2_CH3,
> +							   TIM2_CH4;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer3: gptimer3@40000400 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40000400 0x400>;
> +			clocks = <&rcc 0 129>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			pwm3@0 {
> +				compatible = "st,stm32-pwm";
> +				st,pwm-num-chan = <4>;
> +				status = "disabled";
> +			};
> +
> +			timer3@0 {
> +				compatible = "st,stm32-timer-trigger";
> +				interrupts = <29>;
> +				st,input-triggers-names = TIM1_TRGO,
> +							  TIM8_TRGO,
> +							  TIM5_TRGO,
> +							  TIM4_TRGO;
> +				st,output-triggers-names = TIM3_TRGO,
> +							   TIM3_CH1,
> +							   TIM3_CH2,
> +							   TIM3_CH3,
> +							   TIM3_CH4;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer4: gptimer4@40000800 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40000800 0x400>;
> +			clocks = <&rcc 0 130>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			pwm4@0 {
> +				compatible = "st,stm32-pwm";
> +				st,pwm-num-chan = <4>;
> +				status = "disabled";
> +			};
> +
> +			timer4@0 {
> +				compatible = "st,stm32-timer-trigger";
> +				interrupts = <30>;
> +				st,input-triggers-names = TIM1_TRGO,
> +							  TIM2_TRGO,
> +							  TIM3_TRGO,
> +							  TIM8_TRGO;
> +				st,output-triggers-names = TIM4_TRGO,
> +							   TIM4_CH1,
> +							   TIM4_CH2,
> +							   TIM4_CH3,
> +							   TIM4_CH4;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer5: gptimer5@40000C00 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40000C00 0x400>;
> +			clocks = <&rcc 0 131>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			pwm5@0 {
> +				compatible = "st,stm32-pwm";
> +				st,pwm-num-chan = <4>;
> +				st,32bits-counter;
> +				status = "disabled";
> +			};
> +
> +			timer5@0 {
> +				compatible = "st,stm32-timer-trigger";
> +				interrupts = <50>;
> +				st,input-triggers-names = TIM2_TRGO,
> +							  TIM3_TRGO,
> +							  TIM4_TRGO,
> +							  TIM8_TRGO;
> +				st,output-triggers-names = TIM5_TRGO,
> +							   TIM5_CH1,
> +							   TIM5_CH2,
> +							   TIM5_CH3,
> +							   TIM5_CH4;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer6: gptimer6@40001000 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40001000 0x400>;
> +			clocks = <&rcc 0 132>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			timer6@0 {
> +				compatible = "st,stm32-timer-trigger";
> +				interrupts = <54>;
> +				st,output-triggers-names = TIM6_TRGO;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer7: gptimer7@40001400 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40001400 0x400>;
> +			clocks = <&rcc 0 133>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			timer7@0 {
> +				compatible = "st,stm32-timer-trigger";
> +				interrupts = <55>;
> +				st,output-triggers-names = TIM7_TRGO;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer8: gptimer8@40010400 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40010400 0x400>;
> +			clocks = <&rcc 0 161>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			pwm8@0 {
> +				compatible = "st,stm32-pwm";
> +				st,pwm-num-chan = <4>;
> +				st,complementary;
> +				st,breakinput;
> +				status = "disabled";
> +			};
> +
> +			timer8@0 {
> +				compatible = "st,stm32-timer-trigger";
> +				interrupts = <46>;
> +				st,input-triggers-names = TIM1_TRGO,
> +							  TIM2_TRGO,
> +							  TIM4_TRGO,
> +							  TIM5_TRGO;
> +				st,output-triggers-names = TIM8_TRGO,
> +							   TIM8_CH1,
> +							   TIM8_CH2,
> +							   TIM8_CH3,
> +							   TIM8_CH4;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer9: gptimer9@40014000 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40014000 0x400>;
> +			clocks = <&rcc 0 176>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			pwm9@0 {
> +				compatible = "st,stm32-pwm";
> +				st,pwm-num-chan = <2>;
> +				status = "disabled";
> +			};
> +
> +			timer9@0 {
> +				compatible = "st,stm32-timer-trigger";
> +				interrupts = <24>;
> +				st,input-triggers-names = TIM2_TRGO,
> +							  TIM3_TRGO;
> +				st,output-triggers-names = TIM9_TRGO,
> +							   TIM9_CH1,
> +							   TIM9_CH2;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer10: gptimer10@40014400 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40014400 0x400>;
> +			clocks = <&rcc 0 177>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			pwm10@0 {
> +				compatible = "st,stm32-pwm";
> +				st,pwm-num-chan = <1>;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer11: gptimer11@40014800 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40014800 0x400>;
> +			clocks = <&rcc 0 178>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			pwm11@0 {
> +				compatible = "st,stm32-pwm";
> +				st,pwm-num-chan = <1>;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer12: gptimer12@40001800 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40001800 0x400>;
> +			clocks = <&rcc 0 134>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			pwm12@0 {
> +				compatible = "st,stm32-pwm";
> +				st,pwm-num-chan = <2>;
> +				status = "disabled";
> +			};
> +
> +			timer12@0 {
> +				compatible = "st,stm32-timer-trigger";
> +				interrupts = <43>;
> +				st,input-triggers-names = TIM4_TRGO,
> +							  TIM5_TRGO;
> +				st,output-triggers-names = TIM12_TRGO,
> +							   TIM12_CH1,
> +							   TIM12_CH2;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer13: gptimer13@40001C00 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40001C00 0x400>;
> +			clocks = <&rcc 0 135>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			pwm13@0 {
> +				compatible = "st,stm32-pwm";
> +				st,pwm-num-chan = <1>;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer14: gptimer14@40002000 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40002000 0x400>;
> +			clocks = <&rcc 0 136>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			pwm14@0 {
> +				compatible = "st,stm32-pwm";
> +				st,pwm-num-chan = <1>;
> +				status = "disabled";
> +			};
> +		};
>  	};
>  };
>  
> diff --git a/arch/arm/boot/dts/stm32f469-disco.dts b/arch/arm/boot/dts/stm32f469-disco.dts
> index 8a163d7..df4ca7e 100644
> --- a/arch/arm/boot/dts/stm32f469-disco.dts
> +++ b/arch/arm/boot/dts/stm32f469-disco.dts
> @@ -81,3 +81,31 @@
>  &usart3 {
>  	status = "okay";
>  };
> +
> +&gptimer1 {
> +	status = "okay";
> +
> +	pwm1@0 {
> +		pinctrl-0	= <&pwm1_pins>;
> +		pinctrl-names	= "default";
> +		status = "okay";
> +	};
> +
> +	timer1@0 {
> +		status = "okay";
> +	};
> +};

This is a much *better* format than before.

I still don't like the '&' syntax though.

> +&gptimer3 {
> +	status = "okay";
> +
> +	pwm3@0 {
> +		pinctrl-0	= <&pwm3_pins>;
> +		pinctrl-names	= "default";
> +		status = "okay";
> +	};
> +
> +	timer3@0 {
> +		status = "okay";
> +	};
> +};
Alexandre TORGUE Dec. 2, 2016, 1:41 p.m. UTC | #2
Hi Benjamin,

On 12/02/2016 11:17 AM, Benjamin Gaignard wrote:
> Add general purpose timers and it sub-nodes into DT for stm32f4.
> Define and enable pwm1 and pwm3 for stm32f469 discovery board
>
> version 3:
> - use "st,stm32-timer-trigger" in DT
>
> version 2:
> - use parameters to describe hardware capabilities
> - do not use references for pwm and iio timer subnodes
>
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> ---
>  arch/arm/boot/dts/stm32f429.dtsi      | 333 +++++++++++++++++++++++++++++++++-
>  arch/arm/boot/dts/stm32f469-disco.dts |  28 +++
>  2 files changed, 360 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
> index bca491d..8c50d03 100644
> --- a/arch/arm/boot/dts/stm32f429.dtsi
> +++ b/arch/arm/boot/dts/stm32f429.dtsi
> @@ -48,7 +48,7 @@
>  #include "skeleton.dtsi"
>  #include "armv7-m.dtsi"
>  #include <dt-bindings/pinctrl/stm32f429-pinfunc.h>
> -
> +#include <dt-bindings/iio/timer/st,stm32-timer-triggers.h>
>  / {
>  	clocks {
>  		clk_hse: clk-hse {
> @@ -355,6 +355,21 @@
>  					slew-rate = <2>;
>  				};
>  			};
> +
> +			pwm1_pins: pwm@1 {
> +				pins {
> +					pinmux = <STM32F429_PA8_FUNC_TIM1_CH1>,
> +						 <STM32F429_PB13_FUNC_TIM1_CH1N>,
> +						 <STM32F429_PB12_FUNC_TIM1_BKIN>;
> +				};
> +			};
> +
> +			pwm3_pins: pwm@3 {
> +				pins {
> +					pinmux = <STM32F429_PB4_FUNC_TIM3_CH1>,
> +						 <STM32F429_PB5_FUNC_TIM3_CH2>;
> +				};
> +			};
>  		};
>
>  		rcc: rcc@40023810 {
> @@ -426,6 +441,322 @@
>  			interrupts = <80>;
>  			clocks = <&rcc 0 38>;
>  		};
> +
> +		gptimer1: gptimer1@40010000 {

Currently, nodes are ordered following base address.

> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40010000 0x400>;
> +			clocks = <&rcc 0 160>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			pwm1@0 {
> +				compatible = "st,stm32-pwm";
> +				st,pwm-num-chan = <4>;
> +				st,breakinput;
> +				st,complementary;
> +				status = "disabled";
> +			};
> +
> +			timer1@0 {
> +				compatible = "st,stm32-timer-trigger";
> +				interrupts = <27>;
> +				st,input-triggers-names = TIM5_TRGO,
> +							  TIM2_TRGO,
> +							  TIM4_TRGO,
> +							  TIM3_TRGO;
> +				st,output-triggers-names = TIM1_TRGO,
> +							   TIM1_CH1,
> +							   TIM1_CH2,
> +							   TIM1_CH3,
> +							   TIM1_CH4;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer2: gptimer2@40000000 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40000000 0x400>;
> +			clocks = <&rcc 0 128>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			pwm2@0 {
> +				compatible = "st,stm32-pwm";
> +				st,pwm-num-chan = <4>;
> +				st,32bits-counter;
> +				status = "disabled";
> +			};
> +
> +			timer2@0 {
> +				compatible = "st,stm32-timer-trigger";
> +				interrupts = <28>;
> +				st,input-triggers-names = TIM1_TRGO,
> +							  TIM8_TRGO,
> +							  TIM3_TRGO,
> +							  TIM4_TRGO;
> +				st,output-triggers-names = TIM2_TRGO,
> +							   TIM2_CH1,
> +							   TIM2_CH2,
> +							   TIM2_CH3,
> +							   TIM2_CH4;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer3: gptimer3@40000400 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40000400 0x400>;
> +			clocks = <&rcc 0 129>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			pwm3@0 {
> +				compatible = "st,stm32-pwm";
> +				st,pwm-num-chan = <4>;
> +				status = "disabled";
> +			};
> +
> +			timer3@0 {
> +				compatible = "st,stm32-timer-trigger";
> +				interrupts = <29>;
> +				st,input-triggers-names = TIM1_TRGO,
> +							  TIM8_TRGO,
> +							  TIM5_TRGO,
> +							  TIM4_TRGO;
> +				st,output-triggers-names = TIM3_TRGO,
> +							   TIM3_CH1,
> +							   TIM3_CH2,
> +							   TIM3_CH3,
> +							   TIM3_CH4;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer4: gptimer4@40000800 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40000800 0x400>;
> +			clocks = <&rcc 0 130>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			pwm4@0 {
> +				compatible = "st,stm32-pwm";
> +				st,pwm-num-chan = <4>;
> +				status = "disabled";
> +			};
> +
> +			timer4@0 {
> +				compatible = "st,stm32-timer-trigger";
> +				interrupts = <30>;
> +				st,input-triggers-names = TIM1_TRGO,
> +							  TIM2_TRGO,
> +							  TIM3_TRGO,
> +							  TIM8_TRGO;
> +				st,output-triggers-names = TIM4_TRGO,
> +							   TIM4_CH1,
> +							   TIM4_CH2,
> +							   TIM4_CH3,
> +							   TIM4_CH4;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer5: gptimer5@40000C00 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40000C00 0x400>;
> +			clocks = <&rcc 0 131>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			pwm5@0 {
> +				compatible = "st,stm32-pwm";
> +				st,pwm-num-chan = <4>;
> +				st,32bits-counter;
> +				status = "disabled";
> +			};
> +
> +			timer5@0 {
> +				compatible = "st,stm32-timer-trigger";
> +				interrupts = <50>;
> +				st,input-triggers-names = TIM2_TRGO,
> +							  TIM3_TRGO,
> +							  TIM4_TRGO,
> +							  TIM8_TRGO;
> +				st,output-triggers-names = TIM5_TRGO,
> +							   TIM5_CH1,
> +							   TIM5_CH2,
> +							   TIM5_CH3,
> +							   TIM5_CH4;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer6: gptimer6@40001000 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40001000 0x400>;
> +			clocks = <&rcc 0 132>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			timer6@0 {
> +				compatible = "st,stm32-timer-trigger";
> +				interrupts = <54>;
> +				st,output-triggers-names = TIM6_TRGO;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer7: gptimer7@40001400 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40001400 0x400>;
> +			clocks = <&rcc 0 133>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			timer7@0 {
> +				compatible = "st,stm32-timer-trigger";
> +				interrupts = <55>;
> +				st,output-triggers-names = TIM7_TRGO;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer8: gptimer8@40010400 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40010400 0x400>;
> +			clocks = <&rcc 0 161>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			pwm8@0 {
> +				compatible = "st,stm32-pwm";
> +				st,pwm-num-chan = <4>;
> +				st,complementary;
> +				st,breakinput;
> +				status = "disabled";
> +			};
> +
> +			timer8@0 {
> +				compatible = "st,stm32-timer-trigger";
> +				interrupts = <46>;
> +				st,input-triggers-names = TIM1_TRGO,
> +							  TIM2_TRGO,
> +							  TIM4_TRGO,
> +							  TIM5_TRGO;
> +				st,output-triggers-names = TIM8_TRGO,
> +							   TIM8_CH1,
> +							   TIM8_CH2,
> +							   TIM8_CH3,
> +							   TIM8_CH4;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer9: gptimer9@40014000 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40014000 0x400>;
> +			clocks = <&rcc 0 176>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			pwm9@0 {
> +				compatible = "st,stm32-pwm";
> +				st,pwm-num-chan = <2>;
> +				status = "disabled";
> +			};
> +
> +			timer9@0 {
> +				compatible = "st,stm32-timer-trigger";
> +				interrupts = <24>;
> +				st,input-triggers-names = TIM2_TRGO,
> +							  TIM3_TRGO;
> +				st,output-triggers-names = TIM9_TRGO,
> +							   TIM9_CH1,
> +							   TIM9_CH2;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer10: gptimer10@40014400 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40014400 0x400>;
> +			clocks = <&rcc 0 177>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			pwm10@0 {
> +				compatible = "st,stm32-pwm";
> +				st,pwm-num-chan = <1>;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer11: gptimer11@40014800 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40014800 0x400>;
> +			clocks = <&rcc 0 178>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			pwm11@0 {
> +				compatible = "st,stm32-pwm";
> +				st,pwm-num-chan = <1>;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer12: gptimer12@40001800 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40001800 0x400>;
> +			clocks = <&rcc 0 134>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			pwm12@0 {
> +				compatible = "st,stm32-pwm";
> +				st,pwm-num-chan = <2>;
> +				status = "disabled";
> +			};
> +
> +			timer12@0 {
> +				compatible = "st,stm32-timer-trigger";
> +				interrupts = <43>;
> +				st,input-triggers-names = TIM4_TRGO,
> +							  TIM5_TRGO;
> +				st,output-triggers-names = TIM12_TRGO,
> +							   TIM12_CH1,
> +							   TIM12_CH2;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer13: gptimer13@40001C00 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40001C00 0x400>;
> +			clocks = <&rcc 0 135>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			pwm13@0 {
> +				compatible = "st,stm32-pwm";
> +				st,pwm-num-chan = <1>;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer14: gptimer14@40002000 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40002000 0x400>;
> +			clocks = <&rcc 0 136>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			pwm14@0 {
> +				compatible = "st,stm32-pwm";
> +				st,pwm-num-chan = <1>;
> +				status = "disabled";
> +			};
> +		};
>  	};
>  };
>
> diff --git a/arch/arm/boot/dts/stm32f469-disco.dts b/arch/arm/boot/dts/stm32f469-disco.dts
> index 8a163d7..df4ca7e 100644
> --- a/arch/arm/boot/dts/stm32f469-disco.dts
> +++ b/arch/arm/boot/dts/stm32f469-disco.dts
> @@ -81,3 +81,31 @@
>  &usart3 {
>  	status = "okay";
>  };
> +
> +&gptimer1 {
> +	status = "okay";
> +
> +	pwm1@0 {
> +		pinctrl-0	= <&pwm1_pins>;
> +		pinctrl-names	= "default";
> +		status = "okay";
> +	};
> +
> +	timer1@0 {
> +		status = "okay";
> +	};
> +};
> +
> +&gptimer3 {
> +	status = "okay";
> +
> +	pwm3@0 {
> +		pinctrl-0	= <&pwm3_pins>;
> +		pinctrl-names	= "default";
> +		status = "okay";
> +	};
> +
> +	timer3@0 {
> +		status = "okay";
> +	};
> +};
>
--
To unsubscribe from this list: send the line "unsubscribe linux-pwm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jonathan Cameron Dec. 3, 2016, 9:42 a.m. UTC | #3
On 02/12/16 13:22, Lee Jones wrote:
> On Fri, 02 Dec 2016, Benjamin Gaignard wrote:
> 
>> Add general purpose timers and it sub-nodes into DT for stm32f4.
>> Define and enable pwm1 and pwm3 for stm32f469 discovery board
>>
>> version 3:
>> - use "st,stm32-timer-trigger" in DT
>>
>> version 2:
>> - use parameters to describe hardware capabilities
>> - do not use references for pwm and iio timer subnodes
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>> ---
>>  arch/arm/boot/dts/stm32f429.dtsi      | 333 +++++++++++++++++++++++++++++++++-
>>  arch/arm/boot/dts/stm32f469-disco.dts |  28 +++
>>  2 files changed, 360 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
>> index bca491d..8c50d03 100644
>> --- a/arch/arm/boot/dts/stm32f429.dtsi
>> +++ b/arch/arm/boot/dts/stm32f429.dtsi
>> @@ -48,7 +48,7 @@
>>  #include "skeleton.dtsi"
>>  #include "armv7-m.dtsi"
>>  #include <dt-bindings/pinctrl/stm32f429-pinfunc.h>
>> -
>> +#include <dt-bindings/iio/timer/st,stm32-timer-triggers.h>
>>  / {
>>  	clocks {
>>  		clk_hse: clk-hse {
>> @@ -355,6 +355,21 @@
>>  					slew-rate = <2>;
>>  				};
>>  			};
>> +
>> +			pwm1_pins: pwm@1 {
>> +				pins {
>> +					pinmux = <STM32F429_PA8_FUNC_TIM1_CH1>,
>> +						 <STM32F429_PB13_FUNC_TIM1_CH1N>,
>> +						 <STM32F429_PB12_FUNC_TIM1_BKIN>;
>> +				};
>> +			};
>> +
>> +			pwm3_pins: pwm@3 {
>> +				pins {
>> +					pinmux = <STM32F429_PB4_FUNC_TIM3_CH1>,
>> +						 <STM32F429_PB5_FUNC_TIM3_CH2>;
>> +				};
>> +			};
>>  		};
>>  
>>  		rcc: rcc@40023810 {
>> @@ -426,6 +441,322 @@
>>  			interrupts = <80>;
>>  			clocks = <&rcc 0 38>;
>>  		};
>> +
>> +		gptimer1: gptimer1@40010000 {
> 
> timer@xxxxxxx
> 
> Node names should be generic and not numbered.
> 
> I suggest that this isn't actually a timer either.  Is contains a
> timer (and a PWM), but in it's completeness it is not a timer per
> say.
That's just mean ;)  At least suggest an alternative?

stm32-gptimerish-magic?


> 
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40010000 0x400>;
>> +			clocks = <&rcc 0 160>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			pwm1@0 {
>> +				compatible = "st,stm32-pwm";
>> +				st,pwm-num-chan = <4>;
>> +				st,breakinput;
>> +				st,complementary;
>> +				status = "disabled";
>> +			};
>> +
>> +			timer1@0 {
>> +				compatible = "st,stm32-timer-trigger";
>> +				interrupts = <27>;
>> +				st,input-triggers-names = TIM5_TRGO,
>> +							  TIM2_TRGO,
>> +							  TIM4_TRGO,
>> +							  TIM3_TRGO;
> 
> I'm still dubious with matching by strings.
> 
> I'll take a look at the C code to see what the alternatives could be.
> 
>> +				st,output-triggers-names = TIM1_TRGO,
>> +							   TIM1_CH1,
>> +							   TIM1_CH2,
>> +							   TIM1_CH3,
>> +							   TIM1_CH4;
>> +				status = "disabled";
>> +			};
>> +		};
>> +
>> +		gptimer2: gptimer2@40000000 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40000000 0x400>;
>> +			clocks = <&rcc 0 128>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			pwm2@0 {
>> +				compatible = "st,stm32-pwm";
>> +				st,pwm-num-chan = <4>;
>> +				st,32bits-counter;
>> +				status = "disabled";
>> +			};
>> +
>> +			timer2@0 {
>> +				compatible = "st,stm32-timer-trigger";
>> +				interrupts = <28>;
>> +				st,input-triggers-names = TIM1_TRGO,
>> +							  TIM8_TRGO,
>> +							  TIM3_TRGO,
>> +							  TIM4_TRGO;
>> +				st,output-triggers-names = TIM2_TRGO,
>> +							   TIM2_CH1,
>> +							   TIM2_CH2,
>> +							   TIM2_CH3,
>> +							   TIM2_CH4;
>> +				status = "disabled";
>> +			};
>> +		};
>> +
>> +		gptimer3: gptimer3@40000400 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40000400 0x400>;
>> +			clocks = <&rcc 0 129>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			pwm3@0 {
>> +				compatible = "st,stm32-pwm";
>> +				st,pwm-num-chan = <4>;
>> +				status = "disabled";
>> +			};
>> +
>> +			timer3@0 {
>> +				compatible = "st,stm32-timer-trigger";
>> +				interrupts = <29>;
>> +				st,input-triggers-names = TIM1_TRGO,
>> +							  TIM8_TRGO,
>> +							  TIM5_TRGO,
>> +							  TIM4_TRGO;
>> +				st,output-triggers-names = TIM3_TRGO,
>> +							   TIM3_CH1,
>> +							   TIM3_CH2,
>> +							   TIM3_CH3,
>> +							   TIM3_CH4;
>> +				status = "disabled";
>> +			};
>> +		};
>> +
>> +		gptimer4: gptimer4@40000800 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40000800 0x400>;
>> +			clocks = <&rcc 0 130>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			pwm4@0 {
>> +				compatible = "st,stm32-pwm";
>> +				st,pwm-num-chan = <4>;
>> +				status = "disabled";
>> +			};
>> +
>> +			timer4@0 {
>> +				compatible = "st,stm32-timer-trigger";
>> +				interrupts = <30>;
>> +				st,input-triggers-names = TIM1_TRGO,
>> +							  TIM2_TRGO,
>> +							  TIM3_TRGO,
>> +							  TIM8_TRGO;
>> +				st,output-triggers-names = TIM4_TRGO,
>> +							   TIM4_CH1,
>> +							   TIM4_CH2,
>> +							   TIM4_CH3,
>> +							   TIM4_CH4;
>> +				status = "disabled";
>> +			};
>> +		};
>> +
>> +		gptimer5: gptimer5@40000C00 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40000C00 0x400>;
>> +			clocks = <&rcc 0 131>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			pwm5@0 {
>> +				compatible = "st,stm32-pwm";
>> +				st,pwm-num-chan = <4>;
>> +				st,32bits-counter;
>> +				status = "disabled";
>> +			};
>> +
>> +			timer5@0 {
>> +				compatible = "st,stm32-timer-trigger";
>> +				interrupts = <50>;
>> +				st,input-triggers-names = TIM2_TRGO,
>> +							  TIM3_TRGO,
>> +							  TIM4_TRGO,
>> +							  TIM8_TRGO;
>> +				st,output-triggers-names = TIM5_TRGO,
>> +							   TIM5_CH1,
>> +							   TIM5_CH2,
>> +							   TIM5_CH3,
>> +							   TIM5_CH4;
>> +				status = "disabled";
>> +			};
>> +		};
>> +
>> +		gptimer6: gptimer6@40001000 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40001000 0x400>;
>> +			clocks = <&rcc 0 132>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			timer6@0 {
>> +				compatible = "st,stm32-timer-trigger";
>> +				interrupts = <54>;
>> +				st,output-triggers-names = TIM6_TRGO;
>> +				status = "disabled";
>> +			};
>> +		};
>> +
>> +		gptimer7: gptimer7@40001400 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40001400 0x400>;
>> +			clocks = <&rcc 0 133>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			timer7@0 {
>> +				compatible = "st,stm32-timer-trigger";
>> +				interrupts = <55>;
>> +				st,output-triggers-names = TIM7_TRGO;
>> +				status = "disabled";
>> +			};
>> +		};
>> +
>> +		gptimer8: gptimer8@40010400 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40010400 0x400>;
>> +			clocks = <&rcc 0 161>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			pwm8@0 {
>> +				compatible = "st,stm32-pwm";
>> +				st,pwm-num-chan = <4>;
>> +				st,complementary;
>> +				st,breakinput;
>> +				status = "disabled";
>> +			};
>> +
>> +			timer8@0 {
>> +				compatible = "st,stm32-timer-trigger";
>> +				interrupts = <46>;
>> +				st,input-triggers-names = TIM1_TRGO,
>> +							  TIM2_TRGO,
>> +							  TIM4_TRGO,
>> +							  TIM5_TRGO;
>> +				st,output-triggers-names = TIM8_TRGO,
>> +							   TIM8_CH1,
>> +							   TIM8_CH2,
>> +							   TIM8_CH3,
>> +							   TIM8_CH4;
>> +				status = "disabled";
>> +			};
>> +		};
>> +
>> +		gptimer9: gptimer9@40014000 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40014000 0x400>;
>> +			clocks = <&rcc 0 176>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			pwm9@0 {
>> +				compatible = "st,stm32-pwm";
>> +				st,pwm-num-chan = <2>;
>> +				status = "disabled";
>> +			};
>> +
>> +			timer9@0 {
>> +				compatible = "st,stm32-timer-trigger";
>> +				interrupts = <24>;
>> +				st,input-triggers-names = TIM2_TRGO,
>> +							  TIM3_TRGO;
>> +				st,output-triggers-names = TIM9_TRGO,
>> +							   TIM9_CH1,
>> +							   TIM9_CH2;
>> +				status = "disabled";
>> +			};
>> +		};
>> +
>> +		gptimer10: gptimer10@40014400 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40014400 0x400>;
>> +			clocks = <&rcc 0 177>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			pwm10@0 {
>> +				compatible = "st,stm32-pwm";
>> +				st,pwm-num-chan = <1>;
>> +				status = "disabled";
>> +			};
>> +		};
>> +
>> +		gptimer11: gptimer11@40014800 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40014800 0x400>;
>> +			clocks = <&rcc 0 178>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			pwm11@0 {
>> +				compatible = "st,stm32-pwm";
>> +				st,pwm-num-chan = <1>;
>> +				status = "disabled";
>> +			};
>> +		};
>> +
>> +		gptimer12: gptimer12@40001800 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40001800 0x400>;
>> +			clocks = <&rcc 0 134>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			pwm12@0 {
>> +				compatible = "st,stm32-pwm";
>> +				st,pwm-num-chan = <2>;
>> +				status = "disabled";
>> +			};
>> +
>> +			timer12@0 {
>> +				compatible = "st,stm32-timer-trigger";
>> +				interrupts = <43>;
>> +				st,input-triggers-names = TIM4_TRGO,
>> +							  TIM5_TRGO;
>> +				st,output-triggers-names = TIM12_TRGO,
>> +							   TIM12_CH1,
>> +							   TIM12_CH2;
>> +				status = "disabled";
>> +			};
>> +		};
>> +
>> +		gptimer13: gptimer13@40001C00 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40001C00 0x400>;
>> +			clocks = <&rcc 0 135>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			pwm13@0 {
>> +				compatible = "st,stm32-pwm";
>> +				st,pwm-num-chan = <1>;
>> +				status = "disabled";
>> +			};
>> +		};
>> +
>> +		gptimer14: gptimer14@40002000 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40002000 0x400>;
>> +			clocks = <&rcc 0 136>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			pwm14@0 {
>> +				compatible = "st,stm32-pwm";
>> +				st,pwm-num-chan = <1>;
>> +				status = "disabled";
>> +			};
>> +		};
>>  	};
>>  };
>>  
>> diff --git a/arch/arm/boot/dts/stm32f469-disco.dts b/arch/arm/boot/dts/stm32f469-disco.dts
>> index 8a163d7..df4ca7e 100644
>> --- a/arch/arm/boot/dts/stm32f469-disco.dts
>> +++ b/arch/arm/boot/dts/stm32f469-disco.dts
>> @@ -81,3 +81,31 @@
>>  &usart3 {
>>  	status = "okay";
>>  };
>> +
>> +&gptimer1 {
>> +	status = "okay";
>> +
>> +	pwm1@0 {
>> +		pinctrl-0	= <&pwm1_pins>;
>> +		pinctrl-names	= "default";
>> +		status = "okay";
>> +	};
>> +
>> +	timer1@0 {
>> +		status = "okay";
>> +	};
>> +};
> 
> This is a much *better* format than before.
> 
> I still don't like the '&' syntax though.
> 
>> +&gptimer3 {
>> +	status = "okay";
>> +
>> +	pwm3@0 {
>> +		pinctrl-0	= <&pwm3_pins>;
>> +		pinctrl-names	= "default";
>> +		status = "okay";
>> +	};
>> +
>> +	timer3@0 {
>> +		status = "okay";
>> +	};
>> +};
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-pwm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jonathan Cameron Dec. 3, 2016, 9:45 a.m. UTC | #4
On 02/12/16 10:17, Benjamin Gaignard wrote:
> Add general purpose timers and it sub-nodes into DT for stm32f4.
> Define and enable pwm1 and pwm3 for stm32f469 discovery board
> 
> version 3:
> - use "st,stm32-timer-trigger" in DT
> 
> version 2:
> - use parameters to describe hardware capabilities
> - do not use references for pwm and iio timer subnodes
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
This didn't come out nearly as badly as I thought it would.

Seems we don't need the multiple compatibles which is good!

> ---
>  arch/arm/boot/dts/stm32f429.dtsi      | 333 +++++++++++++++++++++++++++++++++-
>  arch/arm/boot/dts/stm32f469-disco.dts |  28 +++
>  2 files changed, 360 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
> index bca491d..8c50d03 100644
> --- a/arch/arm/boot/dts/stm32f429.dtsi
> +++ b/arch/arm/boot/dts/stm32f429.dtsi
> @@ -48,7 +48,7 @@
>  #include "skeleton.dtsi"
>  #include "armv7-m.dtsi"
>  #include <dt-bindings/pinctrl/stm32f429-pinfunc.h>
> -
> +#include <dt-bindings/iio/timer/st,stm32-timer-triggers.h>
>  / {
>  	clocks {
>  		clk_hse: clk-hse {
> @@ -355,6 +355,21 @@
>  					slew-rate = <2>;
>  				};
>  			};
> +
> +			pwm1_pins: pwm@1 {
> +				pins {
> +					pinmux = <STM32F429_PA8_FUNC_TIM1_CH1>,
> +						 <STM32F429_PB13_FUNC_TIM1_CH1N>,
> +						 <STM32F429_PB12_FUNC_TIM1_BKIN>;
> +				};
> +			};
> +
> +			pwm3_pins: pwm@3 {
> +				pins {
> +					pinmux = <STM32F429_PB4_FUNC_TIM3_CH1>,
> +						 <STM32F429_PB5_FUNC_TIM3_CH2>;
> +				};
> +			};
>  		};
>  
>  		rcc: rcc@40023810 {
> @@ -426,6 +441,322 @@
>  			interrupts = <80>;
>  			clocks = <&rcc 0 38>;
>  		};
> +
> +		gptimer1: gptimer1@40010000 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40010000 0x400>;
> +			clocks = <&rcc 0 160>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			pwm1@0 {
> +				compatible = "st,stm32-pwm";
> +				st,pwm-num-chan = <4>;
> +				st,breakinput;
> +				st,complementary;
> +				status = "disabled";
> +			};
> +
> +			timer1@0 {
> +				compatible = "st,stm32-timer-trigger";
> +				interrupts = <27>;
> +				st,input-triggers-names = TIM5_TRGO,
> +							  TIM2_TRGO,
> +							  TIM4_TRGO,
> +							  TIM3_TRGO;
> +				st,output-triggers-names = TIM1_TRGO,
> +							   TIM1_CH1,
> +							   TIM1_CH2,
> +							   TIM1_CH3,
> +							   TIM1_CH4;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer2: gptimer2@40000000 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40000000 0x400>;
> +			clocks = <&rcc 0 128>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			pwm2@0 {
> +				compatible = "st,stm32-pwm";
> +				st,pwm-num-chan = <4>;
> +				st,32bits-counter;
> +				status = "disabled";
> +			};
> +
> +			timer2@0 {
> +				compatible = "st,stm32-timer-trigger";
> +				interrupts = <28>;
> +				st,input-triggers-names = TIM1_TRGO,
> +							  TIM8_TRGO,
> +							  TIM3_TRGO,
> +							  TIM4_TRGO;
> +				st,output-triggers-names = TIM2_TRGO,
> +							   TIM2_CH1,
> +							   TIM2_CH2,
> +							   TIM2_CH3,
> +							   TIM2_CH4;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer3: gptimer3@40000400 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40000400 0x400>;
> +			clocks = <&rcc 0 129>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			pwm3@0 {
> +				compatible = "st,stm32-pwm";
> +				st,pwm-num-chan = <4>;
> +				status = "disabled";
> +			};
> +
> +			timer3@0 {
> +				compatible = "st,stm32-timer-trigger";
> +				interrupts = <29>;
> +				st,input-triggers-names = TIM1_TRGO,
> +							  TIM8_TRGO,
> +							  TIM5_TRGO,
> +							  TIM4_TRGO;
> +				st,output-triggers-names = TIM3_TRGO,
> +							   TIM3_CH1,
> +							   TIM3_CH2,
> +							   TIM3_CH3,
> +							   TIM3_CH4;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer4: gptimer4@40000800 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40000800 0x400>;
> +			clocks = <&rcc 0 130>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			pwm4@0 {
> +				compatible = "st,stm32-pwm";
> +				st,pwm-num-chan = <4>;
> +				status = "disabled";
> +			};
> +
> +			timer4@0 {
> +				compatible = "st,stm32-timer-trigger";
> +				interrupts = <30>;
> +				st,input-triggers-names = TIM1_TRGO,
> +							  TIM2_TRGO,
> +							  TIM3_TRGO,
> +							  TIM8_TRGO;
> +				st,output-triggers-names = TIM4_TRGO,
> +							   TIM4_CH1,
> +							   TIM4_CH2,
> +							   TIM4_CH3,
> +							   TIM4_CH4;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer5: gptimer5@40000C00 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40000C00 0x400>;
> +			clocks = <&rcc 0 131>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			pwm5@0 {
> +				compatible = "st,stm32-pwm";
> +				st,pwm-num-chan = <4>;
> +				st,32bits-counter;
> +				status = "disabled";
> +			};
> +
> +			timer5@0 {
> +				compatible = "st,stm32-timer-trigger";
> +				interrupts = <50>;
> +				st,input-triggers-names = TIM2_TRGO,
> +							  TIM3_TRGO,
> +							  TIM4_TRGO,
> +							  TIM8_TRGO;
> +				st,output-triggers-names = TIM5_TRGO,
> +							   TIM5_CH1,
> +							   TIM5_CH2,
> +							   TIM5_CH3,
> +							   TIM5_CH4;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer6: gptimer6@40001000 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40001000 0x400>;
> +			clocks = <&rcc 0 132>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			timer6@0 {
> +				compatible = "st,stm32-timer-trigger";
> +				interrupts = <54>;
> +				st,output-triggers-names = TIM6_TRGO;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer7: gptimer7@40001400 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40001400 0x400>;
> +			clocks = <&rcc 0 133>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			timer7@0 {
> +				compatible = "st,stm32-timer-trigger";
> +				interrupts = <55>;
> +				st,output-triggers-names = TIM7_TRGO;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer8: gptimer8@40010400 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40010400 0x400>;
> +			clocks = <&rcc 0 161>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			pwm8@0 {
> +				compatible = "st,stm32-pwm";
> +				st,pwm-num-chan = <4>;
> +				st,complementary;
> +				st,breakinput;
> +				status = "disabled";
> +			};
> +
> +			timer8@0 {
> +				compatible = "st,stm32-timer-trigger";
> +				interrupts = <46>;
> +				st,input-triggers-names = TIM1_TRGO,
> +							  TIM2_TRGO,
> +							  TIM4_TRGO,
> +							  TIM5_TRGO;
> +				st,output-triggers-names = TIM8_TRGO,
> +							   TIM8_CH1,
> +							   TIM8_CH2,
> +							   TIM8_CH3,
> +							   TIM8_CH4;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer9: gptimer9@40014000 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40014000 0x400>;
> +			clocks = <&rcc 0 176>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			pwm9@0 {
> +				compatible = "st,stm32-pwm";
> +				st,pwm-num-chan = <2>;
> +				status = "disabled";
> +			};
> +
> +			timer9@0 {
> +				compatible = "st,stm32-timer-trigger";
> +				interrupts = <24>;
> +				st,input-triggers-names = TIM2_TRGO,
> +							  TIM3_TRGO;
> +				st,output-triggers-names = TIM9_TRGO,
> +							   TIM9_CH1,
> +							   TIM9_CH2;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer10: gptimer10@40014400 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40014400 0x400>;
> +			clocks = <&rcc 0 177>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			pwm10@0 {
> +				compatible = "st,stm32-pwm";
> +				st,pwm-num-chan = <1>;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer11: gptimer11@40014800 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40014800 0x400>;
> +			clocks = <&rcc 0 178>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			pwm11@0 {
> +				compatible = "st,stm32-pwm";
> +				st,pwm-num-chan = <1>;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer12: gptimer12@40001800 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40001800 0x400>;
> +			clocks = <&rcc 0 134>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			pwm12@0 {
> +				compatible = "st,stm32-pwm";
> +				st,pwm-num-chan = <2>;
> +				status = "disabled";
> +			};
> +
> +			timer12@0 {
> +				compatible = "st,stm32-timer-trigger";
> +				interrupts = <43>;
> +				st,input-triggers-names = TIM4_TRGO,
> +							  TIM5_TRGO;
> +				st,output-triggers-names = TIM12_TRGO,
> +							   TIM12_CH1,
> +							   TIM12_CH2;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer13: gptimer13@40001C00 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40001C00 0x400>;
> +			clocks = <&rcc 0 135>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			pwm13@0 {
> +				compatible = "st,stm32-pwm";
> +				st,pwm-num-chan = <1>;
> +				status = "disabled";
> +			};
> +		};
> +
> +		gptimer14: gptimer14@40002000 {
> +			compatible = "st,stm32-gptimer";
> +			reg = <0x40002000 0x400>;
> +			clocks = <&rcc 0 136>;
> +			clock-names = "clk_int";
> +			status = "disabled";
> +
> +			pwm14@0 {
> +				compatible = "st,stm32-pwm";
> +				st,pwm-num-chan = <1>;
> +				status = "disabled";
> +			};
> +		};
>  	};
>  };
>  
> diff --git a/arch/arm/boot/dts/stm32f469-disco.dts b/arch/arm/boot/dts/stm32f469-disco.dts
> index 8a163d7..df4ca7e 100644
> --- a/arch/arm/boot/dts/stm32f469-disco.dts
> +++ b/arch/arm/boot/dts/stm32f469-disco.dts
> @@ -81,3 +81,31 @@
>  &usart3 {
>  	status = "okay";
>  };
> +
> +&gptimer1 {
> +	status = "okay";
> +
> +	pwm1@0 {
> +		pinctrl-0	= <&pwm1_pins>;
> +		pinctrl-names	= "default";
> +		status = "okay";
> +	};
> +
> +	timer1@0 {
> +		status = "okay";
> +	};
> +};
> +
> +&gptimer3 {
> +	status = "okay";
> +
> +	pwm3@0 {
> +		pinctrl-0	= <&pwm3_pins>;
> +		pinctrl-names	= "default";
> +		status = "okay";
> +	};
> +
> +	timer3@0 {
> +		status = "okay";
> +	};
> +};
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-pwm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lee Jones Dec. 5, 2016, 8:54 a.m. UTC | #5
On Sat, 03 Dec 2016, Jonathan Cameron wrote:
> On 02/12/16 13:22, Lee Jones wrote:
> > On Fri, 02 Dec 2016, Benjamin Gaignard wrote:
> > 
> >> Add general purpose timers and it sub-nodes into DT for stm32f4.
> >> Define and enable pwm1 and pwm3 for stm32f469 discovery board
> >>
> >> version 3:
> >> - use "st,stm32-timer-trigger" in DT
> >>
> >> version 2:
> >> - use parameters to describe hardware capabilities
> >> - do not use references for pwm and iio timer subnodes
> >>
> >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> >> ---
> >>  arch/arm/boot/dts/stm32f429.dtsi      | 333 +++++++++++++++++++++++++++++++++-
> >>  arch/arm/boot/dts/stm32f469-disco.dts |  28 +++
> >>  2 files changed, 360 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
> >> index bca491d..8c50d03 100644
> >> --- a/arch/arm/boot/dts/stm32f429.dtsi
> >> +++ b/arch/arm/boot/dts/stm32f429.dtsi
> >> @@ -48,7 +48,7 @@
> >>  #include "skeleton.dtsi"
> >>  #include "armv7-m.dtsi"
> >>  #include <dt-bindings/pinctrl/stm32f429-pinfunc.h>
> >> -
> >> +#include <dt-bindings/iio/timer/st,stm32-timer-triggers.h>
> >>  / {
> >>  	clocks {
> >>  		clk_hse: clk-hse {
> >> @@ -355,6 +355,21 @@
> >>  					slew-rate = <2>;
> >>  				};
> >>  			};
> >> +
> >> +			pwm1_pins: pwm@1 {
> >> +				pins {
> >> +					pinmux = <STM32F429_PA8_FUNC_TIM1_CH1>,
> >> +						 <STM32F429_PB13_FUNC_TIM1_CH1N>,
> >> +						 <STM32F429_PB12_FUNC_TIM1_BKIN>;
> >> +				};
> >> +			};
> >> +
> >> +			pwm3_pins: pwm@3 {
> >> +				pins {
> >> +					pinmux = <STM32F429_PB4_FUNC_TIM3_CH1>,
> >> +						 <STM32F429_PB5_FUNC_TIM3_CH2>;
> >> +				};
> >> +			};
> >>  		};
> >>  
> >>  		rcc: rcc@40023810 {
> >> @@ -426,6 +441,322 @@
> >>  			interrupts = <80>;
> >>  			clocks = <&rcc 0 38>;
> >>  		};
> >> +
> >> +		gptimer1: gptimer1@40010000 {
> > 
> > timer@xxxxxxx
> > 
> > Node names should be generic and not numbered.
> > 
> > I suggest that this isn't actually a timer either.  Is contains a
> > timer (and a PWM), but in it's completeness it is not a timer per
> > say.
> That's just mean ;)  At least suggest an alternative?
> 
> stm32-gptimerish-magic?

Perfect! ;)

I already did:
  https://lkml.org/lkml/2016/11/23/131

> >> +			compatible = "st,stm32-gptimer";
> >> +			reg = <0x40010000 0x400>;
> >> +			clocks = <&rcc 0 160>;
> >> +			clock-names = "clk_int";
> >> +			status = "disabled";
> >> +
> >> +			pwm1@0 {
> >> +				compatible = "st,stm32-pwm";
> >> +				st,pwm-num-chan = <4>;
> >> +				st,breakinput;
> >> +				st,complementary;
> >> +				status = "disabled";
> >> +			};
> >> +
> >> +			timer1@0 {
> >> +				compatible = "st,stm32-timer-trigger";
> >> +				interrupts = <27>;
> >> +				st,input-triggers-names = TIM5_TRGO,
> >> +							  TIM2_TRGO,
> >> +							  TIM4_TRGO,
> >> +							  TIM3_TRGO;
> > 
> > I'm still dubious with matching by strings.
> > 
> > I'll take a look at the C code to see what the alternatives could be.
> > 
> >> +				st,output-triggers-names = TIM1_TRGO,
> >> +							   TIM1_CH1,
> >> +							   TIM1_CH2,
> >> +							   TIM1_CH3,
> >> +							   TIM1_CH4;
> >> +				status = "disabled";
> >> +			};
> >> +		};
> >> +
> >> +		gptimer2: gptimer2@40000000 {
> >> +			compatible = "st,stm32-gptimer";
> >> +			reg = <0x40000000 0x400>;
> >> +			clocks = <&rcc 0 128>;
> >> +			clock-names = "clk_int";
> >> +			status = "disabled";
> >> +
> >> +			pwm2@0 {
> >> +				compatible = "st,stm32-pwm";
> >> +				st,pwm-num-chan = <4>;
> >> +				st,32bits-counter;
> >> +				status = "disabled";
> >> +			};
> >> +
> >> +			timer2@0 {
> >> +				compatible = "st,stm32-timer-trigger";
> >> +				interrupts = <28>;
> >> +				st,input-triggers-names = TIM1_TRGO,
> >> +							  TIM8_TRGO,
> >> +							  TIM3_TRGO,
> >> +							  TIM4_TRGO;
> >> +				st,output-triggers-names = TIM2_TRGO,
> >> +							   TIM2_CH1,
> >> +							   TIM2_CH2,
> >> +							   TIM2_CH3,
> >> +							   TIM2_CH4;
> >> +				status = "disabled";
> >> +			};
> >> +		};
> >> +
> >> +		gptimer3: gptimer3@40000400 {
> >> +			compatible = "st,stm32-gptimer";
> >> +			reg = <0x40000400 0x400>;
> >> +			clocks = <&rcc 0 129>;
> >> +			clock-names = "clk_int";
> >> +			status = "disabled";
> >> +
> >> +			pwm3@0 {
> >> +				compatible = "st,stm32-pwm";
> >> +				st,pwm-num-chan = <4>;
> >> +				status = "disabled";
> >> +			};
> >> +
> >> +			timer3@0 {
> >> +				compatible = "st,stm32-timer-trigger";
> >> +				interrupts = <29>;
> >> +				st,input-triggers-names = TIM1_TRGO,
> >> +							  TIM8_TRGO,
> >> +							  TIM5_TRGO,
> >> +							  TIM4_TRGO;
> >> +				st,output-triggers-names = TIM3_TRGO,
> >> +							   TIM3_CH1,
> >> +							   TIM3_CH2,
> >> +							   TIM3_CH3,
> >> +							   TIM3_CH4;
> >> +				status = "disabled";
> >> +			};
> >> +		};
> >> +
> >> +		gptimer4: gptimer4@40000800 {
> >> +			compatible = "st,stm32-gptimer";
> >> +			reg = <0x40000800 0x400>;
> >> +			clocks = <&rcc 0 130>;
> >> +			clock-names = "clk_int";
> >> +			status = "disabled";
> >> +
> >> +			pwm4@0 {
> >> +				compatible = "st,stm32-pwm";
> >> +				st,pwm-num-chan = <4>;
> >> +				status = "disabled";
> >> +			};
> >> +
> >> +			timer4@0 {
> >> +				compatible = "st,stm32-timer-trigger";
> >> +				interrupts = <30>;
> >> +				st,input-triggers-names = TIM1_TRGO,
> >> +							  TIM2_TRGO,
> >> +							  TIM3_TRGO,
> >> +							  TIM8_TRGO;
> >> +				st,output-triggers-names = TIM4_TRGO,
> >> +							   TIM4_CH1,
> >> +							   TIM4_CH2,
> >> +							   TIM4_CH3,
> >> +							   TIM4_CH4;
> >> +				status = "disabled";
> >> +			};
> >> +		};
> >> +
> >> +		gptimer5: gptimer5@40000C00 {
> >> +			compatible = "st,stm32-gptimer";
> >> +			reg = <0x40000C00 0x400>;
> >> +			clocks = <&rcc 0 131>;
> >> +			clock-names = "clk_int";
> >> +			status = "disabled";
> >> +
> >> +			pwm5@0 {
> >> +				compatible = "st,stm32-pwm";
> >> +				st,pwm-num-chan = <4>;
> >> +				st,32bits-counter;
> >> +				status = "disabled";
> >> +			};
> >> +
> >> +			timer5@0 {
> >> +				compatible = "st,stm32-timer-trigger";
> >> +				interrupts = <50>;
> >> +				st,input-triggers-names = TIM2_TRGO,
> >> +							  TIM3_TRGO,
> >> +							  TIM4_TRGO,
> >> +							  TIM8_TRGO;
> >> +				st,output-triggers-names = TIM5_TRGO,
> >> +							   TIM5_CH1,
> >> +							   TIM5_CH2,
> >> +							   TIM5_CH3,
> >> +							   TIM5_CH4;
> >> +				status = "disabled";
> >> +			};
> >> +		};
> >> +
> >> +		gptimer6: gptimer6@40001000 {
> >> +			compatible = "st,stm32-gptimer";
> >> +			reg = <0x40001000 0x400>;
> >> +			clocks = <&rcc 0 132>;
> >> +			clock-names = "clk_int";
> >> +			status = "disabled";
> >> +
> >> +			timer6@0 {
> >> +				compatible = "st,stm32-timer-trigger";
> >> +				interrupts = <54>;
> >> +				st,output-triggers-names = TIM6_TRGO;
> >> +				status = "disabled";
> >> +			};
> >> +		};
> >> +
> >> +		gptimer7: gptimer7@40001400 {
> >> +			compatible = "st,stm32-gptimer";
> >> +			reg = <0x40001400 0x400>;
> >> +			clocks = <&rcc 0 133>;
> >> +			clock-names = "clk_int";
> >> +			status = "disabled";
> >> +
> >> +			timer7@0 {
> >> +				compatible = "st,stm32-timer-trigger";
> >> +				interrupts = <55>;
> >> +				st,output-triggers-names = TIM7_TRGO;
> >> +				status = "disabled";
> >> +			};
> >> +		};
> >> +
> >> +		gptimer8: gptimer8@40010400 {
> >> +			compatible = "st,stm32-gptimer";
> >> +			reg = <0x40010400 0x400>;
> >> +			clocks = <&rcc 0 161>;
> >> +			clock-names = "clk_int";
> >> +			status = "disabled";
> >> +
> >> +			pwm8@0 {
> >> +				compatible = "st,stm32-pwm";
> >> +				st,pwm-num-chan = <4>;
> >> +				st,complementary;
> >> +				st,breakinput;
> >> +				status = "disabled";
> >> +			};
> >> +
> >> +			timer8@0 {
> >> +				compatible = "st,stm32-timer-trigger";
> >> +				interrupts = <46>;
> >> +				st,input-triggers-names = TIM1_TRGO,
> >> +							  TIM2_TRGO,
> >> +							  TIM4_TRGO,
> >> +							  TIM5_TRGO;
> >> +				st,output-triggers-names = TIM8_TRGO,
> >> +							   TIM8_CH1,
> >> +							   TIM8_CH2,
> >> +							   TIM8_CH3,
> >> +							   TIM8_CH4;
> >> +				status = "disabled";
> >> +			};
> >> +		};
> >> +
> >> +		gptimer9: gptimer9@40014000 {
> >> +			compatible = "st,stm32-gptimer";
> >> +			reg = <0x40014000 0x400>;
> >> +			clocks = <&rcc 0 176>;
> >> +			clock-names = "clk_int";
> >> +			status = "disabled";
> >> +
> >> +			pwm9@0 {
> >> +				compatible = "st,stm32-pwm";
> >> +				st,pwm-num-chan = <2>;
> >> +				status = "disabled";
> >> +			};
> >> +
> >> +			timer9@0 {
> >> +				compatible = "st,stm32-timer-trigger";
> >> +				interrupts = <24>;
> >> +				st,input-triggers-names = TIM2_TRGO,
> >> +							  TIM3_TRGO;
> >> +				st,output-triggers-names = TIM9_TRGO,
> >> +							   TIM9_CH1,
> >> +							   TIM9_CH2;
> >> +				status = "disabled";
> >> +			};
> >> +		};
> >> +
> >> +		gptimer10: gptimer10@40014400 {
> >> +			compatible = "st,stm32-gptimer";
> >> +			reg = <0x40014400 0x400>;
> >> +			clocks = <&rcc 0 177>;
> >> +			clock-names = "clk_int";
> >> +			status = "disabled";
> >> +
> >> +			pwm10@0 {
> >> +				compatible = "st,stm32-pwm";
> >> +				st,pwm-num-chan = <1>;
> >> +				status = "disabled";
> >> +			};
> >> +		};
> >> +
> >> +		gptimer11: gptimer11@40014800 {
> >> +			compatible = "st,stm32-gptimer";
> >> +			reg = <0x40014800 0x400>;
> >> +			clocks = <&rcc 0 178>;
> >> +			clock-names = "clk_int";
> >> +			status = "disabled";
> >> +
> >> +			pwm11@0 {
> >> +				compatible = "st,stm32-pwm";
> >> +				st,pwm-num-chan = <1>;
> >> +				status = "disabled";
> >> +			};
> >> +		};
> >> +
> >> +		gptimer12: gptimer12@40001800 {
> >> +			compatible = "st,stm32-gptimer";
> >> +			reg = <0x40001800 0x400>;
> >> +			clocks = <&rcc 0 134>;
> >> +			clock-names = "clk_int";
> >> +			status = "disabled";
> >> +
> >> +			pwm12@0 {
> >> +				compatible = "st,stm32-pwm";
> >> +				st,pwm-num-chan = <2>;
> >> +				status = "disabled";
> >> +			};
> >> +
> >> +			timer12@0 {
> >> +				compatible = "st,stm32-timer-trigger";
> >> +				interrupts = <43>;
> >> +				st,input-triggers-names = TIM4_TRGO,
> >> +							  TIM5_TRGO;
> >> +				st,output-triggers-names = TIM12_TRGO,
> >> +							   TIM12_CH1,
> >> +							   TIM12_CH2;
> >> +				status = "disabled";
> >> +			};
> >> +		};
> >> +
> >> +		gptimer13: gptimer13@40001C00 {
> >> +			compatible = "st,stm32-gptimer";
> >> +			reg = <0x40001C00 0x400>;
> >> +			clocks = <&rcc 0 135>;
> >> +			clock-names = "clk_int";
> >> +			status = "disabled";
> >> +
> >> +			pwm13@0 {
> >> +				compatible = "st,stm32-pwm";
> >> +				st,pwm-num-chan = <1>;
> >> +				status = "disabled";
> >> +			};
> >> +		};
> >> +
> >> +		gptimer14: gptimer14@40002000 {
> >> +			compatible = "st,stm32-gptimer";
> >> +			reg = <0x40002000 0x400>;
> >> +			clocks = <&rcc 0 136>;
> >> +			clock-names = "clk_int";
> >> +			status = "disabled";
> >> +
> >> +			pwm14@0 {
> >> +				compatible = "st,stm32-pwm";
> >> +				st,pwm-num-chan = <1>;
> >> +				status = "disabled";
> >> +			};
> >> +		};
> >>  	};
> >>  };
> >>  
> >> diff --git a/arch/arm/boot/dts/stm32f469-disco.dts b/arch/arm/boot/dts/stm32f469-disco.dts
> >> index 8a163d7..df4ca7e 100644
> >> --- a/arch/arm/boot/dts/stm32f469-disco.dts
> >> +++ b/arch/arm/boot/dts/stm32f469-disco.dts
> >> @@ -81,3 +81,31 @@
> >>  &usart3 {
> >>  	status = "okay";
> >>  };
> >> +
> >> +&gptimer1 {
> >> +	status = "okay";
> >> +
> >> +	pwm1@0 {
> >> +		pinctrl-0	= <&pwm1_pins>;
> >> +		pinctrl-names	= "default";
> >> +		status = "okay";
> >> +	};
> >> +
> >> +	timer1@0 {
> >> +		status = "okay";
> >> +	};
> >> +};
> > 
> > This is a much *better* format than before.
> > 
> > I still don't like the '&' syntax though.
> > 
> >> +&gptimer3 {
> >> +	status = "okay";
> >> +
> >> +	pwm3@0 {
> >> +		pinctrl-0	= <&pwm3_pins>;
> >> +		pinctrl-names	= "default";
> >> +		status = "okay";
> >> +	};
> >> +
> >> +	timer3@0 {
> >> +		status = "okay";
> >> +	};
> >> +};
> > 
>
Alexandre TORGUE Dec. 5, 2016, 4:09 p.m. UTC | #6
Hi,

On 12/02/2016 02:22 PM, Lee Jones wrote:
> On Fri, 02 Dec 2016, Benjamin Gaignard wrote:
>
>> Add general purpose timers and it sub-nodes into DT for stm32f4.
>> Define and enable pwm1 and pwm3 for stm32f469 discovery board
>>
>> version 3:
>> - use "st,stm32-timer-trigger" in DT
>>
>> version 2:
>> - use parameters to describe hardware capabilities
>> - do not use references for pwm and iio timer subnodes
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>> ---
>>  arch/arm/boot/dts/stm32f429.dtsi      | 333 +++++++++++++++++++++++++++++++++-
>>  arch/arm/boot/dts/stm32f469-disco.dts |  28 +++
>>  2 files changed, 360 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
>> index bca491d..8c50d03 100644
>> --- a/arch/arm/boot/dts/stm32f429.dtsi
>> +++ b/arch/arm/boot/dts/stm32f429.dtsi
>> @@ -48,7 +48,7 @@
>>  #include "skeleton.dtsi"
>>  #include "armv7-m.dtsi"
>>  #include <dt-bindings/pinctrl/stm32f429-pinfunc.h>
>> -
>> +#include <dt-bindings/iio/timer/st,stm32-timer-triggers.h>
>>  / {
>>  	clocks {
>>  		clk_hse: clk-hse {
>> @@ -355,6 +355,21 @@
>>  					slew-rate = <2>;
>>  				};
>>  			};
>> +
>> +			pwm1_pins: pwm@1 {
>> +				pins {
>> +					pinmux = <STM32F429_PA8_FUNC_TIM1_CH1>,
>> +						 <STM32F429_PB13_FUNC_TIM1_CH1N>,
>> +						 <STM32F429_PB12_FUNC_TIM1_BKIN>;
>> +				};
>> +			};
>> +
>> +			pwm3_pins: pwm@3 {
>> +				pins {
>> +					pinmux = <STM32F429_PB4_FUNC_TIM3_CH1>,
>> +						 <STM32F429_PB5_FUNC_TIM3_CH2>;
>> +				};
>> +			};
>>  		};
>>
>>  		rcc: rcc@40023810 {
>> @@ -426,6 +441,322 @@
>>  			interrupts = <80>;
>>  			clocks = <&rcc 0 38>;
>>  		};
>> +
>> +		gptimer1: gptimer1@40010000 {
>
> timer@xxxxxxx
>
> Node names should be generic and not numbered.
>
> I suggest that this isn't actually a timer either.  Is contains a
> timer (and a PWM), but in it's completeness it is not a timer per
> say.
>
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40010000 0x400>;
>> +			clocks = <&rcc 0 160>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			pwm1@0 {
>> +				compatible = "st,stm32-pwm";
>> +				st,pwm-num-chan = <4>;
>> +				st,breakinput;
>> +				st,complementary;
>> +				status = "disabled";
>> +			};
>> +
>> +			timer1@0 {
>> +				compatible = "st,stm32-timer-trigger";
>> +				interrupts = <27>;
>> +				st,input-triggers-names = TIM5_TRGO,
>> +							  TIM2_TRGO,
>> +							  TIM4_TRGO,
>> +							  TIM3_TRGO;
>
> I'm still dubious with matching by strings.
>
> I'll take a look at the C code to see what the alternatives could be.
>
>> +				st,output-triggers-names = TIM1_TRGO,
>> +							   TIM1_CH1,
>> +							   TIM1_CH2,
>> +							   TIM1_CH3,
>> +							   TIM1_CH4;
>> +				status = "disabled";
>> +			};
>> +		};
>> +
>> +		gptimer2: gptimer2@40000000 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40000000 0x400>;
>> +			clocks = <&rcc 0 128>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			pwm2@0 {
>> +				compatible = "st,stm32-pwm";
>> +				st,pwm-num-chan = <4>;
>> +				st,32bits-counter;
>> +				status = "disabled";
>> +			};
>> +
>> +			timer2@0 {
>> +				compatible = "st,stm32-timer-trigger";
>> +				interrupts = <28>;
>> +				st,input-triggers-names = TIM1_TRGO,
>> +							  TIM8_TRGO,
>> +							  TIM3_TRGO,
>> +							  TIM4_TRGO;
>> +				st,output-triggers-names = TIM2_TRGO,
>> +							   TIM2_CH1,
>> +							   TIM2_CH2,
>> +							   TIM2_CH3,
>> +							   TIM2_CH4;
>> +				status = "disabled";
>> +			};
>> +		};
>> +
>> +		gptimer3: gptimer3@40000400 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40000400 0x400>;
>> +			clocks = <&rcc 0 129>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			pwm3@0 {
>> +				compatible = "st,stm32-pwm";
>> +				st,pwm-num-chan = <4>;
>> +				status = "disabled";
>> +			};
>> +
>> +			timer3@0 {
>> +				compatible = "st,stm32-timer-trigger";
>> +				interrupts = <29>;
>> +				st,input-triggers-names = TIM1_TRGO,
>> +							  TIM8_TRGO,
>> +							  TIM5_TRGO,
>> +							  TIM4_TRGO;
>> +				st,output-triggers-names = TIM3_TRGO,
>> +							   TIM3_CH1,
>> +							   TIM3_CH2,
>> +							   TIM3_CH3,
>> +							   TIM3_CH4;
>> +				status = "disabled";
>> +			};
>> +		};
>> +
>> +		gptimer4: gptimer4@40000800 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40000800 0x400>;
>> +			clocks = <&rcc 0 130>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			pwm4@0 {
>> +				compatible = "st,stm32-pwm";
>> +				st,pwm-num-chan = <4>;
>> +				status = "disabled";
>> +			};
>> +
>> +			timer4@0 {
>> +				compatible = "st,stm32-timer-trigger";
>> +				interrupts = <30>;
>> +				st,input-triggers-names = TIM1_TRGO,
>> +							  TIM2_TRGO,
>> +							  TIM3_TRGO,
>> +							  TIM8_TRGO;
>> +				st,output-triggers-names = TIM4_TRGO,
>> +							   TIM4_CH1,
>> +							   TIM4_CH2,
>> +							   TIM4_CH3,
>> +							   TIM4_CH4;
>> +				status = "disabled";
>> +			};
>> +		};
>> +
>> +		gptimer5: gptimer5@40000C00 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40000C00 0x400>;
>> +			clocks = <&rcc 0 131>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			pwm5@0 {
>> +				compatible = "st,stm32-pwm";
>> +				st,pwm-num-chan = <4>;
>> +				st,32bits-counter;
>> +				status = "disabled";
>> +			};
>> +
>> +			timer5@0 {
>> +				compatible = "st,stm32-timer-trigger";
>> +				interrupts = <50>;
>> +				st,input-triggers-names = TIM2_TRGO,
>> +							  TIM3_TRGO,
>> +							  TIM4_TRGO,
>> +							  TIM8_TRGO;
>> +				st,output-triggers-names = TIM5_TRGO,
>> +							   TIM5_CH1,
>> +							   TIM5_CH2,
>> +							   TIM5_CH3,
>> +							   TIM5_CH4;
>> +				status = "disabled";
>> +			};
>> +		};
>> +
>> +		gptimer6: gptimer6@40001000 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40001000 0x400>;
>> +			clocks = <&rcc 0 132>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			timer6@0 {
>> +				compatible = "st,stm32-timer-trigger";
>> +				interrupts = <54>;
>> +				st,output-triggers-names = TIM6_TRGO;
>> +				status = "disabled";
>> +			};
>> +		};
>> +
>> +		gptimer7: gptimer7@40001400 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40001400 0x400>;
>> +			clocks = <&rcc 0 133>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			timer7@0 {
>> +				compatible = "st,stm32-timer-trigger";
>> +				interrupts = <55>;
>> +				st,output-triggers-names = TIM7_TRGO;
>> +				status = "disabled";
>> +			};
>> +		};
>> +
>> +		gptimer8: gptimer8@40010400 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40010400 0x400>;
>> +			clocks = <&rcc 0 161>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			pwm8@0 {
>> +				compatible = "st,stm32-pwm";
>> +				st,pwm-num-chan = <4>;
>> +				st,complementary;
>> +				st,breakinput;
>> +				status = "disabled";
>> +			};
>> +
>> +			timer8@0 {
>> +				compatible = "st,stm32-timer-trigger";
>> +				interrupts = <46>;
>> +				st,input-triggers-names = TIM1_TRGO,
>> +							  TIM2_TRGO,
>> +							  TIM4_TRGO,
>> +							  TIM5_TRGO;
>> +				st,output-triggers-names = TIM8_TRGO,
>> +							   TIM8_CH1,
>> +							   TIM8_CH2,
>> +							   TIM8_CH3,
>> +							   TIM8_CH4;
>> +				status = "disabled";
>> +			};
>> +		};
>> +
>> +		gptimer9: gptimer9@40014000 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40014000 0x400>;
>> +			clocks = <&rcc 0 176>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			pwm9@0 {
>> +				compatible = "st,stm32-pwm";
>> +				st,pwm-num-chan = <2>;
>> +				status = "disabled";
>> +			};
>> +
>> +			timer9@0 {
>> +				compatible = "st,stm32-timer-trigger";
>> +				interrupts = <24>;
>> +				st,input-triggers-names = TIM2_TRGO,
>> +							  TIM3_TRGO;
>> +				st,output-triggers-names = TIM9_TRGO,
>> +							   TIM9_CH1,
>> +							   TIM9_CH2;
>> +				status = "disabled";
>> +			};
>> +		};
>> +
>> +		gptimer10: gptimer10@40014400 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40014400 0x400>;
>> +			clocks = <&rcc 0 177>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			pwm10@0 {
>> +				compatible = "st,stm32-pwm";
>> +				st,pwm-num-chan = <1>;
>> +				status = "disabled";
>> +			};
>> +		};
>> +
>> +		gptimer11: gptimer11@40014800 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40014800 0x400>;
>> +			clocks = <&rcc 0 178>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			pwm11@0 {
>> +				compatible = "st,stm32-pwm";
>> +				st,pwm-num-chan = <1>;
>> +				status = "disabled";
>> +			};
>> +		};
>> +
>> +		gptimer12: gptimer12@40001800 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40001800 0x400>;
>> +			clocks = <&rcc 0 134>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			pwm12@0 {
>> +				compatible = "st,stm32-pwm";
>> +				st,pwm-num-chan = <2>;
>> +				status = "disabled";
>> +			};
>> +
>> +			timer12@0 {
>> +				compatible = "st,stm32-timer-trigger";
>> +				interrupts = <43>;
>> +				st,input-triggers-names = TIM4_TRGO,
>> +							  TIM5_TRGO;
>> +				st,output-triggers-names = TIM12_TRGO,
>> +							   TIM12_CH1,
>> +							   TIM12_CH2;
>> +				status = "disabled";
>> +			};
>> +		};
>> +
>> +		gptimer13: gptimer13@40001C00 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40001C00 0x400>;
>> +			clocks = <&rcc 0 135>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			pwm13@0 {
>> +				compatible = "st,stm32-pwm";
>> +				st,pwm-num-chan = <1>;
>> +				status = "disabled";
>> +			};
>> +		};
>> +
>> +		gptimer14: gptimer14@40002000 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40002000 0x400>;
>> +			clocks = <&rcc 0 136>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			pwm14@0 {
>> +				compatible = "st,stm32-pwm";
>> +				st,pwm-num-chan = <1>;
>> +				status = "disabled";
>> +			};
>> +		};
>>  	};
>>  };
>>
>> diff --git a/arch/arm/boot/dts/stm32f469-disco.dts b/arch/arm/boot/dts/stm32f469-disco.dts
>> index 8a163d7..df4ca7e 100644
>> --- a/arch/arm/boot/dts/stm32f469-disco.dts
>> +++ b/arch/arm/boot/dts/stm32f469-disco.dts
>> @@ -81,3 +81,31 @@
>>  &usart3 {
>>  	status = "okay";
>>  };
>> +
>> +&gptimer1 {
>> +	status = "okay";
>> +
>> +	pwm1@0 {
>> +		pinctrl-0	= <&pwm1_pins>;
>> +		pinctrl-names	= "default";
>> +		status = "okay";
>> +	};
>> +
>> +	timer1@0 {
>> +		status = "okay";
>> +	};
>> +};
>
> This is a much *better* format than before.
>
> I still don't like the '&' syntax though.

Please keep "&" format to match with existing nodes.

>
>> +&gptimer3 {
>> +	status = "okay";
>> +
>> +	pwm3@0 {
>> +		pinctrl-0	= <&pwm3_pins>;
>> +		pinctrl-names	= "default";
>> +		status = "okay";
>> +	};
>> +
>> +	timer3@0 {
>> +		status = "okay";
>> +	};
>> +};
>
--
To unsubscribe from this list: send the line "unsubscribe linux-pwm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lee Jones Dec. 6, 2016, 9:48 a.m. UTC | #7
On Mon, 05 Dec 2016, Alexandre Torgue wrote:
> On 12/02/2016 02:22 PM, Lee Jones wrote:
> > On Fri, 02 Dec 2016, Benjamin Gaignard wrote:
> > 
> > > Add general purpose timers and it sub-nodes into DT for stm32f4.
> > > Define and enable pwm1 and pwm3 for stm32f469 discovery board
> > > 
> > > version 3:
> > > - use "st,stm32-timer-trigger" in DT
> > > 
> > > version 2:
> > > - use parameters to describe hardware capabilities
> > > - do not use references for pwm and iio timer subnodes
> > > 
> > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> > > ---
> > >  arch/arm/boot/dts/stm32f429.dtsi      | 333 +++++++++++++++++++++++++++++++++-
> > >  arch/arm/boot/dts/stm32f469-disco.dts |  28 +++
> > >  2 files changed, 360 insertions(+), 1 deletion(-)

[...]

If you're only commenting on a little piece of the patch, it's always
a good idea to trim the rest.

> > > diff --git a/arch/arm/boot/dts/stm32f469-disco.dts b/arch/arm/boot/dts/stm32f469-disco.dts
> > > index 8a163d7..df4ca7e 100644
> > > --- a/arch/arm/boot/dts/stm32f469-disco.dts
> > > +++ b/arch/arm/boot/dts/stm32f469-disco.dts
> > > @@ -81,3 +81,31 @@
> > >  &usart3 {
> > >  	status = "okay";
> > >  };
> > > +
> > > +&gptimer1 {
> > > +	status = "okay";
> > > +
> > > +	pwm1@0 {
> > > +		pinctrl-0	= <&pwm1_pins>;
> > > +		pinctrl-names	= "default";
> > > +		status = "okay";
> > > +	};
> > > +
> > > +	timer1@0 {
> > > +		status = "okay";
> > > +	};
> > > +};
> > 
> > This is a much *better* format than before.
> > 
> > I still don't like the '&' syntax though.
> 
> Please keep "&" format to match with existing nodes.

Right.  I wasn't suggesting that he differs from the current format in
*this* set.  I am suggesting that we change the format in a subsequent
set though.

> > > +&gptimer3 {
> > > +	status = "okay";
> > > +
> > > +	pwm3@0 {
> > > +		pinctrl-0	= <&pwm3_pins>;
> > > +		pinctrl-names	= "default";
> > > +		status = "okay";
> > > +	};
> > > +
> > > +	timer3@0 {
> > > +		status = "okay";
> > > +	};
> > > +};
> >
Alexandre TORGUE Dec. 6, 2016, 9:56 a.m. UTC | #8
Hi Lee,

On 12/06/2016 10:48 AM, Lee Jones wrote:
> On Mon, 05 Dec 2016, Alexandre Torgue wrote:
>> On 12/02/2016 02:22 PM, Lee Jones wrote:
>>> On Fri, 02 Dec 2016, Benjamin Gaignard wrote:
>>>
>>>> Add general purpose timers and it sub-nodes into DT for stm32f4.
>>>> Define and enable pwm1 and pwm3 for stm32f469 discovery board
>>>>
>>>> version 3:
>>>> - use "st,stm32-timer-trigger" in DT
>>>>
>>>> version 2:
>>>> - use parameters to describe hardware capabilities
>>>> - do not use references for pwm and iio timer subnodes
>>>>
>>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>>>> ---
>>>>  arch/arm/boot/dts/stm32f429.dtsi      | 333 +++++++++++++++++++++++++++++++++-
>>>>  arch/arm/boot/dts/stm32f469-disco.dts |  28 +++
>>>>  2 files changed, 360 insertions(+), 1 deletion(-)
>
> [...]
>
> If you're only commenting on a little piece of the patch, it's always
> a good idea to trim the rest.
>
>>>> diff --git a/arch/arm/boot/dts/stm32f469-disco.dts b/arch/arm/boot/dts/stm32f469-disco.dts
>>>> index 8a163d7..df4ca7e 100644
>>>> --- a/arch/arm/boot/dts/stm32f469-disco.dts
>>>> +++ b/arch/arm/boot/dts/stm32f469-disco.dts
>>>> @@ -81,3 +81,31 @@
>>>>  &usart3 {
>>>>  	status = "okay";
>>>>  };
>>>> +
>>>> +&gptimer1 {
>>>> +	status = "okay";
>>>> +
>>>> +	pwm1@0 {
>>>> +		pinctrl-0	= <&pwm1_pins>;
>>>> +		pinctrl-names	= "default";
>>>> +		status = "okay";
>>>> +	};
>>>> +
>>>> +	timer1@0 {
>>>> +		status = "okay";
>>>> +	};
>>>> +};
>>>
>>> This is a much *better* format than before.
>>>
>>> I still don't like the '&' syntax though.
>>
>> Please keep "&" format to match with existing nodes.
>
> Right.  I wasn't suggesting that he differs from the current format in
> *this* set.  I am suggesting that we change the format in a subsequent
> set though.

Why change? Looking at Linux ARM kernel patchwork, new DT board file 
contains this format. Did you already discuss with Arnd or Olof about it 
?

regards
Alex

>
>>>> +&gptimer3 {
>>>> +	status = "okay";
>>>> +
>>>> +	pwm3@0 {
>>>> +		pinctrl-0	= <&pwm3_pins>;
>>>> +		pinctrl-names	= "default";
>>>> +		status = "okay";
>>>> +	};
>>>> +
>>>> +	timer3@0 {
>>>> +		status = "okay";
>>>> +	};
>>>> +};
>>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-pwm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lee Jones Dec. 6, 2016, 12:54 p.m. UTC | #9
On Tue, 06 Dec 2016, Alexandre Torgue wrote:
> On 12/06/2016 10:48 AM, Lee Jones wrote:
> > On Mon, 05 Dec 2016, Alexandre Torgue wrote:
> > > On 12/02/2016 02:22 PM, Lee Jones wrote:
> > > > On Fri, 02 Dec 2016, Benjamin Gaignard wrote:
> > > > 
> > > > > Add general purpose timers and it sub-nodes into DT for stm32f4.
> > > > > Define and enable pwm1 and pwm3 for stm32f469 discovery board
> > > > > 
> > > > > version 3:
> > > > > - use "st,stm32-timer-trigger" in DT
> > > > > 
> > > > > version 2:
> > > > > - use parameters to describe hardware capabilities
> > > > > - do not use references for pwm and iio timer subnodes
> > > > > 
> > > > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> > > > > ---
> > > > >  arch/arm/boot/dts/stm32f429.dtsi      | 333 +++++++++++++++++++++++++++++++++-
> > > > >  arch/arm/boot/dts/stm32f469-disco.dts |  28 +++
> > > > >  2 files changed, 360 insertions(+), 1 deletion(-)
> > 
> > [...]
> > 
> > If you're only commenting on a little piece of the patch, it's always
> > a good idea to trim the rest.
> > 
> > > > > diff --git a/arch/arm/boot/dts/stm32f469-disco.dts b/arch/arm/boot/dts/stm32f469-disco.dts
> > > > > index 8a163d7..df4ca7e 100644
> > > > > --- a/arch/arm/boot/dts/stm32f469-disco.dts
> > > > > +++ b/arch/arm/boot/dts/stm32f469-disco.dts
> > > > > @@ -81,3 +81,31 @@
> > > > >  &usart3 {
> > > > >  	status = "okay";
> > > > >  };
> > > > > +
> > > > > +&gptimer1 {
> > > > > +	status = "okay";
> > > > > +
> > > > > +	pwm1@0 {
> > > > > +		pinctrl-0	= <&pwm1_pins>;
> > > > > +		pinctrl-names	= "default";
> > > > > +		status = "okay";
> > > > > +	};
> > > > > +
> > > > > +	timer1@0 {
> > > > > +		status = "okay";
> > > > > +	};
> > > > > +};
> > > > 
> > > > This is a much *better* format than before.
> > > > 
> > > > I still don't like the '&' syntax though.
> > > 
> > > Please keep "&" format to match with existing nodes.
> > 
> > Right.  I wasn't suggesting that he differs from the current format in
> > *this* set.  I am suggesting that we change the format in a subsequent
> > set though.
> 
> Why change? Looking at Linux ARM kernel patchwork, new DT board file
> contains this format. Did you already discuss with Arnd or Olof about it ?

Because the syntax is horrible.  It removes any indication of
hierarchy and insists all nodes include labels that would otherwise be
unnecessary.

The syntax is approved, so there is no issue with Arnd/Olof, nor the
DT Maintainers.  The decision is left to the sub-arch maintainer to
choose to use it or not.  My vote (which doesn't really count for
anything in this scenario) is to not use it for the aforementioned
reasons.

> > > > > +&gptimer3 {
> > > > > +	status = "okay";
> > > > > +
> > > > > +	pwm3@0 {
> > > > > +		pinctrl-0	= <&pwm3_pins>;
> > > > > +		pinctrl-names	= "default";
> > > > > +		status = "okay";
> > > > > +	};
> > > > > +
> > > > > +	timer3@0 {
> > > > > +		status = "okay";
> > > > > +	};
> > > > > +};
> > > > 
> >
diff mbox

Patch

diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
index bca491d..8c50d03 100644
--- a/arch/arm/boot/dts/stm32f429.dtsi
+++ b/arch/arm/boot/dts/stm32f429.dtsi
@@ -48,7 +48,7 @@ 
 #include "skeleton.dtsi"
 #include "armv7-m.dtsi"
 #include <dt-bindings/pinctrl/stm32f429-pinfunc.h>
-
+#include <dt-bindings/iio/timer/st,stm32-timer-triggers.h>
 / {
 	clocks {
 		clk_hse: clk-hse {
@@ -355,6 +355,21 @@ 
 					slew-rate = <2>;
 				};
 			};
+
+			pwm1_pins: pwm@1 {
+				pins {
+					pinmux = <STM32F429_PA8_FUNC_TIM1_CH1>,
+						 <STM32F429_PB13_FUNC_TIM1_CH1N>,
+						 <STM32F429_PB12_FUNC_TIM1_BKIN>;
+				};
+			};
+
+			pwm3_pins: pwm@3 {
+				pins {
+					pinmux = <STM32F429_PB4_FUNC_TIM3_CH1>,
+						 <STM32F429_PB5_FUNC_TIM3_CH2>;
+				};
+			};
 		};
 
 		rcc: rcc@40023810 {
@@ -426,6 +441,322 @@ 
 			interrupts = <80>;
 			clocks = <&rcc 0 38>;
 		};
+
+		gptimer1: gptimer1@40010000 {
+			compatible = "st,stm32-gptimer";
+			reg = <0x40010000 0x400>;
+			clocks = <&rcc 0 160>;
+			clock-names = "clk_int";
+			status = "disabled";
+
+			pwm1@0 {
+				compatible = "st,stm32-pwm";
+				st,pwm-num-chan = <4>;
+				st,breakinput;
+				st,complementary;
+				status = "disabled";
+			};
+
+			timer1@0 {
+				compatible = "st,stm32-timer-trigger";
+				interrupts = <27>;
+				st,input-triggers-names = TIM5_TRGO,
+							  TIM2_TRGO,
+							  TIM4_TRGO,
+							  TIM3_TRGO;
+				st,output-triggers-names = TIM1_TRGO,
+							   TIM1_CH1,
+							   TIM1_CH2,
+							   TIM1_CH3,
+							   TIM1_CH4;
+				status = "disabled";
+			};
+		};
+
+		gptimer2: gptimer2@40000000 {
+			compatible = "st,stm32-gptimer";
+			reg = <0x40000000 0x400>;
+			clocks = <&rcc 0 128>;
+			clock-names = "clk_int";
+			status = "disabled";
+
+			pwm2@0 {
+				compatible = "st,stm32-pwm";
+				st,pwm-num-chan = <4>;
+				st,32bits-counter;
+				status = "disabled";
+			};
+
+			timer2@0 {
+				compatible = "st,stm32-timer-trigger";
+				interrupts = <28>;
+				st,input-triggers-names = TIM1_TRGO,
+							  TIM8_TRGO,
+							  TIM3_TRGO,
+							  TIM4_TRGO;
+				st,output-triggers-names = TIM2_TRGO,
+							   TIM2_CH1,
+							   TIM2_CH2,
+							   TIM2_CH3,
+							   TIM2_CH4;
+				status = "disabled";
+			};
+		};
+
+		gptimer3: gptimer3@40000400 {
+			compatible = "st,stm32-gptimer";
+			reg = <0x40000400 0x400>;
+			clocks = <&rcc 0 129>;
+			clock-names = "clk_int";
+			status = "disabled";
+
+			pwm3@0 {
+				compatible = "st,stm32-pwm";
+				st,pwm-num-chan = <4>;
+				status = "disabled";
+			};
+
+			timer3@0 {
+				compatible = "st,stm32-timer-trigger";
+				interrupts = <29>;
+				st,input-triggers-names = TIM1_TRGO,
+							  TIM8_TRGO,
+							  TIM5_TRGO,
+							  TIM4_TRGO;
+				st,output-triggers-names = TIM3_TRGO,
+							   TIM3_CH1,
+							   TIM3_CH2,
+							   TIM3_CH3,
+							   TIM3_CH4;
+				status = "disabled";
+			};
+		};
+
+		gptimer4: gptimer4@40000800 {
+			compatible = "st,stm32-gptimer";
+			reg = <0x40000800 0x400>;
+			clocks = <&rcc 0 130>;
+			clock-names = "clk_int";
+			status = "disabled";
+
+			pwm4@0 {
+				compatible = "st,stm32-pwm";
+				st,pwm-num-chan = <4>;
+				status = "disabled";
+			};
+
+			timer4@0 {
+				compatible = "st,stm32-timer-trigger";
+				interrupts = <30>;
+				st,input-triggers-names = TIM1_TRGO,
+							  TIM2_TRGO,
+							  TIM3_TRGO,
+							  TIM8_TRGO;
+				st,output-triggers-names = TIM4_TRGO,
+							   TIM4_CH1,
+							   TIM4_CH2,
+							   TIM4_CH3,
+							   TIM4_CH4;
+				status = "disabled";
+			};
+		};
+
+		gptimer5: gptimer5@40000C00 {
+			compatible = "st,stm32-gptimer";
+			reg = <0x40000C00 0x400>;
+			clocks = <&rcc 0 131>;
+			clock-names = "clk_int";
+			status = "disabled";
+
+			pwm5@0 {
+				compatible = "st,stm32-pwm";
+				st,pwm-num-chan = <4>;
+				st,32bits-counter;
+				status = "disabled";
+			};
+
+			timer5@0 {
+				compatible = "st,stm32-timer-trigger";
+				interrupts = <50>;
+				st,input-triggers-names = TIM2_TRGO,
+							  TIM3_TRGO,
+							  TIM4_TRGO,
+							  TIM8_TRGO;
+				st,output-triggers-names = TIM5_TRGO,
+							   TIM5_CH1,
+							   TIM5_CH2,
+							   TIM5_CH3,
+							   TIM5_CH4;
+				status = "disabled";
+			};
+		};
+
+		gptimer6: gptimer6@40001000 {
+			compatible = "st,stm32-gptimer";
+			reg = <0x40001000 0x400>;
+			clocks = <&rcc 0 132>;
+			clock-names = "clk_int";
+			status = "disabled";
+
+			timer6@0 {
+				compatible = "st,stm32-timer-trigger";
+				interrupts = <54>;
+				st,output-triggers-names = TIM6_TRGO;
+				status = "disabled";
+			};
+		};
+
+		gptimer7: gptimer7@40001400 {
+			compatible = "st,stm32-gptimer";
+			reg = <0x40001400 0x400>;
+			clocks = <&rcc 0 133>;
+			clock-names = "clk_int";
+			status = "disabled";
+
+			timer7@0 {
+				compatible = "st,stm32-timer-trigger";
+				interrupts = <55>;
+				st,output-triggers-names = TIM7_TRGO;
+				status = "disabled";
+			};
+		};
+
+		gptimer8: gptimer8@40010400 {
+			compatible = "st,stm32-gptimer";
+			reg = <0x40010400 0x400>;
+			clocks = <&rcc 0 161>;
+			clock-names = "clk_int";
+			status = "disabled";
+
+			pwm8@0 {
+				compatible = "st,stm32-pwm";
+				st,pwm-num-chan = <4>;
+				st,complementary;
+				st,breakinput;
+				status = "disabled";
+			};
+
+			timer8@0 {
+				compatible = "st,stm32-timer-trigger";
+				interrupts = <46>;
+				st,input-triggers-names = TIM1_TRGO,
+							  TIM2_TRGO,
+							  TIM4_TRGO,
+							  TIM5_TRGO;
+				st,output-triggers-names = TIM8_TRGO,
+							   TIM8_CH1,
+							   TIM8_CH2,
+							   TIM8_CH3,
+							   TIM8_CH4;
+				status = "disabled";
+			};
+		};
+
+		gptimer9: gptimer9@40014000 {
+			compatible = "st,stm32-gptimer";
+			reg = <0x40014000 0x400>;
+			clocks = <&rcc 0 176>;
+			clock-names = "clk_int";
+			status = "disabled";
+
+			pwm9@0 {
+				compatible = "st,stm32-pwm";
+				st,pwm-num-chan = <2>;
+				status = "disabled";
+			};
+
+			timer9@0 {
+				compatible = "st,stm32-timer-trigger";
+				interrupts = <24>;
+				st,input-triggers-names = TIM2_TRGO,
+							  TIM3_TRGO;
+				st,output-triggers-names = TIM9_TRGO,
+							   TIM9_CH1,
+							   TIM9_CH2;
+				status = "disabled";
+			};
+		};
+
+		gptimer10: gptimer10@40014400 {
+			compatible = "st,stm32-gptimer";
+			reg = <0x40014400 0x400>;
+			clocks = <&rcc 0 177>;
+			clock-names = "clk_int";
+			status = "disabled";
+
+			pwm10@0 {
+				compatible = "st,stm32-pwm";
+				st,pwm-num-chan = <1>;
+				status = "disabled";
+			};
+		};
+
+		gptimer11: gptimer11@40014800 {
+			compatible = "st,stm32-gptimer";
+			reg = <0x40014800 0x400>;
+			clocks = <&rcc 0 178>;
+			clock-names = "clk_int";
+			status = "disabled";
+
+			pwm11@0 {
+				compatible = "st,stm32-pwm";
+				st,pwm-num-chan = <1>;
+				status = "disabled";
+			};
+		};
+
+		gptimer12: gptimer12@40001800 {
+			compatible = "st,stm32-gptimer";
+			reg = <0x40001800 0x400>;
+			clocks = <&rcc 0 134>;
+			clock-names = "clk_int";
+			status = "disabled";
+
+			pwm12@0 {
+				compatible = "st,stm32-pwm";
+				st,pwm-num-chan = <2>;
+				status = "disabled";
+			};
+
+			timer12@0 {
+				compatible = "st,stm32-timer-trigger";
+				interrupts = <43>;
+				st,input-triggers-names = TIM4_TRGO,
+							  TIM5_TRGO;
+				st,output-triggers-names = TIM12_TRGO,
+							   TIM12_CH1,
+							   TIM12_CH2;
+				status = "disabled";
+			};
+		};
+
+		gptimer13: gptimer13@40001C00 {
+			compatible = "st,stm32-gptimer";
+			reg = <0x40001C00 0x400>;
+			clocks = <&rcc 0 135>;
+			clock-names = "clk_int";
+			status = "disabled";
+
+			pwm13@0 {
+				compatible = "st,stm32-pwm";
+				st,pwm-num-chan = <1>;
+				status = "disabled";
+			};
+		};
+
+		gptimer14: gptimer14@40002000 {
+			compatible = "st,stm32-gptimer";
+			reg = <0x40002000 0x400>;
+			clocks = <&rcc 0 136>;
+			clock-names = "clk_int";
+			status = "disabled";
+
+			pwm14@0 {
+				compatible = "st,stm32-pwm";
+				st,pwm-num-chan = <1>;
+				status = "disabled";
+			};
+		};
 	};
 };
 
diff --git a/arch/arm/boot/dts/stm32f469-disco.dts b/arch/arm/boot/dts/stm32f469-disco.dts
index 8a163d7..df4ca7e 100644
--- a/arch/arm/boot/dts/stm32f469-disco.dts
+++ b/arch/arm/boot/dts/stm32f469-disco.dts
@@ -81,3 +81,31 @@ 
 &usart3 {
 	status = "okay";
 };
+
+&gptimer1 {
+	status = "okay";
+
+	pwm1@0 {
+		pinctrl-0	= <&pwm1_pins>;
+		pinctrl-names	= "default";
+		status = "okay";
+	};
+
+	timer1@0 {
+		status = "okay";
+	};
+};
+
+&gptimer3 {
+	status = "okay";
+
+	pwm3@0 {
+		pinctrl-0	= <&pwm3_pins>;
+		pinctrl-names	= "default";
+		status = "okay";
+	};
+
+	timer3@0 {
+		status = "okay";
+	};
+};