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

fix enum walkaround forever check for SE0 when pull up is disabled #700

Merged
merged 1 commit into from
Mar 4, 2021

Conversation

hathach
Copy link
Owner

@hathach hathach commented Mar 4, 2021

Describe the PR
This issue is discovered while troubleshooting adafruit/circuitpython#4066 (comment)

Part of enumeration walkaround is waiting for bus reset to complete i.e line state != SE0.
https://github.com/raspberrypi/pico-sdk/blob/master/src/rp2_common/pico_fix/rp2040_usb_device_enumeration/rp2040_usb_device_enumeration.c#L61

However, when pull-up is disabled e.g application call tud_disconnect() to soft disconnect from Host (while still physically plugged in). SE0 will be permanent present causing the walkround forever checking for the state unitl tud_connect() (pullup is enabled) or cpu reset. Worse if PICO_TIME_DEFAULT_ALARM_POOL_DISABLED = 1, the walkaround will blockingly wait --> sort of hanged there.

To reproduce, add this to any of the example, press the button, rp2040 will disconnect but forever hanged if PICO_TIME_DEFAULT_ALARM_POOL_DISABLED=1

if ( board_button_read() )
{
  printf("Disconnect\n");
  tud_disconnect();
  board_delay(2000);
  printf("Re-connect\n");
  tud_connect();
}

Solution

Only run enumeration walkaround if pull-up is enabled.

Additional Context

I add a bit of TODO for VBus detect and Suspend & Resume later on. Note to myself: the pico schematic use GPIO24 which is is actually VBUS over current detection (FUN9), will need a jump wire to test later on.

@liamfraser
Copy link
Collaborator

Looks good to me. In terms of VBUS, you can use a level high / level low interrupt on any GPIO to provide a vbus changed interrupt. That is to say, you don't need to use vbus detect from the actual usb controller input.

@hathach
Copy link
Owner Author

hathach commented Mar 4, 2021

Looks good to me. In terms of VBUS, you can use a level high / level low interrupt on any GPIO to provide a vbus changed interrupt. That is to say, you don't need to use vbus detect from the actual usb controller input.

Does it trigger the VBUS_DETECT interrupt in sie_status. I would hop to handle it internally by the stack without user to call any API. For example, on nRF52, the PHY is still disabled after tud_init() and only enabled after VBUS is detected. Then it got disabled again after Disconnection is detected to save even more power. I will do more testing with that scheme later on with rp2040.

@hathach hathach merged commit 5285548 into master Mar 4, 2021
@hathach hathach deleted the fix-rp2040-bus-reset branch March 4, 2021 14:01
@liamfraser
Copy link
Collaborator

liamfraser commented Mar 4, 2021

If you look at the gpio function table, only some gpios are connected to USB VBUS DET (which can drive the VBUS_DETECT interrupt in sie status). I am saying that you could use any GPIO interrupt to add and remove vbus detect in software (by forcing and removing vbus detect like we do now when the interrupt fires). This effectively lets you use any GPIO as vbus detect.

@hathach
Copy link
Owner Author

hathach commented Mar 4, 2021

That is what I guess as well. It will needs an extra API for tinyusb stack to interact with application regarding vbus so it could distinguish between disconnected and suspended as mentioned in the datasheet. Thanks for your advice, I will look at this later on.

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

Successfully merging this pull request may close these issues.

2 participants