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

usbdev_synopsys_dwc2: Mask RX FIFO irq when using DMA #18726

Merged
merged 1 commit into from
Oct 12, 2022

Conversation

bergzand
Copy link
Member

Contribution description

When using DMA to transfer endpoint data from the RX FIFO to the endpoint memory, the RXFLVL irq is not needed as that is already handled by the DMA. Furthermore, servicing this irq anyway can cause the event handling to interpret data from the FIFO as the endpoint and status marker during the DMA transfer.

This commit masks the RXFLVL irq while DMA is used for the endpoint transfers.

On my side current master quickly triggers faults because the somewhere halfway through the DMA transaction, the RXFLVL irq is serviced and the logic reads out data from the RX FIFO and assumes it is the first value containing the endpoint number (which has already been removed by the DMA).

Testing procedure

On the nucleo-f746zg everything still works (not using DMA):

[ 3078.031400] usb 7-3.2.2: new full-speed USB device number 11 using xhci_hcd
[ 3078.132010] usb 7-3.2.2: New USB device found, idVendor=1209, idProduct=7d00, bcdDevice= 1.00
[ 3078.132016] usb 7-3.2.2: New USB device strings: Mfr=3, Product=2, SerialNumber=4
[ 3078.132018] usb 7-3.2.2: Product: nucleo-f746zg
[ 3078.132020] usb 7-3.2.2: Manufacturer: RIOT-os.org
[ 3078.132021] usb 7-3.2.2: SerialNumber: 5A8017AD60B13119
[ 3078.190130] cdc_ether 7-3.2.2:1.0 usb0: register 'cdc_ether' at usb-0000:3f:00.3-3.2.2, CDC Ethernet Device, a6:8b:69:85:43:38
[ 3078.190154] usbcore: registered new interface driver cdc_ether
[ 3078.191913] cdc_ether 7-3.2.2:1.0 enp63s0f3u3u2u2: renamed from usb0

And with DMA on the stm32f446re with an ulpi HS phy:

Before

2022-10-12 10:35:25,350 # Test application for the USBUS CDC ECM interface
2022-10-12 10:35:25,350 # 
2022-10-12 10:35:25,350 # This test pulls in parts of the GNRC network stack, use the
2022-10-12 10:35:25,350 # provided shell commands (i.e. ifconfig, ping) to interact with
2022-10-12 10:35:25,350 # the CDC ECM based network interface.
2022-10-12 10:35:25,350 # 
2022-10-12 10:35:25,350 # Starting the shell now...
> ifconfig
2022-10-12 10:35:29,450 # ifconfig
2022-10-12 10:35:29,451 # Iface  4  HWaddr: E6:B2:D4:2B:BF:C5 
2022-10-12 10:35:29,451 #           L2-PDU:1500  MTU:1500  HL:64  RTR  
2022-10-12 10:35:29,451 #           RTR_ADV  
2022-10-12 10:35:29,451 #           Source address length: 6
2022-10-12 10:35:29,451 #           Link type: wired
2022-10-12 10:35:29,452 #           inet6 addr: fe80::e4b2:d4ff:fe2b:bfc5  scope: link  VAL
2022-10-12 10:35:29,452 #           inet6 group: ff02::2
2022-10-12 10:35:29,452 #           inet6 group: ff02::1
2022-10-12 10:35:29,452 #           inet6 group: ff02::1:ff2b:bfc5
2022-10-12 10:35:29,452 #           
2022-10-12 10:35:33,007 # 800c469
2022-10-12 10:35:33,007 # *** RIOT kernel panic:
2022-10-12 10:35:33,008 # FAILED ASSERTION.
2022-10-12 10:35:33,008 # 
2022-10-12 10:35:33,008 #       pid | name                 | state    Q | pri | stack  ( used) ( free) | base addr  | current     
2022-10-12 10:35:33,009 #         - | isr_stack            | -        - |   - |   1024 (  616) (  408) | 0x20000000 | 0x200002f8
2022-10-12 10:35:33,009 #         1 | main                 | bl mutex _ |   7 |   1536 (  864) (  672) | 0x20009038 | 0x20009454 
2022-10-12 10:35:33,010 #         2 | ipv6                 | bl rx    _ |   4 |   1024 (  428) (  596) | 0x2000963c | 0x2000993c 

The address resolves to an assert failure in atomic_utils.h, caused by an invalid endpoint number passed to usbus

with 8cecb81

[Wed Oct 12 10:26:20 2022] usb 7-3.1: new high-speed USB device number 14 using xhci_hcd
[Wed Oct 12 10:26:20 2022] usb 7-3.1: New USB device found, idVendor=1209, idProduct=7d00, bcdDevice= 1.00
[Wed Oct 12 10:26:20 2022] usb 7-3.1: New USB device strings: Mfr=3, Product=2, SerialNumber=4
[Wed Oct 12 10:26:20 2022] usb 7-3.1: Product: custom
[Wed Oct 12 10:26:20 2022] usb 7-3.1: Manufacturer: RIOT-os.org
[Wed Oct 12 10:26:20 2022] usb 7-3.1: SerialNumber: BDBE88053642A6C3
[Wed Oct 12 10:26:20 2022] cdc_ether 7-3.1:1.0 usb0: register 'cdc_ether' at usb-0000:3f:00.3-3.1, CDC Ethernet Device, e6:b2:d4:2b:a3:c4
[Wed Oct 12 10:26:20 2022] cdc_ether 7-3.1:1.0 enp63s0f3u3u1: renamed from usb0

And on the RIOT shell:

2022-10-12 10:43:38,984 # Test application for the USBUS CDC ECM interface
2022-10-12 10:43:38,984 # 
2022-10-12 10:43:38,984 # This test pulls in parts of the GNRC network stack, use the
2022-10-12 10:43:38,984 # provided shell commands (i.e. ifconfig, ping) to interact with
2022-10-12 10:43:38,985 # the CDC ECM based network interface.
2022-10-12 10:43:38,985 # 
2022-10-12 10:43:38,985 # Starting the shell now...
ifconfig 4 add 2001:db8::2/64
2022-10-12 10:44:06,492 # ifconfig 4 add 2001:db8::2/64
2022-10-12 10:44:06,492 # success: added 2001:db8::2/64 to interface 4
ping 2001:db8::1 -s 1438
2022-10-12 10:44:13,792 # ping 2001:db8::1 -s 1438
2022-10-12 10:44:13,793 # 1446 bytes from 2001:db8::1: icmp_seq=0 ttl=64 time=0.864 ms
2022-10-12 10:44:14,784 # 1446 bytes from 2001:db8::1: icmp_seq=1 ttl=64 time=0.597 ms
2022-10-12 10:44:15,792 # 1446 bytes from 2001:db8::1: icmp_seq=2 ttl=64 time=0.616 ms
2022-10-12 10:44:15,793 # 
2022-10-12 10:44:15,793 # --- 2001:db8::1 PING statistics ---
2022-10-12 10:44:15,793 # 3 packets transmitted, 3 packets received, 0% packet loss
2022-10-12 10:44:15,793 # round-trip min/avg/max = 0.597/0.692/0.864 ms

Issues/PRs references

None

When using DMA to transfer endpoint data from the RX FIFO to the
endpoint memory, the RXFLVL irq is not needed as that is already handled
by the DMA. Furthermore, servicing this irq anyway can cause the event
handling to interpret data from the FIFO as the endpoint and status
marker during the DMA transfer.

This commit masks the RXFLVL irq while DMA is used for the endpoint
transfers.
@github-actions github-actions bot added the Area: drivers Area: Device drivers label Oct 12, 2022
@bergzand bergzand added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Oct 12, 2022
@riot-ci
Copy link

riot-ci commented Oct 12, 2022

Murdock results

✔️ PASSED

8cecb81 usbdev_synopsys_dwc2: Mask RX FIFO irq with DMA

Success Failures Total Runtime
1980 0 1980 06m:35s

Artifacts

This only reflects a subset of all builds from https://ci-prod.riot-os.org. Please refer to https://ci.riot-os.org for a complete build for now.

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.

@gschorcht gschorcht merged commit 0a96492 into RIOT-OS:master Oct 12, 2022
@dylad dylad added this to the Release 2022.10 milestone Oct 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants