ESP32/ledc: div_param zero depending on init routine location

[DaBzzz]

Hi,

while tracking down another bug concerning single inverted ports between an old compiled image and a brand new one, I stumbled across this. Maybe (hopefully?) this is extremely simple, but the subroutine version worked with the old image (compiled using esp32 v1.9.x in Arduino 1.8.13), while the current 2.0.2 in 1.8.19 throws me this error:

Code: [Select all] [Expand/Collapse]

1. 22:31:49.793 -> a
2. 22:31:49.793 -> E (1099) ledc: requested frequency and duty resolution can not be achieved, try reducing freq_hz or duty_resolution. div_param=0
3. 22:31:49.793 -> b

GeSHi © Codebox Plus Extension

I'm clearly not exceeding any hardware limits at 5kHz and 8 bit, and the previous version worked fine at 15kHz and 10 bit.
ESP-WROOM32 on a random AliExpress dev board.

Working code for 2.x:

Code: [Select all] [Expand/Collapse]

1. #include <driver/ledc.h>
2. void setup()
3. {
4.   Serial.begin(115200);
5.   delay(1000); // give me time to bring up serial monitor
6.   ledc_timer_config_t ledc_timer;
7.   ledc_timer.speed_mode       = LEDC_HIGH_SPEED_MODE;
8.   ledc_timer.timer_num        = LEDC_TIMER_1;
9.   ledc_timer.duty_resolution  = LEDC_TIMER_8_BIT;
10.   ledc_timer.freq_hz          = 5000;
11.   ledc_channel_config_t ledc_channel0;
12.   ledc_channel0.channel       = LEDC_CHANNEL_0;
13.   ledc_channel0.gpio_num      = 2;
14.   ledc_channel0.speed_mode    = LEDC_HIGH_SPEED_MODE;  
15.   ledc_channel0.intr_type     = LEDC_INTR_FADE_END;
16.   ledc_channel0.timer_sel     = LEDC_TIMER_1;
17.   ledc_channel0.hpoint        = 0;
18.   ledc_channel0.duty          = 255;
19.   Serial.println("a");
20.   ledc_timer_config(&ledc_timer);
21.   Serial.println("b");
22.   ledc_channel_config(&ledc_channel0);  //onboard
23.   ledc_fade_func_install(ESP_INTR_FLAG_SHARED);
24. }
25.  
26. void loop(){}

GeSHi © Codebox Plus Extension

Code producing the above error:

Code: [Select all] [Expand/Collapse]

1. #include <driver/ledc.h>
2. void setup()
3. {
4.   Serial.begin(115200);
5.   delay(1000); // give me time to bring up serial monitor
6.   setupLEDs();
7. }
8.  
9. void setupLEDs() {
10.   ledc_timer_config_t ledc_timer;
11.   ledc_timer.speed_mode       = LEDC_HIGH_SPEED_MODE;
12.   ledc_timer.timer_num        = LEDC_TIMER_1;
13.   ledc_timer.duty_resolution  = LEDC_TIMER_8_BIT;
14.   ledc_timer.freq_hz          = 5000;
15.   ledc_channel_config_t ledc_channel0;
16.   ledc_channel0.channel       = LEDC_CHANNEL_0;
17.   ledc_channel0.gpio_num      = 2;
18.   ledc_channel0.speed_mode    = LEDC_HIGH_SPEED_MODE;  
19.   ledc_channel0.intr_type     = LEDC_INTR_FADE_END;
20.   ledc_channel0.timer_sel     = LEDC_TIMER_1;
21.   ledc_channel0.hpoint        = 0;
22.   ledc_channel0.duty          = 255;
23.   Serial.println("a");
24.   ledc_timer_config(&ledc_timer);
25.   Serial.println("b");
26.   ledc_channel_config(&ledc_channel0);  //onboard
27.   ledc_fade_func_install(ESP_INTR_FLAG_SHARED);
28. }
29.  
30. void loop(){}

GeSHi © Codebox Plus Extension

Everything is the same except for the fact that it is put in a function that is called from setup(). Originally, this is even a separate .ino file since there are seven channels and a couple of fading routines used across the project. I'd really like to have this separated from setup(), but I cannot figure out why it doesn't work anymore.

[ESP_Sprite]

Code: Select all

ledc_timer_config_t ledc_timer;
...
ledc_channel_config_t ledc_channel0;
...

Not sure if that's the issue, but Don't Do This. It defines ledc_timer as a stack-allocated variable that has undefined data (i.e. random crap) in it. If we add a field in a newer version of a driver and you don't explicitly set a value to it, it means you have undefined crap in the newly-allocated field, making the driver do weird things.

Instead, change all occurences of this pattern to

Code: Select all

ledc_timer_config_t ledc_timer={};
...
ledc_channel_config_t ledc_channel0={};
...

to zero-initialize them. If we add new fields, in general we make them work so if the field has 0 in it, it'll behave like the old version of the driver.

[DaBzzz]

Haven't heard that so I gave it a try...

Code: Select all

16:32:12.882 -> ELF file SHA256: 0000000000000000
16:32:12.882 -> 
16:32:12.882 -> Rebooting...
16:32:12.882 -> ets Jun  8 2016 00:22:57
16:32:12.882 -> 
16:32:12.882 -> rst:0xc (SW_CPU_RESET),boot:0x13 (SPI_FAST_FLASH_BOOT)
16:32:12.916 -> configsip: 0, SPIWP:0xee
16:32:12.916 -> clk_drv:0x00,q_drv:0x00,d_drv:0x00,cs0_drv:0x00,hd_drv:0x00,wp_drv:0x00
16:32:12.916 -> mode:DIO, clock div:2
16:32:12.916 -> load:0x3fff0030,len:1324
16:32:12.916 -> ho 0 tail 12 room 4
16:32:12.916 -> load:0x40078000,len:13480
16:32:12.916 -> ho 0 tail 12 room 4
16:32:12.916 -> load:0x40080400,len:3604
16:32:12.916 -> entry 0x400805f0
16:32:14.109 -> a
16:32:14.109 -> b
16:32:14.109 -> 
16:32:14.109 -> assert failed: xQueueGiveFromISR queue.c:1217 (pxQueue)
16:32:14.142 -> 
16:32:14.142 -> 
16:32:14.142 -> Backtrace:0x4008306d:0x3ffbec6c |<-CORRUPTED

Code: Select all

Decoding stack results
0x4008306d: panic_abort at /home/runner/work/esp32-arduino-lib-builder/esp32-arduino-lib-builder/esp-idf/components/esp_system/panic.c line 402

Same behaviour in both init styles. Don't think that's it, can you maybe provide a MWE with the ={} addition?

[ESP_Sprite]

MWE?

[DaBzzz]

Minimal working example?

[ESP_Sprite]

Hm. Looking again at your output, it looks like the original issue is fixed but you're having some other issue in the ISR. Is there more code after the ledc init routine that could mess something up (e.g. memory corruption)?

[DaBzzz]

The original issue with the partially inverted output channels is not fixed and can be traced back to the 2.0.0 esp32 revision - anything compiled with the 1.x version does work, everything with 2.x has the strange behaviour. But I'd like to get this one resolved so that I can provide a good example for the next one...
The above code is everything needed to reproduce the issue, loop() can be empty as it crashes before setup() is finished. The strange corruption message can be reproduced on different boards by using ledc_timer={} / ledc_channel0={}, so I don't think that is a crazy one-time exception. Hence my need for a working example using those exact commands.