zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.12k stars 6.21k forks source link

usb: STM32F4 reset when usb enabled, disabled, and enabled again, canceled callback invoked #63262

Closed akaiserUS closed 8 months ago

akaiserUS commented 9 months ago

Describe the bug

System resets due to a divide by zero in in stm32f4xx_II_usb.c at the line:

ktcnt = (uint16_t)((ep->xfer_len + ep->maxpacket - 1U) / ep->maxpacket);

System resets due to a divide by zero in in modules/hal/stm32/stm32cube/stm32f4xx/drivers/src/stm32f4xx_ll_usb.c at the line pktcnt = (uint16_t)((ep->xfer_len + ep->maxpacket - 1U) / ep->maxpacket);

due to ep->maxpacket being 0. It appears that the reason is due a canceled usb transfer (receive) invoking a callback still. See usb_transfer.c in function usb_transfer_work();

To Reproduce

  1. west init -m https://github.com/zephyrproject-rtos/zephyr
  2. west update
  3. west build -b stm32f412g_disco samples/subsys/usb/console
  4. Inside main() (at: zephyr/samples/subsys/usb/console/src/main.c)

modify the function to:

int main(void)
{
    const struct device *const dev = DEVICE_DT_GET(DT_CHOSEN(zephyr_console));
    uint32_t dtr = 0;

#if defined(CONFIG_USB_DEVICE_STACK_NEXT)
    if (enable_usb_device_next()) {
        return 0;
    }
#else
    if (usb_enable(NULL)) {
        return 0;
    }
#endif
    k_sleep(K_SECONDS(1));
    usb_disable();
    k_sleep(K_SECONDS(1));
    usb_enable(NULL);
    k_sleep(K_SECONDS(1));
    usb_disable();
    k_sleep(K_SECONDS(1));
    usb_enable(NULL);

    /* Poll if the DTR flag was set */
    while (!dtr) {
        uart_line_ctrl_get(dev, UART_LINE_CTRL_DTR, &dtr);
        /* Give CPU resources to low priority threads. */
        k_sleep(K_MSEC(100));
    }

    while (1) {
        printk("Hello World! %s\n", CONFIG_ARCH);
        k_sleep(K_SECONDS(1));
    }
}
  1. The second usb_enable(NULL) call may return but the next k_sleep() will not.
  2. System will reset

Expected behavior

When the usb is enabled and disabled and enabled again, the usb connection should be re-established without causing the system to reset (due to the divide by zero).

Impact Impact is annoyance, currently have a patch that reverts the commit that is causing the issue.

Environment (please complete the following information):

Additional context Reverting commit: 81e4934ebd37a5d134ebd02e689bab622e582ff1 with a patch resolves my issue

github-actions[bot] commented 9 months ago

Hi @akaiserUS! We appreciate you submitting your first issue for our open-source project. šŸŒŸ

Even though I'm a bot, I can assure you that the whole community is genuinely grateful for your time and effort. šŸ¤–šŸ’™

Desvauxm-st commented 9 months ago

@akaiserUS I confirme the issue you found but it is not a STM32 driver issue because even with a frdm_k64f it is a crash.

when you add :

    k_sleep(K_SECONDS(1));
usb_disable();
k_sleep(K_SECONDS(1));
usb_enable(NULL);

and complile like that: west build -p always -b frdm_k64f samples/subsys/usb/console/ after the message Hello World! arm is not display on the console

akaiserUS commented 9 months ago

Thank you for confirming that it is cross-platform

Desvauxm-st commented 9 months ago

@akaiserUS and @jfischer-no I confirm that with the modifications proposed by jfischer-no ( #63333) next this solves the problem: image

akaiserUS commented 9 months ago

@Desvauxm-st Thank you for letting me know, I will use that locally until I can pull in the change through Zephyr