Allow setting external 32khz quartz for slow rtc clock
Hello
This is just a working draft for setting the external 32khz quartz for slow rtc clock. I would like to discuss some potential fixes for issues I encountered. The final goal is to get this merged some day, some how.
So to the issues:
- For this to work it is needed to enable CONFIG_RTC_CLK_SRC_EXT_CRYS for the bootloader and rebuild it, reflash it. Otherwise it's simply not turned on and the program tries to calibrate the quartz, so it freezes at that point. The possible fixes are: 2 set of bootloader binaries? A simple note that you need a custom bootloader binary?
post_initat esp-hal/src/soc/esp32c6/mod.rs turns the RTC on, I don't know why it is the case, but this requires choosing the rtc clock as the default variable in the code, as there is no config system yet. The obvious solution is to not do that? I don't know the reason behind it, but it messes things up. Now, if the user wants the rtc variable, it will be initialized / calibrated 2 times for no reason- To create the RTC now you need to specify the clock:
Rtc::new(LPWR::steal(), RtcSlowClock::default());this breaks all current examples and programs. In my experience, this will not be an issue :smiley:
We might be able to place this into esp_hal::init once #1970 and its follow-up are merged.
I'm gonna adjust this draft once the other pull request is merged then
@bugadani I have questions on how to exactly do it
https://github.com/esp-rs/esp-hal/blob/08d406ee2b240384610bea8199d444f44942f157/esp-hal/src/lib.rs#L726-L742
Here, I should probably add the enum RtcSlowClock to Config and then configure it in init the question is, should init always return now the Rtc class, even when the user never asked for it (as he probably wanted the default). This will introduce another "Api" change and all examples would need to be adjusted. I don't know, a possible fix for it is a struct that returns those variables instead of returning them manually.
I'm not very familiar with the rtc API, but I think you should add RtcSlowClock to Config, call rtc::configure_clock here and remove the clock source parameter from Rtc::new. There's no need to return anything more than what we currently do.
Oh, this will not work at all because post_init still configures the RTC clock with the default RtcSlowClock, there is no way to pass to it the config
@MabezDev can we get rid of post_init in favour of esp_hal::init?
Why does it exist in the first place?
To run code before main but after initialisation. Useful for stuff like psram and until now, watchdog.
Rather than get rid of the post_init , maybe just move the watchdog disabling code to the init function and have that be configurable. That would help fix https://github.com/esp-rs/esp-hal/issues/1698 as well. (Arguably out of scope for this PR but it should solve your problem I think)
Okay, I did it for the esp32c6 for now. Does it look fine? If yes, I can do it for the rest of the cpu's.
Also I tested it on esp32c6-devkit-m-1.0 with a soldered in xtal 32khz, tested if quartz turns on with an oscilloscope, it did. It required a special bootloader as I said, otherwise it won't output hello world.
Silence means it's fine, right :smiley: ?
I think so. Please rebase :)
Does this look fine? It's able to return to the default clock and everything seems to work fine, but I can't guarantee it
Also how do I enable logs from esp-hal, the log feature is already enabled when running examples
To enable logs via esp-println your need to initialize the logger - e.g. esp_println::logger::init_logger_from_env(); early in your code. And you need to specify the log-level via the env variable ESP_LOGLEVEL - e.g. debug. See https://crates.io/crates/esp-println
To enable logs via
esp-printlnyour need to initialize the logger - e.g.esp_println::logger::init_logger_from_env();early in your code. And you need to specify the log-level via the env variableESP_LOGLEVEL- e.g.debug. See https://crates.io/crates/esp-println
Shouldn't there be an option / feature for esp-hal to enable it by itself? The logs I want are being executed before user code. Basically show esp-hal logs if the user wants it, because otherwise it's not possible to enable them directly from the user code and I have no idea how early can I put the log init for it to work
Well anyway
Does the esp32c6 code for setting the 32 khz oscillator look fine? If yes, can I implement it for other cpu's. I don't want to rework it later for all of them again
Shouldn't there be an option / feature for esp-hal to enable it by itself? The logs I want are being executed before user code. Basically show esp-hal logs if the user wants it, because otherwise it's not possible to enable them directly from the user code and I have no idea how early can I put the log init for it to work
Enable it before calling esp_hal::init() and you'll get the logs. Before main almost nothing else happens, at least nothing interesting.
I think this PR should contain the code to initialize the external quartz before we can merge it, depending on a custom bootloader to do it isn't acceptable IMO. Is this something you're willing to work on @Szybet or shall we close this?
I definitely want to do something with it, but working on a custom rust bootloader? that's out of my league. We maybe could just do a delay, but it would need to be huge to make it cpu independent. Also that's just a bad solution.
Maybe just accept the custom bootloader? both probe-rs and espflash support a custom bootloader binary argument
You don't need to work on a custom bootloader, you just need to port the code from the bootloader that does the initialization to esp-hal.
Maybe just accept the custom bootloader?
As I previously stated, depending on a specific bootloader is not an option. Based on #2376, it's extremely likely we won't be using the esp-idf bootloader at all soon.
If you would like to open a new PR with the initialization built in, we'd gladly accept it!
Thanks for your work so far, but I'm going to close this for now.