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.

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

usbus: Add support for full speed with high speed phy #19438

Merged
merged 6 commits into from
Mar 31, 2023
Merged

usbus: Add support for full speed with high speed phy #19438

merged 6 commits into from
Mar 31, 2023

Conversation

bergzand
Copy link
Member

Contribution description

This adds infrastructure around usbus and usbdev to query the speed of the USB link after enumeration. This as the maximum speed of the link might be slower than the maximum speed of the peripheral. This is the case with the stm32f429i-disco board that has a full speed phy coupled with the high speed peripheral.

This also adds the necessary code to the cdc_ecm code to use the correct packet size. The allocated buffer size is not modified with this PR unfortunately.

Testing procedure

The cdc_ecm handler should work with a HS peripheral coupled with a FS phy.

Issues/PRs references

Fixes an issue caused by #19358

@bergzand bergzand added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Mar 31, 2023
@bergzand bergzand requested a review from gschorcht March 31, 2023 13:34
@bergzand bergzand requested review from dylad and aabadie as code owners March 31, 2023 13:34
@github-actions github-actions bot added Area: drivers Area: Device drivers Area: sys Area: System Area: USB Area: Universal Serial Bus labels Mar 31, 2023
gschorcht
gschorcht reviewed Mar 31, 2023
View reviewed changes
sys/usb/usbus/usbus.c Outdated
}

if (res < 0) {
return 0; /* Misbehaving usbdev device not implementing any speed indication */
Copy link
Contributor

@gschorcht gschorcht Mar 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Shouldn't it be an assert(false)?

Copy link
Member Author

Choose a reason for hiding this comment

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

probably better than softly failing yes.

@riot-ci
Copy link

riot-ci commented Mar 31, 2023 •
edited
Loading

Murdock results

✔️ PASSED

ca0d075 Revert "tests/usbus_cdc_ecm: blacklist stm32f429i-disco"

Success Failures Total Runtime
6882 0 6882 08m:47s

Artifacts

@bergzand bergzand force-pushed the pr/usbopt_enumerated_speed branch from 49fa7bc to 39bb057 Compare March 31, 2023 13:45
@gschorcht
Copy link
Contributor

I know it sounds a bit weird after we just merged PR #19437. But if we merge the PR without rebasing onto the current master and reverting PR #19437, only the definitions (Number of EPs, EP data size) of the FS peripheral are used again.

@bergzand bergzand force-pushed the pr/usbopt_enumerated_speed branch 2 times, most recently from 58e2624 to 3881101 Compare March 31, 2023 13:58
gschorcht
gschorcht reviewed Mar 31, 2023
View reviewed changes
drivers/usbdev_synopsys_dwc2/usbdev_synopsys_dwc2.c Outdated
{
dwc2_usb_otg_fshs_t *usbdev = (dwc2_usb_otg_fshs_t *)dev;
const dwc2_usb_otg_fshs_config_t *conf = usbdev->config;
unsigned speed = (_device_regs(conf)->DSTS & USB_OTG_DSTS_ENUMSPD_Msk) >> USB_OTG_DSTS_ENUMSPD_Pos;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
unsigned speed = (_device_regs(conf)->DSTS & USB_OTG_DSTS_ENUMSPD_Msk) >> USB_OTG_DSTS_ENUMSPD_Pos;
unsigned speed = (_device_regs(conf)->DSTS & USB_OTG_DSTS_ENUMSPD_Msk) >>
USB_OTG_DSTS_ENUMSPD_Pos;

Copy link
Contributor

@gschorcht gschorcht Mar 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Squash it directly. Please rebase and include the revert for PR #19437.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed, squashed and added two revert commits for the ones in #19437

@bergzand bergzand force-pushed the pr/usbopt_enumerated_speed branch from 3881101 to b342f76 Compare March 31, 2023 14:43
@bergzand bergzand force-pushed the pr/usbopt_enumerated_speed branch from b342f76 to 8044e57 Compare March 31, 2023 14:43
@bergzand bergzand requested a review from fjmolinas as a code owner March 31, 2023 14:44
@github-actions github-actions bot added Area: boards Area: Board ports Area: Kconfig Area: Kconfig integration Area: tests Area: tests and testing framework labels Mar 31, 2023
gschorcht
gschorcht approved these changes Mar 31, 2023
View reviewed changes
Copy link
Contributor

@gschorcht gschorcht left a comment

Choose a reason for hiding this comment

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

Looks good and works as expected.

@gschorcht
Copy link
Contributor

bors merge

@gschorcht
Copy link
Contributor

bors cancel

@gschorcht gschorcht added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Mar 31, 2023
@miri64
Copy link
Member

miri64 commented Mar 31, 2023

bors merge

@bors
Copy link
Contributor

bors bot commented Mar 31, 2023

Build succeeded:

@bors bors bot merged commit f797bbe into RIOT-OS:master Mar 31, 2023
@gschorcht
Copy link
Contributor

@bergzand Thanks for contributing this fix.

@miri64 Again, sorry for my confusion with the bors Expected - Waiting for status to be reported status in the checks. I confused it with Murdock - The job succeeded. But as said, it was caused by the fact that I missed the staging build at https://ci.riot-os.org after I started bors merge. Therefore I canceled it 1 hour later. The staging build did not start for some reason. Maybe I should have just commanded bors retry. I'll know better next time.

@MrKevinWeiss MrKevinWeiss added this to the Release 2023.04 milestone Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gschorcht gschorcht gschorcht approved these changes

@dylad dylad Awaiting requested review from dylad dylad is a code owner

@aabadie aabadie Awaiting requested review from aabadie aabadie is a code owner

@leandrolanzieri leandrolanzieri Awaiting requested review from leandrolanzieri leandrolanzieri is a code owner

@jia200x jia200x Awaiting requested review from jia200x jia200x is a code owner

@MrKevinWeiss MrKevinWeiss Awaiting requested review from MrKevinWeiss MrKevinWeiss is a code owner

@smlng smlng Awaiting requested review from smlng

@MichelRottleuthner MichelRottleuthner Awaiting requested review from MichelRottleuthner MichelRottleuthner is a code owner

@fjmolinas fjmolinas Awaiting requested review from fjmolinas

Assignees
No one assigned
Labels
Area: boards Area: Board ports Area: drivers Area: Device drivers Area: Kconfig Area: Kconfig integration Area: sys Area: System Area: tests Area: tests and testing framework Area: USB Area: Universal Serial Bus CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Milestone
Release 2023.04
Development

Successfully merging this pull request may close these issues.
5 participants
@bergzand @riot-ci @gschorcht @miri64 @MrKevinWeiss