usb: device_next: use k_event instead of msg_q #96814
Conversation
Define USB device thread data and stack for each device context. Signed-off-by: Johann Fischer <[email protected]>
| k_msgq_get(&usbd_msgq, &event, K_FOREVER); | ||
| uint32_t events; | ||
|
|
||
| events = k_event_wait(&uds_ctx->events, UINT32_MAX, false, K_FOREVER); |
There was a problem hiding this comment.
What happens if multiple events come? The msgq was going to keep them, this will seemingly lose duplicates?
There was a problem hiding this comment.
Well, yes, that could be an issue, that is why it is a draft. We also lose some metadata from the events. Losing some duplicates of the bus event may be intended.
| __ASSERT(uds_ctx != NULL && usbd_is_initialized(uds_ctx), | ||
| "USB device is not initialized"); | ||
| usbd_event_handler(uds_ctx, &event); | ||
| if (events == BIT(UDC_EVT_EP_REQUEST)) { |
There was a problem hiding this comment.
You could plausibly have both UDC_EVT_EP_REQUEST and other events flagged here but only one path would be handled (EP_REQUEST).
There was a problem hiding this comment.
Yes, that could happen. Thanks for the point.
| ctx->stack_size, | ||
| usbd_thread, | ||
| ctx, NULL, NULL, | ||
| K_PRIO_COOP(8), 0, K_NO_WAIT); |
There was a problem hiding this comment.
Why are there not one but many threads for USB here, and cooperative at that? Is there a time limit on how long these coop tasks are allowed to run before yielding?
There was a problem hiding this comment.
Why are there not one but many threads for USB here, and cooperative at that?
With this commit there are as many threads as there are USB device contexts used in the application. Usually one.
Is there a time limit on how long these coop tasks are allowed to run before yielding?
The thread is active to process current events, which are always initiated by the host. The longest path is for control transfers, but nothing is unpredictable except user code. Simple USB device functions could run in preemptive context not sure about UAC/UVC. I had a patch to run the stack from main thread for USB DFU, but I am not convinced whether it makes sense.
There was a problem hiding this comment.
Since they are coop and all at the same priority anyways why not share a stack and use a common usbd work queue instead?
USBD_MAX_UDC_MSG configures the number of events coming from the UDC driver that the stack can keep. This can be filled very quickly on high speed devices, and subsequent events would be dropped. It seems that only endpoint events, which are net_bufs, need to be queued, and we can use single linked lists for that. Then, we can replace msg_q with k_event. Likely, some work is needed to filter bus events, as now the stack may see them simultaneously. Signed-off-by: Johann Fischer <[email protected]>
jfischer-no
force-pushed
the
pr-device_next-use-k_events
branch
from
c5540d0 to
87427d8
Compare
|
USBD_MAX_UDC_MSG configures the number of events coming from the UDC
driver that the stack can keep. This can be filled very quickly on high
speed devices, and subsequent events would be dropped. It seems that
only endpoint events, which are net_bufs, need to be queued, and we can
use single linked lists for that. Then, we can replace msg_q with
k_event. Likely, some work is needed to filter bus events, as now the
stack may see them simultaneously.
The first commit introduces some drawbacks in regards to memory usage for use cases like samples/subsys/usb/dfu.