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@xxxxxx> > >> --- > >> 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"; > >> + }; > >> +}; > > > -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html