STM32 Synopsis doesn't seems to detect disconnection event

[hathach]

Describe the bug A clear and concise description of what the bug is.

Set up (please complete the following information):

• OS: Ubuntu 18.04
• Board: stm32f407disco, stm32f411disco, stm32f767nucleo
• Firmware Code: examples/device/cdc_msc

To Reproduce Steps to reproduce the behavior:

1. Run example without USB: led blink fast indicate no connection
2. Plug USB cable, device enumerate, also led blink slow
3. Unplug USB cable (keep stlink for power), led still blink slow --> no disconnect event detected

Expected behavior LED should blink slow in the above example indicating there is no connections. maybe related to #179

[hathach]

on 407disco and f767nucleo host couldn't communicate with stm32 after reconnection.

[hathach]

on stm32f411disco, control endpoint works after reconnection, but non-control like msc endpoint seems to have troubles :worried:

[cr1901]

Yes, the Synopsys driver doesn't properly detect disconnection events because I never added the code for that :P.

I'm a bit overwhelmed at the moment, but this is on my TODO list...

In fact, I have a half-complete implementation of disconnect stashed away somewhere...

[PanRe]

I added this feature for an STM32F411 by changing these two functions:

void dcd_init (uint8_t rhport)
{
    (void) rhport;

    // Added to TinyUSB
    // Change configuration of PA10 (ID Pin) since this seems to be wrong done by CubeMX
    GPIO_InitTypeDef GPIO_InitStruct = {0};
    GPIO_InitStruct.Pin = GPIO_PIN_10;
    GPIO_InitStruct.Mode = GPIO_MODE_AF_OD;
    GPIO_InitStruct.Pull = GPIO_PULLUP;
    GPIO_InitStruct.Speed = GPIO_SPEED_FREQ_VERY_HIGH;
    GPIO_InitStruct.Alternate = GPIO_AF10_OTG_FS;
    HAL_GPIO_Init(GPIOA, &GPIO_InitStruct);

    // Enable clocks since this is not covered by tinyusb
    // enabling USB_OTG in RCC
    RCC->AHB2ENR |= (RCC_AHB2ENR_OTGFSEN);

    // Wait until AHB is ready
    while(((USB_OTG_FS->GRSTCTL) & (USB_OTG_GRSTCTL_AHBIDL)) == 0);

    // Clear TRDT register since tinyusb does not set this value correctly, they just or 0x6 into that register without clearing it before and thus 0x7 is the result...
    USB_OTG_FS->GUSBCFG &= ~(0x0F << USB_OTG_GUSBCFG_TRDT_Pos);

    // Enable VBUS sensing - this might not work for some eval boards as corresponding pin needs to be connected to usb plug
    USB_OTG_FS->GCCFG &= ~USB_OTG_GCCFG_NOVBUSSENS;
    USB_OTG_FS->GCCFG |= USB_OTG_GCCFG_VBUSBSEN;

    // Enable physical clock
    volatile uint32_t * const OTGPCTL  = (void*)(USB_OTG_FS_PERIPH_BASE + USB_OTG_PCGCCTL_BASE);
    *OTGPCTL = 0;

    // Soft disconnect
    USB_OTG_DeviceTypeDef * dev = (USB_OTG_DeviceTypeDef *) (USB_OTG_FS_PERIPH_BASE + USB_OTG_DEVICE_BASE);
    dev->DCTL |= USB_OTG_DCTL_SDIS;

    // Programming model begins in the last section of the chapter on the USB
    // peripheral in each Reference Manual.
    USB_OTG_FS->GAHBCFG |= USB_OTG_GAHBCFG_TXFELVL | USB_OTG_GAHBCFG_GINT;

    // No HNP/SRP (no OTG support), program timeout later, turnaround
    // programmed for 32+ MHz.
    // TODO: PHYSEL is read-only on some cores (STM32F407). Worth gating?
    USB_OTG_FS->GUSBCFG |= (0x06 << USB_OTG_GUSBCFG_TRDT_Pos) | USB_OTG_GUSBCFG_PHYSEL;

    // Required as part of core initialization. Disable OTGINT as we don't use
    // it right now OLD. TODO: How should mode mismatch be handled? It will cause
    // the core to stop working/require reset.
    USB_OTG_FS->GINTMSK |=  USB_OTG_GINTMSK_OTGINT |  USB_OTG_GINTMSK_MMISM;

    USB_OTG_DeviceTypeDef * dev = DEVICE_BASE;

    // If USB host misbehaves during status portion of control xfer
    // (non zero-length packet), send STALL back and discard. Full speed.
    dev->DCFG |=  USB_OTG_DCFG_NZLSOHSK | (3 << USB_OTG_DCFG_DSPD_Pos);

    USB_OTG_FS->GINTMSK |= USB_OTG_GINTMSK_USBRST | USB_OTG_GINTMSK_ENUMDNEM | \
    USB_OTG_GINTMSK_SOFM | USB_OTG_GINTMSK_RXFLVLM /* SB_OTG_GINTMSK_ESUSPM */ | \
    USB_OTG_GINTMSK_USBSUSPM | USB_OTG_GINTMSK_WUIM;

    // Clear all interrupts
    USB_OTG_FS->GINTSTS = 0xFFFFFFFF;

    // Enable USB transceiver.
    USB_OTG_FS->GCCFG |= USB_OTG_GCCFG_PWRDWN;

    // Soft Connect -> Enable pullup on D+/D-.
    // This step does not appear to be specified in the programmer's model.
    dev->DCTL &= ~USB_OTG_DCTL_SDIS;
}

and

void OTG_FS_IRQHandler(void) {
  USB_OTG_DeviceTypeDef * dev = DEVICE_BASE;
  USB_OTG_OUTEndpointTypeDef * out_ep = OUT_EP_BASE;
  USB_OTG_INEndpointTypeDef * in_ep = IN_EP_BASE;

  uint32_t int_status = USB_OTG_FS->GINTSTS;

  if(int_status & USB_OTG_GINTSTS_USBRST) {
    // USBRST is start of reset.
    USB_OTG_FS->GINTSTS = USB_OTG_GINTSTS_USBRST;
    bus_reset();
  }

  if(int_status & USB_OTG_GINTSTS_ENUMDNE) {
    // ENUMDNE detects speed of the link. For full-speed, we
    // always expect the same value. This interrupt is considered
    // the end of reset.
    USB_OTG_FS->GINTSTS = USB_OTG_GINTSTS_ENUMDNE;
    end_of_reset();
    dcd_event_bus_signal(0, DCD_EVENT_BUS_RESET, true);
  }

  if(int_status & USB_OTG_GINTSTS_SOF) {
    USB_OTG_FS->GINTSTS = USB_OTG_GINTSTS_SOF;
    dcd_event_bus_signal(0, DCD_EVENT_SOF, true);
  }

  if(int_status & USB_OTG_GINTSTS_RXFLVL) {
    read_rx_fifo(out_ep);
  }

  // OUT endpoint interrupt handling.
  if(int_status & USB_OTG_GINTSTS_OEPINT) {
    handle_epout_ints(dev, out_ep);
  }

  // IN endpoint interrupt handling.
  if(int_status & USB_OTG_GINTSTS_IEPINT) {
    handle_epin_ints(dev, in_ep);
  }

  // Remote wake up interrupt handling.
  if(int_status & USB_OTG_GINTSTS_WKUINT) {
    USB_OTG_FS->GINTSTS = USB_OTG_GINTSTS_WKUINT;
    dcd_event_bus_signal(0, DCD_EVENT_RESUME, true);
  }

  // Suspend interrupt handling.
  if(int_status & USB_OTG_GINTMSK_USBSUSPM) {
    USB_OTG_FS->GINTSTS = USB_OTG_GINTMSK_USBSUSPM;
    dcd_event_bus_signal(0, DCD_EVENT_SUSPEND, true);
  }

  // Check for unplugging event
  if(int_status & USB_OTG_GINTSTS_OTGINT) {

    uint32_t int_otg_status = USB_OTG_FS->GOTGINT;

    if(int_otg_status & USB_OTG_GOTGINT_SEDET)  // This event also provokes a suspend interrupt (more than 3ms no SOFs). However, the stack handles this case well so we do not need to do anything else here.
    {
        // Reset interrupt.
        USB_OTG_FS->GOTGINT |= USB_OTG_GOTGINT_SEDET;
        // Check if device is disconnected
        if(!(USB_OTG_FS->GOTGCTL & USB_OTG_GOTGCTL_BSVLD))
        {
            dcd_event_bus_signal(0, DCD_EVENT_UNPLUGGED, true);
        }
    }
  }
}

I am just starting with USB, so i have no clue if all of this is perfectly right. However, it worked for me!

[hathach]

@PanRe thanks for sharing, I definitely take a look at this next time working with stm32f4 :)

[hathach]

thanks to @PanRe suggestion, I have implement the fix with pr #357 .