-
Notifications
You must be signed in to change notification settings - Fork 7k
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
[topic-gpio] drivers: gpio_stm32: update to use new GPIO API #19579
[topic-gpio] drivers: gpio_stm32: update to use new GPIO API #19579
Conversation
@mniestroj, FYI |
Tested ok on |
Seems to be intended behavior. |
Yeah, seen the same thing myself |
While maybe not the most efficient, here's an example to help convert the dts files over:
|
Marking dnm since there are some todo items. |
All checks are passing now. Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages. |
recheck |
89ea7b0
to
a6958f1
Compare
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.
Observed one flag test using the zero-valued variant, which needs to be fixed. One or two other comments.
Checked on nucleo_l476rg, things passed.
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.
I believe it would be better to have all stm32 changes in one PR instead of two. It makes it difficult to do the review since both PRs modify driver code. Also by not running checks on all the boards we may miss some issues. In case of Shippable timeout it's better to add a temporary patch from @galak that fixes the issue.
@mnkp Thanks for review!
This should not be the case PR #19067 only adds commit 717de74 which impact is limited to boards
Agree it's better to actually get the results on all boards. @galak how can we make this happen? |
@erwango As suggested to me by @carlescufi:
Also #19562 (comment) |
Thanks, will use that. |
Status following last push: Tested on all STM32 series. Test status:
Test failures on
|
@erwango could you please rebase? |
This allows compiler to inline function body and reduce overall code size. Signed-off-by: Marcin Niestroj <[email protected]>
This patch doesn't change functionality, but is only related to improved readability and reusability. Signed-off-by: Marcin Niestroj <[email protected]>
Up to now interrupts could be only configured once, with no way to disable them in runtime. Allow interrupts to be disabled in runtime and then properly reenabled on user request. This allows to ignore interrupts when software is not expecting them. The improvement over previously reverted patch [1] is that we disable interrupts only when we configure port for which interrupt line was previously selected. This for example prevents to disable interrupts line 2 in case PA2 was previously configured as interrupt source, but we are currently configuring PB2 as output. [1] 0951ce2 ("gpio: stm32: support disabling and reenabling interrupts on pin") Signed-off-by: Marcin Niestroj <[email protected]>
Update STM32 GPIO driver to support new GPIO API. Signed-off-by: Erwan Gouriou <[email protected]>
Perform few clean up in stm32 gpio driver: *Clean up uint32_t occurrences left over. *Move function gpio_stm32_flags_to_conf from const to static and remove useless parameter check *Rework error handling in functoin gpio_stm32_config *Remove gpio_stm32_int_enabled_port function and have direct call to gpio_stm32_get_exti_source Signed-off-by: Erwan Gouriou <[email protected]>
Since it is now possible to disable/re-enable interrupts and also to reconfigure an already configured interrupt, it is now required to clear non requested triggers. While it is not strictly requested, triggers are also cleared when interrupt is disabled (assuming trigger should be configured when interrupt is enabled). Signed-off-by: Erwan Gouriou <[email protected]> fixup exti
Move GPIO_ACTIVE_INT_HIGH/LOW to GPIO_ACTIVE_HIGH/LOW. Signed-off-by: Erwan Gouriou <[email protected]>
Add boards overlay to enable gpio_basic tests. Selected boards should reflect various stm32 series. Signed-off-by: Erwan Gouriou <[email protected]>
On some stm32 based boards buttons and leds configuration was wrong. Fix that. Signed-off-by: Erwan Gouriou <[email protected]>
Add arduino connection dts description and update target yaml files. Signed-off-by: Erwan Gouriou <[email protected]>
Argument 'port' in stm32_exti_set_callback function is not used, remove it. Signed-off-by: Erwan Gouriou <[email protected]>
stm32_exti_enable was returning errors on line > 32 or line pointing to non implemented line. Both conditions are hard-coded, hence there is no use to detect them dynamically in the code. Check them with assert. As a consequence, function could now be void. Additionally, enable exti irq line only if both checks are passed. Signed-off-by: Erwan Gouriou <[email protected]>
Add led and button to board description to enable basic gpio tests. Signed-off-by: Erwan Gouriou <[email protected]>
Test generates lengthy log. Signed-off-by: Erwan Gouriou <[email protected]>
Change codeowner following driver indeep rework. Signed-off-by: Erwan Gouriou <[email protected]>
@carlescufi, done. |
/* trigger on rising edge */ | ||
STM32_EXTI_TRIG_RISING = 0x1, | ||
/* trigger on falling endge */ | ||
STM32_EXTI_TRIG_FALLING = 0x2, | ||
/* trigger on falling endge */ |
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.
endge?
Update driver code to use new configuration flags such as GPIO_ACTIVE_LOW.
STM32F1 series not taken into account.Only one board converted.No backward compatibility:gpio_configure
don't configure interrupts anymoreBased on:
TODO:
Done in dedicated PR [topic-gpio] boards: stm32: update to use new GPIO API #19607 in order to get useful Shippable result in current PRDone in current PRTo be corrected in [topic-gpio] boards: stm32: update to use new GPIO API #19607Might be trickier than pure nucleo_64 vs nucleo_144. Need to check manuals in detail.DoneEDIT:
PR description rework after push on 09-09-19