From b0bde58521b1869da6d3aa7d6fcc0f8abfb71060 Mon Sep 17 00:00:00 2001 From: Eduardo Lacerda Campos Date: Fri, 1 Sep 2023 18:33:08 -0400 Subject: [PATCH 1/3] fix(uart): Fix uart_ll_set_baudrate div-by-zero crash due to uint32_t overflow Merges https://github.com/espressif/esp-idf/pull/12179 --- components/hal/esp32c2/include/hal/uart_ll.h | 6 ++++-- components/hal/esp32c3/include/hal/uart_ll.h | 6 ++++-- components/hal/esp32c6/include/hal/uart_ll.h | 4 +++- components/hal/esp32h2/include/hal/uart_ll.h | 4 +++- components/hal/esp32s3/include/hal/uart_ll.h | 6 ++++-- 5 files changed, 18 insertions(+), 8 deletions(-) diff --git a/components/hal/esp32c2/include/hal/uart_ll.h b/components/hal/esp32c2/include/hal/uart_ll.h index 9bb37c71d4..f6cd7915cf 100644 --- a/components/hal/esp32c2/include/hal/uart_ll.h +++ b/components/hal/esp32c2/include/hal/uart_ll.h @@ -159,13 +159,15 @@ FORCE_INLINE_ATTR void uart_ll_set_baudrate(uart_dev_t *hw, uint32_t baud, uint3 { #define DIV_UP(a, b) (((a) + (b) - 1) / (b)) const uint32_t max_div = BIT(12) - 1; // UART divider integer part only has 12 bits - int sclk_div = DIV_UP(sclk_freq, max_div * baud); + uint32_t sclk_div = DIV_UP(sclk_freq, (uint64_t)max_div * baud); + + if (sclk_div == 0) abort(); uint32_t clk_div = ((sclk_freq) << 4) / (baud * sclk_div); // The baud rate configuration register is divided into // an integer part and a fractional part. hw->clk_div.div_int = clk_div >> 4; - hw->clk_div.div_frag = clk_div & 0xf; + hw->clk_div.div_frag = clk_div & 0xf; hw->clk_conf.sclk_div_num = sclk_div - 1; #undef DIV_UP } diff --git a/components/hal/esp32c3/include/hal/uart_ll.h b/components/hal/esp32c3/include/hal/uart_ll.h index ea29b56317..636622f1ed 100644 --- a/components/hal/esp32c3/include/hal/uart_ll.h +++ b/components/hal/esp32c3/include/hal/uart_ll.h @@ -161,13 +161,15 @@ FORCE_INLINE_ATTR void uart_ll_set_baudrate(uart_dev_t *hw, uint32_t baud, uint3 { #define DIV_UP(a, b) (((a) + (b) - 1) / (b)) const uint32_t max_div = BIT(12) - 1; // UART divider integer part only has 12 bits - int sclk_div = DIV_UP(sclk_freq, max_div * baud); + uint32_t sclk_div = DIV_UP(sclk_freq, (uint64_t)max_div * baud); + + if (sclk_div == 0) abort(); uint32_t clk_div = ((sclk_freq) << 4) / (baud * sclk_div); // The baud rate configuration register is divided into // an integer part and a fractional part. hw->clk_div.div_int = clk_div >> 4; - hw->clk_div.div_frag = clk_div & 0xf; + hw->clk_div.div_frag = clk_div & 0xf; HAL_FORCE_MODIFY_U32_REG_FIELD(hw->clk_conf, sclk_div_num, sclk_div - 1); #undef DIV_UP } diff --git a/components/hal/esp32c6/include/hal/uart_ll.h b/components/hal/esp32c6/include/hal/uart_ll.h index 5024188792..e8c403cedc 100644 --- a/components/hal/esp32c6/include/hal/uart_ll.h +++ b/components/hal/esp32c6/include/hal/uart_ll.h @@ -305,7 +305,9 @@ FORCE_INLINE_ATTR void uart_ll_set_baudrate(uart_dev_t *hw, uint32_t baud, uint3 { #define DIV_UP(a, b) (((a) + (b) - 1) / (b)) const uint32_t max_div = BIT(12) - 1; // UART divider integer part only has 12 bits - int sclk_div = DIV_UP(sclk_freq, max_div * baud); + uint32_t sclk_div = DIV_UP(sclk_freq, (uint64_t)max_div * baud); + + if (sclk_div == 0) abort(); uint32_t clk_div = ((sclk_freq) << 4) / (baud * sclk_div); // The baud rate configuration register is divided into diff --git a/components/hal/esp32h2/include/hal/uart_ll.h b/components/hal/esp32h2/include/hal/uart_ll.h index f513c23e4e..931c871eff 100644 --- a/components/hal/esp32h2/include/hal/uart_ll.h +++ b/components/hal/esp32h2/include/hal/uart_ll.h @@ -194,7 +194,9 @@ FORCE_INLINE_ATTR void uart_ll_set_baudrate(uart_dev_t *hw, uint32_t baud, uint3 { #define DIV_UP(a, b) (((a) + (b) - 1) / (b)) const uint32_t max_div = BIT(12) - 1; // UART divider integer part only has 12 bits - int sclk_div = DIV_UP(sclk_freq, max_div * baud); + uint32_t sclk_div = DIV_UP(sclk_freq, (uint64_t)max_div * baud); + + if (sclk_div == 0) abort(); uint32_t clk_div = ((sclk_freq) << 4) / (baud * sclk_div); // The baud rate configuration register is divided into diff --git a/components/hal/esp32s3/include/hal/uart_ll.h b/components/hal/esp32s3/include/hal/uart_ll.h index ac75036664..d0edfb18eb 100644 --- a/components/hal/esp32s3/include/hal/uart_ll.h +++ b/components/hal/esp32s3/include/hal/uart_ll.h @@ -133,13 +133,15 @@ FORCE_INLINE_ATTR void uart_ll_set_baudrate(uart_dev_t *hw, uint32_t baud, uint3 { #define DIV_UP(a, b) (((a) + (b) - 1) / (b)) const uint32_t max_div = BIT(12) - 1; // UART divider integer part only has 12 bits - int sclk_div = DIV_UP(sclk_freq, max_div * baud); + uint32_t sclk_div = DIV_UP(sclk_freq, (uint64_t)max_div * baud); + + if (sclk_div == 0) abort(); uint32_t clk_div = ((sclk_freq) << 4) / (baud * sclk_div); // The baud rate configuration register is divided into // an integer part and a fractional part. hw->clkdiv.clkdiv = clk_div >> 4; - hw->clkdiv.clkdiv_frag = clk_div & 0xf; + hw->clkdiv.clkdiv_frag = clk_div & 0xf; HAL_FORCE_MODIFY_U32_REG_FIELD(hw->clk_conf, sclk_div_num, sclk_div - 1); #undef DIV_UP } From f227cbca1ba0fed66de75bf79231fd5f21e8d754 Mon Sep 17 00:00:00 2001 From: Song Ruo Jing Date: Mon, 11 Sep 2023 16:57:25 +0800 Subject: [PATCH 2/3] fix(uart): Add 8/16-bit register field access workaround to ESP32C2 --- components/hal/esp32c2/include/hal/uart_ll.h | 31 +++++++++++--------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/components/hal/esp32c2/include/hal/uart_ll.h b/components/hal/esp32c2/include/hal/uart_ll.h index f6cd7915cf..f42aded269 100644 --- a/components/hal/esp32c2/include/hal/uart_ll.h +++ b/components/hal/esp32c2/include/hal/uart_ll.h @@ -7,12 +7,14 @@ // The LL layer for UART register operations. // Note that most of the register operations in this layer are non-atomic operations. - #pragma once + +#include #include "hal/uart_types.h" #include "soc/uart_reg.h" #include "soc/uart_struct.h" -#include "hal/clk_tree_ll.h" +#include "soc/clk_tree_defs.h" +#include "hal/misc.h" #include "esp_attr.h" #ifdef __cplusplus @@ -168,7 +170,7 @@ FORCE_INLINE_ATTR void uart_ll_set_baudrate(uart_dev_t *hw, uint32_t baud, uint3 // an integer part and a fractional part. hw->clk_div.div_int = clk_div >> 4; hw->clk_div.div_frag = clk_div & 0xf; - hw->clk_conf.sclk_div_num = sclk_div - 1; + HAL_FORCE_MODIFY_U32_REG_FIELD(hw->clk_conf, sclk_div_num, sclk_div - 1); #undef DIV_UP } @@ -184,7 +186,8 @@ FORCE_INLINE_ATTR uint32_t uart_ll_get_baudrate(uart_dev_t *hw, uint32_t sclk_fr { typeof(hw->clk_div) div_reg; div_reg.val = hw->clk_div.val; - return ((sclk_freq << 4)) / (((div_reg.div_int << 4) | div_reg.div_frag) * (hw->clk_conf.sclk_div_num + 1)); + return ((sclk_freq << 4)) / + (((div_reg.div_int << 4) | div_reg.div_frag) * (HAL_FORCE_READ_U32_REG_FIELD(hw->clk_conf, sclk_div_num) + 1)); } /** @@ -469,7 +472,7 @@ FORCE_INLINE_ATTR void uart_ll_set_tx_idle_num(uart_dev_t *hw, uint32_t idle_num FORCE_INLINE_ATTR void uart_ll_tx_break(uart_dev_t *hw, uint32_t break_num) { if (break_num > 0) { - hw->txbrk_conf.tx_brk_num = break_num; + HAL_FORCE_MODIFY_U32_REG_FIELD(hw->txbrk_conf, tx_brk_num, break_num); hw->conf0.txd_brk = 1; } else { hw->conf0.txd_brk = 0; @@ -536,8 +539,8 @@ FORCE_INLINE_ATTR void uart_ll_set_sw_flow_ctrl(uart_dev_t *hw, uart_sw_flowctrl hw->flow_conf.sw_flow_con_en = 1; hw->swfc_conf1.xon_threshold = flow_ctrl->xon_thrd; hw->swfc_conf0.xoff_threshold = flow_ctrl->xoff_thrd; - hw->swfc_conf1.xon_char = flow_ctrl->xon_char; - hw->swfc_conf0.xoff_char = flow_ctrl->xoff_char; + HAL_FORCE_MODIFY_U32_REG_FIELD(hw->swfc_conf1, xon_char, flow_ctrl->xon_char); + HAL_FORCE_MODIFY_U32_REG_FIELD(hw->swfc_conf0, xoff_char, flow_ctrl->xoff_char); } else { hw->flow_conf.sw_flow_con_en = 0; hw->flow_conf.xonoff_del = 0; @@ -559,11 +562,11 @@ FORCE_INLINE_ATTR void uart_ll_set_sw_flow_ctrl(uart_dev_t *hw, uart_sw_flowctrl */ FORCE_INLINE_ATTR void uart_ll_set_at_cmd_char(uart_dev_t *hw, uart_at_cmd_t *cmd_char) { - hw->at_cmd_char.data = cmd_char->cmd_char; - hw->at_cmd_char.char_num = cmd_char->char_num; - hw->at_cmd_postcnt.post_idle_num = cmd_char->post_idle; - hw->at_cmd_precnt.pre_idle_num = cmd_char->pre_idle; - hw->at_cmd_gaptout.rx_gap_tout = cmd_char->gap_tout; + HAL_FORCE_MODIFY_U32_REG_FIELD(hw->at_cmd_char, data, cmd_char->cmd_char); + HAL_FORCE_MODIFY_U32_REG_FIELD(hw->at_cmd_char, char_num, cmd_char->char_num); + HAL_FORCE_MODIFY_U32_REG_FIELD(hw->at_cmd_postcnt, post_idle_num, cmd_char->post_idle); + HAL_FORCE_MODIFY_U32_REG_FIELD(hw->at_cmd_precnt, pre_idle_num, cmd_char->pre_idle); + HAL_FORCE_MODIFY_U32_REG_FIELD(hw->at_cmd_gaptout, rx_gap_tout, cmd_char->gap_tout); } /** @@ -752,8 +755,8 @@ FORCE_INLINE_ATTR void uart_ll_set_mode(uart_dev_t *hw, uart_mode_t mode) */ FORCE_INLINE_ATTR void uart_ll_get_at_cmd_char(uart_dev_t *hw, uint8_t *cmd_char, uint8_t *char_num) { - *cmd_char = hw->at_cmd_char.data; - *char_num = hw->at_cmd_char.char_num; + *cmd_char = HAL_FORCE_READ_U32_REG_FIELD(hw->at_cmd_char, data); + *char_num = HAL_FORCE_READ_U32_REG_FIELD(hw->at_cmd_char, char_num); } /** From 901eb026595f9f3781cd9440f9d5f67189636b73 Mon Sep 17 00:00:00 2001 From: Song Ruo Jing Date: Mon, 11 Sep 2023 19:33:33 +0800 Subject: [PATCH 3/3] fix(docs): Improve the DFS and Peripheral Drivers section in power_management.rst --- docs/en/api-reference/system/power_management.rst | 9 ++------- docs/zh_CN/api-reference/system/power_management.rst | 9 ++------- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/docs/en/api-reference/system/power_management.rst b/docs/en/api-reference/system/power_management.rst index 33eaee50c7..d752ab1e1b 100644 --- a/docs/en/api-reference/system/power_management.rst +++ b/docs/en/api-reference/system/power_management.rst @@ -92,13 +92,7 @@ Dynamic Frequency Scaling and Peripheral Drivers When DFS is enabled, the APB frequency can be changed multiple times within a single RTOS tick. The APB frequency change does not affect the operation of some peripherals, while other peripherals may have issues. For example, Timer Group peripheral timers keeps counting, however, the speed at which they count changes proportionally to the APB frequency. -The following peripherals work normally even when the APB frequency is changing: - -- **UART**: if ``REF_TICK`` or XTAL is used as a clock source. See :cpp:member:`uart_config_t::source_clk`. -- **LEDC**: if ``REF_TICK`` is used as a clock source. See :cpp:func:`ledc_timer_config` function. -- **RMT**: if ``REF_TICK`` or XTAL is used as a clock source. See :cpp:member:`rmt_config_t::flags` and macro `RMT_CHANNEL_FLAGS_AWARE_DFS`. -- **GPTimer**: if APB is used as the clock source. See :cpp:member:`gptimer_config_t::clk_src`. -- **TSENS**: if XTAL or ``RTC_8M`` is used as a clock source. So, APB frequency changing will not influence it. +Peripheral clock sources such as ``REF_TICK``, ``XTAL``, ``RC_FAST`` (i.e. ``RTC_8M``), their frequencies will not be inflenced by APB frequency. And therefore, to ensure the peripheral behaves consistently during DFS, it is recommanded to select one of these clocks as the peripheral clock source. For more specific guidelines, please refer to the "Power Management" section of each peripheral's "API Reference > Peripherals API" page. Currently, the following peripheral drivers are aware of DFS and use the ``ESP_PM_APB_FREQ_MAX`` lock for the duration of the transaction: @@ -112,6 +106,7 @@ The following drivers hold the ``ESP_PM_APB_FREQ_MAX`` lock while the driver is .. list:: - **SPI slave**: between calls to :cpp:func:`spi_slave_initialize` and :cpp:func:`spi_slave_free`. + - **GPTimer**: between calls to :cpp:func:`gptimer_enable` and :cpp:func:`gptimer_disable`. - **Ethernet**: between calls to :cpp:func:`esp_eth_driver_install` and :cpp:func:`esp_eth_driver_uninstall`. - **WiFi**: between calls to :cpp:func:`esp_wifi_start` and :cpp:func:`esp_wifi_stop`. If modem sleep is enabled, the lock will be released for the periods of time when radio is disabled. :SOC_TWAI_SUPPORTED: - **TWAI**: between calls to :cpp:func:`twai_driver_install` and :cpp:func:`twai_driver_uninstall` (only when the clock source is set to :cpp:enumerator:`TWAI_CLK_SRC_APB`). diff --git a/docs/zh_CN/api-reference/system/power_management.rst b/docs/zh_CN/api-reference/system/power_management.rst index 17e4952ef4..a24c66a423 100644 --- a/docs/zh_CN/api-reference/system/power_management.rst +++ b/docs/zh_CN/api-reference/system/power_management.rst @@ -92,13 +92,7 @@ ESP-IDF 中集成的电源管理算法可以根据应用程序组件的需求， 启用动态调频后，APB 频率可在一个 RTOS 滴答周期内多次更改。有些外设不受 APB 频率变更的影响，但有些外设可能会出现问题。例如，Timer Group 外设定时器会继续计数，但定时器计数的速度将随 APB 频率的变更而变更。 -以下外设不受 APB 频率变更的影响： - -- **UART**：如果 ``REF_TICK`` 或者 XTAL 用作时钟源，则 UART 不受 APB 频率变更影响。请查看 :cpp:member:`uart_config_t::source_clk`。 -- **LEDC**：如果 ``REF_TICK`` 用作时钟源，则 LEDC 不受 APB 频率变更影响。请查看 :cpp:func:`ledc_timer_config` 函数。 -- **RMT**：如果 ``REF_TICK`` 或者 XTAL 被用作时钟源，则 RMT 不受 APB 频率变更影响。请查看 :cpp:member:`rmt_config_t::flags` 以及 `RMT_CHANNEL_FLAGS_AWARE_DFS` 宏。 -- **GPTimer**：如果 XTAL 用作时钟源，则 GPTimer 不受 APB 频率变更影响。请查看 :cpp:member:`gptimer_config_t::clk_src`。 -- **TSENS**：XTAL 或 ``RTC_8M`` 用作时钟源，因此不受 APB 频率变化影响。 +时钟频率不受 APB 频率影响的外设时钟源通常有 ``REF_TICK``, ``XTAL``, ``RC_FAST`` (i.e. ``RTC_8M``)。因此，为了保证外设在 DFS 期间的所有行为一致，建议在上述时钟中选择其一作为外设的时钟源。如果想要了解更多详情可以浏览每个外设 ”API 参考 > 外设 API“ 页面的 “电源管理” 章节。 目前以下外设驱动程序可感知动态调频，并在调频期间使用 ``ESP_PM_APB_FREQ_MAX`` 锁： @@ -112,6 +106,7 @@ ESP-IDF 中集成的电源管理算法可以根据应用程序组件的需求， .. list:: - **SPI slave**：从调用 :cpp:func:`spi_slave_initialize` 至 :cpp:func:`spi_slave_free` 期间。 + - **GPTimer**：从调用 :cpp:func:`gptimer_enable` 至 :cpp:func:`gptimer_disable` 期间。 - **Ethernet**：从调用 :cpp:func:`esp_eth_driver_install` 至 :cpp:func:`esp_eth_driver_uninstall` 期间。 - **WiFi**：从调用 :cpp:func:`esp_wifi_start` 至 :cpp:func:`esp_wifi_stop` 期间。如果启用了调制解调器睡眠模式，广播关闭时将释放此管理锁。 :SOC_TWAI_SUPPORTED: - **TWAI**：从调用 :cpp:func:`twai_driver_install` 至 :cpp:func:`twai_driver_uninstall` 期间 (只有在 TWAI 时钟源选择为 :cpp:enumerator:`TWAI_CLK_SRC_APB` 的时候生效)。