-
Notifications
You must be signed in to change notification settings - Fork 724
[collab-nrfx-4.0] drivers: gpio: nrf: align to nrfx_gpiote extracted cb #3403
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
[collab-nrfx-4.0] drivers: gpio: nrf: align to nrfx_gpiote extracted cb #3403
Conversation
17612bd to
d3b73fd
Compare
drivers/gpio/gpio_nrfx.c
Outdated
| }; | ||
|
|
||
| static nrfx_gpiote_t gpiote_nrfx[] = { | ||
| DT_FOREACH_STATUS_OKAY(nordic_nrf_gpiote, GPIOTE_NRFX_INST) |
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 wonder if those instances should not be created in soc file? What about cases where gpiote is used without CONFIG_GPIO=y? Unless we assume that it is required to enable GPIO driver to use gpiote.
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.
then we would need to have a software component (like dmm.c ) to create and own the nrfx_gpiote instances, and some nordic-soc-level API to extract those instances, correct? Then both gpio shim and non-shim users could use that to obtain given nrfx_gpiote instance.
I wonder if this is worth the hassle - if any code runs with CONFIG_GPIO=n it means no one is the owner of GPIOTE instance so its the application responsibility to manage nrfx_gpiote instance.
I can try the suggested approach, however I am afraid it might be problematic when it comes to IRQ_CONNECT macro in gpio shim which expects the arguments to be constant. I guess with suggested approach the instance would need to be obtained in run-time? Alternatively via linker magic and global variables, but it won't be pretty
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.
@nordic-krch I have extracted nrfx_gpiote instances into new gpiote_nrfx.c component in soc/common + macrology to allow to obtain any nrfx_gpiote via global variable. This should ease the nrfx_gpiote instances sharing across the system as well as CONFIG_GPIO=n variant.
A big plus of this approach is quite small impact on nrfx_gpiote users - instead of creating NRFX_GPIOTE_INSTANCE macrology needs to be used to obtain reference to global nrfx_gpiote instance.
Please take a look if this is what you had in mind.
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 had in mind something like that. Imo, it's better to have one approach for gpiote instantiation otherwise it would become cumbersome for modules that use gpiote instance (like PPI trace). It would need to use 2 APIs for getting the instance depending on CONFIG_GPIO. Not to mention that user would need to create gpiote instance in application code with GPIO is not used.
d3b73fd to
3391840
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.
shouldn't interrupt be connected here as well?
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 see your point, however I suggest to keep it as it is now, as current approach is aligned with what we have in main branch (i.e. IRQ installed by gpio shim when CONFIG_GPIO=y )
3391840 to
6fe974a
Compare
6fe974a to
139641f
Compare
…extraction Analogously to existing macro for extracting GPIOTE instance property. Upstream PR #: 98527 Signed-off-by: Nikodem Kastelik <[email protected]>
…ponent GPIOTE driver instances are no longer defined within nrfx. Add a component supplementing missing functionality, as GPIOTE driver instances are often shared across the system. Upstream PR #: 98527 Signed-off-by: Nikodem Kastelik <[email protected]>
…trol block Align GPIOTE shim to changes in nrfx instantiation. Upstream PR #: 98527 Signed-off-by: Nikodem Kastelik <[email protected]>
… control block GPIOTE driver instances are no longer defined within nrfx. Upstream PR #: 98527 Signed-off-by: Nikodem Kastelik <[email protected]>
…tracted cb GPIOTE driver instances are no longer defined within nrfx. Upstream PR #: 98527 Signed-off-by: Nikodem Kastelik <[email protected]>
… with extracted cb GPIOTE driver instances are no longer defined within nrfx. Upstream PR #: 98527 Signed-off-by: Nikodem Kastelik <[email protected]>
…with extracted cb GPIOTE driver instances are no longer defined within nrfx. Upstream PR #: 98527 Signed-off-by: Nikodem Kastelik <[email protected]>
…xtracted cb GPIOTE driver instances are no longer defined within nrfx. Upstream PR #: 98527 Signed-off-by: Nikodem Kastelik <[email protected]>
…th extracted cb GPIOTE driver instances are no longer defined within nrfx. Upstream PR #: 98527 Signed-off-by: Nikodem Kastelik <[email protected]>
Errno codes are returned now. Upstream PR #: 98569 Signed-off-by: Nikodem Kastelik <[email protected]>
abd7e9d to
fd6783b
Compare
d482a7d
into
nrfconnect:collab-nrfx-4.0
Align GPIOTE shim to changes in nrfx instantiation.
Draft PR before pulling the trigger on the approach in nrfx.Upstream PR: zephyrproject-rtos/zephyr#98527