From c5a5b34ba4e360794e7f32c234c87f47e7545cca Mon Sep 17 00:00:00 2001
From: Angus Gratton <angus@espressif.com>
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.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
---
 components/driver/include/driver/ledc.h | 16 ++++++++++------
 components/driver/ledc.c                | 16 +++++++++++++---
 2 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/components/driver/include/driver/ledc.h b/components/driver/include/driver/ledc.h
index 5ef668263..e257b70f5 100644
--- a/components/driver/include/driver/ledc.h
+++ b/components/driver/include/driver/ledc.h
@@ -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 */
diff --git a/components/driver/ledc.c b/components/driver/ledc.c
index 1d677e9d1..65ee486a3 100644
--- a/components/driver/ledc.c
+++ b/components/driver/ledc.c
@@ -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);