rp/usb: return error on disconnect

[birktj]

This PR causes USB to return EndpointError::Disabled when trying to read or write to a disconnected usb port.

It seems to work as expected, but I haven't tested it very heavily and I am not super familiar with USB internals so there might be some bugs.

This fixes #3426

[birktj]

I could not find any documentation that says why this has to be - 1, but it is what the wait_enabled functions do so I figure it must be correct.

[Dirbaio]

in USB, "suspend" and "disconnect" are different things. It's only on actual disconnection that we should disable all endpoints and reset the USB device state. On suspend, the host expects everything to stay as-is when it unsuspends.

Here's how you tell which state you're in:

VBUS present, activity on the bus -> USB is active
VBUS present, no activity on the bus -> USB is suspended
VBUS no present -> USB is disconnected

the USB peripheral only looks at the DP/DM lines so it can only tell whether there's activity on the bus or not. To distinguish "suspended" and "disconnected" states you need to use GPIO. On the Pico VBUS is wired to GPIO24, but it varies across boards so it has to be configurable. Some boards don't wire it, in which case it's simply impossible to support the "disconnected" state.

Also, embassy-usb has builtin support for handling the "disconnected" state. The driver should return PowerDetected, PowerRemoved events from poll() when VBUS comes/goes.

[birktj]

Thanks for the explanation.

So if I understand correctly the correct approach here would be to remove the code that disables the endpoints on suspend and instead add some code for vbus detection?

But should there also be a way to check for the suspended case? It is not very practical with read and writes that can potentially block forever. Would a EndpointError::Suspended be an option in this case?

In my case my hardware does not support VBUS detection, but I need to be able to react to a disconnect so this code works well for me. But if this is a unstandard usecase it probably doesn't need to be upstreamed.

BTW, do you have any good resources on USB internals? I have been looking a bit around for some time, but never found a nice overview over all the terminology.

[Dirbaio]

So if I understand correctly the correct approach here would be to remove the code that disables the endpoints on suspend and instead add some code for vbus detection?

yes

But should there also be a way to check for the suspended case? It is not very practical with read and writes that can potentially block forever. Would a EndpointError::Suspended be an option in this case?

I think blocking forever is the intended behavior. consider the case where you're copying a file from a USB drive and you suspend your laptop. You don't want the transfer to fail, it'll just hang and then resume when you unsuspend whic hcan be days, weeks later.

[birktj]

Okay great, then I will try to go for a model similar to embasy-nrf with a VbusDetect trait. Possibly moving it into emabssy-usb?

I think there also are cases where you would not want to block on suspend, e.g. a keyboard where you don't want to automatically send the first keystroke after suspend when the suspend ends. But it is maybe a bit of-topic here, I will open an issue where it can be discussed a bit.