hathach / tinyusb

An open source cross-platform USB stack for embedded system
https://www.tinyusb.org
MIT License
4.77k stars 1.02k forks source link

No DCD_EVENT_UNPLUGGED event for STM32 dwc2 #1476

Open mauriciocirelli opened 2 years ago

mauriciocirelli commented 2 years ago

Operating System

Windows 10

Board

Custom based on STM32H7B3

Firmware

portable/st/stm32_fsdev/dcd_stm32_fsdev.c

What happened ?

Dear,

I have followed the example for CDC/MSC using FreeRTOS and have got pretty much everything working. Except for the disconnection event: unmount callback (tud_umount_cb) is not being called for this platform and driver. Mount callback (tud_mount_cb) happens as expected.

I am new to this library, so maybe I have messed up something, but as everything else is working, I guess I maybe facing a bug somewhere. Looking into the code, the old synopsys driver evokes the DCD_EVENT_UNPLUGGED event, but the new stm32_fsdev does not.

Any help would be much appreciated.

How to reproduce ?

Just connect the board using the new stm32_fsdev and look for the mount and unmount callbacks.

Debug Log as txt file

No response

Screenshots

No response

tfx2001 commented 2 years ago

It seems that STM32H7B3 is using synopsys dwc2 driver, not stm32_fsdev driver.

mauriciocirelli commented 2 years ago

It seems that STM32H7B3 is using synopsys dwc2 driver, not stm32_fsdev driver.

Dear, thank you for pointing this out. I have confirmed that I am using the synopsys dwc2 driver.

However, debugging dcd_int_handler function I have confirmed that the following code is not being executed when I unplug the device from the computer:

dcd_event_bus_signal(rhport, DCD_EVENT_UNPLUGGED, true);

I have enabled VBUS sensing in CubeMX and it is directly connected from the USB connector to PA9.

hathach commented 2 years ago

@mauriciocirelli indeed, the disconnect detection is a bit confusing on dwc2. The specs is a bit fuzzy. dwc2 driver may miss some logic here and there since it is a generic driver which work on all configuration of dwc2. The old stm synopsys driver is very specific to stm configured controller.

I will try to revise and test this out when having time.

mauriciocirelli commented 2 years ago

@hathach thank you for your reply. Detecting the disconnection event is important for us, because we have a FATFs running on this device and we need to unmount/re-mount it when connected/disconnected from USB. Otherwise, if our firmware tries to access the FATFs while Windows is accessing it from USB, the file system gets corrupted.

hathach commented 2 years ago

unfortunately, I am currently busy with other works. will check this out when possible. If you manage to get it work, please consider to make an PR to help others.

BennyEvans commented 2 years ago

I'm experiencing the same issue after recently updating tinyUSB and moving to the dwc2 driver. I'll have a quick look through the new driver to see if it's anything obvious but might revert to the old driver to keep the device working for now.

BennyEvans commented 2 years ago

Removing the line below seems to resolve the issue for me. May someone else please confirm this also works for them. I'm running in HS mode on an STM32h742 (custom hardware). https://github.com/hathach/tinyusb/blob/fd5bb6e5db8e8e997d66775e689cc73f149e7fc1/src/portable/synopsys/dwc2/dcd_dwc2.c#L525

Edit: I'm also currently using the 0.13.0 release. Will likely switch to head on main.

BennyEvans commented 2 years ago

Switched to head and it also seems to be working with the fix above. Note however, there is another bug if running in HS mode with an STM32H7. You will need to modify a section in tusb_mcu.h to add #define TUP_RHPORT_HIGHSPEED 1 - in this section: https://github.com/hathach/tinyusb/blob/fd5bb6e5db8e8e997d66775e689cc73f149e7fc1/src/common/tusb_mcu.h#L158-L163

I can create a PR to add #define TUP_RHPORT_HIGHSPEED 1 if needed.

hathach commented 2 years ago

h7 does not have built-in HS PHY, which is the TUP_RHPORT_HIGHSPEED is about. For board with external PHY, you need to have define CFG_TUD_MAX_SPEED = OPT_MODE_HIGH_SPEED

BennyEvans commented 2 years ago

@hathach my mistake! I see now. I'll give that a try. Thanks!

Edit: After adding #define CFG_TUD_MAX_SPEED BOARD_TUD_MAX_SPEED and removing #define TUP_RHPORT_HIGHSPEED 1 it all seems to work.

mauriciocirelli commented 1 year ago

@BennyEvans I can confirm that removing that line from the driver triggers the unmount event. I am testing on v0.13, and the respective line is 514 instead of 525.

However, I am getting several triggers of this unmount event when I unplug the USB cable. Is it expected?

Also, I would like to try v0.14 to get things up to date, but it seems that there are some configurations that changed and I am stuck on check_dwc2 function.

This is the setup I have for v0.13:

#define CFG_TUSB_MCU                  OPT_MCU_STM32H7
#define CFG_TUSB_OS                   OPT_OS_FREERTOS
#define BOARD_DEVICE_RHPORT_SPEED     OPT_MODE_FULL_SPEED
#define BOARD_DEVICE_RHPORT_NUM       1
#define CFG_TUSB_RHPORT1_MODE         (OPT_MODE_DEVICE | OPT_MODE_FULL_SPEED)
#define CFG_TUSB_DEBUG                0

Is there any doc explaining the new equivalent settings?

BennyEvans commented 1 year ago

@mauriciocirelli I don't have the board with me at the moment so can't help. But will try to get to it over the next few days.

mauriciocirelli commented 1 year ago

Dear @BennyEvans,

The issue is worse than at first... It is calling the _tud_umountcb callback so many times that it is giving me a STACK OVERFLOW error at the TinyUSB task.

I had to work around it by setting a flag like this:

void tud_mount_cb(void)
{
  if(bIsMounted)
    return;

  // Do something here
  bIsMounted = true;
}

// Invoked when device is unmounted
void tud_umount_cb(void)
{
  if(!bIsMounted)
    return;

  bIsMounted = false;
  // Do something here
}
benishor commented 1 year ago

I am using a daisy seed board which runs a STM32H7 and I can confirm I too do not get the unplugged event. The hack described above does not work either.