boards/nucleo-f401: configure PWM on 7 pins by aabadie · Pull Request #6192 · RIOT-OS/RIOT

aabadie · 2016-12-08T18:56:43Z

This PR is based on #6186.

temmihoo · 2016-12-09T07:21:37Z

Hi

As this seems to touch all STM32F[234] in some ways and not only nucleo-f4 the board I suggest indicating this in the PR subject.

Support for F2 PWM is temporarily dropped and hopefully better support for F[34] PWM is added.

This is an area where perhaps nonclamenture could be more exact. The STM32 family has several different PWM peripherals, none of which are specific to any particular Fx model, except possibly HRTIM, which seems to be available only in F334.

Anyhow the STM32 timers are complex, their clock architecture elaborate and the whole issue around timing takes roughly quarter of the STM32 manual, so tackling this yak shaving in small pieces would make sense to me. ST doesn't help in not having a common to all STM32 reference manual but seems to treat the subfamilies as separate entities in the documentation when in reality they're built from the same modular blocks.

I found a decent article about STM32 timers but even that forgets to mention F334 HRTIM completely :D All in all as the peripherals aren't model specific I'd rather see the timers supported as peripherals and then the support matrix (which timers in which models of mcu) as separate entities. This would in my view reflect the architecture of RIOT that a spefic board has a specific mcu that consists of a specific core and specific peripherals.

Thus I suggest naming these peripherals stm32-gptimer stm32-basictimer stm32-advancedtimer stm32-hrtimer and then possibly have or not have similar naming scheme for the clocks and their interconnects available in the whole STM32 family. (this area needs better thinking than my fast off the cuff suggestion)

Anyway the STM32 family is pretty modular so it is completely not necessary to see the models as separate from each other as the whole family shares possibility of having pretty much any STM32 peripheral available. Not all have but new models are prone to have any combination.

This modularity is present especially if you're having a custom chip made for you as you could select for example Cortex M0 core, MAC and HRTIM only and nothing else (well really just about any combination, they all share the interconnect mechanisms). I'm hoping that in some not too far future those guys having chips made for products would be willing to look at RIOT... :)

t

aabadie · 2016-12-09T10:13:01Z

@temmihoo. Thanks for commenting.

As this seems to touch all STM32F[234] in some ways and not only nucleo-f4 the board I suggest indicating this in the PR subject.

As said in the PR description (which is minimal), this PR is based on #6186 where most of the job is done. This PR only adds PWM definitions for the nucleo-f401 board on top of #6186.

temmihoo · 2016-12-12T07:39:26Z

@aabadie Thx for notifying, I commented more or less the same in parent PR, including the link given.

haukepetersen · 2016-12-15T12:09:14Z

boards/nucleo-f401/include/periph_conf.h

+ * @{
+ */
+static const pwm_conf_t pwm_config[] = {
+    { /* D3 */


I would prefer not to use TIM2 for PWM, as the same timer is already mapped to the timer_ interface. If really wanted/needed for PWM, we should change the default TIMER_DEV(0) to TIM5 above in the timer configuration.

haukepetersen · 2016-12-15T12:12:22Z

boards/nucleo-f401/include/periph_conf.h

+        .dev      = TIM3,
+        .rcc_mask = RCC_APB1ENR_TIM3EN,
+        .pins     = { GPIO_PIN(PORT_B, 4) },
+        .af       = GPIO_AF2,


with this configuration, TIM3 is overloaded -> this would lead to trouble. I think what you actually want is to configure TIM3 with 2 channels:

... { /* D5 and D9 */ .dev = TIM3, .rcc_mask = RCC_APB1ENR_TIM3EN, .pins = { GPIO_PIN(PORT_B, 4), GPIO_PIN(PORT_C, 7), GPIO_UNDEF, GPIO_UNDEF }, .af = GPIO_AF2, .chan = 2, .bus = APB1 }, ...

Note also, that you always need to map 4 pins, but you may set them to GPIO_UNDEF for un-used channels.

aabadie · 2017-01-03T14:25:37Z

@haukepetersen, thanks for reviewing. I was confused with the PWM configuration and could fix the issue thanks to your comments.
Tested on board, the 4 PWM pins mentionned (D3, 5, 6, 9) work with the test/periph_pwm test.

I also had to remove TIM2 from the timers list and switch the default timer to TIM5. This is required to make the PWM work on pins D3 and D6.
Following the datasheets here (from page 45) and here (page 32), I also configured 3 pins on the ST morpho connectors, just to fill the first channels of each timers (TIM2 and 3), otherwise I ended up in a kernel hard fault or not working PWM.

aabadie · 2017-01-09T10:02:11Z

@haukepetersen, added the periph_pwm module to the FEATURES_PROVIDED by this board. Should be ok now.

aabadie · 2017-01-09T12:22:55Z

ping @haukepetersen

aabadie · 2017-01-18T14:23:33Z

@haukepetersen, rebased and adapted following changes from#6393.

haukepetersen

minor typo?

haukepetersen · 2017-01-18T18:58:55Z

boards/nucleo-f401/include/periph_conf.h

+        .dev      = TIM3,
+        .rcc_mask = RCC_APB1ENR_TIM3EN,
+        .chan     = { { .pin = GPIO_PIN(PORT_B, 4) /* D5 */, .cc_chan = 0 },
+                      { .pin = GPIO_PIN(PORT_C, 7) /* D9 */, .cc_chan = 1 },


channel setting seems wrong

Arf, fixed.

aabadie · 2017-01-19T06:40:50Z

All green, merging

aabadie assigned haukepetersen Dec 9, 2016

aabadie added Platform: ARM Platform: This PR/issue effects ARM-based platforms State: waiting for other PR State: The PR requires another PR to be merged first labels Dec 9, 2016

temmihoo mentioned this pull request Dec 12, 2016

cpu/stm32x: unified PWM driver implementations #6186

Merged

haukepetersen requested changes Dec 15, 2016

View reviewed changes

aabadie force-pushed the nucleo_f401_pwm branch from 1f6cc71 to 2619a9d Compare January 3, 2017 14:16

aabadie force-pushed the nucleo_f401_pwm branch 2 times, most recently from c23f530 to c0947c4 Compare January 9, 2017 10:00

aabadie removed the State: waiting for other PR State: The PR requires another PR to be merged first label Jan 9, 2017

aabadie changed the title ~~Add Nucleo f401 PWM configuration on 4 pins~~ Add Nucleo f401 PWM configuration on 7 pins Jan 11, 2017

aabadie added this to the Release 2017.01 milestone Jan 13, 2017

aabadie changed the title ~~Add Nucleo f401 PWM configuration on 7 pins~~ boards/nucleo: configure PWM on 7 pins Jan 17, 2017

aabadie changed the title ~~boards/nucleo: configure PWM on 7 pins~~ boards/nucleo-f401: configure PWM on 7 pins Jan 17, 2017

aabadie force-pushed the nucleo_f401_pwm branch 2 times, most recently from 8b93bbd to aacaf10 Compare January 18, 2017 14:22

PeterKietzmann added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 18, 2017

haukepetersen requested changes Jan 18, 2017

View reviewed changes

boards/nucleo-f401: add pwm configuration on pins d{3,5,6,9}

8217ed1

aabadie force-pushed the nucleo_f401_pwm branch from aacaf10 to 8217ed1 Compare January 18, 2017 21:19

haukepetersen approved these changes Jan 18, 2017

View reviewed changes

aabadie merged commit 93abe83 into RIOT-OS:master Jan 19, 2017

aabadie deleted the nucleo_f401_pwm branch March 6, 2017 12:00

Conversation

aabadie commented Dec 8, 2016

Uh oh!

temmihoo commented Dec 9, 2016

Uh oh!

aabadie commented Dec 9, 2016

Uh oh!

temmihoo commented Dec 12, 2016

Uh oh!

haukepetersen Dec 15, 2016

Choose a reason for hiding this comment

Uh oh!

haukepetersen Dec 15, 2016

Choose a reason for hiding this comment

Uh oh!

aabadie commented Jan 3, 2017

Uh oh!

aabadie commented Jan 9, 2017

Uh oh!

aabadie commented Jan 9, 2017

Uh oh!

aabadie commented Jan 18, 2017

Uh oh!

haukepetersen left a comment

Choose a reason for hiding this comment

Uh oh!

haukepetersen Jan 18, 2017

Choose a reason for hiding this comment

Uh oh!

aabadie Jan 18, 2017

Choose a reason for hiding this comment

Uh oh!

aabadie commented Jan 19, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants