From 435dd546cc6ef45fb7646072a6f54381b45fab09 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Mon, 16 Dec 2019 17:38:00 +1100 Subject: [PATCH] driver: Avoid possible accidental mismatch between ledc_clk_src_t & ledc_clk_cfg_t enum ledc_types.h includes two similar enums, ledc_clk_src_t & ledc_clk_cfg_t. Latter was added in ESP-IDF v4.0. 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 --- components/soc/esp32/include/hal/ledc_ll.h | 8 ++++++-- components/soc/include/hal/ledc_types.h | 14 +++++++++----- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/components/soc/esp32/include/hal/ledc_ll.h b/components/soc/esp32/include/hal/ledc_ll.h index 2082d9a70..674b55cff 100644 --- a/components/soc/esp32/include/hal/ledc_ll.h +++ b/components/soc/esp32/include/hal/ledc_ll.h @@ -138,7 +138,7 @@ static inline void ledc_ll_get_clock_divider(ledc_dev_t *hw, ledc_mode_t speed_m * @return None */ static inline void ledc_ll_set_clock_source(ledc_dev_t *hw, ledc_mode_t speed_mode, ledc_timer_t timer_sel, ledc_clk_src_t clk_src){ - hw->timer_group[speed_mode].timer[timer_sel].conf.tick_sel = clk_src; + hw->timer_group[speed_mode].timer[timer_sel].conf.tick_sel = (clk_src == LEDC_APB_CLK); } /** @@ -152,7 +152,11 @@ static inline void ledc_ll_set_clock_source(ledc_dev_t *hw, ledc_mode_t speed_mo * @return None */ static inline void ledc_ll_get_clock_source(ledc_dev_t *hw, ledc_mode_t speed_mode, ledc_timer_t timer_sel, ledc_clk_src_t *clk_src){ - *clk_src = hw->timer_group[speed_mode].timer[timer_sel].conf.tick_sel; + if (hw->timer_group[speed_mode].timer[timer_sel].conf.tick_sel) { + *clk_src = LEDC_APB_CLK; + } else { + *clk_src = LEDC_REF_TICK; + } } /** diff --git a/components/soc/include/hal/ledc_types.h b/components/soc/include/hal/ledc_types.h index 855c18c6c..92352cedc 100644 --- a/components/soc/include/hal/ledc_types.h +++ b/components/soc/include/hal/ledc_types.h @@ -41,11 +41,6 @@ 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_SLOW_CLK_RTC8M = 0, /*!< LEDC low speed timer clock source is 8MHz RTC clock*/ LEDC_SLOW_CLK_APB, /*!< LEDC low speed timer clock source is 80MHz APB clock*/ @@ -64,6 +59,15 @@ typedef enum { #endif } 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 */