USB fixes and improvements #5874
Conversation
c1728p9
commented
Jan 17, 2018
c1728p9
commented
- Fix local static initialization in USB
- Remove use of deprecated attach in USB
- Fix clearing of ISTAT in Kinetis USB
|
/morph build |
|
@Sissors this doesn't fix the Kinetis USB issues discussed earlier. That will be in a separate PR. This is just general fixes and improvements. |
Build : SUCCESSBuild number : 887 Triggering tests/morph test |
Exporter Build : SUCCESSBuild number : 558 |
Test : SUCCESSBuild number : 732 |
| @@ -619,20 +621,27 @@ uint8_t * USBAudio::configurationDesc() { | |||
| // Terminator | |||
| 0 // bLength | |||
| }; | |||
|
|
|||
| MBED_ASSERT(sizeof(configDescriptorTemp) == sizeof(deviceDescriptor)); | |||
| memcpy(deviceDescriptor, configDescriptorTemp, sizeof(configDescriptor)); | |||
There was a problem hiding this comment.
Wrong variable in memcpy
The local static initialization in USB was causing multiple problems: -Configurable descriptor values are set only once -USB descriptor initialization causes a trap in debug builds since this is acquiring a mutex in an interrupt handler -Extra ram used since all descriptors are in RAM This patch fixes these problems by making fixed descriptors static const so they are stored in flash and never need to be initialized and by making descriptors that do change a member of the class so they are always initialized when requested rather than once though lazy static local initialization.
Attach callbacks with the assignment operator rather than with the deprecated attach function. This fixes deprecation warnings. This patch also adds the ability to attach a Callback directly.
The ISTAT register is write 1 to clear. Because of this ORing this register with itself fill clear all bits that are set. This patch updates the code to use plain assignment so only desired bit is cleared.
c1728p9
force-pushed
the
usb_fixes_and_improvements
branch
from
fcd3f81 to
beaac15
Compare
|
/morph build |
Build : SUCCESSBuild number : 890 Triggering tests/morph test |
Exporter Build : SUCCESSBuild number : 563 |
Test : SUCCESSBuild number : 735 |
|
@c1728p9 Is this backward compatible changes? I would say it is not based on the diff here. Is this intentional? |
|
@0xc0170 the intent was to be backwards compatible. What are you concerned with? The only place this may cause issues is with USB classes outside of mbed-os which override the descriptor functions (as the return value was changes to const). If this is the problem you are concerned with then I could remove that part of this PR. |
Yes, those changes I had on my mind as they are not binary compatible. Just concerned about breaking a user space. The rest looks good. |
|
I tried to see if this solved my issues, however USBDevice lib is not linked to here apparently, and hasn't been updated in half a year (https://os.mbed.com/users/mbed_official/code/USBDevice/). It says it is legacy and refers to mbed-os, but as far as I can see mbed-os does not include USBDevice. Then again these days I don't understand mbed anymore anyway, so maybe just me. |
|
@Sissors unfortunately, the changes are only in mbed-os right now. To try out the changes copy this directory from mbed-os: |
|
Makes you wonder first why it is not synced to the USBDevice lib. But still. I got mbd-os imported in my project in the online compiler, and maybe I am missing something, but I can't find a USBDevice class, and the compiler also does not find it if I just include it. Of course it could be setup in an offline compiler, but then I first needs to setup everything again. Edit: Online compiler also cannot handle importing from sub-folders of a Github. I guess I'll try just importing all files via manual import, but still would be nice if USBDevice was actually get up to date. |
|
EDIT: moved to #7928. |
|
@bmcdonnell-ionx Thanks for the review, can you create a new issue please ? It improves the visibility. |
