Skip to content

drivers: pinctrl: rp2040: output override config #88612

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
Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

drivers: pinctrl: rp2040: output override config #88612

wants to merge 2 commits into from
+93 −27

Conversation

ebmmy
Copy link

@ebmmy ebmmy commented Apr 14, 2025

Add a device-tree property to configure the output override functionnality of RP2040 GPIO pins.

@github-actions github-actions bot added platform: Raspberry Pi Pico Raspberry Pi Pico (RPi Pico) area: Pinctrl labels Apr 14, 2025
@github-actions GitHub Actions
Copy link

Hello @ebmmy, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

@ebmmy ebmmy force-pushed the support-output-override-rpi-pico-pinctrl branch from fc4a066 to 4bdd8b8 Compare April 14, 2025 20:25
@ebmmy ebmmy force-pushed the support-output-override-rpi-pico-pinctrl branch from 4bdd8b8 to 35ff987 Compare April 15, 2025 07:55
@ebmmy ebmmy requested a review from soburi April 15, 2025 07:57
ajf58
ajf58 suggested changes Apr 17, 2025
View reviewed changes
Copy link
Collaborator

@ajf58 ajf58 left a comment

Choose a reason for hiding this comment

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

The functional change looks good.

I would like to see the whitespace changes moved to a separate commit

soc/raspberrypi/rpi_pico/common/pinctrl_soc.h
Comment on lines +34 to +35
/** Output override */
uint32_t out_override: 2;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use a descriptive variable name, and then the comment becomes redundant, i.e.

Suggested change
/** Output override */
uint32_t out_override: 2;
/** Output override */
uint32_t output_override: 2;

Copy link
Member

@soburi soburi Apr 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Since it matches the original naming that follows the SDK's gpio_set_outover(), I think the current version is preferable.

Copy link
Author

Choose a reason for hiding this comment

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

@soburi qnd @ajf58: Please let me know whether I should keep this naming or adapt with input and output, thanks!

Copy link
Member

Choose a reason for hiding this comment

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

This is an area of ​​contention and a matter of style, so it's up to you to decide based on both sides' arguments.

Copy link
Author

Choose a reason for hiding this comment

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

I would leave it as it is, so this is similar to the `oe-override config

soc/raspberrypi/rpi_pico/common/pinctrl_soc.h
/** Output override */
uint32_t out_override: 2;
/** Input override */
uint32_t in_override: 2;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
uint32_t in_override: 2;
uint32_t input_override: 2;

soc/raspberrypi/rpi_pico/common/pinctrl_soc.h
Comment on lines +62 to +63
DT_PROP(node_id, raspberrypi_out_override), \
DT_PROP(node_id, raspberrypi_in_override), \
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
DT_PROP(node_id, raspberrypi_out_override), \
DT_PROP(node_id, raspberrypi_in_override), \
DT_PROP(node_id, raspberrypi_output_override), \
DT_PROP(node_id, raspberrypi_input_override), \

In combination with the suggestion above.

drivers/pinctrl/pinctrl_rpi_pico.c
Comment on lines +23 to +24
gpio_set_outover(pin->pin_num, pin->out_override);
gpio_set_inover(pin->pin_num, pin->in_override);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
gpio_set_outover(pin->pin_num, pin->out_override);
gpio_set_inover(pin->pin_num, pin->in_override);
gpio_set_outover(pin->pin_num, pin->output_override);
gpio_set_inover(pin->pin_num, pin->input_override);

@soburi
Copy link
Member

soburi commented Apr 20, 2025

@ThreeEights Could you take a look if you have time?

@ebmmy ebmmy force-pushed the support-output-override-rpi-pico-pinctrl branch from 35ff987 to cc78880 Compare April 21, 2025 08:33
@ebmmy ebmmy requested a review from ajf58 April 21, 2025 08:37
ebmmy added 2 commits April 24, 2025 16:52
@ebmmy
drivers: pinctrl: rp2040: extend pin override config
9e953ef 
Add a device-tree property to configure the override
functionalities of RP2040 GPIO pins.

Signed-off-by: Martin Meyer <[email protected]>
@ebmmy
soc: raspberrypi: common: reformat pinctrl include
4936dae 
Apply clang-format on source files.

Signed-off-by: Martin Meyer <[email protected]>
@ebmmy ebmmy force-pushed the support-output-override-rpi-pico-pinctrl branch from cc78880 to 4936dae Compare April 24, 2025 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@soburi soburi soburi approved these changes

@gmarull gmarull Awaiting requested review from gmarull

@ThreeEights ThreeEights Awaiting requested review from ThreeEights

@yonsch yonsch Awaiting requested review from yonsch

@ajf58 ajf58 Awaiting requested review from ajf58

At least 2 approving reviews are required to merge this pull request.

Assignees

@soburi soburi

Labels
area: Pinctrl platform: Raspberry Pi Pico Raspberry Pi Pico (RPi Pico)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ebmmy @soburi @ajf58