From fdeaf805d330543ce317eccfed662048d83064ec Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Sun, 13 Nov 2022 18:17:47 -0500 Subject: [PATCH] STM: off-by-one TIMx reference; other code cleanup and minor fixes --- ports/stm/common-hal/pulseio/PulseIn.c | 3 +- ports/stm/common-hal/pwmio/PWMOut.c | 97 ++++++++++++-------------- ports/stm/common-hal/pwmio/PWMOut.h | 6 +- ports/stm/peripherals/timers.c | 58 ++++++++++++--- 4 files changed, 100 insertions(+), 64 deletions(-) diff --git a/ports/stm/common-hal/pulseio/PulseIn.c b/ports/stm/common-hal/pulseio/PulseIn.c index 4ed2600d55..5b7602d9bf 100644 --- a/ports/stm/common-hal/pulseio/PulseIn.c +++ b/ports/stm/common-hal/pulseio/PulseIn.c @@ -106,7 +106,8 @@ void pulsein_reset(void) { memset(callback_obj_ref, 0, sizeof(callback_obj_ref)); HAL_TIM_Base_DeInit(&tim_handle); - tim_clock_disable(stm_peripherals_timer_get_index(tim_handle.Instance)); + // tim_clock_disable() takes a bitmask of timers. + tim_clock_disable(1 << stm_peripherals_timer_get_index(tim_handle.Instance)); memset(&tim_handle, 0, sizeof(tim_handle)); refcount = 0; } diff --git a/ports/stm/common-hal/pwmio/PWMOut.c b/ports/stm/common-hal/pwmio/PWMOut.c index 16d510b605..cc712497d2 100644 --- a/ports/stm/common-hal/pwmio/PWMOut.c +++ b/ports/stm/common-hal/pwmio/PWMOut.c @@ -36,23 +36,24 @@ #include "timers.h" -#define ALL_CLOCKS 0xFFFF - -STATIC uint8_t reserved_tim[TIM_BANK_ARRAY_LEN]; +// Bitmask of channels taken. +STATIC uint8_t tim_channels_taken[TIM_BANK_ARRAY_LEN]; +// Initial frequency timer is set to. STATIC uint32_t tim_frequencies[TIM_BANK_ARRAY_LEN]; STATIC bool never_reset_tim[TIM_BANK_ARRAY_LEN]; STATIC uint32_t timer_get_internal_duty(uint16_t duty, uint32_t period) { // duty cycle is duty/0xFFFF fraction x (number of pulses per period) - return (duty * period) / ((1 << 16) - 1); + return (duty * period) / 0xffff; } STATIC bool timer_get_optimal_divisors(uint32_t *period, uint32_t *prescaler, uint32_t frequency, uint32_t source_freq) { // Find the largest possible period supported by this frequency - for (int i = 0; i < (1 << 16); i++) { + *prescaler = 0; + for (uint32_t i = 1; i <= 0xffff; i++) { *period = source_freq / (i * frequency); - if (*period < (1 << 16) && *period >= 2) { + if (*period <= 0xffff && *period >= 2) { *prescaler = i; break; } @@ -62,13 +63,10 @@ STATIC bool timer_get_optimal_divisors(uint32_t *period, uint32_t *prescaler, } void pwmout_reset(void) { - uint16_t never_reset_mask = 0x00; for (int i = 0; i < TIM_BANK_ARRAY_LEN; i++) { if (!never_reset_tim[i]) { - reserved_tim[i] = 0x00; - tim_frequencies[i] = 0x00; - } else { - never_reset_mask |= 1 << i; + tim_channels_taken[i] = 0x00; + tim_frequencies[i] = 0; } } } @@ -78,73 +76,69 @@ pwmout_result_t common_hal_pwmio_pwmout_construct(pwmio_pwmout_obj_t *self, uint16_t duty, uint32_t frequency, bool variable_frequency) { - TIM_TypeDef *TIMx; - uint8_t tim_num = MP_ARRAY_SIZE(mcu_tim_pin_list); - bool tim_taken_internal = false; - bool tim_chan_taken = false; - bool tim_taken_f_mismatch = false; - bool var_freq_mismatch = false; + // Default error is no timer at all on pin. + pwmout_result_t last_failure = PWMOUT_INVALID_PIN; bool first_time_setup = true; - for (uint i = 0; i < tim_num; i++) { - const mcu_tim_pin_obj_t *l_tim = &mcu_tim_pin_list[i]; - uint8_t l_tim_index = l_tim->tim_index; - uint8_t l_tim_channel = l_tim->channel_index; + uint8_t tim_index; + uint8_t tim_channel_index; + + self->tim = NULL; + for (uint i = 0; i < MP_ARRAY_SIZE(mcu_tim_pin_list); i++) { + const mcu_tim_pin_obj_t *tim = &mcu_tim_pin_list[i]; + tim_index = tim->tim_index; + tim_channel_index = tim->channel_index; // if pin is same - if (l_tim->pin == pin) { + if (tim->pin == pin) { // check if the timer has a channel active, or is reserved by main timer system - if (l_tim_index < TIM_BANK_ARRAY_LEN && reserved_tim[l_tim_index] != 0) { + if (tim_index < TIM_BANK_ARRAY_LEN && tim_channels_taken[tim_index] != 0) { // Timer has already been reserved by an internal module - if (stm_peripherals_timer_is_reserved(mcu_tim_banks[l_tim_index])) { - tim_taken_internal = true; + if (stm_peripherals_timer_is_reserved(mcu_tim_banks[tim_index])) { + last_failure = PWMOUT_ALL_TIMERS_ON_PIN_IN_USE; continue; // keep looking } // is it the same channel? (or all channels reserved by a var-freq) - if (reserved_tim[l_tim_index] & 1 << (l_tim_channel)) { - tim_chan_taken = true; + if (tim_channels_taken[tim_index] & (1 << tim_channel_index)) { + last_failure = PWMOUT_ALL_TIMERS_ON_PIN_IN_USE; continue; // keep looking, might be another viable option } // If the frequencies are the same it's ok - if (tim_frequencies[l_tim_index] != frequency) { - tim_taken_f_mismatch = true; + if (tim_frequencies[tim_index] != frequency) { + last_failure = PWMOUT_INVALID_FREQUENCY_ON_PIN; continue; // keep looking } // you can't put a variable frequency on a partially reserved timer if (variable_frequency) { - var_freq_mismatch = true; + last_failure = PWMOUT_VARIABLE_FREQUENCY_NOT_AVAILABLE; continue; // keep looking } first_time_setup = false; // skip setting up the timer } // No problems taken, so set it up - self->tim = l_tim; + self->tim = tim; break; } } + TIM_TypeDef *TIMx; + // handle valid/invalid timer instance if (self->tim != NULL) { // create instance - TIMx = mcu_tim_banks[self->tim->tim_index]; + TIMx = mcu_tim_banks[tim_index]; // reserve timer/channel if (variable_frequency) { - reserved_tim[self->tim->tim_index] = 0x0F; + // Take all the channels. + tim_channels_taken[tim_index] = 0x0F; } else { - reserved_tim[self->tim->tim_index] |= 1 << self->tim->channel_index; + tim_channels_taken[tim_index] |= 1 << tim_channel_index; } - tim_frequencies[self->tim->tim_index] = frequency; + tim_frequencies[tim_index] = frequency; stm_peripherals_timer_reserve(TIMx); - } else { // no match found - if (tim_chan_taken || tim_taken_internal) { - return PWMOUT_ALL_TIMERS_ON_PIN_IN_USE; - } else if (tim_taken_f_mismatch) { - return PWMOUT_INVALID_FREQUENCY_ON_PIN; - } else if (var_freq_mismatch) { - return PWMOUT_VARIABLE_FREQUENCY_NOT_AVAILABLE; - } else { - return PWMOUT_INVALID_PIN; - } + } else { + // no match found + return last_failure; } uint32_t prescaler = 0; // prescaler is 15 bit @@ -163,10 +157,10 @@ pwmout_result_t common_hal_pwmio_pwmout_construct(pwmio_pwmout_obj_t *self, HAL_GPIO_Init(pin_port(pin->port), &GPIO_InitStruct); self->pin = pin; - tim_clock_enable(1 << (self->tim->tim_index)); + tim_clock_enable(1 << tim_index); - // translate channel into handle value - self->channel = 4 * self->tim->channel_index; + // translate channel into handle value: TIM_CHANNEL_1, _2, _3, _4. + self->channel = 4 * tim_channel_index; // Timer init self->handle.Instance = TIMx; @@ -175,6 +169,7 @@ pwmout_result_t common_hal_pwmio_pwmout_construct(pwmio_pwmout_obj_t *self, self->handle.Init.ClockDivision = TIM_CLOCKDIVISION_DIV1; self->handle.Init.CounterMode = TIM_COUNTERMODE_UP; self->handle.Init.RepetitionCounter = 0; + self->handle.Init.AutoReloadPreload = TIM_AUTORELOAD_PRELOAD_DISABLE; // only run init if this is the first instance of this timer if (first_time_setup) { @@ -232,15 +227,15 @@ void common_hal_pwmio_pwmout_deinit(pwmio_pwmout_obj_t *self) { } // var freq shuts down entire timer, others just their channel if (self->variable_frequency) { - reserved_tim[self->tim->tim_index] = 0x00; + tim_channels_taken[self->tim->tim_index] = 0x00; } else { - reserved_tim[self->tim->tim_index] &= ~(1 << self->tim->channel_index); + tim_channels_taken[self->tim->tim_index] &= ~(1 << self->tim->channel_index); HAL_TIM_PWM_Stop(&self->handle, self->channel); } common_hal_reset_pin(self->pin); // if reserved timer has no active channels, we can disable it - if (reserved_tim[self->tim->tim_index] == 0) { + if (tim_channels_taken[self->tim->tim_index] == 0) { tim_frequencies[self->tim->tim_index] = 0x00; stm_peripherals_timer_free(self->handle.Instance); } diff --git a/ports/stm/common-hal/pwmio/PWMOut.h b/ports/stm/common-hal/pwmio/PWMOut.h index de3a304721..9a8b897c6c 100644 --- a/ports/stm/common-hal/pwmio/PWMOut.h +++ b/ports/stm/common-hal/pwmio/PWMOut.h @@ -39,12 +39,12 @@ typedef struct { TIM_HandleTypeDef handle; TIM_OC_InitTypeDef chan_handle; const mcu_tim_pin_obj_t *tim; - uint8_t channel : 7; - bool variable_frequency : 1; - uint16_t duty_cycle; uint32_t frequency; uint32_t period; const mcu_pin_obj_t *pin; + uint16_t duty_cycle; + uint8_t channel; + bool variable_frequency; } pwmio_pwmout_obj_t; void pwmout_reset(void); diff --git a/ports/stm/peripherals/timers.c b/ports/stm/peripherals/timers.c index ed0f2ec38a..20b6bcc759 100644 --- a/ports/stm/peripherals/timers.c +++ b/ports/stm/peripherals/timers.c @@ -150,26 +150,65 @@ static size_t irq_map[] = { }; // Get the frequency (in Hz) of the source clock for the given timer. -// On STM32F405/407/415/417 there are 2 cases for how the clock freq is set. -// If the APB prescaler is 1, then the timer clock is equal to its respective -// APB clock. Otherwise (APB prescaler > 1) the timer clock is twice its -// respective APB clock. See DM00031020 Rev 4, page 115. +// +// From STM ref manual: DM00031020 Rev 19, section 7.2, page 217: +// +// The timer clock frequencies for STM32F405xx/07xx and STM32F415xx/17xx are +// automatically set by hardware. There are two cases: +// 1. If the APB prescaler is 1, the timer clock frequencies are set to the same frequency as +// that of the APB domain to which the timers are connected. +// 2. Otherwise, they are set to twice (×2) the frequency of the APB domain to which the +// timers are connected. + +// From STM ref manual: DM00031020 Rev 19, section 6.2, page 153: +// +// The timer clock frequencies for STM32F42xxx and STM32F43xxx are automatically set by +// hardware. There are two cases depending on the value of TIMPRE bit in RCC_CFGR [sic - should be RCC_DKCFGR] +// register: +// * If TIMPRE bit in RCC_DKCFGR register is reset: +// If the APB prescaler is configured to a division factor of 1, the timer clock frequencies +// (TIMxCLK) are set to PCLKx. Otherwise, the timer clock frequencies are twice the +// frequency of the APB domain to which the timers are connected: TIMxCLK = 2xPCLKx. +// * If TIMPRE bit in RCC_DKCFGR register is set: +// If the APB prescaler is configured to a division factor of 1, 2 or 4, the timer clock +// frequencies (TIMxCLK) are set to HCLK. Otherwise, the timer clock frequencies is four +// times the frequency of the APB domain to which the timers are connected: TIMxCLK = 4xPCLKx. + uint32_t stm_peripherals_timer_get_source_freq(TIM_TypeDef *timer) { - size_t tim_id = stm_peripherals_timer_get_index(timer); + // The timer index starts at 0, but the timer numbers start at TIM1. + size_t tim_id = stm_peripherals_timer_get_index(timer) + 1; uint32_t source, clk_div; if (tim_id == 1 || (8 <= tim_id && tim_id <= 11)) { // TIM{1,8,9,10,11} are on APB2 source = HAL_RCC_GetPCLK2Freq(); - clk_div = RCC->CFGR & RCC_CFGR_PPRE2; + // 0b0xx means not divided; 0b100 is divide by 2; 0b101 by 4; 0b110 by 8; 0b111 by 16. + clk_div = (RCC->CFGR & RCC_CFGR_PPRE2) >> RCC_CFGR_PPRE2_Pos; } else { // TIM{2,3,4,5,6,7,12,13,14} are on APB1 source = HAL_RCC_GetPCLK1Freq(); - clk_div = RCC->CFGR & RCC_CFGR_PPRE1; + // 0b0xx means not divided; 0b100 is divide by 2; 0b101 by 4; 0b110 by 8; 0b111 by 16. + clk_div = (RCC->CFGR & RCC_CFGR_PPRE1) >> RCC_CFGR_PPRE1_Pos; } - if (clk_div != 0) { - // APB prescaler for this timer is > 1 + + // Only some STM32's have TIMPRE. + #if defined(RCC_CFGR_TIMPRE) + uint32_t timpre = RCC->DCKCFGR & RCC_CFGR_TIMPRE; + if (timpre == 0) { + if (clk_div >= 0b100) { + source *= 2; + } + } else { + if (clk_div > 0b101) { + source *= 4; + } else { + source = HAL_RCC_GetHCLKFreq(); + } + } + #else + if (clk_div >= 0b100) { source *= 2; } + #endif return source; } @@ -271,6 +310,7 @@ bool stm_peripherals_timer_is_reserved(TIM_TypeDef *instance) { return stm_timer_reserved[tim_idx]; } +// Note this returns a timer index starting at zero, corresponding to TIM1. size_t stm_peripherals_timer_get_index(TIM_TypeDef *instance) { for (size_t i = 0; i < MP_ARRAY_SIZE(mcu_tim_banks); i++) { if (instance == mcu_tim_banks[i]) {