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

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";
  

• };

+};