-
Notifications
You must be signed in to change notification settings - Fork 474
ledc: add esp32c5 support and model ledc_sclk source #4966
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 5 commits
45fb3f5
c55f36b
0fd76aa
df95b60
6b9ddd0
846d86e
50b9fe5
4125937
85c5569
d82e5f7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -142,18 +142,33 @@ impl<'d> Ledc<'d> { | |
| #[cfg(not(esp32))] | ||
| /// Set global slow clock source | ||
| pub fn set_global_slow_clock(&mut self, clock_source: LSGlobalClkSource) { | ||
| #[cfg(any(esp32c6, esp32h2))] | ||
| #[cfg(soc_has_clock_node_ledc_sclk)] | ||
| let ledc_sclk = match clock_source { | ||
| LSGlobalClkSource::APBClk => crate::soc::clocks::LedcSclkConfig::PllF80m, | ||
| }; | ||
|
|
||
| #[cfg(soc_has_clock_node_ledc_sclk)] | ||
| crate::soc::clocks::ClockTree::with(|clocks| { | ||
| crate::soc::clocks::configure_ledc_sclk(clocks, ledc_sclk); | ||
| }); | ||
|
|
||
| #[cfg(any(esp32c5, esp32c6, esp32h2))] | ||
| let pcr = unsafe { &*crate::peripherals::PCR::ptr() }; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok I have to ask if this is AI generated
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh yeah (Codex 5.3 to be exact) - is it strictly forbidden here? Upon some reflection - I should've marked it somewhere. I needed RMT and LEDC with C5 in my project - once I got it working and tested it locally I thought I could contribute here. If I'm not being usefull here (or even mallicious) just let me know :)
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well we ask people to disclose the use of AI, but my main observation is that this is outdated code (use
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks - I’ll review the current project conventions |
||
|
|
||
| #[cfg(any(esp32c6, esp32h2))] | ||
| #[cfg(any(esp32c5, esp32c6, esp32h2))] | ||
| pcr.ledc_sclk_conf().write(|w| w.ledc_sclk_en().set_bit()); | ||
|
|
||
| match clock_source { | ||
| LSGlobalClkSource::APBClk => { | ||
| #[cfg(not(any(esp32c6, esp32h2)))] | ||
| #[cfg(not(any(esp32c5, esp32c6, esp32h2)))] | ||
| self.ledc | ||
| .conf() | ||
| .write(|w| unsafe { w.apb_clk_sel().bits(1) }); | ||
| #[cfg(esp32c5)] | ||
| pcr.ledc_sclk_conf().modify(|_, w| unsafe { | ||
| w.ledc_sclk_sel().bits(2); | ||
| w.ledc_sclk_en().set_bit() | ||
| }); | ||
| #[cfg(esp32c6)] | ||
| pcr.ledc_sclk_conf() | ||
| .write(|w| unsafe { w.ledc_sclk_sel().bits(1) }); | ||
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The esp32c5
start_duty_fade_innerimplementation ignores all fade parameters and only setsduty_start. As a result,start_duty_fade()can returnOk(())but not perform a fade (andis_duty_fade_running()may not reflect reality). Either implement the actual fade configuration for ESP32-C5 or makestart_duty_fadereturn an error/unsupported on esp32c5.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented for ESP32-C5 in the latest commit (
846d86e5).What changed:
LEDC + 0x400, 16 ranges/channel), since the current C5 PAC does not exposech_gamma_wr*registers.int_clr+int_ena) and madeis_duty_fade_running()readint_stfor C5.PCR.LEDC_PD_CTRL.LEDC_MEM_FORCE_PDinLedc::new(); this bit resets to force power-down and otherwise gamma RAM writes do not take effect.duty_fade_changes_output_and_completes) that verifies fade starts, completes, and actually changes waveform duty.Verification run on hardware (
esp32c5): 4/4 tests passing, repeated multiple times.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Follow-up: I opened
esp-rs/esp-pacs#397to expose C5 gamma RAM in PAC as a typedLEDC_GAMMA_RAMperipheral (0x6000_7400, 96 entries = 16 ranges x 6 channels).Small wording correction to my previous note: on C5 the missing piece is gamma RAM range register exposure (not
ch_gamma_wr*, which is the C6/H2 register model).