Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

usb: dfu: refactor dfu class implementation #30611

Merged
merged 3 commits into from
Dec 20, 2020

Conversation

mrrosen
Copy link
Contributor

@mrrosen mrrosen commented Dec 10, 2020

From #30562, some smaller fixes for USB DFU class implementation were included in this PR that have been separated out in this PR as separate commits:

  1. Implement appDETACH timer as required by USB DFU 1.1 specification (Section 5.1) so if the host fails to perform a USB reset within a certain windows, the device will return to appIDLE and future USB resets will not cause the device to incorrectly transition into DFU mode
  2. Move the time when the device changes the descriptors to the DFU mode descriptors to when the host sends a USB reset rather than after receiving a DFU_DETACH command since while in appDETACH, the device is still in Run-time mode and should still return the Run-time descriptors if queried.
  3. Refactor the code to allow for separate VID and PID for the DFU mode descriptor. At very least, separate PID is required for DFU mode as some host OS's will cache descriptors depending on the VID and PID combination. Using the same VID and PID for both the Run-time and DFU mode descriptors has been observed to cause issues with USB DFU on Windows 10 where the device is not found after USB reset due to descriptor caching.

Michael Rosen added 2 commits December 10, 2020 12:08
In accordance with USB DFU 1.1 Section 5.1, a device should only
stay in appDETACH for a given period of time, either from the
DFU_DETACH request (wValue ms) or from the wDetachTimeout property,
after which the device should return to appIDLE.

Signed-off-by: Michael Rosen <[email protected]>
The USB device descriptors for DFU mode should only change after a
USB reset, not in appDETACH as the device is still in run-time mode
until reset; thus should still return the run-time descriptors when
requested.

Signed-off-by: Michael Rosen <[email protected]>
@mrrosen
Copy link
Contributor Author

mrrosen commented Dec 10, 2020

@jfischer-no There was a bit of open discussion about the new VID/PID, I did a bit of what you had suggested as part of these commits but Im not sure we reached a final conclusion on whether there should be a separate VID, and what if anything cmake for DFU should check; since this is probably the most important fix in this commit due to it causing the most issues (again, Windows 10, dfu-utl 0.9 will always fail any DFU transaction the first time due to descriptor caching)


zephyr_sources(usb_dfu.c)

if(CONFIG_USB_DEVICE_VID EQUAL CONFIG_USB_DEVICE_DFU_VID)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove everything about USB_DEVICE_DFU_VID, it is not necessary to change VID.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Refactor the file structure for USB DFU class to facilitate
separate PID for USB DFU when in DFU mode. As required by USB DFU
1.1 Section 2, the PID in the USB device descriptor must be
different between the Run-time and DFU mode device descriptor to
avoid problems caused by the host OS caching the remaining
descriptors when switching to DFU mode, thus hiding the new
interface descriptors from applications on the host and reporting
the Run-time descriptors when the device is in DFU mode.

To avoid adding too much clutter to the root USB class Kconfig and
CMakeLists files, move the DFU class files into their own directory
with dedicated Kconfig and CMakeLists.txt.

Signed-off-by: Michael Rosen <[email protected]>
@mrrosen
Copy link
Contributor Author

mrrosen commented Dec 16, 2020

@jfischer-no Any other changes need to be made to this PR?

@nashif nashif merged commit 1c89837 into zephyrproject-rtos:master Dec 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants