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

STM32L4: Add support of the new USB Device API #8665

Conversation

bcostm
Copy link
Contributor

@bcostm bcostm commented Nov 7, 2018

Description

Enable new USB Device API on the following STM32L4 platforms:

  • NUCLEO_L496ZG
  • NUCLEO_L496ZG_P
  • NUCLEO_L4R5ZI
  • DISCO_L496AG
  • DISCO_L475VG_IOT01A
  • DISCO_L476VG

The same USB HAL patch as done for STM32F4 and STM32F2 has been applied (see PRs #7322 and #8583)

Tests

  • tests-usb_device-basic tests are OK on ARM, IAR and GCC_ARM
  • tests-usb_device-serial tests are FAIL

Pull request type

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

@adbridge
Copy link
Contributor

@c1728p9 This review has been waiting for 7 days could you please take a look asap

@@ -311,10 +310,10 @@ __weak void HAL_PCD_MspDeInit(PCD_HandleTypeDef *hpcd)
*/
HAL_StatusTypeDef HAL_PCD_Start(PCD_HandleTypeDef *hpcd)
{
__HAL_LOCK(hpcd);
// MBED PATCH __HAL_LOCK(hpcd);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was the lock removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It has been removed in the F4 file so I just did the same...

hpcd->EPLock[index].Lock = HAL_UNLOCKED;

for (index = 0; index < hpcd->Init.dev_endpoints ; index++) { // MBED PATCH
hpcd->EPLock[index].Lock = HAL_UNLOCKED;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this just cosmetic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 15, 2018

Note: we are currently having high load on the CI. It might take some time.

@c1728p9 this is not scheduled for 5.11 , is it?

@0xc0170 0xc0170 mentioned this pull request Nov 16, 2018
@0xc0170
Copy link
Contributor

0xc0170 commented Nov 16, 2018

Info: This PR has been re-bundled into a new rollup PR (#8768).

No further work is needed here, as once that PR is merged, this PR will also be closed and marked as merged.
If any more commits are made in this PR, this PR will remain open and have to go through CI on its own.

@bcostm
Copy link
Contributor Author

bcostm commented Nov 16, 2018

OK. I will not add more commits here.

I will send a new PR later to align the PCD functions (Init, Start, Stop) in the F2, F4, F7 and L4 families.

@cmonr
Copy link
Contributor

cmonr commented Nov 16, 2018

@bcostm It looks like both PRs in the last rollup PR had issues. Please take a look at the L4-related build failures: #8768 (comment)

@bcostm bcostm force-pushed the L4_feature-hal-spec-usb-device branch from 18037c6 to da471ab Compare November 20, 2018 09:27
@bcostm
Copy link
Contributor Author

bcostm commented Nov 20, 2018

Rebased. I don't understand the build errors with the rollup PR for the L4 boards related to USB ? Because these boards don't have the USBDEVICE macro in targets.json. How is it possible ?

@cmonr
Copy link
Contributor

cmonr commented Nov 22, 2018

@bcostm It's possible it was an interpretation error on my part.

Restarting CI.

@screamerbg
Copy link
Contributor

@0xc0170 @cmonr What's the issue with this PR and CI? We're very close to feature freeze for Mbed OS 5.11 and we need to get this in (assuming it's CI issues)

@bcostm
Copy link
Contributor Author

bcostm commented Nov 22, 2018

I have reproduced the build error and found the root cause. I'll send a new commit to fix it.

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 22, 2018

@0xc0170 @cmonr What's the issue with this PR and CI? We're very close to feature freeze for Mbed OS 5.11 and we need to get this in (assuming it's CI issues)

This answers my question I asked a week ago above - this is targeting 5.11.

There were build problems and CI issues, we migrated to the new CI so improve. We will start CI soon (rollup PR and kvstore need to pass first , they should finish within few hours).

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 24, 2018

CI started

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 26, 2018

This is the build failure:

21:11:16 [MTS_DRAGONFLY_F411RE:ARM] # mbed-os-example-cellular MTS_DRAGONFLY_F411RE ARM

21:11:16 [MTS_DRAGONFLY_F411RE:ARM] UndefinedParameter: Attempt to override undefined parameter 'nsapi.default-cellular-plmn' in 'application[*]'

I do not see any change related to this. @jarvte CAn you review the failure? I looked at the cellular example, there were recent commits (related to this failure) ? Did anything break?

@cmonr
Copy link
Contributor

cmonr commented Jan 10, 2019

sigh

And now the feature branch itself needs a rebase because of Travis CI fixes...

@bcostm Will start in a bit once the feature branch is sorted out. The TLDR is that the feature branch needs this PR (#9289) and a fix for an astyle simplification which I'm resolving now (ref: #9287 (comment)).

@cmonr
Copy link
Contributor

cmonr commented Jan 11, 2019

Note: #7976 (comment)

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 14, 2019

@c1728p9 Can you help rebasing the branch please?

@bcostm bcostm force-pushed the L4_feature-hal-spec-usb-device branch from e8c5762 to 72e0a98 Compare January 18, 2019 09:22
@bcostm
Copy link
Contributor Author

bcostm commented Jan 18, 2019

Rebased

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 18, 2019

I restarted travis failure, and will schedule CI as soon as CI is back from updates

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 18, 2019

CI restarted

@mbed-ci
Copy link

mbed-ci commented Jan 18, 2019

Test run: FAILED

Summary: 3 of 7 test jobs failed
Build number : 6
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_build-GCC_ARM
  • jenkins-ci/mbed-os-ci_build-ARM
  • jenkins-ci/mbed-os-ci_build-IAR

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 18, 2019

[DEBUG] Output: "./usb/device/targets/TARGET_STM/USBPhy_STM32.cpp", line 518: Error: #20: identifier "HAL_PCD_EP_Abort" is undefined found in the build logs, please review

@bcostm
Copy link
Contributor Author

bcostm commented Jan 18, 2019

I don't understand what happens here ??? It is the same error as 10 days ago which I already resolved. Now it comes back again after a Rebase ? @jeromecoutant can you please have a look ?

@@ -3777,6 +3777,7 @@
"TRNG",
"FLASH",
"QSPI",
"USBDEVICE",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"USBDEVICE",

@bcostm bcostm force-pushed the L4_feature-hal-spec-usb-device branch from 72e0a98 to bd1c700 Compare January 21, 2019 07:35
@jeromecoutant
Copy link
Collaborator

CI restart :-)

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 21, 2019

CI restarted

@mbed-ci
Copy link

mbed-ci commented Jan 22, 2019

Test run: FAILED

Summary: 1 of 11 test jobs failed
Build number : 7
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_greentea-test

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 22, 2019

Test restarted (CI agent failures)

@0xc0170 0xc0170 merged commit 091b0e7 into ARMmbed:feature-hal-spec-usb-device Jan 22, 2019
@bcostm bcostm deleted the L4_feature-hal-spec-usb-device branch January 22, 2019 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants