espressif / esp-idf

Espressif IoT Development Framework. Official development framework for Espressif SoCs.
Apache License 2.0
13.77k stars 7.31k forks source link

When ESP32_RTC_CLK_SRC_INT_8MD256=y , wakeup from deep sleep time wrong (IDFGH-7938) #9455

Open cssword opened 2 years ago

cssword commented 2 years ago

I'm using the sntp example .

When by menuconfig, I set :

CONFIG_RTC_CLK_SRC = Internal 150KHz , it's working and the cpu is waked up after 10 s CONFIG_RTC_CLK_SRC = Internal 8MHz, it's working but the cpu is waked up after about 3 min

Did I missing anything ?

cssword commented 2 years ago

I used ESP32-C3-MINI-1.
And the time printed when waked up is the first time + 10s.
Not the correct time. Ex. Start time 15:00:00 Waked up time 15:03:05 but printed time is 15:00:10 .

AxelLin commented 2 years ago

Which esp-idf version are you using? (git describe --tags)

cssword commented 2 years ago

Which esp-idf version are you using? (git describe --tags)

v5.0-dev-4379-g36f49f361c

AxelLin commented 2 years ago

Confirm the same issue on v5.1-dev-224-g4d14c2ef2d with ESP32-C3-MINI-1.

WereCatf commented 2 years ago

I just encountered this issue myself in my attempts to test how much drift there is in deep-sleep with 8MHz RTC-clock. It's clear that the time to spend in sleep is calculated wrong with the 8MHz-clock.

lennyJL commented 2 years ago

Hi all, the fix is under review.

WereCatf commented 2 years ago

@AxelLin I assume they were referring to the pull-request that's been sitting around for half a year now with zero activity. I don't see anything else relevant there, nor does @lennyJL seem to be associated with Espressif anyways, so I doubt they have any insider information on some non-public pull-requests.

lennyJL commented 2 years ago

Hi @WereCatf. I am working in Espressif. And we have an internal gitlab MR for this issue. The MR contains more, still need some time to merge and sync to github. If you really need this fix now(for esp32c3), can you please try to change:

out_config->dbg_atten_slp = atten_deep_sleep;

to:

#define RTC_CNTL_DBG_ATTEN_DEEPSLEEP_NODROP   0
out_config->dbg_atten_slp = !(sleep_flags & RTC_SLEEP_PD_INT_8M) ? RTC_CNTL_DBG_ATTEN_DEEPSLEEP_NODROP : atten_deep_sleep;

The code is in components/esp_hw_support/port/esp32c3/rtc_sleep.c, and in function rtc_sleep_get_default_config. I think the fix is similar for other chips. But note that this may not be the final fix.

WereCatf commented 2 years ago

@lennyJL Ok, I stand corrected. I only went by what was on your profile, which did not mention any association with Espressif.

That said, it is nice to get a clarification on what's going on. Appreciated.

AxelLin commented 1 year ago

Hi @WereCatf. I am working in Espressif. And we have an internal gitlab MR for this issue. The MR contains more, still need some time to merge and sync to github.

@lennyJL It seems does not make sense that you need more than 1 month to merge and sync to github.

lennyJL commented 1 year ago

Sorry @AxelLin, the related MR was not provided by me. Currently, you can use the temporary fix for your project.

AxelLin commented 1 year ago

Sorry @AxelLin, the related MR was not provided by me. Currently, you can use the temporary fix for your project.

Any chance to contact related people for handling the MR?

AxelLin commented 1 year ago

@KonstantinKondrashov any update? This initial issue was "the deep sleep mode doesn't work when CONFIG_ESP32_RTC_CLK_SRC_INT_8MD256", which was reported a year ago (https://github.com/espressif/esp-idf/issues/8007). It still does not work well due to the wrong wakeup time (this issue). This is not a pleasent experience.

KonstantinKondrashov commented 1 year ago

@AxelLin I asked the responsible for this issue person to get in touch here. Probably he can provide a patch here. (There is an MR in our internal repo, but it is still under review).

AxelLin commented 1 year ago

@AxelLin I asked the responsible for this issue person to get in touch here. Probably he can provide a patch here. (There is an MR in our internal repo, but it is still under review).

The fix is under review since Oct 19 (https://github.com/espressif/esp-idf/issues/9455#issuecomment-1284026549). This makes me wonder what is the reasonable time for review. I cannot tell if it's forgotten or something wrong or blocked for some reason if it takes such long time. People are waitting for the bug fix for stable trees, which will take even more time.

ginkgm commented 1 year ago

Hi @cssword @WereCatf @AxelLin ,

Sorry for bad experience...

We noticed this issue, but the reviewing and testing work is a bit above our expectation. The issue is an analog issue, where there is a trade-off between power consumption and the functionalities of each part of hardware. Moreover, the behavior is different on different chips, so we need to do testing on plenty of chips...

As an alternative, I will give you a patch on v5.0-dev-4379-g36f49f361c with the latest parameter we are trying, to see if it solves your problem. But please understand that we still need some time to do more testing before it can be merged, and there may also be some changes in the merged version.

WereCatf commented 1 year ago

@ginkgm Well, I wouldn't mind trying the patch out. Tacking on an external crystal to existing PCBs that were designed without a crystal there is pretty much a no-go and the 32768Hz internal oscillator is too imprecise to be even remotely useful.

ginkgm commented 1 year ago

I see the patch is already provided by one of my colleagues: https://github.com/espressif/esp-idf/issues/6687#issuecomment-1316769420

ginkgm commented 1 year ago

@WereCatf oh , sorry, I ignored the chip you are using is ESP32-C3.

The condition there is more complex, because we are having a big refactoring (similar to S3), splitting power modes into smaller ones, to optimize the power consumption in different use cases. https://github.com/espressif/esp-idf/blob/master/components/esp_hw_support/port/esp32s3/rtc_sleep.c#L77

To fix your issue quickly. May I know whether you are using ADC or Temp sensor in deep sleep mode? (via ULP)

WereCatf commented 1 year ago

@ginkgm I actually use ESP32 and ESP32-S2; it was another commenter who uses C3.

AxelLin commented 1 year ago

Hi @cssword @WereCatf @AxelLin ,

Sorry for bad experience...

We noticed this issue, but the reviewing and testing work is a bit above our expectation. The issue is an analog issue, where there is a trade-off between power consumption and the functionalities of each part of hardware. Moreover, the behavior is different on different chips, so we need to do testing on plenty of chips...

@ginkgm How is the status now? Could you list which chips are tested with fixes, which chips still need fix?

ginkgm commented 1 year ago

Hi @AxelLin ,

I think all existing chips need further adjustment. Among them ESP32 have already some progress:

ESP32:

Target Branch Merged Supported Last N.A.
release/v5.0 null null v5.0
release/v4.4 e1e9e10 null v4.4.4
release/v4.3 null null v4.3.4
master 84a6c6f v5.1 null

Sorry again, but there is still no progress for other chips. And I don't expect this to happen in a short time..

AxelLin commented 1 year ago

@ginkgm This is marked as "Resolution: Done", does that mean it's fixed for all chips?

AxelLin commented 1 year ago

Hi @AxelLin ,

I think all existing chips need further adjustment. Among them ESP32 have already some progress: Sorry again, but there is still no progress for other chips. And I don't expect this to happen in a short time..

@ginkgm It has been a while since your last reply. Any update for other chips? (There is an issue reported for ESP32S3 in above link)