dcd/dwc2: fix suspend interrupt indefinitely disabled on AT32F405

[peppapighs]

Describe the PR
Fix #3315 by only process SUSPEND event if the device is connected. Tested locally to confirm that _usb_dev state is as intended when the bus is suspended and resume. We probably don't need to repeat this change for RESUME event since enabling suspend interrupt during initialization should have no side effect.

Closes #3315

[HiFiPhile]

In TinyUSB's structure, DCD layer shouldn't call exported device stack functions (tud_*)

I found the cause of the issue. On initialization, AT32F405 receives a suspend interrupt once (usbsusp = usbsuspm = 1) before the device is connected. As a result, dcd_int_handler disables the suspend interrupt, even though the device is still unconnected.

When plugging/unplugging device, the D+/D- state are unstable and can accidentally meet the SUSPEND condition. But this shouldn't be an issue since the interrupt is enabled after the usb reset:

tinyusb/src/portable/synopsys/dwc2/dcd_dwc2.c

Line 1057 in ac37176

dwc2->gintmsk |= GINTMSK_USBSUSPM;

Unless GINTSTS_USBSUSP is already pending and should be cleared before enabling.

[peppapighs]

In TinyUSB's structure, DCD layer shouldn't call exported device stack functions (tud_*)

I found the cause of the issue. On initialization, AT32F405 receives a suspend interrupt once (usbsusp = usbsuspm = 1) before the device is connected. As a result, dcd_int_handler disables the suspend interrupt, even though the device is still unconnected.

When plugging/unplugging device, the D+/D- state are unstable and can accidentally meet the SUSPEND condition. But this shouldn't be an issue since the interrupt is enabled after the usb reset:

tinyusb/src/portable/synopsys/dwc2/dcd_dwc2.c

Line 1057 in ac37176

dwc2->gintmsk |= GINTMSK_USBSUSPM;

Unless GINTSTS_USBSUSP is already pending and should be cleared before enabling.

You are right. I put a breakpoint at USB reset event, and usbsusp = 1, so if I understand this correctly, there is a pending suspend interrupt before USB reset. I'll make the change to clear it first.

[HiFiPhile]

I think you can simply write like this, otherwise pending Enum Done interrupt will trigger the IRQ again:

if (gintsts & GINTSTS_ENUMDNE) {
    // ENUMDNE is the end of reset where speed of the link is detected
    dwc2->gintsts = GINTSTS_ENUMDNE;
    // There may be a pending suspend event, so we clear it first
    dwc2->gintsts = GINTSTS_USBSUSP;
    dwc2->gintmsk |= GINTMSK_USBSUSPM;
    handle_enum_done(rhport);
  }

[peppapighs]

I think you can simply write like this, otherwise pending Enum Done interrupt will trigger the IRQ again:

if (gintsts & GINTSTS_ENUMDNE) {
    // ENUMDNE is the end of reset where speed of the link is detected
    dwc2->gintsts = GINTSTS_ENUMDNE;
    // There may be a pending suspend event, so we clear it first
    dwc2->gintsts = GINTSTS_USBSUSP;
    dwc2->gintmsk |= GINTMSK_USBSUSPM;
    handle_enum_done(rhport);
  }

Right, I wasn't aware you can clear multiple interrupts at once. I've made the change.

[HiFiPhile]

Thank you !