From 245074ec42d4699d2411575bd21b757655be694a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Proch=C3=A1zka?= <90197375+P-R-O-C-H-Y@users.noreply.github.com> Date: Mon, 9 Jun 2025 12:01:34 +0200 Subject: [PATCH 1/4] feat(ledc): Improve timer management with frequency/resolution matching --- cores/esp32/esp32-hal-ledc.c | 129 +++++++++++++++++++++++++++++++---- cores/esp32/esp32-hal-ledc.h | 2 + 2 files changed, 119 insertions(+), 12 deletions(-) diff --git a/cores/esp32/esp32-hal-ledc.c b/cores/esp32/esp32-hal-ledc.c index 039fa1312f1..c26d0a740b9 100644 --- a/cores/esp32/esp32-hal-ledc.c +++ b/cores/esp32/esp32-hal-ledc.c @@ -45,6 +45,96 @@ typedef struct { ledc_periph_t ledc_handle = {0}; +// Helper function to find a timer with matching frequency and resolution +static bool find_matching_timer(uint8_t speed_mode, uint32_t freq, uint8_t resolution, uint8_t *timer_num) { + log_d("Searching for timer with freq=%u, resolution=%u", freq, resolution); + // Check all channels to find one with matching frequency and resolution + for (uint8_t i = 0; i < SOC_GPIO_PIN_COUNT; i++) { + if (!perimanPinIsValid(i)) { + continue; + } + peripheral_bus_type_t type = perimanGetPinBusType(i); + if (type == ESP32_BUS_TYPE_LEDC) { + ledc_channel_handle_t *bus = (ledc_channel_handle_t *)perimanGetPinBus(i, ESP32_BUS_TYPE_LEDC); + if (bus != NULL && (bus->channel / 8) == speed_mode) { + if (bus->freq_hz == freq && bus->channel_resolution == resolution) { + log_d("Found matching timer %u for freq=%u, resolution=%u", bus->timer_num, freq, resolution); + *timer_num = bus->timer_num; + return true; + } + } + } + } + log_d("No matching timer found for freq=%u, resolution=%u", freq, resolution); + return false; +} + +// Helper function to find an unused timer +static bool find_free_timer(uint8_t speed_mode, uint8_t *timer_num) { + // Check which timers are in use + uint8_t used_timers = 0; + for (uint8_t i = 0; i < SOC_GPIO_PIN_COUNT; i++) { + if (!perimanPinIsValid(i)) { + continue; + } + peripheral_bus_type_t type = perimanGetPinBusType(i); + if (type == ESP32_BUS_TYPE_LEDC) { + ledc_channel_handle_t *bus = (ledc_channel_handle_t *)perimanGetPinBus(i, ESP32_BUS_TYPE_LEDC); + if (bus != NULL && (bus->channel / 8) == speed_mode) { + log_d("Timer %u is in use by channel %u", bus->timer_num, bus->channel); + used_timers |= (1 << bus->timer_num); + } + } + } + + // Find first unused timer + for (uint8_t i = 0; i < SOC_LEDC_TIMER_NUM; i++) { + if (!(used_timers & (1 << i))) { + log_d("Found free timer %u", i); + *timer_num = i; + return true; + } + } + log_e("No free timers available"); + return false; +} + +// Helper function to remove a channel from a timer and clear timer if no channels are using it +static void remove_channel_from_timer(uint8_t speed_mode, uint8_t timer_num, uint8_t channel) { + log_d("Removing channel %u from timer %u in speed_mode %u", channel, timer_num, speed_mode); + + // Check if any other channels are using this timer + bool timer_in_use = false; + for (uint8_t i = 0; i < SOC_GPIO_PIN_COUNT; i++) { + if (!perimanPinIsValid(i)) { + continue; + } + peripheral_bus_type_t type = perimanGetPinBusType(i); + if (type == ESP32_BUS_TYPE_LEDC) { + ledc_channel_handle_t *bus = (ledc_channel_handle_t *)perimanGetPinBus(i, ESP32_BUS_TYPE_LEDC); + if (bus != NULL && (bus->channel / 8) == speed_mode && + bus->timer_num == timer_num && bus->channel != channel) { + log_d("Timer %u is still in use by channel %u", timer_num, bus->channel); + timer_in_use = true; + break; + } + } + } + + if (!timer_in_use) { + log_d("No other channels using timer %u, deconfiguring timer", timer_num); + // Stop the timer + ledc_timer_pause(speed_mode, timer_num); + // Deconfigure the timer + ledc_timer_config_t ledc_timer; + memset((void *)&ledc_timer, 0, sizeof(ledc_timer_config_t)); + ledc_timer.speed_mode = speed_mode; + ledc_timer.timer_num = timer_num; + ledc_timer.deconfigure = true; + ledc_timer_config(&ledc_timer); + } +} + static bool fade_initialized = false; static ledc_clk_cfg_t clock_source = LEDC_DEFAULT_CLK; @@ -81,6 +171,8 @@ static bool ledcDetachBus(void *bus) { } pinMatrixOutDetach(handle->pin, false, false); if (!channel_found) { + uint8_t group = (handle->channel / 8); + remove_channel_from_timer(group, handle->timer_num, handle->channel % 8); ledc_handle.used_channels &= ~(1UL << handle->channel); } free(handle); @@ -117,8 +209,10 @@ bool ledcAttachChannel(uint8_t pin, uint32_t freq, uint8_t resolution, uint8_t c return false; } - uint8_t group = (channel / 8), timer = ((channel / 2) % 4); + uint8_t group = (channel / 8); + uint8_t timer; bool channel_used = ledc_handle.used_channels & (1UL << channel); + if (channel_used) { log_i("Channel %u is already set up, given frequency and resolution will be ignored", channel); if (ledc_set_pin(pin, group, channel % 8) != ESP_OK) { @@ -126,17 +220,26 @@ bool ledcAttachChannel(uint8_t pin, uint32_t freq, uint8_t resolution, uint8_t c return false; } } else { - ledc_timer_config_t ledc_timer; - memset((void *)&ledc_timer, 0, sizeof(ledc_timer_config_t)); - ledc_timer.speed_mode = group; - ledc_timer.timer_num = timer; - ledc_timer.duty_resolution = resolution; - ledc_timer.freq_hz = freq; - ledc_timer.clk_cfg = clock_source; - - if (ledc_timer_config(&ledc_timer) != ESP_OK) { - log_e("ledc setup failed!"); - return false; + // Find a timer with matching frequency and resolution, or a free timer + if (!find_matching_timer(group, freq, resolution, &timer)) { + if (!find_free_timer(group, &timer)) { + log_e("No free timers available for speed mode %u", group); + return false; + } + + // Configure the timer if we're using a new one + ledc_timer_config_t ledc_timer; + memset((void *)&ledc_timer, 0, sizeof(ledc_timer_config_t)); + ledc_timer.speed_mode = group; + ledc_timer.timer_num = timer; + ledc_timer.duty_resolution = resolution; + ledc_timer.freq_hz = freq; + ledc_timer.clk_cfg = clock_source; + + if (ledc_timer_config(&ledc_timer) != ESP_OK) { + log_e("ledc setup failed!"); + return false; + } } uint32_t duty = ledc_get_duty(group, (channel % 8)); @@ -157,6 +260,8 @@ bool ledcAttachChannel(uint8_t pin, uint32_t freq, uint8_t resolution, uint8_t c ledc_channel_handle_t *handle = (ledc_channel_handle_t *)malloc(sizeof(ledc_channel_handle_t)); handle->pin = pin; handle->channel = channel; + handle->timer_num = timer; + handle->freq_hz = freq; #ifndef SOC_LEDC_SUPPORT_FADE_STOP handle->lock = NULL; #endif diff --git a/cores/esp32/esp32-hal-ledc.h b/cores/esp32/esp32-hal-ledc.h index 5b44aaad452..0417501b7f8 100644 --- a/cores/esp32/esp32-hal-ledc.h +++ b/cores/esp32/esp32-hal-ledc.h @@ -51,6 +51,8 @@ typedef struct { uint8_t pin; // Pin assigned to channel uint8_t channel; // Channel number uint8_t channel_resolution; // Resolution of channel + uint8_t timer_num; // Timer number used by this channel + uint32_t freq_hz; // Frequency configured for this channel voidFuncPtr fn; void *arg; #ifndef SOC_LEDC_SUPPORT_FADE_STOP From 42bf774e871fcb76fa698d1d605701369ee1e353 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Proch=C3=A1zka?= <90197375+P-R-O-C-H-Y@users.noreply.github.com> Date: Mon, 9 Jun 2025 12:38:28 +0200 Subject: [PATCH 2/4] fix(ci): Fix uninitialized timer variable warning --- cores/esp32/esp32-hal-ledc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cores/esp32/esp32-hal-ledc.c b/cores/esp32/esp32-hal-ledc.c index c26d0a740b9..b04ba255591 100644 --- a/cores/esp32/esp32-hal-ledc.c +++ b/cores/esp32/esp32-hal-ledc.c @@ -210,7 +210,7 @@ bool ledcAttachChannel(uint8_t pin, uint32_t freq, uint8_t resolution, uint8_t c } uint8_t group = (channel / 8); - uint8_t timer; + uint8_t timer = 0; bool channel_used = ledc_handle.used_channels & (1UL << channel); if (channel_used) { From b37a764d97f3ce3ad61444ffdaf8f051687f1ce3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Proch=C3=A1zka?= <90197375+P-R-O-C-H-Y@users.noreply.github.com> Date: Tue, 10 Jun 2025 07:45:16 +0200 Subject: [PATCH 3/4] Update cores/esp32/esp32-hal-ledc.c Co-authored-by: Lucas Saavedra Vaz <32426024+lucasssvaz@users.noreply.github.com> --- cores/esp32/esp32-hal-ledc.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/cores/esp32/esp32-hal-ledc.c b/cores/esp32/esp32-hal-ledc.c index b04ba255591..e84a4176c41 100644 --- a/cores/esp32/esp32-hal-ledc.c +++ b/cores/esp32/esp32-hal-ledc.c @@ -56,12 +56,10 @@ static bool find_matching_timer(uint8_t speed_mode, uint32_t freq, uint8_t resol peripheral_bus_type_t type = perimanGetPinBusType(i); if (type == ESP32_BUS_TYPE_LEDC) { ledc_channel_handle_t *bus = (ledc_channel_handle_t *)perimanGetPinBus(i, ESP32_BUS_TYPE_LEDC); - if (bus != NULL && (bus->channel / 8) == speed_mode) { - if (bus->freq_hz == freq && bus->channel_resolution == resolution) { - log_d("Found matching timer %u for freq=%u, resolution=%u", bus->timer_num, freq, resolution); - *timer_num = bus->timer_num; - return true; - } + if (bus != NULL && (bus->channel / 8) == speed_mode && bus->freq_hz == freq && bus->channel_resolution == resolution) { + log_d("Found matching timer %u for freq=%u, resolution=%u", bus->timer_num, freq, resolution); + *timer_num = bus->timer_num; + return true; } } } From d68a1466183612f95bbca0c121a40777d64c7b94 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci-lite[bot]" <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> Date: Tue, 10 Jun 2025 08:01:29 +0000 Subject: [PATCH 4/4] ci(pre-commit): Apply automatic fixes --- cores/esp32/esp32-hal-ledc.c | 13 ++++++------- cores/esp32/esp32-hal-ledc.h | 4 ++-- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/cores/esp32/esp32-hal-ledc.c b/cores/esp32/esp32-hal-ledc.c index e84a4176c41..764c2803b4b 100644 --- a/cores/esp32/esp32-hal-ledc.c +++ b/cores/esp32/esp32-hal-ledc.c @@ -84,7 +84,7 @@ static bool find_free_timer(uint8_t speed_mode, uint8_t *timer_num) { } } } - + // Find first unused timer for (uint8_t i = 0; i < SOC_LEDC_TIMER_NUM; i++) { if (!(used_timers & (1 << i))) { @@ -100,7 +100,7 @@ static bool find_free_timer(uint8_t speed_mode, uint8_t *timer_num) { // Helper function to remove a channel from a timer and clear timer if no channels are using it static void remove_channel_from_timer(uint8_t speed_mode, uint8_t timer_num, uint8_t channel) { log_d("Removing channel %u from timer %u in speed_mode %u", channel, timer_num, speed_mode); - + // Check if any other channels are using this timer bool timer_in_use = false; for (uint8_t i = 0; i < SOC_GPIO_PIN_COUNT; i++) { @@ -110,8 +110,7 @@ static void remove_channel_from_timer(uint8_t speed_mode, uint8_t timer_num, uin peripheral_bus_type_t type = perimanGetPinBusType(i); if (type == ESP32_BUS_TYPE_LEDC) { ledc_channel_handle_t *bus = (ledc_channel_handle_t *)perimanGetPinBus(i, ESP32_BUS_TYPE_LEDC); - if (bus != NULL && (bus->channel / 8) == speed_mode && - bus->timer_num == timer_num && bus->channel != channel) { + if (bus != NULL && (bus->channel / 8) == speed_mode && bus->timer_num == timer_num && bus->channel != channel) { log_d("Timer %u is still in use by channel %u", timer_num, bus->channel); timer_in_use = true; break; @@ -208,9 +207,9 @@ bool ledcAttachChannel(uint8_t pin, uint32_t freq, uint8_t resolution, uint8_t c } uint8_t group = (channel / 8); - uint8_t timer = 0; + uint8_t timer = 0; bool channel_used = ledc_handle.used_channels & (1UL << channel); - + if (channel_used) { log_i("Channel %u is already set up, given frequency and resolution will be ignored", channel); if (ledc_set_pin(pin, group, channel % 8) != ESP_OK) { @@ -224,7 +223,7 @@ bool ledcAttachChannel(uint8_t pin, uint32_t freq, uint8_t resolution, uint8_t c log_e("No free timers available for speed mode %u", group); return false; } - + // Configure the timer if we're using a new one ledc_timer_config_t ledc_timer; memset((void *)&ledc_timer, 0, sizeof(ledc_timer_config_t)); diff --git a/cores/esp32/esp32-hal-ledc.h b/cores/esp32/esp32-hal-ledc.h index 0417501b7f8..f1a27dd4f7a 100644 --- a/cores/esp32/esp32-hal-ledc.h +++ b/cores/esp32/esp32-hal-ledc.h @@ -51,8 +51,8 @@ typedef struct { uint8_t pin; // Pin assigned to channel uint8_t channel; // Channel number uint8_t channel_resolution; // Resolution of channel - uint8_t timer_num; // Timer number used by this channel - uint32_t freq_hz; // Frequency configured for this channel + uint8_t timer_num; // Timer number used by this channel + uint32_t freq_hz; // Frequency configured for this channel voidFuncPtr fn; void *arg; #ifndef SOC_LEDC_SUPPORT_FADE_STOP