Update RWDT and refactor RTC #3

JurajSadel · 2023-02-21T17:32:16Z

update and add RTC example
refactor RTC and add a very modified example

Regarding RTC:
This code should give similar calibration values (cal_val) as ESP-IDF does except for RtcCal32kRc.
I'm really not sure if we want to add/keep the clock_monitor example and also the estimate_xtal_frequency() function - I let it here just fur testing purposes, besides that, I don't think it's useful tbh and it also crashes with external clock sources because they return cal_val = 0

Probably the whole RTC will need some additional cleanup and fixes but we can address it after we get the initial PR into the main.

We can discuss how to proceed further with RTC here.

Note: For some reason, I have troubles patching the LP_WDT interrupt even if it works for Jesse fine locally - I'll look into that later so the CI will not be happy with rtc_watchdog example

jessebraham

Seems fine in general, though there's a lot of low-level code here so only so much review I can do.

I'm fine with just ignoring the clock_monitor example for now, for the sake of wrapping this up.

@MabezDev @bjoernQ would you mind taking a peek at this if you get a chance?

esp-hal-common/src/rtc_cntl/mod.rs

esp-hal-common/src/rtc_cntl/rtc/esp32c6.rs

jessebraham · 2023-02-21T17:59:04Z

I've updated esp32c6 to include the LP_WDT interrupt with esp-rs/esp-pacs#96

bjoernQ · 2023-02-22T07:09:17Z

I think there are some #[allow(unused)] etc. left over from before moving the code to esp32c6.rs , so probably worth to double check that

besides that, I think I followed this topic a bit and it seems this is really the best we can come up with for now

so just those minor things and we should be ready for the next step with ESP32-C6 - even if not everything is perfect yet it will probably help to get it into main

MabezDev

Overall LGTM, just a couple of points to address from me :)

esp-hal-common/src/rtc_cntl/rtc/esp32c6.rs

esp32c6-hal/examples/clock_monitor.rs

MabezDev · 2023-02-22T13:33:33Z

It seems me and @bjoernQ figured out the direct boot issue at the same time :D - I opened #4 which includes the same fix as Bjoern's here, plus a couple of binary size improvements.

bjoernQ · 2023-02-22T13:45:16Z

It seems me and @bjoernQ figured out the direct boot issue at the same time :D - I opened #4 which includes the same fix as Bjoern's here, plus a couple of binary size improvements.

haha what a coincidence - seems like my fix is not enough to also make pcnt_encoder work in direct-boot mode
we should first get yours in and then look into pcnt_encoder (if that is still an issue after that PR)

bjoernQ · 2023-02-22T13:55:02Z

Gave the changes from #4 a try and with that all examples build in direct-boot mode - however pcnt_encoder doesn't work yet

MabezDev · 2023-02-22T13:57:51Z

The error I get for pcnt_encoder is a weird one, I get already borrowed: BorrowMutError, which I guess means critical_section impl isn't working correctly?

bjoernQ · 2023-02-22T14:02:57Z

The error I get for pcnt_encoder is a weird one, I get already borrowed: BorrowMutError, which I guess means critical_section impl isn't working correctly?

I think that some garbage gets copied into .data and that makes it believe it's already borrowed (or it's the right data copied to a wrong place)

jessebraham · 2023-02-22T14:26:39Z

I've merged #4, @JurajSadel could you please rebase?

MabezDev · 2023-02-22T14:26:48Z

I think I fixed it in #4, mind trying again?

bjoernQ · 2023-02-22T14:58:15Z

I think I fixed it in #4, mind trying again?

Seems like that helped! (I still haven't found my rotary encoder but I don't see the crash anymore) 🎉

… example

…rgo fmt

JurajSadel · 2023-02-23T09:48:00Z

I think this should be good to go, CI is green

jessebraham

Just a few things to clean up, looks good otherwise. I'll go test some examples in the meantime.

.github/workflows/ci.yml

esp32c6-hal/Cargo.toml

jessebraham · 2023-02-23T15:16:12Z

@JurajSadel the examples I tested (including rtc_watchdog) seem to be working, so looks good there. In addition to my comments, could you please also address the earlier comments made by @MabezDev?

esp-hal-common/src/clock/clocks_ll/esp32c6.rs

jessebraham

CI is green and the examples are working as expected, so I think we're good to go here! Thank you again @JurajSadel for all your work on this, I know it was challenging!

JurajSadel · 2023-02-23T16:00:04Z

@bjoernQ @jessebraham @MabezDev Thank you guys for all the help, reviews and testing!

* C6: Update RWDT and add example, refactor RTC and add not-really-good example * Update based on review comments, resolve bunch of warnings and run cargo fmt * Update C6 esp-pacs rev commit * Fix clocks_ll/esp32c6.rs * Fix riscv interrupts * Remove clock_monitor example for now * RAM example works in direct-boot mode * Add a TODO for &mut TIMG0 and cargo fmt * Fix linker script after a bad rebase * Update CI and Cargo.toml embassy required features * use riscv32imac-unknown-none-elf target for C6 in CI * change default target to riscv32imac-unknown-none-elf * add riscv32imac-unknown-none-elf target to MSRV job * another cleanup --------- Co-authored-by: bjoernQ <[email protected]> Co-authored-by: Jesse Braham <[email protected]>

* Create the `esp32c6-hal` package * Teach `esp-hal-common` about the ESP32-C6 * Get a number of peripheral drivers building for the ESP32-C6 bckup initial clocks_ii * Create the `esp32c6-hal` package C6: update * Simplify and fix the linker script update * C6: add I2S * Create the `esp32c6-hal` package * Teach `esp-hal-common` about the ESP32-C6 * Get a number of peripheral drivers building for the ESP32-C6 bckup initial clocks_ii * Create the `esp32c6-hal` package * C6: update * Simplify and fix the linker script * update * C6: add I2S * update * C6 Interrupts * C6: Update build.rs, linker scripts and initial examples * C6: RMT * Fix interrupt handling * Fix `ClockControl::configure` * C6: revert to I2S0 instead of just I2S * C6: rebase and update * RTC not buildable * Implement RWDT and SWD disable * C6: working LEDC * C6: working RMT * C6: add aes * C6: add mcpwm * C6: add rtc_cntln - not finished * C6: update and formatting * C6: add pcnt * C6: add examples and format * Remove inline assembly, fix interrupts and linker scripts * Remove unused features, update cargo config for atomic emu, misc cleanup * Get ADC building and example "working" (as much as it ever does) * Remove a bunch of unused constants which were copied from ESP-IDF * The `mcpwm` example now works correctly * Get `TWAI` peripheral driver building for C6 * Clean up the `rtc_cntl` module and get all the other HALs building again * Add the C6 to our CI workflow * Fix various things that have been missed when rebasing Still missing a few examples (`clock_monitor`, `embassy_spi`, `ram`) * C6: Small updates in wdt (#1) * C6: Update WDT * C6: Update examples with WDT update * Update `esp-println` dependency to fix build errors * Fix formatting issues causing pre-commit hook to fail * Get some more examples working * Working `ram` example * Sync with changes in `main` after rebasing * Working `embassy_spi` example * Use a git dependency for the PAC until we publish a release * Fix I2S for ESP32-C6 * Fix esp32c6 direct boot (#4) * Add direct boot support for C6 * Fix direct boot for c6 - Actually copy into rtc ram - remove dummy section that is no longer needed (was just a waste of flash space) - Move RTC stuff before the no load sections * Update RWDT and refactor RTC (#3) * C6: Update RWDT and add example, refactor RTC and add not-really-good example * Update based on review comments, resolve bunch of warnings and run cargo fmt * Update C6 esp-pacs rev commit * Fix clocks_ll/esp32c6.rs * Fix riscv interrupts * Remove clock_monitor example for now * RAM example works in direct-boot mode * Add a TODO for &mut TIMG0 and cargo fmt * Fix linker script after a bad rebase * Update CI and Cargo.toml embassy required features * use riscv32imac-unknown-none-elf target for C6 in CI * change default target to riscv32imac-unknown-none-elf * add riscv32imac-unknown-none-elf target to MSRV job * another cleanup --------- Co-authored-by: bjoernQ <[email protected]> Co-authored-by: Jesse Braham <[email protected]> * Make required changes to include new `RADIO` peripheral * Use published versions of PAC and `esp-println` * Use the correct target extensions (`imac`) * Fix the super watchdog timer, plus a few more examples * Fix UART clock configuration * Make sure to sync UART registers when configuring AT cmd detection * Disable APM in direct-boot mode * Address a number of review comments * Fix `SPI` clocks and `rtc_watchdog` example (#6) * fix SPI clocks * run cargo fmt * Add comment about used default clk src * Fix rtc_watchdog example in BL mode * run cargo fmt * Update rtc_watchdog example that it works in DB mode * README and example fixes/cleanup * Add I2C peripheral enable and reset * Fix `ApbSarAdc` configuration in `system.rs` --------- Co-authored-by: bjoernQ <[email protected]> Co-authored-by: Juraj Sadel <[email protected]> Co-authored-by: Juraj Sadel <[email protected]> Co-authored-by: Scott Mabin <[email protected]>

jessebraham reviewed Feb 21, 2023

View reviewed changes

esp-hal-common/src/rtc_cntl/mod.rs Outdated Show resolved Hide resolved

esp-hal-common/src/rtc_cntl/mod.rs Outdated Show resolved Hide resolved

esp-hal-common/src/rtc_cntl/rtc/esp32c6.rs Show resolved Hide resolved

esp-hal-common/src/rtc_cntl/rtc/esp32c6.rs Show resolved Hide resolved

MabezDev approved these changes Feb 22, 2023

View reviewed changes

esp-hal-common/src/rtc_cntl/rtc/esp32c6.rs Show resolved Hide resolved

esp-hal-common/src/rtc_cntl/rtc/esp32c6.rs Show resolved Hide resolved

esp32c6-hal/examples/clock_monitor.rs Outdated Show resolved Hide resolved

JurajSadel and others added 9 commits February 22, 2023 08:47

C6: Update RWDT and add example, refactor RTC and add not-really-good…

419b398

… example

Update based on review comments, resolve bunch of warnings and run ca…

c11bf1a

…rgo fmt

Update C6 esp-pacs rev commit

4b7aa68

Fix clocks_ll/esp32c6.rs

5bd744a

Fix riscv interrupts

d5bb63a

Remove clock_monitor example for now

9957732

RAM example works in direct-boot mode

1047c34

Add a TODO for &mut TIMG0 and cargo fmt

952e270

Fix linker script after a bad rebase

8168893

JurajSadel force-pushed the feature/esp32c6_rtc_refactor branch from b462895 to 8168893 Compare February 22, 2023 19:02

JurajSadel added 4 commits February 22, 2023 20:30

Update CI and Cargo.toml embassy required features

0543cb7

use riscv32imac-unknown-none-elf target for C6 in CI

d485b43

change default target to riscv32imac-unknown-none-elf

cc0be7e

add riscv32imac-unknown-none-elf target to MSRV job

8fc314f

jessebraham reviewed Feb 23, 2023

View reviewed changes

.github/workflows/ci.yml Outdated Show resolved Hide resolved

.github/workflows/ci.yml Outdated Show resolved Hide resolved

esp32c6-hal/Cargo.toml Outdated Show resolved Hide resolved

MabezDev suggested changes Feb 23, 2023

View reviewed changes

esp-hal-common/src/clock/clocks_ll/esp32c6.rs Show resolved Hide resolved

MabezDev approved these changes Feb 23, 2023

View reviewed changes

another cleanup

912401d

jessebraham approved these changes Feb 23, 2023

View reviewed changes

jessebraham merged commit a931c16 into jessebraham:feature/esp32c6 Feb 23, 2023

JurajSadel deleted the feature/esp32c6_rtc_refactor branch April 10, 2024 15:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update RWDT and refactor RTC #3

Update RWDT and refactor RTC #3

JurajSadel commented Feb 21, 2023

jessebraham left a comment

jessebraham commented Feb 21, 2023

bjoernQ commented Feb 22, 2023

MabezDev left a comment

MabezDev commented Feb 22, 2023

bjoernQ commented Feb 22, 2023

bjoernQ commented Feb 22, 2023

MabezDev commented Feb 22, 2023

bjoernQ commented Feb 22, 2023

jessebraham commented Feb 22, 2023

MabezDev commented Feb 22, 2023 •

edited

Loading

bjoernQ commented Feb 22, 2023

JurajSadel commented Feb 23, 2023

jessebraham left a comment

jessebraham commented Feb 23, 2023 •

edited

Loading

jessebraham left a comment

JurajSadel commented Feb 23, 2023

Update RWDT and refactor RTC #3

Update RWDT and refactor RTC #3

Conversation

JurajSadel commented Feb 21, 2023

jessebraham left a comment

Choose a reason for hiding this comment

jessebraham commented Feb 21, 2023

bjoernQ commented Feb 22, 2023

MabezDev left a comment

Choose a reason for hiding this comment

MabezDev commented Feb 22, 2023

bjoernQ commented Feb 22, 2023

bjoernQ commented Feb 22, 2023

MabezDev commented Feb 22, 2023

bjoernQ commented Feb 22, 2023

jessebraham commented Feb 22, 2023

MabezDev commented Feb 22, 2023 • edited Loading

bjoernQ commented Feb 22, 2023

JurajSadel commented Feb 23, 2023

jessebraham left a comment

Choose a reason for hiding this comment

jessebraham commented Feb 23, 2023 • edited Loading

jessebraham left a comment

Choose a reason for hiding this comment

JurajSadel commented Feb 23, 2023

MabezDev commented Feb 22, 2023 •

edited

Loading

jessebraham commented Feb 23, 2023 •

edited

Loading