STM32WB: enable USB Device #12751

jeromecoutant · 2020-04-02T15:21:37Z

Summary of changes

This enable USBDEVICE for STM32WB55

Impact of changes

Migration actions required

Documentation

Pull request type

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

USB tests are OK.

Also tested with an internal application

[] No Tests required for this change (E.g docs only update)
[x] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers

ciarmcom · 2020-04-02T17:00:37Z

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

donatieng · 2020-04-08T09:07:47Z

Thanks @jeromecoutant, we'll review and provide feedback ASAP

fkjagodzinski · 2020-04-14T14:43:48Z

Thank you for the PR @jeromecoutant! I checked our USB tests with mbed test -DUSB_DEVICE_TESTS -t GCC_ARM -m NUCLEO_WB55RG -n tests-usb_device*.

| target                | platform_name | test suite              | result | elapsed_time (sec) | copy_method |
|-----------------------|---------------|-------------------------|--------|--------------------|-------------|
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-basic  | FAIL   | 13.95              | default     |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-hid    | OK     | 20.54              | default     |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-msd    | FAIL   | 45.98              | default     |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-serial | OK     | 36.44              | default     |
mbedgt: test suite results: 2 FAIL / 2 OK

See the full table here.

| target                | platform_name | test suite              | test case                                       | passed | failed | result  | elapsed_time (sec) |
|-----------------------|---------------|-------------------------|-------------------------------------------------|--------|--------|---------|--------------------|
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-basic  | endpoint test abort                             | 0      | 0      | SKIPPED | 0.0                |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-basic  | endpoint test data correctness                  | 0      | 0      | SKIPPED | 0.0                |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-basic  | endpoint test data toggle reset                 | 0      | 0      | SKIPPED | 0.0                |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-basic  | endpoint test halt                              | 0      | 0      | SKIPPED | 0.0                |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-basic  | endpoint test parallel transfers                | 0      | 0      | SKIPPED | 0.0                |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-basic  | endpoint test parallel transfers ctrl           | 0      | 0      | SKIPPED | 0.0                |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-basic  | usb control basic test                          | 0      | 1      | FAIL    | 0.46               |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-basic  | usb control sizes test                          | 0      | 0      | SKIPPED | 0.0                |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-basic  | usb control stall test                          | 0      | 0      | SKIPPED | 0.0                |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-basic  | usb control stress test                         | 0      | 0      | SKIPPED | 0.0                |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-basic  | usb device reset test                           | 0      | 0      | SKIPPED | 0.0                |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-basic  | usb repeated construction destruction test      | 0      | 0      | SKIPPED | 0.0                |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-basic  | usb soft reconnection test                      | 0      | 0      | SKIPPED | 0.0                |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-hid    | Configuration descriptor, generic               | 1      | 0      | OK      | 0.6                |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-hid    | Configuration descriptor, keyboard              | 1      | 0      | OK      | 0.55               |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-hid    | Configuration descriptor, mouse                 | 1      | 0      | OK      | 0.56               |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-hid    | HID class descriptors, generic                  | 1      | 0      | OK      | 0.56               |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-hid    | HID class descriptors, keyboard                 | 1      | 0      | OK      | 0.5                |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-hid    | HID class descriptors, mouse                    | 1      | 0      | OK      | 0.5                |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-hid    | Raw input/output, 1-byte reports                | 1      | 0      | OK      | 0.61               |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-hid    | Raw input/output, 20-byte reports               | 1      | 0      | OK      | 0.58               |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-hid    | Raw input/output, 64-byte reports               | 1      | 0      | OK      | 0.54               |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-msd    | mount/unmount and data test - Heap block device | 0      | 0      | SKIPPED | 0.0                |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-msd    | mount/unmount test - Heap block device          | 0      | 1      | FAIL    | 30.46              |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-msd    | storage initialization                          | 1      | 0      | OK      | 0.06               |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-serial | CDC RX multiple bytes                           | 1      | 0      | OK      | 5.13               |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-serial | CDC RX multiple bytes concurrent                | 1      | 0      | OK      | 6.96               |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-serial | CDC RX single bytes                             | 1      | 0      | OK      | 0.54               |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-serial | CDC RX single bytes concurrent                  | 1      | 0      | OK      | 0.59               |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-serial | CDC USB reconnect                               | 1      | 0      | OK      | 1.04               |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-serial | CDC loopback                                    | 1      | 0      | OK      | 1.32               |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-serial | Serial USB reconnect                            | 1      | 0      | OK      | 1.07               |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-serial | Serial getc                                     | 1      | 0      | OK      | 0.55               |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-serial | Serial line coding change                       | 1      | 0      | OK      | 1.08               |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-serial | Serial printf/scanf                             | 1      | 0      | OK      | 1.61               |
| NUCLEO_WB55RG-GCC_ARM | NUCLEO_WB55RG | tests-usb_device-serial | Serial terminal reopen                          | 1      | 0      | OK      | 0.92               |
mbedgt: test case results: 13 SKIPPED / 2 FAIL / 21 OK
mbedgt: completed in 117.87 sec

A couple of comments regarding the results:

tests-usb_device-msd -- this may be currently ignored because of a Python3 compatibility issue on my machine,
tests-usb_device-serial | Serial line coding change -- this test case fails on master because of recent printf updates; I've prepared a fix in Tests: USBSerial: Handle minimal printf limitations #12805.
I had to rebase on top of current master to successfully build the tests. Some APIs are currently being removed, tests updated.
tests-usb_device-basic -- this requires further investigation. Should not fail.

@jeromecoutant, could you re-check with mbed test -DUSB_DEVICE_TESTS -t GCC_ARM -m NUCLEO_WB55RG -n tests-usb_device-basic -vv, please?

fkjagodzinski · 2020-04-14T14:47:28Z

@maciejbocianski could you run mbed test -DUSB_DEVICE_TESTS -t GCC_ARM -m NUCLEO_WB55RG -n tests-usb_device-basic -vv on your NUCLEO_WB55RG, please?

I can see errors related to USB configuration setting on my setup. E.g. this one:

[1586875382.49][CONN][RXD] >>> Running case #1: 'usb control basic test'...
[1586875382.53][CONN][INF] found KV pair in stream: {{__testcase_start;usb control basic test}}, queued...
[1586875382.80][CONN][INF] found KV pair in stream: {{control_basic_test;0123456789 3368 517}}, queued...
<<< get_set_configuration_test >>>
device deconfigured - OK
configuration 1 set - OK
configuration 2 set - OK

<<< get_set_interface_test >>>
[1586875382.81][HTST][INF] TEST FAILED: [C:\Users\filjag01\workspace\mbed-os\TESTS\host_tests\pyusb_basic.py]:466, Dev-host transfer error ([Errno None] b'libusb0-dll:err [set_configuration] could not set config 1: win error: A device attached to the system is not functioning.\r\n').
[1586875382.82][SERI][TXD] {{fail;0}}
mbedgt: :129::FAIL: Expected 'pass' Was 'fail'
[1586875382.88][CONN][RXD] :129::FAIL: Expected 'pass' Was 'fail'
[1586875382.93][CONN][INF] found KV pair in stream: {{__testcase_finish;usb control basic test;0;1}}, queued...
[1586875383.01][CONN][RXD] >>> 'usb control basic test': 0 passed, 1 failed with reason 'Assertion Failed'

maciejbocianski · 2020-04-15T11:32:06Z

@fkjagodzinski
Similar problem on my side:

[1586949921.16][CONN][RXD] >>> Running case #1: 'usb control basic test'...
[1586949921.21][CONN][INF] found KV pair in stream: {{__testcase_start;usb control basic test}}, queued...
[1586949921.64][CONN][INF] found KV pair in stream: {{control_basic_test;0123456789 3368 517}}, queued...
<<< get_set_configuration_test >>>
device deconfigured - OK
[1586949921.68][HTST][INF] TEST FAILED: [C:\Users\macboc01.ARM\work\mbed\dev\mbed-os\TESTS\host_tests\pyusb_basic.pyc]:440, set_configuration request failed ([Errno None] libusb0-dll:err [set_configuration] could not set config 1: win error: A device attached to the system is not functioning.).
[1586949921.69][SERI][TXD] {{fail;0}}
mbedgt: :129::FAIL: Expected 'pass' Was 'fail'
[1586949921.76][CONN][RXD] :129::FAIL: Expected 'pass' Was 'fail'

0xc0170 · 2020-04-20T15:37:43Z

@jeromecoutant When you get a chance, please review above tests

mergify · 2020-04-22T09:00:37Z

This PR cannot be merged due to conflicts. Please rebase to resolve them.

jeromecoutant · 2020-06-08T11:49:44Z

@MarceloSalazar
I propose to merge this PR in order to share the USB feature with community.
And in parallel, to open an issue on the tests-usb_device-basic issue, as this issue is not really related to this PR, but common to all STM targets.

MarceloSalazar · 2020-06-08T16:01:27Z

Thanks Jerome, this makes sense to me.
@0xc0170 @fkjagodzinski are you ok to proceed?

adbridge · 2020-06-11T11:31:42Z

@donatieng could you review please? In the meantime I will start CI

mbed-ci · 2020-06-11T12:11:58Z

Test run: FAILED

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

Failed test jobs:

jenkins-ci/mbed-os-ci_cloud-client-pytest

jeromecoutant · 2020-06-15T07:15:58Z

Failed test jobs:
* jenkins-ci/mbed-os-ci_cloud-client-pytest

What's this ?

Thx

0xc0170 · 2020-06-17T18:13:44Z

CI restarted

@jeromecoutant should be cleared once CI completes

mbed-ci · 2020-06-17T18:55:48Z

Test run: SUCCESS

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

jeromecoutant · 2020-07-01T14:28:45Z

And in parallel, to open an issue on the tests-usb_device-basic issue, as this issue is not really related to this PR, but common to all STM targets.

See #13215

maciejbocianski · 2020-08-21T06:59:33Z

targets/TARGET_STM/USBPhy_STM32.cpp

+#define PMA_EP2_OUT_ADDR   ((PMA_EP2_OUT_BASE + MAX_PACKET_SIZE_NON_ISO) | (PMA_EP2_OUT_BASE << 16U))
+#define PMA_EP2_IN_ADDR     (PMA_EP2_OUT_BASE + MAX_PACKET_SIZE_NON_ISO * 2)
+#define PMA_EP2_CMD_ADDR    (PMA_EP2_IN_ADDR + MAX_PACKET_SIZE_NON_ISO)
+    HAL_PCDEx_PMAConfig(&hpcd, LOG_OUT_TO_EP(2), PCD_DBL_BUF, PMA_EP2_OUT_ADDR);  // HAL_PCDEx_PMAConfig always returns HAL_OK


Looks like we have a bug here causing endpoint wrong RAM buffer address
Shouldn't be PCD_SNG_BUF here ? @jeromecoutant
@donatieng

Did you meet some issue with STM32WB target?
or it is a question during review ?

I checked this commit: 4b29c4f
Tests are still OK, no regression.
Can you check it in your application ?

I was implementing two interface HID device and noticed that on EP2_OUT the driver read other RAM location than expected causing wrong data to be send to host. Changing to SNG_BUF fixes this problem.

ciarmcom requested a review from a team April 2, 2020 17:00

ciarmcom added the needs: review label Apr 2, 2020

0xc0170 requested a review from donatieng April 17, 2020 13:49

mergify bot added needs: work and removed needs: review labels Apr 22, 2020

jeromecoutant added 2 commits June 8, 2020 13:14

NUCLEO_F439ZI: align USB config from NUCLEO_F429ZI

26223e6

NUCLEO_WB55RG: enable USBDEVICE

551f8b4

jeromecoutant force-pushed the PR_WB_USB branch from 6c8c514 to 551f8b4 Compare June 8, 2020 11:46

adbridge added the release-type: patch Indentifies a PR as containing just a patch label Jun 11, 2020

adbridge approved these changes Jun 11, 2020

View reviewed changes

mergify bot added needs: CI and removed needs: work labels Jun 11, 2020

mergify bot added needs: work and removed needs: CI labels Jun 11, 2020

donatieng approved these changes Jun 17, 2020

View reviewed changes

0xc0170 added needs: CI and removed needs: work labels Jun 17, 2020

mergify bot added ready for merge and removed needs: CI labels Jun 17, 2020

0xc0170 merged commit aafae5d into ARMmbed:master Jun 18, 2020

mergify bot removed the ready for merge label Jun 18, 2020

jeromecoutant deleted the PR_WB_USB branch June 18, 2020 07:47

adbridge added release-version: 6.1.0 Release-pending and removed release-type: patch Indentifies a PR as containing just a patch Release-pending labels Jun 24, 2020

maciejbocianski reviewed Aug 21, 2020

View reviewed changes

jeromecoutant mentioned this pull request Aug 31, 2020

STM32 USB: use SNG_BUF #13514

Merged

STM32WB: enable USB Device #12751

STM32WB: enable USB Device #12751

Uh oh!

Conversation

jeromecoutant commented Apr 2, 2020

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

Uh oh!

ciarmcom commented Apr 2, 2020

Uh oh!

donatieng commented Apr 8, 2020

Uh oh!

fkjagodzinski commented Apr 14, 2020

Uh oh!

fkjagodzinski commented Apr 14, 2020

Uh oh!

maciejbocianski commented Apr 15, 2020

Uh oh!

0xc0170 commented Apr 20, 2020

Uh oh!

mergify bot commented Apr 22, 2020

Uh oh!

jeromecoutant commented Jun 8, 2020

Uh oh!

MarceloSalazar commented Jun 8, 2020

Uh oh!

adbridge commented Jun 11, 2020

Uh oh!

mbed-ci commented Jun 11, 2020

Test run: FAILED

Uh oh!

jeromecoutant commented Jun 15, 2020

Uh oh!

0xc0170 commented Jun 17, 2020

Uh oh!

mbed-ci commented Jun 17, 2020

Test run: SUCCESS

Uh oh!

jeromecoutant commented Jul 1, 2020

Uh oh!

maciejbocianski Aug 21, 2020

Choose a reason for hiding this comment

Uh oh!

jeromecoutant Aug 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maciejbocianski Aug 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jeromecoutant Aug 27, 2020 •

edited

Loading

maciejbocianski Aug 30, 2020 •

edited

Loading