subsys: usbd_msc: fix the issues that occur when the user calls usbd_disable. #97769
Conversation
zephyrbot
added
size: XS
| if (err == -ECONNABORTED) { | ||
| LOG_WRN("request ep 0x%02x, len %u cancelled", | ||
| bi->ep, buf->len); | ||
| atomic_clear_bit(&ctx->bits, MSC_CLASS_ENABLED); |
There was a problem hiding this comment.
This looks simply wrong. Why msc_bot_disable() is not called in your use case?
There was a problem hiding this comment.
usbd_disable() first calls usbd_config_set(uds_ctx, 0);
zephyr/subsys/usb/device_next/usbd_device.c
Lines 311 to 322 in 2cfd924
which will in turns call
usbd_config_reset()zephyr/subsys/usb/device_next/usbd_config.c
Lines 113 to 114 in 2cfd924
which disables all classes
so it will call
msc_bot_disable().
There was a problem hiding this comment.
ep_dequeue exists in step 1, which first submits -ECONNABORTED, causing the request to be executed first.
There was a problem hiding this comment.
usbd_interface_shutdown -> usbd_ep_disable -> udc_ep_dequeue -> udc_submit_ep_event
There was a problem hiding this comment.
The thread priority behind the USBD preempted the current thread’s priority, causing the USBD request to be executed first.
There was a problem hiding this comment.
Yes, and we also need to know how to completely resolve it. It seems that currently all UDC drivers are affected by this issue.
There was a problem hiding this comment.
@tmon-nordic In this commit, k_sched_lock is called in usbd_disable to disable the scheduler, just like in usbd_enable.
There was a problem hiding this comment.
@tmon-nordic However, I’m not sure why k_yield is called inside usbd_ep_disable. Since calling k_yield forces a context switch, it causes the MSC thread to run first. That’s why I disabled it here.
There was a problem hiding this comment.
The new approach is much better. However, I think it is still important to look into why the subsequent enqueue is not failing.
There was a problem hiding this comment.
I don't know why the yield is called at that position. The reason why ep_enqueue did not fail is that ep_dequeue cleared busy, and ep_enqueue did not determine that it was busy.
|
@jfischer-no FYI |
clear MSC_CLASS_ENABLED to prevent issuing an OUT request after a soft disconnect at the application layer. Otherwise, during a subsequent soft connect, MSC_BULK_OUT_QUEUED would not be cleared, causing the OUT transfer to fail and leading to enumeration failure. Signed-off-by: tangzhenye <[email protected]>
|
4 participants
When ECONNABORTED is received, clear MSC_CLASS_ENABLED to prevent issuing an OUT request after a soft disconnect at the application layer. Otherwise, during a subsequent soft connect, MSC_BULK_OUT_QUEUED would not be cleared, causing the OUT transfer to fail and leading to enumeration failure.