Skip to content

drivers: pwm: Channel > 1 not working on rp2040 (Raspberry Pi Pico) #50876

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

Merged
merged 1 commit into from
Oct 4, 2022

Conversation

68ec020
Copy link
Contributor

@68ec020 68ec020 commented Oct 1, 2022

pwm_set_chan_level uses slice channels A(=0) or B(=1) and not Zephyr channel (0..15). So PWM doesn't work for channels > 1. There is already a function (pwm_rpi_channel_to_pico_channel) which does the right thing, but it isn't used for pwm_set_chan_level.

@zephyrbot zephyrbot added the area: PWM Pulse Width Modulation label Oct 1, 2022
@nordicjm nordicjm requested a review from yonsch October 3, 2022 11:34
@nordicjm nordicjm added the platform: Raspberry Pi Pico Raspberry Pi Pico (RPi Pico) label Oct 3, 2022
yonsch
yonsch previously approved these changes Oct 3, 2022
Copy link
Collaborator

@yonsch yonsch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Did you test this on a few PWM channels to make sure that it works?

@68ec020
Copy link
Contributor Author

68ec020 commented Oct 3, 2022

It took me a long time to figure out, why PWM doesn't work. Even the "samples" don't work on a RPi Pico. I testet Pin 2 (with a scope) and Pin 25 (onboard LED). And if you just look at valid parameters for "pwm_set_chan_level" it's clear the "full" channel number (2, 25) cannot work.

@68ec020
Copy link
Contributor Author

68ec020 commented Oct 3, 2022

I just realised, I used my work email. And I didn't sign-off the commit. It is fixed (I hope) in the force-push.

@68ec020 68ec020 requested review from yonsch and removed request for henrikbrixandersen, anangl and gmarull October 3, 2022 16:18
pwm_set_chan_level uses slice channels A(=0) or B(=1) and not Zephyr
channel (0..15). So PWM doesn't work for channels > 1. There is already
a function (pwm_rpi_channel_to_pico_channel) which does the right thing,
but it isn't used for pwm_set_chan_level.

Signed-off-by: Jan Hilsdorf <[email protected]>
@68ec020
Copy link
Contributor Author

68ec020 commented Oct 3, 2022

Sorry, another force push, to get the compliance checks fixed. I'm really sorry.

@yonsch
Copy link
Collaborator

yonsch commented Oct 3, 2022

Don't worry, we all do that :)
Did you test it on a few channels to make sure it's working now?

@68ec020
Copy link
Contributor Author

68ec020 commented Oct 3, 2022

I tested it successfully on two pins (2 and 25). If you want, I can try it on all available pins (maybe except one pair of UART pins).

@yonsch
Copy link
Collaborator

yonsch commented Oct 3, 2022

A few random pins should be fine

@carlescufi carlescufi merged commit c888d6a into zephyrproject-rtos:main Oct 4, 2022
@68ec020 68ec020 deleted the pico_pwm_bugfix branch November 16, 2022 06:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: PWM Pulse Width Modulation platform: Raspberry Pi Pico Raspberry Pi Pico (RPi Pico)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants