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

assertion using nRF5 power clock with BLE and nRF5 temp sensor #12114

Closed
dhananjaygj opened this issue Dec 14, 2018 · 5 comments · Fixed by #17293
Closed

assertion using nRF5 power clock with BLE and nRF5 temp sensor #12114

dhananjaygj opened this issue Dec 14, 2018 · 5 comments · Fixed by #17293
Assignees
Labels
bug The issue is a bug, or the PR is fixing a bug platform: nRF Nordic nRFx priority: low Low impact/importance bug

Comments

@dhananjaygj
Copy link
Contributor

Describe the bug
When nRF5 temperature sensor is used during BLE communication, assertion is hit in temp_nrf5_sample_fetch() with -EBUSY error (temp_nrf5.c line 54). Assertion is hit during clock_control_off() since both BLE controller and nRF Temperature uses the same nRF power clock.

To Reproduce
Steps to reproduce the behavior:

  1. Create sample BLE peripheral application with nRF temperature sensor.
  2. Poll the temperature sensor every 2 seconds.
  3. Connect the sample BLE application with any peer
  4. At some point assertion is hit as explained above.

Expected behavior
Assertion should not occur or if it occurs API documentation can give more information

Impact
Even if this issue is minor this affects just the Debug build of our application and release build works fine. Hopefully there is no problem using BLE along with nRF temperature sensor in parallel.

Environment (please complete the following information):

  • Zephyr
  • nRF52840 board
@dhananjaygj dhananjaygj added the bug The issue is a bug, or the PR is fixing a bug label Dec 14, 2018
@dhananjaygj
Copy link
Contributor Author

Below is part of the discussion with @anangl regarding this observation

ok, so the assertion fails when the clock is released
then I think the assertion in line 54 is incorrect
I am not sure, because the clock_control API does not precise what error codes should clock_control_off() return
and from what I see in the code, the nrf5 clock_control returns -EBUSY when there is another module that requested the clock and still needs it, so the clock cannot be turned off at the moment
so only the reference counter is decremented and the clock will be turned off after the last module that previously requested it releases it as well (edited)
therefore, I think what you are observing is a pretty normal situation, the clock is no longer needed for the temperature sensor, but it cannot be just turned off because it is still needed for BLE
so the -EBUSY error is not actually signaling that something is wrong, it is rather an information that the request of releasing the clock was accepted, but the clock itself cannot be turned of yet

@carlescufi
Copy link
Member

@cvinayak and @anangl can you please reply?

@carlescufi carlescufi added the priority: low Low impact/importance bug label Dec 17, 2018
@dhananjaygj
Copy link
Contributor Author

I submitted a patch based on suggestions from @anangl here #12133

@dhananjaygj
Copy link
Contributor Author

The assertion issue when BLE and nRF temperature sensor is used concurrently is now seen at a different place

https://github.com/zephyrproject-rtos/zephyr/blob/master/drivers/sensor/nrf5/temp_nrf5.c#L47

The returned error code being -EINPROGRESS

After discussing with @anangl, he suggested that

clock driver should not return that the operation is in progress when it is instructed to do it in the blocking manner.

@cvinayak
I have created a sample application to reproduce the same. It just a modified peripheral_hids sample.
The precondition is to have the peripheral connected and notice the assertion after sometime. It happens mostly within 1 minute but it will eventually happen.

peripheral_hids_temp_sensor_issue.zip

@nashif nashif added the platform: nRF Nordic nRFx label Apr 2, 2019
@nashif nashif assigned anangl and unassigned cvinayak Apr 2, 2019
@nashif
Copy link
Member

nashif commented Apr 2, 2019

@anangl ping

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue is a bug, or the PR is fixing a bug platform: nRF Nordic nRFx priority: low Low impact/importance bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants