Invoke power state changes on DCD_EVENT_BUS_RESET

[Rocky04]

Should fix a bug that the device state isn't reset when a bus reset happened as discussed here: #2169

[hathach]

can you provide context of your setup and how to reproduce & test the issue.

[Rocky04]

I use a USB setup with multiple USB hubs. I have a laptop where a screen is connected via USB-C to my laptop and that has a built-in hub. A USB switch is connected to that, to that there is an active USB hub connected because the switch don't have enough ports and and can't provide enough power.

With that setup I noticed that my Wooting (which uses TinyUSB) sometimes remains lit after I unplug the screen from the laptop. So I checked out the TinyUSB example and was able to reproduce the issue on a GD32 dev kit which uses the STM32 code. Even when the single-ended zero state is constantly hold the device still blinks in the mounted state. As a workaround and test I scheduling a DCD_EVENT_UNPLUGGED in addition to the DCD_EVENT_BUS_RESET in the interrupt handler for the bus reset.

This is a general issue. As mentioned in the discussion, a device should reset the internal USB state when a reset on the bus occur. I understood a bus reset as compete reset where even the device speed is reset. But this must include the device state.

[hathach]

thank you for the PR, What is your GD32 mcu specifically ? even though it is generic, we will still want to reproduce the issue and verify the fix. Is your monitor has built-in battery. If yes, it looks to me that the reproducing steps would be

• Attached to self-power hub -> PC
• remove hub from PC, but still keeping power

right ?

[Rocky04]

Not sure how much you want spend but this is my setup:

Asus Flow 13 (2021) <- (PD sink) USB-C (PD source) -> Cooler Master Tempest GP27U <- USB-A 2.0 -> ATEN US424 (passive) <- USB-A 3.0 -> TP-Link UH720 (active) <- USB-A -> GD32303C-START

It likely mainly comes to the TP-Link UH720. It shows a connection LED which is off if there is no data transfer over the port and it provides power because it's active. I let it on all the time. Besides a USB sound card other devices went dark as soon as the laptop is disconnected.

Edit:
So I noticed the problem recently because I changed the setup (the screen is new). I'm also aware that the Wootings had rare issues where the enumeration failed due to a port reset problem. I think this is maybe related to this.

The MCU is a GD32F303CGT6. I found the issue because I'm currently reviewing the code here: https://github.com/WootingKb/tinyusb/tree/feature/gd32-support

[hathach]

thank you for your detail setup explanation, I will try to simulate it with self-power hub of some sort.

[Rocky04]

• Looks like there must be a note that the fall through is wanted. Not sure if [[fallthrough]], /* FALLTHROUGH */ or may __attribute__((__fallthrough__)) does the trick.

• Sadly the USB3CV compliance test program don't seems to test for the reset handling.

• I cross checked the signal state (SE0) via a USB power tester (AVHzY CT3+).

• The problem to reproduce the issue is likely to trigger a reset without an enumeration attempt afterwards. It should be possible to indirectly test this by checking if the _usbd_dev.connected parameter is set to 0 after the second device reset. Because that should come after the first device descriptor request and so this value would be 1 right before.

[Rocky04]

The issue is triggered on my system by USB switch which is in-between the host and the active USB hub.
The hub shuts all its ports down if its up facing port isn't active but the switch provides the idling J state to its down facing ports even if the selected host isn't present. Every time the host is toggled on the switch the DFPs are reset and then the J state of the plugged in device is present again.

To be clear, this setup only revealed the reset issue and shows that TinyUSB don't seems to prober react to the reset. Because the host should only reset the device if there was an issue. The reset event is kind of rare but can may lead into weird problems.

Here is how it looks like when the switch is toggled through its four outputs:

The short spikes on D+ are the resets.

[hathach]

• Looks like there must be a note that the fall through is wanted. Not sure if [[fallthrough]], /* FALLTHROUGH */ or may __attribute__((__fallthrough__)) does the trick.

you should use TU_ATTR_FALLTHROUGH; like https://github.com/hathach/tinyusb/blob/master/src/host/usbh.c#L664

• I cross checked the signal state (SE0) via a USB power tester (AVHzY CT3+).
• The problem to reproduce the issue is likely to trigger a reset without an enumeration attempt afterwards. It should be possible to indirectly test this by checking if the _usbd_dev.connected parameter is set to 0 after the second device reset. Because that should come after the first device descriptor request and so this value would be 1 right before.

Thank you for detailed breakout, I will try to reproduce it whenever I could

[Rocky04]

@hathach
As a warming, Wooting sent me a dev build with the changes of this PR for testing. Because before I was only able to test it on a dev board with the TinyUSB example. It seems that now the Wooting get stuck in the sleep mode every time I disconnect my host. This behavior maybe depends on the MCU specific implementation. For the SMT32 implementation this should be addressed by the optimizations in the PR #2178. So it can be that fixing the bug from this PR causes the same issue for other MCUs as well which likely don't have the Start of Frame fallback to ensure to leave the sleep mode. But it can also be that this is just related how the Wootings handle the device power states. I can't tell but it looks to me that it's maybe related to a missed resume...

Edit 1:
But on the other side I wasn't able to reproduce this issue with the dev board (when only the changes of this PR are applied, so without the mentioned optimizations), which indicate that this is maybe just a Wooting specific issue.

Edit 2:
The Wootings no longer inform the host that they support the remote wakeup function in the configuration descriptor. I didn't checked that for a while because I never imagined that this was maybe turned off. I assume the wakeup interrupt isn't triggered for the Wooting for some reason. But for the dev board that is still the case even if the remote wakeup support is also not announced in the device descriptor because it should react to bus activity.
False flag, I'm checking the descriptor inside a VM and due to this it got suppressed. Meaning the Wooting indeed offer this feature in the descriptor.

[Rocky04]

The problem with the Wooting is addressed now, it was related to a bug and missing improvements for the MCU implementation form PR #2178.

So together with the the PR #2202 these changes should help making the USB handling overall more reliable which likely addresses issues for some edge cases.

[Rocky04]

Had some time again recently to check this issue out...

The problem is that when a TinyUSB device is used on an active USB hub it don't react properly when the host is either shutdown or disconnected from the hub. In that case the hub will may provide a single-ended zero signal on both data wires resulting in a continuous reset state for the device while the device is still being powered.

If the device was in the suspended state before the reset tud_resume_cb wasn't invoked resulting in the device may remaining in sleep mode, while tud_umount_cb was never invoked in that case leading that the device still thought it's enumerated while it should be unmounted instead.

I also removed redundant code which clears the current device state.

[Rocky04]

Fixed some bugs...

• The USB speed is now kept on a USB reset.
• Repeating bus state events are now suppressed preventing that a spamming MCU can fills up the queue quickly.
• Adding a fix for the DWC2 driver that the start of a reset is suppressed.

The later is related when such a device is connected to an active USB hub while that gets disconnected from the host. In that case the hub may continuously send a USB reset signal. Because it can take a long time until the reset state is released the device would otherwise not be aware that it's no longer enumerated.

[HiFiPhile]

This will also ignore DCD_EVENT_SOF

[Rocky04]

Despite that this can't be merged easily anymore it's also outdated and should contain a bug.

Maybe next week I can provide the newer version which should work on the current branch.