-
Notifications
You must be signed in to change notification settings - Fork 7.3k
drivers: gpio: gpio_dw: add optional API to set data and control source #85438
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?
Conversation
1c49855
to
4e4020a
Compare
* | ||
* @retval 0 If successful. | ||
*/ | ||
int gpio_dw_set_hw_mode(const struct device *port, gpio_pin_t pin, bool hw_mode); |
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.
Will this be called once per pin? or the mode can be changed many 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.
Yes, once per pin, it's basically a binary pinmux and either SW (GPIO) or HW controlled.
If supported, the data and control source for a signal can come from either software or hardware. This change adds a driver specific API to set this for a specific pin. Signed-off-by: Corey Wharton <[email protected]>
This variable should be const and inside the driver config struct. Signed-off-by: Corey Wharton <[email protected]>
4e4020a
to
bb5e615
Compare
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |
Ping @kartben @JarmouniA |
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.
Pull Request Overview
This PR adds an optional API for the DW GPIO driver to set the data/control source for a pin, allowing it to be controlled by hardware if supported. It also updates various driver functions to consistently retrieve the base address from the configuration structure and removes the redundant base_addr field from the runtime structure.
- Added a new API function (gpio_dw_set_hw_mode) in the header and its implementation in the driver.
- Updated internal functions to use config->base_addr rather than runtime->base_addr.
- Removed duplication in the runtime structure by eliminating an unnecessary base_addr field.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
File | Description |
---|---|
include/zephyr/drivers/gpio/gpio_dw.h | Added the new API declaration and documentation for setting hardware control mode |
drivers/gpio/gpio_dw.h | Added a base_addr field in the configuration structure and removed base_addr from runtime |
drivers/gpio/gpio_dw.c | Updated multiple functions to use config->base_addr and implemented the gpio_dw_set_hw_mode API |
Comments suppressed due to low confidence (1)
drivers/gpio/gpio_dw.c:408
- [nitpick] Consider enhancing the assert message to include the pin number (e.g., 'Unsupported pin %d') to provide clearer debugging information.
__ASSERT((config->common.port_pin_mask & (gpio_port_pins_t)BIT(pin)) != 0U, "Unsupported pin");
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.
Why not use custom GPIO flags for this?
If supported, the data and control source for a signal can come from either software or hardware. This change adds a driver specific API to set this for a specific pin.