Skip to content

Crash while disconnecting device from USB #12016

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

Closed
pdunaj opened this issue Dec 11, 2018 · 1 comment
Closed

Crash while disconnecting device from USB #12016

pdunaj opened this issue Dec 11, 2018 · 1 comment
Assignees
Labels
area: USB Universal Serial Bus bug The issue is a bug, or the PR is fixing a bug platform: nRF Nordic nRFx priority: medium Medium impact/importance bug

Comments

@pdunaj
Copy link
Collaborator

pdunaj commented Dec 11, 2018

Describe the bug
Device starts as connected, disconnect it promptly.

ASSERTION FAIL [m_drv_state == NRFX_DRV_STATE_POWERED_ON] @ /home/padu/work/fw-nrfconnect-zephyr/ext/hal/nordic/nrfx/drivers/src/nrfx_usbd.c:1865:

To Reproduce
This is a race. Will add more info in the comment.
Start with the device connected. Just after booting up disconnect the device.

Expected behavior
Device must not crash.

Impact
Probably showstopper although it is not easy to replicate (so far happened once).

Screenshots or console output
Assert above.

Environment (please complete the following information):
rev:{{e21d7cc}}
Replicated using older revision but the bug is still there.

@pdunaj pdunaj added the bug The issue is a bug, or the PR is fixing a bug label Dec 11, 2018
@pdunaj
Copy link
Collaborator Author

pdunaj commented Dec 11, 2018

I was unable to replicate it while testing today but I see how this can be triggered.
usb_dc_nrfx_power_event_callback is being called with event from hw, set the state and then submits the work.

usb_dc_nrfx_power_event_callback is called from an interrupt context while work is executed from a workqueue thread.

There are two cases that can lead to the assert (situation where we detached while not being attached). Common part is that device starts with USB detached, then device is attached and work is submitted. Now the cases:

  1. Before work is executed device detaches. Work will execute with detached state and call nrfx detach again.
  2. While work executes device detaches. In this case state will change while we process.

In both cases we should get an assert.

Hi @pawelzadrozniak , could you review this part. I think we need to handle the state check directly in usb_dc_nrfx_power_event_callback (e.g. if state is not changed do not schedule work) or enqueue all events and process them in the same order as were received from HW.

@carlescufi carlescufi added area: USB Universal Serial Bus platform: nRF Nordic nRFx labels Dec 11, 2018
@carlescufi carlescufi added the priority: medium Medium impact/importance bug label Dec 11, 2018
pawelzadrozniak added a commit to pawelzadrozniak/zephyr that referenced this issue Jan 23, 2019
Some of the events from USBD peripheral (i.e. cable disconnect)
were handled in IRQ context and some of them (i.e. ep r/w events) in
system workqueue (inherited from initial driver implementation).
This may lead to race condition in some specific situations.
Currently, all of the events are enqueued in ISR and processed in
workqueue. Driver is reinitialized on queue overflow and queue size
is configurable in KConfig.

Fixes zephyrproject-rtos#12016

Signed-off-by: Paweł Zadrożniak <[email protected]>
carlescufi pushed a commit that referenced this issue Jan 23, 2019
Some of the events from USBD peripheral (i.e. cable disconnect)
were handled in IRQ context and some of them (i.e. ep r/w events) in
system workqueue (inherited from initial driver implementation).
This may lead to race condition in some specific situations.
Currently, all of the events are enqueued in ISR and processed in
workqueue. Driver is reinitialized on queue overflow and queue size
is configurable in KConfig.

Fixes #12016

Signed-off-by: Paweł Zadrożniak <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: USB Universal Serial Bus bug The issue is a bug, or the PR is fixing a bug platform: nRF Nordic nRFx priority: medium Medium impact/importance bug
Projects
None yet
Development

No branches or pull requests

3 participants