drivers/usbdev_synopsys_dwc2: add GD32V support #19389

gschorcht · 2023-03-14T14:36:03Z

Contribution description

This PR provides the GD32V support in drivers/usbdev_synopsys_dwc2. It also includes some cleanup and bug fixes.

To support GD32V (CID 1.000) the PR includes the following changes:

XFRC interrupts are also used in Non-DMA mode to complete a transfer
SETUP phase done (STUP) interrupt is used in Non-DMA mode
additional XFRC interrupts in SETUP stage are ignored
RX FIFO handling doesn't complete a transfer anymore and is handled directly in ISR
OTG interrupt handling added

The following fixes, improvements and cleanup has been added:

USB OTG HS definition dependencies for platforms that don't support USB OTG HS
thread context switch at ISR exit was added for all platforms
units in USB FIFO size definitions are clarified
USB EP number as defined in CMSIS is used
FIFO allocation documentation is improved
power modes for STM32 are fixed
CID/HW debug info added

The PR could be split into a PRs with cleanups and improvements and a PR with GD32V support if necessary.

Testing procedure

BOARD=sipeed-longan-nano make -j8 -C tests/shell

should work with stdio_cdc_acm.

periph_usbdev should still work for other platforms. The PR was tested with tests/usbus_cdc_ecm together with stdio_cdc_acm if possible:

esp32s2      FS  CID: 0000, HWREV: 4f54400a, HWCFG: 224dd930  (esp32s2-devkit)
esp32s3      FS  CID: 0000, HWREV: 4f54400a, HWCFG: 224dd930  (esp32s3-devkit)
efm32gg12    FS  CID: 0000, HWREV: 4f54330a, HWCFG: 228f5910  (sltb009a)
gd32vf103cb  FS  CID: 1000, HWREV: 00000000, HWCFG: 00000000  (sipeed-longan-nano)
stm32f407vg  FS  CID: 1200, HWREV: 4f54281a, HWCFG: 229dcd20  (stm32f4discovery) 
stm32f429zi  HS  CID: 1100, HWREV: 4f54281a, HWCFG: 229ed590  (stm32f429i-disc1)
stm32f439zi  FS  CID: 1200, HWREV: 4f54281a, HWCFG: 229dcd20  (nucleo-f439ze)
stm32f446ze  FS  CID: 2000, HWREV: 4f54320a, HWCFG: 229ed520  (nucleo-f446ze)
stm32f723ie  FS  CID: 3000, HWREV: 4f54330a, HWCFG: 229ed520  (stm32f723e-disco)
stm32f723ie  HS  CID: 3100, HWREV: 4f54330a, HWCFG: 229fe1d0  (stm32f723e-disco)
stm32f746ng  FS  CID: 2000, HWREV: 4f54320a, HWCFG: 229ed520  (stm32f746g-disco)
stm32f746ng  HS  CID: 2100, HWREV: 4f54320a, HWCFG: 229fe190  (stm32f746g-disco)
stm32f767zi  FS  CID: 2000, HWREV: 4f54320a, HWCFG: 229ed520  (nucleo-f767zi)

Boards with HS interfaces were tested in DMA and none-DMA mode.

Issues/PRs references

riot-ci · 2023-03-15T10:58:52Z

Murdock results

✔️ PASSED

1e983f3 boards/common/gd32v: enable usb_board_reset if USB CDC ACM is used

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

Artifacts

Documentation preview

benpicco

Works like a charm and I see you already have tested this on a wide range of implementations.

Please squash.

drivers/usbdev_synopsys_dwc2/usbdev_synopsys_dwc2.c

gschorcht · 2023-03-16T07:52:51Z

Works like a charm and I see you already have tested this on a wide range of implementations.

I would like to have look from @bergzand onto the driver since its interrupt handling has been changed completely (commits bcc8c2b, 38620fd, ad191b1)

bergzand · 2023-03-16T09:54:04Z

I would like to have look from @bergzand onto the driver since its interrupt handling has been changed completely (commits bcc8c2b, 38620fd, ad191b1)

I see one potential race condition here, but not sure if it is the case. For the non-DMA case both the rxfifolvl and the transfer complete interrupts are enabled. I previously had a similar issue with the DMA case where both the transfer complete and the rxfifolvl interrupts were enabled (see #18726). On fast microcontrollers with HS phy this could trigger unstable behaviour quickly when using cdc ecm and pinging the device with large icmp packets. I wonder if this issue returns here but for the non-dma case, with a HS phy.

I'm also curious why a different interrupt is used during the address state.

gschorcht · 2023-03-16T10:36:56Z

I see one potential race condition here, but not sure if it is the case. For the non-DMA case both the rxfifolvl and the transfer complete interrupts are enabled.

The causal order should be that RXFLVL comes before XFRC. There shouldn't be a race condition because only one interrupt is handled per ISR call and RXFLVL is always handled before XFRC even if RXFLVL and XFRC are triggered at the same time. In addition, RXFLVL is handled synchronously directly in the ISR while XFRC is handled asynchronously using thread events. Also, RXFLVL is only used to copy the FIFO contents and to set a flag indicating that a SETUP transfer has been started. The latter is seems to be needed for certain core versions such as the GD32V core version 1.000.

I'm also curious why a different interrupt is used during the address state.

Because TXFE interrupt doesn't seem to work in default state in core version 1.000 😟

gschorcht · 2023-03-16T10:45:40Z

drivers/usbdev_synopsys_dwc2/usbdev_synopsys_dwc2.c

-     * while (un)blocking works on the stm32f401, needs more
-     * investigation with a larger set of chips */
-#if defined(STM32_USB_OTG_CID_1x)
+#if defined(MCU_STM32)


@bergzand I am not sure if this change could cause it to stop working on the stm32f446. I have tested it with a number of non CID 1.x core versions but could not find a problem.

gschorcht · 2023-03-20T12:01:20Z

I had to rebase to resolve the conflicts.

XFRC (Transfer Complete) interrupts are now also used for IN EPs in non-DMA mode. TXFE (TX FIFO Empty) interrupts are not necessarily needed but are still enabled.

gschorcht · 2023-04-16T21:04:58Z

Please rebase now that #19471 is merged

Thanks for merging PR #19471. I rebased the PR and squashed the small change with static assert right away.

XFRC (Transfer Complete) interrupts are now also used for OUT EPs in non-DMA mode. RXFLVL (RX FIFO Level) interrupts are no longer used to signal completed transfers, but only to copy data from FIFO to memory and to set a flag indicating that a SETUP stage is in progress. STUP (SETUP phase done) interrupts are then used to signal a completed SETUP stage and to reset the flags that indicates the SETUP stage. The flag that indicates the SETUP stage in progress is used to ignore additional XFRC interrupts for EP0 during the SETUP stage.

Since RXFLVL (RX FIFO Level) interrupt doen't complete transfers anymore, they are handled now directly in the ISR.

After changing IN EPs also to use XFRC (Transfer Complete) interrupts in non-DMA mode, the TXFE (TX FIFO Empty) interrupt is no longer needed to signal the completion of an IN transfer.

bors · 2023-04-16T23:36:18Z

try

Build succeeded:

bergzand

Ack, thanks for the many improvements!

bors merge

bors · 2023-04-17T10:35:58Z

Build succeeded:

gschorcht · 2023-04-17T13:12:10Z

Thanks for reviewing, testing und merging.

gschorcht requested review from leandrolanzieri, jia200x, MrKevinWeiss, aabadie, fjmolinas, DipSwitch, vincent-d and basilfx as code owners March 14, 2023 14:36

gschorcht requested a review from bergzand March 14, 2023 17:45

benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 15, 2023

gschorcht force-pushed the cpu/gd32v/periph_usbdev branch from 30ee6cb to cdc62d0 Compare March 15, 2023 17:22

gschorcht requested review from smlng and MichelRottleuthner as code owners March 15, 2023 17:24

github-actions bot added the Area: tests Area: tests and testing framework label Mar 15, 2023

benpicco approved these changes Mar 15, 2023

View reviewed changes

drivers/usbdev_synopsys_dwc2/usbdev_synopsys_dwc2.c Outdated Show resolved Hide resolved

drivers/usbdev_synopsys_dwc2/usbdev_synopsys_dwc2.c Outdated Show resolved Hide resolved

gschorcht added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Type: new feature The issue requests / The PR implemements a new feature for RIOT labels Mar 16, 2023

gschorcht commented Mar 16, 2023

View reviewed changes

gschorcht force-pushed the cpu/gd32v/periph_usbdev branch from 925d38c to 670951a Compare March 20, 2023 12:01

gschorcht added 6 commits April 16, 2023 22:59

drivers/usbdev_synopsys_dwc2: fix a typo

d35fa91

drivers/usbdev_synopsys_dwc2: fix power modes for STM32

ff9e9e3

drivers/usbdev_synopsys_dwc2: print CID/HW debug info

1d72246

drivers/usbdev_synopsys_dwc2: add OTG interrupt handling

e3fb4e7

drivers/usbdev_synopsys_dwc2: several cleanups

1a37d3b

drivers/usbdev_synopsys_dwc2: use XFRC for IN EPs in non-DMA mode

1ccafe1

XFRC (Transfer Complete) interrupts are now also used for IN EPs in non-DMA mode. TXFE (TX FIFO Empty) interrupts are not necessarily needed but are still enabled.

gschorcht force-pushed the cpu/gd32v/periph_usbdev branch from 1f671ac to ce97aa6 Compare April 16, 2023 21:00

github-actions bot removed Area: USB Area: Universal Serial Bus Area: sys Area: System labels Apr 16, 2023

gschorcht force-pushed the cpu/gd32v/periph_usbdev branch from ce97aa6 to 7c807d1 Compare April 16, 2023 21:06

gschorcht added 10 commits April 16, 2023 23:24

drivers/usbdev_synopsys_dwc2: handle RXFLVL interruprs in ISR

ea776c9

Since RXFLVL (RX FIFO Level) interrupt doen't complete transfers anymore, they are handled now directly in the ISR.

drivers/periph_usbdev: TXFE not used any longer

ee83893

After changing IN EPs also to use XFRC (Transfer Complete) interrupts in non-DMA mode, the TXFE (TX FIFO Empty) interrupt is no longer needed to signal the completion of an IN transfer.

drivers/usbdev_synopsys_dwc2: add GD32V support

26d435a

cpu/gd32v: add periph_usbdev support

81cf8a5

boards/sipeed-longan-nano: add periph_usbdev support

ac8f221

boards/seeedstudio-gd32: add periph_usbdev support

6d7c6c1

boards/seeedstudio-gd32: use stdio_cdc_acm by default

ff6e3d4

boards/spieed-longan-nano: use stdio_cdc_acm by default

c92b981

boards/common/gd32v: enable usb_board_reset if USB CDC ACM is used

1e983f3

gschorcht force-pushed the cpu/gd32v/periph_usbdev branch from 7c807d1 to 1e983f3 Compare April 16, 2023 21:27

bors bot added a commit that referenced this pull request Apr 16, 2023

Try #19389:

07c5022

bergzand approved these changes Apr 17, 2023

View reviewed changes

bors bot merged commit 82234b1 into RIOT-OS:master Apr 17, 2023

gschorcht deleted the cpu/gd32v/periph_usbdev branch April 26, 2023 09:39

benpicco added this to the Release 2023.07 milestone Aug 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

drivers/usbdev_synopsys_dwc2: add GD32V support #19389

drivers/usbdev_synopsys_dwc2: add GD32V support #19389

gschorcht commented Mar 14, 2023 •

edited

Loading

riot-ci commented Mar 15, 2023 •

edited

Loading

benpicco left a comment

gschorcht commented Mar 16, 2023

bergzand commented Mar 16, 2023

gschorcht commented Mar 16, 2023 •

edited

Loading

gschorcht Mar 16, 2023 •

edited

Loading

gschorcht commented Mar 20, 2023

gschorcht commented Apr 16, 2023

bors bot commented Apr 16, 2023

bergzand left a comment

bors bot commented Apr 17, 2023

gschorcht commented Apr 17, 2023

drivers/usbdev_synopsys_dwc2: add GD32V support #19389

drivers/usbdev_synopsys_dwc2: add GD32V support #19389

Conversation

gschorcht commented Mar 14, 2023 • edited Loading

Contribution description

Testing procedure

Issues/PRs references

riot-ci commented Mar 15, 2023 • edited Loading

Murdock results

Artifacts

benpicco left a comment

Choose a reason for hiding this comment

gschorcht commented Mar 16, 2023

bergzand commented Mar 16, 2023

gschorcht commented Mar 16, 2023 • edited Loading

gschorcht Mar 16, 2023 • edited Loading

Choose a reason for hiding this comment

gschorcht commented Mar 20, 2023

gschorcht commented Apr 16, 2023

bors bot commented Apr 16, 2023

try

bergzand left a comment

Choose a reason for hiding this comment

bors bot commented Apr 17, 2023

gschorcht commented Apr 17, 2023

gschorcht commented Mar 14, 2023 •

edited

Loading

riot-ci commented Mar 15, 2023 •

edited

Loading

gschorcht commented Mar 16, 2023 •

edited

Loading

gschorcht Mar 16, 2023 •

edited

Loading