Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[embassy-rp] pwm::Pwm drop leaves gpios in incorrect state #3615

Open
Jojo-1000 opened this issue Dec 5, 2024 · 0 comments
Open

[embassy-rp] pwm::Pwm drop leaves gpios in incorrect state #3615

Jojo-1000 opened this issue Dec 5, 2024 · 0 comments

Comments

@Jojo-1000
Copy link

Hi, I am using a Pwm to control some motors on Pico 2.

I noticed that when the Pwm goes out of scope, it leaves them running. I looked at the drop implementation and there seems to be a difference between Pwm and PwmOutput. Using the two channels split as PwmOutput works as expected. Here is the implementation for reference:

This one works as expected:

embassy/embassy-rp/src/pwm.rs

Lines 398 to 427 in af02310

impl<'d> Drop for PwmOutput<'d> {
fn drop(&mut self) {
if self.is_owned {
let p = pac::PWM.ch(self.slice);
match &self.channel_pin {
PwmChannelPin::A(pin) => {
p.cc().modify(|w| {
w.set_a(0);
});
pin.gpio().ctrl().write(|w| w.set_funcsel(31));
//Enable pin PULL-DOWN
pin.pad_ctrl().modify(|w| {
w.set_pde(true);
});
}
PwmChannelPin::B(pin) => {
p.cc().modify(|w| {
w.set_b(0);
});
pin.gpio().ctrl().write(|w| w.set_funcsel(31));
//Enable pin PULL-DOWN
pin.pad_ctrl().modify(|w| {
w.set_pde(true);
});
}
}
}
}
}

This one leaves the pins in an incorrect state, the part about pull down is missing:

embassy/embassy-rp/src/pwm.rs

Lines 485 to 495 in af02310

impl<'d> Drop for Pwm<'d> {
fn drop(&mut self) {
pac::PWM.ch(self.slice).csr().write_clear(|w| w.set_en(false));
if let Some(pin) = &self.pin_a {
pin.gpio().ctrl().write(|w| w.set_funcsel(31));
}
if let Some(pin) = &self.pin_b {
pin.gpio().ctrl().write(|w| w.set_funcsel(31));
}
}
}

Reproduction

Unexpected:

{
    let mut c = embassy_rp::pwm::Config::default();
    let pwm_freq = 20_000;
    let clock_freq = embassy_rp::clocks::clk_sys_freq();
    c.top = (clock_freq / pwm_freq) as u16 - 1;
    
    let mut pwm = embassy_rp::pwm::Pwm::new_output_ab(pwm_slice6, pin_11, pin_10, c.clone());
    pwm.set_duty_cycle_fully_off().unwrap();
}
// There is an output signal

Expected:

{
    let mut c = embassy_rp::pwm::Config::default();
    let pwm_freq = 20_000;
    let clock_freq = embassy_rp::clocks::clk_sys_freq();
    c.top = (clock_freq / pwm_freq) as u16 - 1;
    
    let pwm = embassy_rp::pwm::Pwm::new_output_ab(pwm_slice6, pin_11, pin_10, c.clone());
    let (pwm_a,pwm_b) = pwm.split();
    pwm_a.unwrap().set_duty_cycle_fully_off().unwrap();
    pwm_b.unwrap().set_duty_cycle_fully_off().unwrap();
}
// There is no output signal
vivi202 added a commit to vivi202/embassy-rp2040SplitPwm that referenced this issue Dec 23, 2024
@vivi202
embassy-rs#3615 fix.
c1197e4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@Jojo-1000