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

DWC2: Assert on replug of device after update from 0.16.0 to 0.17.0 #2815

Closed
1 task done
Olstyle opened this issue Sep 25, 2024 · 3 comments · Fixed by #2820
Closed
1 task done

DWC2: Assert on replug of device after update from 0.16.0 to 0.17.0 #2815

Olstyle opened this issue Sep 25, 2024 · 3 comments · Fixed by #2820
Labels

Comments

@Olstyle
Copy link

Olstyle commented Sep 25, 2024

Operating System

Windows 10

Board

Custom STM32U585 PCB

Firmware

tinyusb + FreeRTOS based project which unfortunately I can not share completely
With the following initialisation of an MSC and Vendor Class device

/* This variable uses the defines from usb_dwc2_cfg.h */
uint8_t const g_ConfigDesc[] =
{
  /* Config number, interface count, string index, total length, attribute, power in mA */
  TUD_CONFIG_DESCRIPTOR(1, ITF_NUM_TOTAL, 0, CONFIG_TOTAL_LEN, 0x00, 100),

  /* Interface number, string descriptor index, EP Out & EP In address, EP size */
  TUD_MSC_DESCRIPTOR(ITF_NUM_MSC, 4, EPNUM_MSC_OUT, EPNUM_MSC_IN, 64),

  /* Interface number, string descriptor index, EP Out & EP In address, EP size */
  TUD_VENDOR_DESCRIPTOR(ITF_NUM_VENDOR_WCID, 5, EPNUM_WCID_OUT, EPNUM_WCID_IN, 64)
};


const char g_VendorId[] = "Me";
const char g_ProductId[] = "My Prog. Dev 3";
const char g_ProductRev[] = "1.0";

static uint8_t s_Serial[] = "0000000000";
/* array of pointer to string descriptors */
char const* g_StringDesc_P[] =
{
  (char[]) { 0x09, 0x04 },        /* 0: is supported language is English (0x0409) */
  "Me",                          /* 1: Manufacturer */
  "My Prog. Dev 3",              /* 2: Product */
  (char*)&s_Serial,               /* 3: Serial */
  "My MSC",                      /* 4: MSC Interface */
  "My WCID",                     /* 5: WCID Interface */
};

/* Public Function Definition ***************************************/

/* USB Device Driver task
 * This top level thread process all usb events and invoke callbacks
 */
void usbHdl_deviceTask(void* Param)
{
    uint8_t SerialArray[4];
    uint32_t Serial;
    (void) Param;

    usbHdl_reOpenMSC();

    portALLOCATE_SECURE_CONTEXT( configMINIMAL_SECURE_STACK_SIZE * 2 );
    //read clock as reference for the USB driver
    SystemCoreClock = mcu_getSystemClock();

    /* Init MSC */
    usb_msc_init(g_VendorId, sizeof(g_VendorId),
                 g_ProductId, sizeof(g_ProductId),
                 g_ProductRev, sizeof(g_ProductRev));


    vendord_init();

    devinfo_getSerial(SerialArray);
    Serial = SerialArray[0] << 24 | SerialArray[1] << 16 | SerialArray[2] << 8 | SerialArray[0];
    convU32ToAsciiString(Serial, s_Serial, 10);

    /* Init module */
    usb_dwc2_init(g_ConfigDesc, g_StringDesc_P, sizeof(g_StringDesc_P)/sizeof(g_StringDesc_P[0]));

    /* set status once via pin read */
    usb_dwc2_setPlug( (dio_readChannel(PIN_USB_SENSE_E) == DIO_HIGH_E) );

    /* RTOS forever loop */
    while (1)
    {
        /* put this thread to waiting state until there is new events */
        usb_dwc2_task(WDGMAN_MAX_TRIG_MS);
    }
}

What happened ?

After switching from version 01.16.0 to 0.17.01, If I plug out the device and plug it in again, I get stuck at an assert in usbd.c line 1049

 // Failed if there is no supported drivers
   TU_ASSERT(drv_id < TOTAL_DRIVER_COUNT);
 }

(the device has a battery which keeps it running after disconnect)
Could this be due to bad descriptors or is this a bug in tinyusb?

How to reproduce ?

  1. plug in device
  2. plug out device
  3. plug device in again

Debug Log as txt file (LOG/CFG_TUSB_DEBUG=2)

logComplete.txt
logFromSuspend.txt

Screenshots

No response

I have checked existing issues, dicussion and documentation

  • I confirm I have checked existing issues, dicussion and documentation.
@Olstyle Olstyle changed the title Assert on replug of device after update from 0.16.0 to 0.17.0 DWC2: Assert on replug of device after update from 0.16.0 to 0.17.0 Sep 26, 2024
@HiFiPhile
Copy link
Collaborator

Please try #2820

@Olstyle
Copy link
Author

Olstyle commented Sep 30, 2024

#2820 fixes it, thank you. Though looking at the change in the PR and the changes from, 0.16.0 to 0.17.0, I ask myself the typical "How did that ever work?". The old code was a direct call to tu_fifo_clear, and tu_edpt_stream_clear is just an alias for that. Why is the additional close suddenly necessary?

@HiFiPhile
Copy link
Collaborator

Why is the additional close suddenly necessary?

It's not the fifo who cause the issue. In old version vendor class is erased on reset

tu_memclr(p_itf, ITF_MEM_RESET_SIZE);

Since 0.17.0 transfer management has been migrated to stream api but forgot to erase endpoint on reset, the next class opening attempt will fail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants