ChibiOS USB: Add a dummy IN callback to work around LLD bugs #18811
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In qmk#18631 some IN notification callbacks that were doing nothing were removed, which should be a valid thing to do (ChibiOS HAL checks the `in_cb` and `out_cb` pointers for being non-NULL before invoking those optional callbacks). However, it turned out that some less popular USB LLDs (KINETIS and MIMXRT1062) have their own checks for those pointers, and (incorrectly) skip the ChibiOS callback handling when those pointers are NULL, which breaks the code for the `USB_USE_WAIT` configuration option (the waiting thread never gets resumed if the corresponding callback pointer is NULL). Add those dummy callbacks again (but use a single function for all of them instead of individual ones for each endpoint); this restores the KINETIS and MIMXRT1062 boards to the working state while the LLDs are getting fixed.
|
The actual fixes for the underlying driver bugs should be:
The KINETIS fix was minimally tested on a Teensy 3.2 (MK20DX256); ideally it should be tested on other Teensy variations which have different MCUs but still use the same driver (LC, 3.5, 3,6), but I don't have that hardware. The MIMXRT1062 fix was just compile-tested with When these fixes finally land in the |
2 tasks
zvecr
approved these changes
Oct 22, 2022
This was
linked to
issues
Oct 22, 2022
|
Thanks, can confirm that this one fixes the issue as well on Teensy 4.1. |
ramonimbao
pushed a commit
to ramonimbao/qmk_firmware
that referenced
this pull request
Nov 28, 2022
In qmk#18631 some IN notification callbacks that were doing nothing were removed, which should be a valid thing to do (ChibiOS HAL checks the `in_cb` and `out_cb` pointers for being non-NULL before invoking those optional callbacks). However, it turned out that some less popular USB LLDs (KINETIS and MIMXRT1062) have their own checks for those pointers, and (incorrectly) skip the ChibiOS callback handling when those pointers are NULL, which breaks the code for the `USB_USE_WAIT` configuration option (the waiting thread never gets resumed if the corresponding callback pointer is NULL). Add those dummy callbacks again (but use a single function for all of them instead of individual ones for each endpoint); this restores the KINETIS and MIMXRT1062 boards to the working state while the LLDs are getting fixed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
In #18631 some IN notification callbacks that were doing nothing were removed, which should be a valid thing to do (ChibiOS HAL checks the
in_cbandout_cbpointers for being non-NULL before invoking those optional callbacks). However, it turned out that some less popular USB LLDs (KINETIS and MIMXRT1062) have their own checks for those pointers, and (incorrectly) skip the ChibiOS callback handling when those pointers are NULL, which breaks the code for theUSB_USE_WAITconfiguration option (the waiting thread never gets resumed if the corresponding callback pointer is NULL).Add those dummy callbacks again (but use a single function for all of them instead of individual ones for each endpoint); this restores the KINETIS and MIMXRT1062 boards to the working state while the LLDs are getting fixed.
A more in-depth analysis of what happens in the code is in #18805 (comment).
Tested on a Teensy 3.2 board (
qmk flash -kb handwired/onekey/teensy_32 -km default). On that hardware without this fix the lockup happens immediately after pressing a key mapped toKC_A; this does not match the MIMXRT1062 behavior described in #18805 (where plain keys still work), but that problem should hopefully be fixed by this change too (given that a revert of #18631 helps there).Types of Changes
Issues Fixed or Closed by This PR
Checklist