Allow to configure sleep RTC calibration (IDFGH-12371)

[DarkZeros]

• The default behaviour is to run some calibration cycles to ensure RTC slow clock is vlaid during the sleep time.
• However, depending on the application this might be redundant or detrimental
• Added an option to configure it if required

[CLAassistant]

All committers have signed the CLA.

[github-actions]

Warnings

⚠️

Some issues found for the commit messages in this PR: the commit message "Allow to configure sleep RTC calibration": summary looks empty type/action looks empty Please fix these commit messages - here are some basic tips: follow Conventional Commits style correct format of commit message should be: <type/action>(<scope/component>): <summary>, for example fix(esp32): Fixed startup timeout issue allowed types are: change,ci,docs,feat,fix,refactor,remove,revert,test sufficiently descriptive message summary should be between 20 to 72 characters and start with upper case letter avoid Jira references in commit messages (unavailable/irrelevant for our customers) TIP: Install pre-commit hooks and run this check when committing (uses the Conventional Precommit Linter).

👋 Hello DarkZeros, we appreciate your contribution to this project!

---

📘 Please review the project's Contributions Guide for key guidelines on code, documentation, testing, and more.

---

🖊️ Please also make sure you have read and signed the Contributor License Agreement for this project.

---

Click to see more instructions ...

This automated output is generated by the PR linter DangerJS, which checks if your Pull Request meets the project's requirements and helps you fix potential issues.

DangerJS is triggered with each push event to a Pull Request and modify the contents of this comment.

Please consider the following:
- Danger mainly focuses on the PR structure and formatting and can't understand the meaning behind your code or changes.
- Danger is not a substitute for human code reviews; it's still important to request a code review from your colleagues.
- Resolve all warnings (⚠️ ) before requesting a review from human reviewers - they will appreciate it.
- To manually retry these Danger checks, please navigate to the Actions tab and re-run last Danger workflow.

Review and merge process you can expect ...

We do welcome contributions in the form of bug reports, feature requests and pull requests via this public GitHub repository.

This GitHub project is public mirror of our internal git repository

1. An internal issue has been created for the PR, we assign it to the relevant engineer.
2. They review the PR and either approve it or ask you for changes or clarifications.
3. Once the GitHub PR is approved, we synchronize it into our internal git repository.
4. In the internal git repository we do the final review, collect approvals from core owners and make sure all the automated tests are passing.
- At this point we may do some adjustments to the proposed change, or extend it by adding tests or documentation.
5. If the change is approved and passes the tests it is merged into the default branch.
5. On next sync from the internal git repository merged change will appear in this public GitHub repository.

Generated by 🚫 dangerJS against 3eed53a

[esp-wzh]

@DarkZeros

Thank you for your contribution!!

But in fact we do not expect users to configure the number of calibration cycles each time they enter sleep. If the number of calibration cycles is too small, not all configuration values may be able to calibrate accurate frequency values.

Of course, I have also noticed that too many calibrations in certain scenarios lead to too much overhead in and out of sleep. You could try configuring PM_LIGHTSLEEP_RTC_OSC_CAL_INTERVAL this option to configure an interval to skip part of the calibration. Could this resolve the issue you are experiencing?

I think the best solution is to implement the global calibration service of rtc_timer. It is possible to automatically calibrate rtc_timer at regular intervals. I am trying to implement it (but the priority is not high).

[DarkZeros]

@esp-wzh Thanks for the reply.

I understand that the current code has its value, this is why I left the defaults as before.

My use case is an ultra low power display update code. The time is handled by external RTC (+interrupt), therefore exact slow RTC is non needed, and the extra calibration cycles in/out sleep is causing 2ms total extra time (around 4% total power usage).
In my case a fixed call to esp_clk_slowclk_cal_set(16000000) makes more sense than a constant calibration in an out of sleep. Saves power and is also better because the external RTC is quite accurate.

I think this might also be useful for cases where time accuracy is not critical. So it may be interesting for other people to set this to 0, or even higher values for long sleep time (ie: 1day sleep). But feel freeto take it or not. I have forked the IDF project for my case, so it is ok if it is not taken upstream. :)

[esp-wzh]

@DarkZeros

Yes, I understand the problem you encountered, and this solution is reasonable in your case.

But what I am worried about is that when using the automatic sleep mode hosted by the esp_pm component (with FreeRTOS tickless idle enabled), there are certain requirements for the accuracy of the tick source provided by slow_clock. For example, freertos suppresses a 100ms ticks, butt if rtc_timer is particularly inaccurate, the chip sleeps for 101ms, so the events that need to be processed on the 100ms tick will be lost, and then FreeRTOS will crash.

[DarkZeros]

@DarkZeros

Yes, I understand the problem you encountered, and this solution is reasonable in your case.

But what I am worried about is that when using the automatic sleep mode hosted by the esp_pm component (with FreeRTOS tickless idle enabled), there are certain requirements for the accuracy of the tick source provided by slow_clock. For example, freertos suppresses a 100ms ticks, butt if rtc_timer is particularly inaccurate, the chip sleeps for 101ms, so the events that need to be processed on the 100ms tick will be lost, and then FreeRTOS will crash.

I won´t claim myself an expert in the ESP32 system. So take everything I say with caution.
But the calibration, as I see it, it is a way to estimate the slow RTC vs high frequency internal XTAL (40MHz tipically), assuming the slow RTC clock is quite bad (and tipically it is the case if RC 150kHz is used) (?), so it is better to calibrate it vs the FAST clock for less ppm drift.

Therefore, if the chip goes to sleep for ie: 100ms, needs to calibrate the RTC clock to know 100ms how many ticks it means of the RTC clock. To set it to as-closer as possible to 100ms.
When the ESP wakes up after RTC timer has finished, the ESP will "think" that 100ms have passed (maybe 101ms have passed), however it has no clue of knowing that, and the RTOS will function properly, I think.

So even if this is set to 0, a crash in RTOS I think is non possible, however it can cause a de-sync with other timers and external chips.

However, in my case the assumptions are the other way around, the RTC clock is very very accurate. So calibrating it makes no sense, it always gives 16000000, and a fast calibration of just 10 cycles tipically gives a bad calibration of 15999980, ...and so on.
Therefore is detrimental to perform it.

[esp-wzh]

Yes, you're right, I didn't think clearly.

After discussion, we do not intend to solve this problem using the method in this PR.

We would like to add an menuconfig option to configure the calibration interval (count in rtc_timer time instead the number of calibration times) for calibrating slow_clock.

During the calibration request process, only when the time from the current moment to the last calibration time is greater than the configured threshold, the hardware will actually be called to calibrate, otherwise the previous calibration value will be returned.

How do you think? If it's ok, we will complete it in the near future.

[DarkZeros]

That would definitely be a step forward, because it would not incur multiple calibrations that are redundant.
(Unless the calibration wants to take into account certain thermal drift)

I think in general, for RTC calibration it doesn´t really need to be a blocking process.
2 samples can be performed (ie: 1 at startup, and 1 at a later time, and compute the total RTC/FAST cycles).
The only condition is that freqs cannot change, and device can´t sleep.

[esp-wzh]

@DarkZeros It's indeed a good suggestion. That's what I'm planning to do.

We can add an new API rtc_clk_cal_non_blocking_start.

After starting the calibration, the CPU can continue to run the code, and the RTC clock calibration module is calibrating in parallel at this time.

When the program needs to use the calibration value, it calls the rtc_clk_cal_non_blocking_end API to block and wait for the calibration to be completed.

You are very thoughtful. The use of this pair of interfaces should have restrictions on the behavior between them. Actions that disrupt the calibration process are not allowed to occur. It should only be used in scenarios that have been strictly evaluated, but I think that at least in the current IDF sleep process, this solution can be used to improve the time overhead of entering and exiting sleep.