Adding support for a generic SOF callback #2213
Conversation
|
I'm unsure if the device port should be part of the parameter of the callback or not. I started the PR with it but removed it for now. |
Rocky04
changed the title
|
I've move sof status into usbd_device_t to ensure it's cleared on bus reset or configuration change.
|
HiFiPhile
dismissed
hathach’s stale review
tud_sof_cb_enable() added for user land sof control.
| _usbd_dev.sof_consumer &= (uint8_t)(~(1 << consumer)); | ||
| } | ||
|
|
||
| // Test logically unequal |
There was a problem hiding this comment.
Seems like a typo, should be: // Test logically XOR
But I don't get why to mention the specific implementation instead of the goal. While a hint that this is a logical XOR is indeed nice I think a comment should primary explain the meaning and not the implementation detail. Like: // Logically XOR to only handle a state change
There was a problem hiding this comment.
Not a typo, it could be other consumers later, should be only called when whole value change to true or false.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
| TU_ATTR_WEAK void tud_sof_cb(uint32_t frame_count) { | ||
| (void)frame_count; | ||
| } |
There was a problem hiding this comment.
Why not have a weak declaration in the header file? After all it's used only once at #L629 anyway. So a check there for a definition similar how it's done for other things should be more inline.
Edit:
In that case there can be a sanity check. Like to return false at #L394 if there is no function definition.
There was a problem hiding this comment.
We are converting to weak default implementation for keil and clang compatibility.
| bool tud_sof_cb_enable(bool en) | ||
| { | ||
| usbd_sof_enable(_usbd_rhport, SOF_CONSUMER_USER, en); | ||
| return true; |
There was a problem hiding this comment.
A return value makes no sense here.
While also true for my original PR that wasn't as obvious. I wasn't sure if every driver has implemented dcd_sof_enable() and so used TU_VERIFY() to check for that.
It would make sense if there is a check if an implementation of a weak declared tud_sof_cb() is present.
There was a problem hiding this comment.
You are right, forgot to change.
Can be used to synchronize the scanning to the polling for HID devices or other device functions. Not useful for time critical things because the time which has passed between the interrupt and the execution is unknown. For that it would be necessary to save the timestamp when the SOF interrupt handling is done in the ISR.
Proper example implementation of the PR #2180.
Edit:
A draft for a vendor specific SOF callback which is executed in the ISR via
dcd_event_handler()can be found here: #2220