driver: Avoid possible accidental mismatch between ledc_clk_src_t & ledc_clk_cfg_t enum

ledc.h includes two similar enums, ledc_clk_src_t & ledc_clk_cfg_t.

The two enums do different things but there are two similar names: LEDC_REF_TICK / LEDC_USE_REF_TICK
and LEDC_APB_CLK / LEDC_USE_APB_CLK.

Because C will accept any enum or integer value for an enum argument, there's no easy way to check
the correct enum is passed without using static analysis.

To avoid accidental errors, make the numeric values for the two similarly named enums the same.,

Noticed when looking into https://github.com/espressif/esp-idf/issues/4476
This commit is contained in:
Angus Gratton 2019-12-16 17:38:00 +11:00 committed by Angus Gratton
parent 376703bf05
commit c5a5b34ba4
2 changed files with 23 additions and 9 deletions

View File

@ -46,18 +46,22 @@ typedef enum {
LEDC_DUTY_DIR_MAX,
} ledc_duty_direction_t;
typedef enum {
LEDC_REF_TICK = 0, /*!< LEDC timer clock divided from reference tick (1Mhz) */
LEDC_APB_CLK, /*!< LEDC timer clock divided from APB clock (80Mhz) */
} ledc_clk_src_t;
typedef enum {
LEDC_AUTO_CLK, /*!< The driver will automatically select the source clock(REF_TICK or APB) based on the giving resolution and duty parameter when init the timer*/
LEDC_AUTO_CLK, /*!< The driver will automatically select the source clock(REF_TICK or APB) based on the giving resolution and duty parameter when init the timer*/
LEDC_USE_REF_TICK, /*!< LEDC timer select REF_TICK clock as source clock*/
LEDC_USE_APB_CLK, /*!< LEDC timer select APB clock as source clock*/
LEDC_USE_RTC8M_CLK, /*!< LEDC timer select RTC8M_CLK as source clock. Only for low speed channels and this parameter must be the same for all low speed channels*/
} ledc_clk_cfg_t;
/* Note: Setting numeric values to match ledc_clk_cfg_t values are a hack to avoid collision with
LEDC_AUTO_CLK in the driver, as these enums have very similar names and user may pass
one of these by mistake. */
typedef enum {
LEDC_REF_TICK = LEDC_USE_REF_TICK, /*!< LEDC timer clock divided from reference tick (1Mhz) */
LEDC_APB_CLK = LEDC_USE_APB_CLK, /*!< LEDC timer clock divided from APB clock (80Mhz) */
} ledc_clk_src_t;
typedef enum {
LEDC_TIMER_0 = 0, /*!< LEDC timer 0 */
LEDC_TIMER_1, /*!< LEDC timer 1 */

View File

@ -162,7 +162,7 @@ esp_err_t ledc_timer_set(ledc_mode_t speed_mode, ledc_timer_t timer_sel, uint32_
LEDC_ARG_CHECK(timer_sel < LEDC_TIMER_MAX, "timer_select");
portENTER_CRITICAL(&ledc_spinlock);
LEDC.timer_group[speed_mode].timer[timer_sel].conf.clock_divider = clock_divider;
LEDC.timer_group[speed_mode].timer[timer_sel].conf.tick_sel = clk_src;
LEDC.timer_group[speed_mode].timer[timer_sel].conf.tick_sel = (clk_src == LEDC_APB_CLK);
LEDC.timer_group[speed_mode].timer[timer_sel].conf.duty_resolution = duty_resolution;
ledc_ls_timer_update(speed_mode, timer_sel);
portEXIT_CRITICAL(&ledc_spinlock);
@ -481,7 +481,12 @@ esp_err_t ledc_set_freq(ledc_mode_t speed_mode, ledc_timer_t timer_num, uint32_t
esp_err_t ret = ESP_OK;
uint32_t clock_divider = 0;
uint32_t duty_resolution = LEDC.timer_group[speed_mode].timer[timer_num].conf.duty_resolution;
uint32_t timer_source_clk = LEDC.timer_group[speed_mode].timer[timer_num].conf.tick_sel;
ledc_clk_src_t timer_source_clk;
if (LEDC.timer_group[speed_mode].timer[timer_num].conf.tick_sel) {
timer_source_clk = LEDC_APB_CLK;
} else {
timer_source_clk = LEDC_REF_TICK;
}
uint32_t precision = (0x1 << duty_resolution);
if (timer_source_clk == LEDC_APB_CLK) {
clock_divider = ((uint64_t) LEDC_APB_CLK_HZ << 8) / freq_hz / precision;
@ -503,7 +508,12 @@ uint32_t ledc_get_freq(ledc_mode_t speed_mode, ledc_timer_t timer_num)
LEDC_ARG_CHECK(speed_mode < LEDC_SPEED_MODE_MAX, "speed_mode");
portENTER_CRITICAL(&ledc_spinlock);
uint32_t freq = 0;
uint32_t timer_source_clk = LEDC.timer_group[speed_mode].timer[timer_num].conf.tick_sel;
ledc_clk_src_t timer_source_clk;
if (LEDC.timer_group[speed_mode].timer[timer_num].conf.tick_sel) {
timer_source_clk = LEDC_APB_CLK;
} else {
timer_source_clk = LEDC_REF_TICK;
}
uint32_t duty_resolution = LEDC.timer_group[speed_mode].timer[timer_num].conf.duty_resolution;
uint32_t clock_divider = LEDC.timer_group[speed_mode].timer[timer_num].conf.clock_divider;
uint32_t precision = (0x1 << duty_resolution);