[PATCH] usb: gadget: atmel: reliably generate disconnect by disabling controller instead of toggling PULLD_DIS

[Zixun LI]

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 <eugen.hristev at linaro.org> 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 <admin at hifiphile.com> 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 <admin at hifiphile.com>
>>>  ---
>>>   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 <admin at hifiphile.com>
>
>