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

Bring USB Feature branch into master #9768

Merged
merged 154 commits into from Feb 27, 2019

Conversation

@c1728p9
Copy link
Contributor

commented Feb 19, 2019

Description

Bring feature-hal-spec-usb-device into master.

Pull request type

[ ] Fix
[ ] Refactor
[ ] Target update
[x] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

Release Notes

This feature brings USB Device support into Mbed OS from a feature branch. Targets that support USB include the LPC1768, ARCH_PRO, K64F, NUCLEO_F207ZG, NUCLEO_F412ZG, DISCO_F413ZH, NUCLEO_F413ZH, NUCLEO_F429ZI, NUCLEO_F446ZE, NUCLEO_F746ZG, NUCLEO_F756ZG, NUCLEO_F767ZI, DISCO_F469NI, DISCO_F746NG, DISCO_F769NI, DISCO_L475VG_IOT01A, DISCO_L476VG, RZ_A1H, GR_LYCHEE, DISCO_L496AG, NUCLEO_L496ZG, NUCLEO_L4R5ZI. Supported USB device classes include USBAudio, USBHID, USBMouse, USBKeyboard, USBMIDI, USBMSD, USBSerial and USBCDC.

@ciarmcom ciarmcom requested review from ARMmbed/mbed-os-maintainers Feb 19, 2019

@ciarmcom

This comment has been minimized.

Copy link
Member

commented Feb 19, 2019

@cmonr cmonr requested review from donatieng, bulislaw and SenRamakri Feb 19, 2019

@cmonr cmonr requested a review from ARMmbed/mbed-os-test Feb 19, 2019

@juhaylinen

This comment has been minimized.

Copy link
Contributor

commented Feb 20, 2019

@c1728p9 Support for USBCDC_ECM class seems to be missing from the PR. Should I add the support here? My original PR #9443

@cmonr cmonr added the do not merge label Feb 20, 2019

@cmonr

This comment has been minimized.

Copy link
Contributor

commented Feb 20, 2019

@c1728p9 Why is this PR your fork instead of ARMmbed:feature-hal-spec-usb-device

@cmonr

This comment has been minimized.

Copy link
Contributor

commented Feb 20, 2019

Also, any chance of progressing these other PRs coming into the ARMmbed feature branch?
https://github.com/ARMmbed/mbed-os/pulls?utf8=%E2%9C%93&q=is%3Apr+is%3Aopen+base%3Afeature-hal-spec-usb-device+

@c1728p9

This comment has been minimized.

Copy link
Contributor Author

commented Feb 20, 2019

Also, any chance of progressing these other PRs coming into the ARMmbed feature branch?

Both of these need updates. It may be best re-create these PRs against master once this goes in.

@cmonr cmonr removed the do not merge label Feb 20, 2019

@cmonr

This comment has been minimized.

Copy link
Contributor

commented Feb 20, 2019

@ARMmbed/mbed-os-maintainers Note, this is a rebase ARMmbed:feature-hal-spec-usb-device and feature branch merge.

I'm fine with letting this come in asis, and removing ARMmbed:feature-hal-spec-usb-device once it's merged. Thoughts?

@c1728p9

This comment has been minimized.

Copy link
Contributor Author

commented Feb 20, 2019

Added SPDX license identifiers and fixed astyle problems.

@bridadan
Copy link
Contributor

left a comment

Seems ok to me!

Will there be a future PR to remove the unsupported USB stack? https://github.com/ARMmbed/mbed-os/tree/master/features/unsupported/USBDevice

"""

list = [64, 256]
dev = y

This comment has been minimized.

Copy link
@bridadan

bridadan Feb 21, 2019

Contributor

Holy smokes, what a host test! 🚢

When you run the `tests-usb_device-basic` test suite on a Linux machine, please make
sure that at least one of the following prerequisites are met:
* using the Linux kernel ***4.17* or newer**,
* using the ***eHCI*** USB driver instead of *xHCI*.

This comment has been minimized.

Copy link
@bridadan

This comment has been minimized.

Copy link
@cmonr

cmonr Feb 21, 2019

Contributor

@c1728p9 What's the technical reason for this limitation?

Are we expecting devlopers to have to load a specific OS driver to be able to test in Linux?

This comment has been minimized.

Copy link
@fkjagodzinski

fkjagodzinski Feb 21, 2019

Member

@c1728p9 What's the technical reason for this limitation?

Basically, an incomplete implementation of xHCI driver in kernels prior to 4.17. Please see the rest of this readme. I've also included some links for reference.

This comment has been minimized.

Copy link
@cmonr

cmonr Feb 21, 2019

Contributor

Basically, an incomplete implementation of xHCI driver in kernels prior to 4.17. Please see the rest of this readme. I've also included some links for reference.

Damn, those are actually good technical reasons...

Thanks for having them in the readme.

Show resolved Hide resolved requirements.txt Outdated
@c1728p9

This comment has been minimized.

Copy link
Contributor Author

commented Feb 21, 2019

@ARMmbed/mbed-os-maintainers be aware, USB tests are currently not part of CI. @OPpuolitaival will need to make some infrastructure changes to enable this. The two biggest limitations are:

  • USB tests require that host tests run locally
  • CI needs to have the target USB cables connected

c1728p9 added some commits Feb 7, 2018

Copy USBDevice from unsupported
Copy the USBDevice code out of unsupported and into a top level USB
folder in preparation for development.

squash
Add USBPhy, USB HAL and Utility class
Add the USBPhy and USBPhyEvents abstract classes, the HAL header using
this class and the EndpointResolver utility class.
Add USBDevice test code
Add a USB test and the class USBTester.cpp to go along with it.
Add USBPhy for the LPC17xx
Move the LPC17xx USB driver files from
mbed-os\features\unsupported\USBDevice\targets\TARGET_NXP
and update them to match the new USBPhy API.
Remove mbed 2 USB tests from Travis
Remove the mbed 2 USB tests since USB support will be part of mbed 5.

bcostm and others added some commits Nov 8, 2018

STM32L4 USB: move HAL_PCD_EP_Abort function
This function is for USB_OTG_FS devices only. Move it in the correct place (in "#ifdef USB_OTG_FS" area).
Tests: USB: Add a README for Linux users
Explain how to overcome xHCI limitations in kernels prior to v4.17 and
successfully run USB tests.
Renesas : Implement USB Device feature
I implemented USB Device feature for Renesas mbed boards.
The code referenced the following code as a starting point and is implemented it by inheritting USBPhy same as other boards.
(mbed-os\features\unsupported\USBDevice\targets\TARGET_RENESAS)
Tests: USB: Fix 'endpoint halt' test
Abort all endpoint transfers before running the test again.
Use an updated vendor request to explicitly restart device reads.
Remove inclusion of mbed.h from USB
Remove mbed.h from USB files and fix the build errors this causes.
This is required to pass CI.
Update files to include SPDX-License-Identifier
Update the header of all files to use a newer license template which
includes SPDX-License-Identifier.
Update requirements.txt
Add an upper bound to the API version of pyusb.

Co-Authored-By: c1728p9 <butleja10511@hotmail.com>

@c1728p9 c1728p9 force-pushed the c1728p9:feature-hal-spec-usb-device branch to 0e6056b Feb 22, 2019

@c1728p9

This comment has been minimized.

Copy link
Contributor Author

commented Feb 22, 2019

Rebased to fix conflicts with requirements.txt changes

@0xc0170

This comment has been minimized.

Copy link
Member

commented Feb 25, 2019

While reviewing, early CI run now

@mbed-ci

This comment has been minimized.

Copy link

commented Feb 25, 2019

Test run: SUCCESS

Summary: 12 of 12 test jobs passed
Build number : 1
Build artifacts

@cmonr

This comment has been minimized.

Copy link
Contributor

commented Feb 25, 2019

@cmonr cmonr added the risk: A label Feb 25, 2019

@fkjagodzinski
Copy link
Member

left a comment

Looks good for me 👍

@bulislaw

This comment has been minimized.

Copy link
Member

commented Feb 26, 2019

This PR is at risk of missing 5.12 release as it's marked as "needs: review". Code freeze is coming! On Friday 1st. Please review the changes ASAP.

@ARMmbed/mbed-os-hal

1 similar comment
@bulislaw

This comment has been minimized.

Copy link
Member

commented Feb 26, 2019

This PR is at risk of missing 5.12 release as it's marked as "needs: review". Code freeze is coming! On Friday 1st. Please review the changes ASAP.

@ARMmbed/mbed-os-hal

@cmonr cmonr added ready for merge and removed needs: review labels Feb 26, 2019

@cmonr cmonr merged commit 4b13c8a into ARMmbed:master Feb 27, 2019

27 checks passed

continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jenkins-ci/build-ARM Success
Details
jenkins-ci/build-ARMC6 Success
Details
jenkins-ci/build-GCC_ARM Success
Details
jenkins-ci/build-IAR Success
Details
jenkins-ci/cloud-client-test Success
Details
jenkins-ci/dynamic-memory-usage RTOS ROM(+0 bytes) RAM(+0 bytes)
Details
jenkins-ci/exporter Success
Details
jenkins-ci/greentea-test Success
Details
jenkins-ci/mbed2-build-ARM Success
Details
jenkins-ci/mbed2-build-GCC_ARM Success
Details
jenkins-ci/mbed2-build-IAR Success
Details
jenkins-ci/unittests Success
Details
travis-ci/astyle Local astyle testing has passed
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/doxy-spellcheck Local doxy-spellcheck testing has passed
Details
travis-ci/events Passed, runtime is 9117 cycles (-1090 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/include_check Local include_check testing has passed
Details
travis-ci/licence_check Local licence_check testing has passed
Details
travis-ci/littlefs Passed, code size is 8408B (+0.00%)
Details
travis-ci/psa-autogen Local psa-autogen testing has passed
Details
travis-ci/tools-py2.7 Local tools-py2.7 testing has passed
Details
travis-ci/tools-py3.5 Local tools-py3.5 testing has passed
Details
travis-ci/tools-py3.6 Local tools-py3.6 testing has passed
Details
travis-ci/tools-py3.7 Local tools-py3.7 testing has passed
Details

@cmonr cmonr removed the ready for merge label Feb 27, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.