Skip to content
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

Nordic: Directly accessing GPIOTE might create unstable firmware (GPIO, PWM, BLE) #8815

Closed
Olivier-ProGlove opened this issue Jul 9, 2018 · 5 comments
Assignees

Comments

@Olivier-ProGlove
Copy link
Contributor

Olivier-ProGlove commented Jul 9, 2018

I found the issue causing #8252 (GPIO interrupt only called once on nRF52832).

The issue is accessing GPIOTE registers responsible for GPIO pin configuration (ie: gpiote->CONFIG[]) from multiple Zephyr drivers.

Nordic Zephyr drivers directly access these registers without checking if other drivers also access it. So they overwrite each other GPIO configurations.
In the case of #8252, it was the PWM driver that was overwriting the GPIO driver configurations. As @JoeHut mentioned, there are two other drivers that uses GPIO interrupts. And following which one was started first, the other one was not working (as soon as the PWM driver was doing stuff with gpiote->CONFIG[]).

At the moment, there are 3 drivers directly accessing gpiote->CONFIG[]:

Not sure if the new Nordic PWN driver https://github.com/zephyrproject-rtos/zephyr/blob/master/drivers/pwm/pwm_nrfx.c accesses GPIOTE through NRFX SDK.

I see two possibles fixes for this issue:

  • either we introduce a Zephyr Nordic GPIOTE driver (that could live in arch/arm/soc/nordic_nrf/) that manages all GPIOTE accesses (mainly GPIO pin configuration allocations).
  • or we only use NRFX SDK to access GPIOTE registers (I guess that will be the preferred solution as some Zephyr drivers are migrating to this API).

Note: This issue is already raised in GPIO driver: https://github.com/zephyrproject-rtos/zephyr/blob/master/drivers/gpio/gpio_nrf5.c#L68

@henrikbrixandersen Your pull-request #8628 would be affected

@cvinayak
Copy link
Contributor

cvinayak commented Jul 9, 2018

@Olivier-ProGlove, I have been the author of both pwm and radio that is using the GPIOTE channels, and have been careful in allocating the channels 0-2 for pwm and 3 for PA/LNA in BLE radio. I assume if your application uses GPIOTE channels 4 and above upto 7, you should not face any problem. Which GPIOTE channels did your application use?

@oliviermartin
Copy link

I am not allocating any GPIOTE channels manually. But if you use GPIO driver with interrupt support (see https://github.com/zephyrproject-rtos/zephyr/blob/master/drivers/gpio/gpio_nrf5.c#L153), it will not consider the possible reserved channels 0-3.

@cvinayak
Copy link
Contributor

cvinayak commented Jul 10, 2018

@Olivier-ProGlove, yes, sorry, GPIO driver does not consider other users unless we get a GPIOTE channel manager. Two options, first, a build time allocation/resolution, second, a runtime dynamic manager. I favor the former, considering resource constrains in designing something runtime.

To summarise, the issue stated here is genuine, and we need a fix/design to handle GPIOTE channel use through the system.

If in urgency, we can update GPIO driver to consider other GPIOTE channel users, and adjust the starting channel index using Kconfig. I will try to send a PR.

cvinayak added a commit to cvinayak/zephyr that referenced this issue Jul 10, 2018
Fixes issues caused in GPIO driver due to overlapping GPIOTE
channel use in nRF5 software PWM driver and in Bluetooth
controller for implementing PA/LNA feature.

The issue is solved by assigning the base and available
channel count for GPIOTE considering whether PWM and/or
PA/LNA feature is selected in the Kconfig.

Fixes zephyrproject-rtos#8815.

Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
carlescufi pushed a commit that referenced this issue Jul 10, 2018
Fixes issues caused in GPIO driver due to overlapping GPIOTE
channel use in nRF5 software PWM driver and in Bluetooth
controller for implementing PA/LNA feature.

The issue is solved by assigning the base and available
channel count for GPIOTE considering whether PWM and/or
PA/LNA feature is selected in the Kconfig.

Fixes #8815.

Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
@oliviermartin
Copy link

While the static solution works for now, if a new driver uses GPIOTE, it will make the GPIOTE channel management more complicated and probably revive this issue.

The runtime dynamic allocation does not need to be complicated. Updating all Nordic Zephyr drivers that use GPIOTE to use NRFX driver would make the job: https://github.com/zephyrproject-rtos/zephyr/blob/master/ext/hal/nordic/nrfx/drivers/src/nrfx_gpiote.c#L100

The NRFX GPIOTE driver is already used by https://github.com/zephyrproject-rtos/zephyr/blob/master/drivers/pwm/pwm_nrfx.c

@cvinayak
Copy link
Contributor

As part of moving gpio_nrf5 to using nrfx source, we can consider it. But in short in such design changes, I will consider race-to-idle systems which do not want to have runtime allocation in their time critical control paths where they wakeup from off and return back to off as quickly as possible.

Yes, we can revive this issue, when we come to it.

walter-xie pushed a commit to walter-xie/zephyr that referenced this issue Jul 13, 2018
Fixes issues caused in GPIO driver due to overlapping GPIOTE
channel use in nRF5 software PWM driver and in Bluetooth
controller for implementing PA/LNA feature.

The issue is solved by assigning the base and available
channel count for GPIOTE considering whether PWM and/or
PA/LNA feature is selected in the Kconfig.

Fixes zephyrproject-rtos#8815.

Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants