Skip to content

STM32 USB update step 1 #11675

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 16 commits into from
Oct 28, 2019
Merged

STM32 USB update step 1 #11675

merged 16 commits into from
Oct 28, 2019

Conversation

jeromecoutant
Copy link
Collaborator

@jeromecoutant jeromecoutant commented Oct 11, 2019 •
edited
Loading

Description

Hi

Goal is to make USB Device integration easy for each STM32 targets.

Step 1 (this PR):

  • USB pins are declared for all current supported ST boards
    even if USB_DEVICE feature is not supported yet
    even is the board doesn't have any connector
  • So, no more need to manually hardcode pins value for each target
    This fixes How to configure USB pins for custom target based on STM32F746? #10806
    Only addition of USB_DEVICE in the local mbed_app.json file is necessary
  • removes compilation warning
    This fixes Above array bounds access warning in ST USB implementation #10822
  • this should work for every targets supporting OTG and with full speed connector
    So targets, which was supporting USB, still supports USB (no removed target, no target added)
  • Old and not supported MBED2 USB lib is removed from targets.json

Next step (other PR coming soon):

  • STM32WB, STM32H7, STM32L0
  • USB High speed

@bulislaw @MarceloSalazar @LMESTM

Pull request type

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

Reviewers

Release Notes

@jeromecoutant jeromecoutant mentioned this pull request Oct 11, 2019
Closed
@ciarmcom ciarmcom requested review from a team October 11, 2019 17:00
@ciarmcom
Copy link
Member

@jeromecoutant, thank you for your changes.
@ARMmbed/mbed-os-maintainers @ARMmbed/mbed-os-tools please review.

mark-edgeworth
mark-edgeworth previously requested changes Oct 14, 2019
View reviewed changes
tools/targets/STM32_gen_PeripheralPins.py Outdated
@@ -1,6 +1,6 @@
"""
* mbed Microcontroller Library
* Copyright (c) 2006-2018 ARM Limited
* Copyright (c) 2019 ARM Limited
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be 2006-2019, unless you are sure that none of this was developed in the years inbetween.

tools/targets/STM32_gen_PeripheralPins.py Outdated
Comment on lines 337 to 342
if "OTG" not in signal:
usb_list.append([pin, name, signal])
if signal.startswith("USB_OTG_FS"):
usb_otgfs_list.append([pin, name, signal])
if signal.startswith("USB_OTG_HS"):
usb_otghs_list.append([pin, name, signal])
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be incorrect as it changes the behaviour for all signals that contain 'OTG' that do not match the special case. Whilst there may be none of these today, this will fail to add the pin in future if the signal contains 'OTG'. Suggest an if..elif..elif...else construct could be better here.
If this is not the intent then there should be a big comment explaining why. Ideally there should be a comment to explain what this new code is trying to do in any case.

tools/targets/STM32_gen_PeripheralPins.py Outdated
use_hs_in_fs = False
nb_loop = 1
inst = "USB_FS"
if lst == usb_otgfs_list:
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a list element comparison - use 'is' instead. However, the pattern of accessing a global variable from inside the function in this way makes the code harder to understand, harder to maintain and harder to test.

tools/targets/STM32_gen_PeripheralPins.py Outdated
inst = "USB_FS"
if lst == usb_otgfs_list:
inst = "USB_FS"
elif lst == usb_otghs_list:
Copy link
Contributor

Choose a reason for hiding this comment

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

same here.

tools/targets/STM32_gen_PeripheralPins.py Outdated
s1 += "\n"
out_c_file.write(s1)
if lst:
if lst == usb_otghs_list:
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

@jeromecoutant
Copy link
Collaborator Author

Rebased after @mark-edgeworth comments

@0xc0170 0xc0170 requested a review from a team October 21, 2019 10:08
@0xc0170
Copy link
Contributor

0xc0170 commented Oct 21, 2019

Note, there is a conflict now.

@jeromecoutant
Copy link
Collaborator Author

Conflict solved
Thx

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 21, 2019

CI started

@mbed-ci
Copy link

mbed-ci commented Oct 21, 2019

Test run: FAILED

Summary: 1 of 4 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

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

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 21, 2019

The errors look related , please review

@jeromecoutant
STM32WB USB pins addition
0e1a04b
@jeromecoutant
STM32 clock configuration depending on USB
01e798f
@jeromecoutant
STM32 targets.json update for USB
3950fd0 
Removed unsupported MBED2 features:
- USB_STM_HAL
- USBHOST_OTHER
@jeromecoutant
STM32 USB redesign step 1
dab09f3 
No more need to explicitly configure each targets.
Pins are now defined in the PeripheralPin.c file which is build by a script.
@jeromecoutant
Copy link
Collaborator Author

Error corrected.
Please restart CI
Thx

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 22, 2019

CI restarted

@mbed-ci
Copy link

mbed-ci commented Oct 22, 2019

Test run: SUCCESS

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

@0xc0170 0xc0170 added ready for merge release-version: 6.0.0-alpha-1 First pre-release version of 6.0.0 and removed needs: review labels Oct 28, 2019
@0xc0170 0xc0170 merged commit df79609 into ARMmbed:master Oct 28, 2019
@jeromecoutant jeromecoutant deleted the PR_USB_STEP1 branch October 30, 2019 08:20
This was referenced Oct 30, 2019
Closed
Closed
This was referenced Nov 18, 2019
Merged
Merged
@0xc0170 0xc0170 added release-version: 5.15.0-rc1 and removed release-version: 6.0.0-alpha-1 First pre-release version of 6.0.0 labels Nov 19, 2019
@0xc0170 0xc0170 mentioned this pull request Nov 19, 2019
Merged
@jeromecoutant jeromecoutant mentioned this pull request Nov 27, 2019
Merged
@moshe-borochov
Copy link

Hi @jeromecoutant,
I tried to work with the Usb Device on STM32H743
and it didn't work then I saw this PR .
Can you give me some information on how to add support to Usb Device on STM32H743.
Thank you in advance :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0xc0170 0xc0170 0xc0170 approved these changes

@mark-edgeworth mark-edgeworth Awaiting requested review from mark-edgeworth

Assignees
No one assigned
Labels
release-version: 5.15.0-rc1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Above array bounds access warning in ST USB implementation How to configure USB pins for custom target based on STM32F746?
6 participants
@jeromecoutant @ciarmcom @0xc0170 @mbed-ci @moshe-borochov @mark-edgeworth