From 16e454ccaf5f6df2945befd3ba421087c6f682f8 Mon Sep 17 00:00:00 2001 From: hugues Date: Thu, 9 Jul 2020 23:44:49 +0200 Subject: [PATCH 1/5] cpu/stm32/periph/pwm: some bugfixes... --- cpu/stm32/periph/pwm.c | 41 ++++++++++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/cpu/stm32/periph/pwm.c b/cpu/stm32/periph/pwm.c index 3fbf5c0e7e54..05ab58b66736 100644 --- a/cpu/stm32/periph/pwm.c +++ b/cpu/stm32/periph/pwm.c @@ -29,21 +29,29 @@ #include "periph/pwm.h" #include "periph/gpio.h" -#define CCMR_LEFT (TIM_CCMR1_OC1M_1 | TIM_CCMR1_OC1M_2 | \ +#define CCMR_MODE1 (TIM_CCMR1_OC1M_1 | TIM_CCMR1_OC1M_2 | \ TIM_CCMR1_OC2M_1 | TIM_CCMR1_OC2M_2) -#define CCMR_RIGHT (TIM_CCMR1_OC1M_0 | TIM_CCMR1_OC1M_1 | \ +#define CCMR_MODE2 (TIM_CCMR1_OC1M_0 | TIM_CCMR1_OC1M_1 | \ TIM_CCMR1_OC1M_2 | TIM_CCMR1_OC2M_0 | \ TIM_CCMR1_OC2M_1 | TIM_CCMR1_OC2M_2); +static pwm_mode_t mode; + static inline TIM_TypeDef *dev(pwm_t pwm) { return pwm_config[pwm].dev; } -uint32_t pwm_init(pwm_t pwm, pwm_mode_t mode, uint32_t freq, uint16_t res) +uint32_t pwm_init(pwm_t pwm, pwm_mode_t m, uint32_t freq, uint16_t res) { uint32_t timer_clk = periph_timer_clk(pwm_config[pwm].bus); + mode = m; + + /* in PWM_CENTER mode the counter counts up and down at each periode + * so the resolution had to be divided by 2 */ + res *= (mode == PWM_CENTER) ? 2 : 1; + /* verify parameters */ assert((pwm < PWM_NUMOF) && ((freq * res) <= timer_clk)); @@ -53,7 +61,7 @@ uint32_t pwm_init(pwm_t pwm, pwm_mode_t mode, uint32_t freq, uint16_t res) dev(pwm)->CR1 = 0; dev(pwm)->CR2 = 0; for (unsigned i = 0; i < TIMER_CHANNEL_NUMOF; ++i) { - TIM_CHAN(pwm, i) = 0; + TIM_CHAN(pwm, i) = (mode == PWM_RIGHT) ? res : 0; } /* configure the used pins */ @@ -67,21 +75,22 @@ uint32_t pwm_init(pwm_t pwm, pwm_mode_t mode, uint32_t freq, uint16_t res) /* configure the PWM frequency and resolution by setting the auto-reload * and prescaler registers */ dev(pwm)->PSC = (timer_clk / (res * freq)) - 1; - dev(pwm)->ARR = res - 1; + dev(pwm)->ARR = (mode == PWM_CENTER) ? (res / 2) : res - 1; /* set PWM mode */ switch (mode) { case PWM_LEFT: - dev(pwm)->CCMR1 = CCMR_LEFT; - dev(pwm)->CCMR2 = CCMR_LEFT; + dev(pwm)->CCMR1 = CCMR_MODE1; + dev(pwm)->CCMR1 = CCMR_MODE1; break; case PWM_RIGHT: - dev(pwm)->CCMR1 = CCMR_RIGHT; - dev(pwm)->CCMR2 = CCMR_RIGHT; + dev(pwm)->CCMR1 = CCMR_MODE2; + dev(pwm)->CCMR1 = CCMR_MODE2; break; case PWM_CENTER: - dev(pwm)->CCMR1 = 0; - dev(pwm)->CCMR2 = 0; + dev(pwm)->CCMR1 = CCMR_MODE1; + dev(pwm)->CCMR1 = CCMR_MODE1; + /* center-aligned mode 3 */ dev(pwm)->CR1 |= (TIM_CR1_CMS_0 | TIM_CR1_CMS_1); break; } @@ -116,9 +125,15 @@ void pwm_set(pwm_t pwm, uint8_t channel, uint16_t value) (pwm_config[pwm].chan[channel].pin != GPIO_UNDEF)); /* norm value to maximum possible value */ - if (value > dev(pwm)->ARR) { - value = (uint16_t)dev(pwm)->ARR; + if (value > dev(pwm)->ARR + 1) { + value = (uint16_t)dev(pwm)->ARR + 1; } + + if (mode == PWM_RIGHT) { + /* reverse the value */ + value = (uint16_t)dev(pwm)->ARR + 1 - value; + } + /* set new value */ TIM_CHAN(pwm, pwm_config[pwm].chan[channel].cc_chan) = value; } From a5da5953b257e22adfb712100544393b771ac50f Mon Sep 17 00:00:00 2001 From: hugues Date: Fri, 10 Jul 2020 10:22:13 +0200 Subject: [PATCH 2/5] cpu/stm32/periph/pwm: multiple devices PWM_RIGHT mode bugfix --- cpu/stm32/periph/pwm.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/cpu/stm32/periph/pwm.c b/cpu/stm32/periph/pwm.c index 05ab58b66736..65edea3fcd8e 100644 --- a/cpu/stm32/periph/pwm.c +++ b/cpu/stm32/periph/pwm.c @@ -35,19 +35,17 @@ TIM_CCMR1_OC1M_2 | TIM_CCMR1_OC2M_0 | \ TIM_CCMR1_OC2M_1 | TIM_CCMR1_OC2M_2); -static pwm_mode_t mode; +static uint32_t dc_reverse = 0; static inline TIM_TypeDef *dev(pwm_t pwm) { return pwm_config[pwm].dev; } -uint32_t pwm_init(pwm_t pwm, pwm_mode_t m, uint32_t freq, uint16_t res) +uint32_t pwm_init(pwm_t pwm, pwm_mode_t mode, uint32_t freq, uint16_t res) { uint32_t timer_clk = periph_timer_clk(pwm_config[pwm].bus); - mode = m; - /* in PWM_CENTER mode the counter counts up and down at each periode * so the resolution had to be divided by 2 */ res *= (mode == PWM_CENTER) ? 2 : 1; @@ -77,6 +75,8 @@ uint32_t pwm_init(pwm_t pwm, pwm_mode_t m, uint32_t freq, uint16_t res) dev(pwm)->PSC = (timer_clk / (res * freq)) - 1; dev(pwm)->ARR = (mode == PWM_CENTER) ? (res / 2) : res - 1; + dc_reverse &= ~(1 << pwm); + /* set PWM mode */ switch (mode) { case PWM_LEFT: @@ -86,6 +86,8 @@ uint32_t pwm_init(pwm_t pwm, pwm_mode_t m, uint32_t freq, uint16_t res) case PWM_RIGHT: dev(pwm)->CCMR1 = CCMR_MODE2; dev(pwm)->CCMR1 = CCMR_MODE2; + /* duty cycle should be reversed */ + dc_reverse |= (1 << pwm); break; case PWM_CENTER: dev(pwm)->CCMR1 = CCMR_MODE1; @@ -129,7 +131,7 @@ void pwm_set(pwm_t pwm, uint8_t channel, uint16_t value) value = (uint16_t)dev(pwm)->ARR + 1; } - if (mode == PWM_RIGHT) { + if (dc_reverse & (1 << pwm)) { /* reverse the value */ value = (uint16_t)dev(pwm)->ARR + 1 - value; } From d069c6e7873889666d2b331c06a1078ab0a64c1b Mon Sep 17 00:00:00 2001 From: hugues Date: Fri, 10 Jul 2020 10:41:26 +0200 Subject: [PATCH 3/5] cpu/stm32/periph/pwm: CCMR1 was defined a second time instead of CCMR2 --- cpu/stm32/periph/pwm.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cpu/stm32/periph/pwm.c b/cpu/stm32/periph/pwm.c index 65edea3fcd8e..2c9a4dfcae47 100644 --- a/cpu/stm32/periph/pwm.c +++ b/cpu/stm32/periph/pwm.c @@ -81,17 +81,17 @@ uint32_t pwm_init(pwm_t pwm, pwm_mode_t mode, uint32_t freq, uint16_t res) switch (mode) { case PWM_LEFT: dev(pwm)->CCMR1 = CCMR_MODE1; - dev(pwm)->CCMR1 = CCMR_MODE1; + dev(pwm)->CCMR2 = CCMR_MODE1; break; case PWM_RIGHT: dev(pwm)->CCMR1 = CCMR_MODE2; - dev(pwm)->CCMR1 = CCMR_MODE2; + dev(pwm)->CCMR2 = CCMR_MODE2; /* duty cycle should be reversed */ dc_reverse |= (1 << pwm); break; case PWM_CENTER: dev(pwm)->CCMR1 = CCMR_MODE1; - dev(pwm)->CCMR1 = CCMR_MODE1; + dev(pwm)->CCMR2 = CCMR_MODE1; /* center-aligned mode 3 */ dev(pwm)->CR1 |= (TIM_CR1_CMS_0 | TIM_CR1_CMS_1); break; From 304d3f9e8d2accb5acec9de10fa948d46ec47088 Mon Sep 17 00:00:00 2001 From: hugues Date: Fri, 10 Jul 2020 12:47:05 +0200 Subject: [PATCH 4/5] cpu/stm32/periph/pwm: some bugfixes... --- cpu/stm32/periph/pwm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpu/stm32/periph/pwm.c b/cpu/stm32/periph/pwm.c index 2c9a4dfcae47..3f049b3f01e4 100644 --- a/cpu/stm32/periph/pwm.c +++ b/cpu/stm32/periph/pwm.c @@ -46,7 +46,7 @@ uint32_t pwm_init(pwm_t pwm, pwm_mode_t mode, uint32_t freq, uint16_t res) { uint32_t timer_clk = periph_timer_clk(pwm_config[pwm].bus); - /* in PWM_CENTER mode the counter counts up and down at each periode + /* in PWM_CENTER mode the counter counts up and down at each period * so the resolution had to be divided by 2 */ res *= (mode == PWM_CENTER) ? 2 : 1; @@ -131,7 +131,7 @@ void pwm_set(pwm_t pwm, uint8_t channel, uint16_t value) value = (uint16_t)dev(pwm)->ARR + 1; } - if (dc_reverse & (1 << pwm)) { + if ((dc_reverse & (1 << pwm)) != 0) { /* reverse the value */ value = (uint16_t)dev(pwm)->ARR + 1 - value; } From 0926a04b08e8e1ab65e131dffd2fccd7d41440b9 Mon Sep 17 00:00:00 2001 From: hugues Date: Tue, 14 Jul 2020 01:41:16 +0200 Subject: [PATCH 5/5] cpu/stm32/periph/pwm: useless static var and a semicolon removed --- cpu/stm32/periph/pwm.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/cpu/stm32/periph/pwm.c b/cpu/stm32/periph/pwm.c index 3f049b3f01e4..c96ff970501b 100644 --- a/cpu/stm32/periph/pwm.c +++ b/cpu/stm32/periph/pwm.c @@ -33,9 +33,7 @@ TIM_CCMR1_OC2M_1 | TIM_CCMR1_OC2M_2) #define CCMR_MODE2 (TIM_CCMR1_OC1M_0 | TIM_CCMR1_OC1M_1 | \ TIM_CCMR1_OC1M_2 | TIM_CCMR1_OC2M_0 | \ - TIM_CCMR1_OC2M_1 | TIM_CCMR1_OC2M_2); - -static uint32_t dc_reverse = 0; + TIM_CCMR1_OC2M_1 | TIM_CCMR1_OC2M_2) static inline TIM_TypeDef *dev(pwm_t pwm) { @@ -75,8 +73,6 @@ uint32_t pwm_init(pwm_t pwm, pwm_mode_t mode, uint32_t freq, uint16_t res) dev(pwm)->PSC = (timer_clk / (res * freq)) - 1; dev(pwm)->ARR = (mode == PWM_CENTER) ? (res / 2) : res - 1; - dc_reverse &= ~(1 << pwm); - /* set PWM mode */ switch (mode) { case PWM_LEFT: @@ -87,7 +83,6 @@ uint32_t pwm_init(pwm_t pwm, pwm_mode_t mode, uint32_t freq, uint16_t res) dev(pwm)->CCMR1 = CCMR_MODE2; dev(pwm)->CCMR2 = CCMR_MODE2; /* duty cycle should be reversed */ - dc_reverse |= (1 << pwm); break; case PWM_CENTER: dev(pwm)->CCMR1 = CCMR_MODE1; @@ -131,7 +126,7 @@ void pwm_set(pwm_t pwm, uint8_t channel, uint16_t value) value = (uint16_t)dev(pwm)->ARR + 1; } - if ((dc_reverse & (1 << pwm)) != 0) { + if (dev(pwm)->CCMR1 == CCMR_MODE2) { /* reverse the value */ value = (uint16_t)dev(pwm)->ARR + 1 - value; }