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

Alexandre Torgue alexandre.torgue at st.com
Mon Dec 5 08:09:04 PST 2016


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 at 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 at 1 {
>> +				pins {
>> +					pinmux = <STM32F429_PA8_FUNC_TIM1_CH1>,
>> +						 <STM32F429_PB13_FUNC_TIM1_CH1N>,
>> +						 <STM32F429_PB12_FUNC_TIM1_BKIN>;
>> +				};
>> +			};
>> +
>> +			pwm3_pins: pwm at 3 {
>> +				pins {
>> +					pinmux = <STM32F429_PB4_FUNC_TIM3_CH1>,
>> +						 <STM32F429_PB5_FUNC_TIM3_CH2>;
>> +				};
>> +			};
>>  		};
>>
>>  		rcc: rcc at 40023810 {
>> @@ -426,6 +441,322 @@
>>  			interrupts = <80>;
>>  			clocks = <&rcc 0 38>;
>>  		};
>> +
>> +		gptimer1: gptimer1 at 40010000 {
>
> timer at 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 at 0 {
>> +				compatible = "st,stm32-pwm";
>> +				st,pwm-num-chan = <4>;
>> +				st,breakinput;
>> +				st,complementary;
>> +				status = "disabled";
>> +			};
>> +
>> +			timer1 at 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 at 40000000 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40000000 0x400>;
>> +			clocks = <&rcc 0 128>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			pwm2 at 0 {
>> +				compatible = "st,stm32-pwm";
>> +				st,pwm-num-chan = <4>;
>> +				st,32bits-counter;
>> +				status = "disabled";
>> +			};
>> +
>> +			timer2 at 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 at 40000400 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40000400 0x400>;
>> +			clocks = <&rcc 0 129>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			pwm3 at 0 {
>> +				compatible = "st,stm32-pwm";
>> +				st,pwm-num-chan = <4>;
>> +				status = "disabled";
>> +			};
>> +
>> +			timer3 at 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 at 40000800 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40000800 0x400>;
>> +			clocks = <&rcc 0 130>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			pwm4 at 0 {
>> +				compatible = "st,stm32-pwm";
>> +				st,pwm-num-chan = <4>;
>> +				status = "disabled";
>> +			};
>> +
>> +			timer4 at 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 at 40000C00 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40000C00 0x400>;
>> +			clocks = <&rcc 0 131>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			pwm5 at 0 {
>> +				compatible = "st,stm32-pwm";
>> +				st,pwm-num-chan = <4>;
>> +				st,32bits-counter;
>> +				status = "disabled";
>> +			};
>> +
>> +			timer5 at 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 at 40001000 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40001000 0x400>;
>> +			clocks = <&rcc 0 132>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			timer6 at 0 {
>> +				compatible = "st,stm32-timer-trigger";
>> +				interrupts = <54>;
>> +				st,output-triggers-names = TIM6_TRGO;
>> +				status = "disabled";
>> +			};
>> +		};
>> +
>> +		gptimer7: gptimer7 at 40001400 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40001400 0x400>;
>> +			clocks = <&rcc 0 133>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			timer7 at 0 {
>> +				compatible = "st,stm32-timer-trigger";
>> +				interrupts = <55>;
>> +				st,output-triggers-names = TIM7_TRGO;
>> +				status = "disabled";
>> +			};
>> +		};
>> +
>> +		gptimer8: gptimer8 at 40010400 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40010400 0x400>;
>> +			clocks = <&rcc 0 161>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			pwm8 at 0 {
>> +				compatible = "st,stm32-pwm";
>> +				st,pwm-num-chan = <4>;
>> +				st,complementary;
>> +				st,breakinput;
>> +				status = "disabled";
>> +			};
>> +
>> +			timer8 at 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 at 40014000 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40014000 0x400>;
>> +			clocks = <&rcc 0 176>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			pwm9 at 0 {
>> +				compatible = "st,stm32-pwm";
>> +				st,pwm-num-chan = <2>;
>> +				status = "disabled";
>> +			};
>> +
>> +			timer9 at 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 at 40014400 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40014400 0x400>;
>> +			clocks = <&rcc 0 177>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			pwm10 at 0 {
>> +				compatible = "st,stm32-pwm";
>> +				st,pwm-num-chan = <1>;
>> +				status = "disabled";
>> +			};
>> +		};
>> +
>> +		gptimer11: gptimer11 at 40014800 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40014800 0x400>;
>> +			clocks = <&rcc 0 178>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			pwm11 at 0 {
>> +				compatible = "st,stm32-pwm";
>> +				st,pwm-num-chan = <1>;
>> +				status = "disabled";
>> +			};
>> +		};
>> +
>> +		gptimer12: gptimer12 at 40001800 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40001800 0x400>;
>> +			clocks = <&rcc 0 134>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			pwm12 at 0 {
>> +				compatible = "st,stm32-pwm";
>> +				st,pwm-num-chan = <2>;
>> +				status = "disabled";
>> +			};
>> +
>> +			timer12 at 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 at 40001C00 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40001C00 0x400>;
>> +			clocks = <&rcc 0 135>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			pwm13 at 0 {
>> +				compatible = "st,stm32-pwm";
>> +				st,pwm-num-chan = <1>;
>> +				status = "disabled";
>> +			};
>> +		};
>> +
>> +		gptimer14: gptimer14 at 40002000 {
>> +			compatible = "st,stm32-gptimer";
>> +			reg = <0x40002000 0x400>;
>> +			clocks = <&rcc 0 136>;
>> +			clock-names = "clk_int";
>> +			status = "disabled";
>> +
>> +			pwm14 at 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 at 0 {
>> +		pinctrl-0	= <&pwm1_pins>;
>> +		pinctrl-names	= "default";
>> +		status = "okay";
>> +	};
>> +
>> +	timer1 at 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 at 0 {
>> +		pinctrl-0	= <&pwm3_pins>;
>> +		pinctrl-names	= "default";
>> +		status = "okay";
>> +	};
>> +
>> +	timer3 at 0 {
>> +		status = "okay";
>> +	};
>> +};
>



More information about the linux-arm-kernel mailing list