Hi all, The case has been reported with No.01589331.
In brief line high-impedance is not observed when both PULLD_DIS & DETACH bits are 1: - In unenumerated state (no cable attached) DP is still pulled up - In enumerated state, disconnection is not detected by host [1] [2] only by [3]. The test has been done in 3 ways on SAM9X60-Curiosity and AT91SAM9G25-EK boards: - Modify UDPHS_CTRL on u-boot-at91 with mw command - Modify UDPHS_CTRL on linux4sam-poky-sam9x60_curiosity-headless-2024.10 with devmem2 utils - On linux-mchp 6.6, comment out gadget_udc_start/stop in soft_connect_store() of drivers/usb/gadget/udc/core.c, leaving only gadget_connect/disconnect. Then run echo disconnect > /sys/class/udc/500000.gadget/soft_connect Host used: 1. AMD Ryzen 5800H native port, Linux 6.14 2. Intel i7-8750H native port, Windows 10 3. Genesys GL3523 hub attached to [1] Zixun LI On Jun 4, 2025, at 11:51, Eugen Hristev <[email protected]> wrote: > > > > On 6/4/25 11:20, Mattijs Korpershoek wrote: >> >> Hi Zixun, >> >> Thank you for the patch. >> >> On Mon, Jun 02, 2025 at 17:45, Zixun LI <[email protected]> wrote: >> >> I'm surprised that checkpatch.pl does not catch this, but the subject >> (title) is too long (it should be less than 70 chars): >> >> https://docs.u-boot.org/en/latest/develop/sending_patches.html#commit-message-conventions >> >>> >>> Contrary to the datasheet, setting both DETACH and PULLD_DIS bits to 1 >>> does not always drive the DP and DM lines to high-impedance. This >>> prevents the host from reliably detecting a USB disconnect and subsequent >>> reconnect. >>> >>> The symptom is that the first gadget command (e.g., dhcp) succeeds, while >>> subsequent commands (e.g., nfs) fail. >>> >>> Disabling and re-enabling the controller entirely, instead of toggling the >>> PULLD_DIS bit, reliably generates a disconnect event. >>> >>> The Linux driver works correctly because gadget_disconnect/gadget_connect >>> are always followed by gadget_udc_start/gadget_udc_stop. In U-Boot >>> pullup() is used solely. >>> >>> This behavior has been observed on the SAM9X60-Curiosity and >>> AT91SAM9G25-EK boards and has been reported to Microchip. >> >> >> I'm not against this, but the driver supports multiple gadget >> families according to the compatible. >> >> I'm wondering if this issue is reproduced on other atmel >> boards that are supported in U-Boot. >> >> Eugen, have you noticed the above limitation on a different >> board family that you maintain? > > > I have not seen this before, or don't remember about it. > > Adding Mihai from Microchip team to see if we can get more info, whether > this is a general gadget thing or it's specific to these chipsets. If it > was reported to Microchip it would be interesting to see the response. > > Eugen > >> >> If this issue is chip specific, I think we should test for compatible in >> usba_udc_pullup() to only use USBA_ENABLE/USBA_DISABLE for this >> particular compatible (microchip,sam9x60-udc). >> >>> >>> >>> Signed-off-by: Zixun LI <[email protected]> >>> --- >>> drivers/usb/gadget/atmel_usba_udc.c | 9 ++------- >>> 1 file changed, 2 insertions(+), 7 deletions(-) >>> >>> diff --git a/drivers/usb/gadget/atmel_usba_udc.c >>> b/drivers/usb/gadget/atmel_usba_udc.c >>> index >>> f9326f0a7e7d913bcf201ba3e4aeca9a099e7be7..a38e70957402fb3ade3de611d73e29b990d00703 >>> 100644 >>> --- a/drivers/usb/gadget/atmel_usba_udc.c >>> +++ b/drivers/usb/gadget/atmel_usba_udc.c >>> @@ -521,16 +521,11 @@ usba_udc_set_selfpowered(struct usb_gadget *gadget, >>> int is_selfpowered) >>> static int usba_udc_pullup(struct usb_gadget *gadget, int is_on) >>> { >>> struct usba_udc *udc = to_usba_udc(gadget); >>> - u32 ctrl; >>> - >>> - ctrl = usba_readl(udc, CTRL); >>> >>> if (is_on) >>> - ctrl &= ~USBA_DETACH; >>> + usba_writel(udc, CTRL, USBA_ENABLE_MASK); >> >> >> Could we add a comment in the function as well? Something similar to >> what's in the commit message: >> >> /* Some chips don't reliably drive DP/DM lines to high impedance when >> * using the DETACH/PULLD_DIS bits. >> * To ensure a reliable disconnect, power cycle the controller instead >> */ >> >> Feel free to rephrase your preference. >> >>> >>> else >>> - ctrl |= USBA_DETACH; >>> - >>> - usba_writel(udc, CTRL, ctrl); >>> + usba_writel(udc, CTRL, USBA_DISABLE_MASK); >>> >>> return 0; >>> } >>> >>> --- >>> base-commit: 4d3b5c679bc9d5c6cbbeedcc1e4a186f1cc35541 >>> change-id: 20250602-pullup-83c5540db5cd >>> >>> Best regards, >>> -- >>> Zixun LI <[email protected]> > >
