at91_udc Bug: USB gadget unplug not noticed by kernel

[Felipe Balbi <balbi@kernel.org>]

Hi,

Ana Rodriguez Lopez <rodrilopez.ana@gmail.com> writes:
> Hello Felipe,
>
> I am not an expert on Linux Kernel but I have a problem with the USB
> gadget and the origin seems to be there.
>
> I am working with Kernel 4.13.0 on Stamp 9G20 from Taskit GmbH (CPU:

rather recent, kudos :-)

> Atmel AT91SAM9G20 Embedded Processor featuring an ARM926EJ-STM ARM®
> Thumb® Core).
>
> The problem is, the disconnection of the USB gadget cable is not
> acquainted by the Kernel. There is nothing in dmesg.
>
> When I connect the cable, there is a reaction visible in dmesg:
>
> [ 1711.406250] g_ether gadget: full-speed config #1: CDC Ethernet (ECM)
> [ 1711.414062] IPv6: ADDRCONF(NETDEV_CHANGE): usb0: link becomes ready
>
> Kernel modules loaded:
>
> Module                  Size  Used by
> usb_f_ecm               9123  1
> g_ether                 4488  0
> usb_f_rndis            20935  2 g_ether
> u_ether                10799  3 usb_f_ecm,g_ether,usb_f_rndis
> libcomposite           44168  3 usb_f_ecm,g_ether,usb_f_rndis
>
> After disconnecting the cable, usb0 is still shown as connected:
>
> 4: usb0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast
> state UP group default qlen 1000
>     link/ether 8e:8e:76:28:44:ab brd ff:ff:ff:ff:ff:ff
>     inet 169.254.169.4/16 brd 169.254.255.255 scope link usb0
>        valid_lft forever preferred_lft forever
>     inet6 fe80::8c8e:76ff:fe28:44ab/64 scope link
>        valid_lft forever preferred_lft forever
>
>
> We used to work with an older Kernel (3.2) and we had a patch for this
> problem. We expected it to be solved in 4.13 but it isn't.

if the patch isn't sent to the appropriate mailing list, there's nothing
we can do about it :-)

> diff --git a/drivers/usb/gadget/at91_udc.c b/drivers/usb/gadget/at91_udc.c
> index 8efe0fa..423fc6a 100644
> --- a/drivers/usb/gadget/at91_udc.c
> +++ b/drivers/usb/gadget/at91_udc.c
> @@ -1395,6 +1395,7 @@ static irqreturn_t at91_udc_irq (int irq, void *_udc)
>      u32            rescans = 5;
>      int            disable_clock = 0;
>      unsigned long        flags;
> +    struct usb_ctrlrequest    req;
>  
>      spin_lock_irqsave(&udc->lock, flags);
>  
> @@ -1443,7 +1444,18 @@ static irqreturn_t at91_udc_irq (int irq, void *_udc)
>              at91_udp_write(udc, AT91_UDP_ICR, AT91_UDP_RXSUSP);
>              /* VDBG("bus suspend\n"); */
>              if (udc->suspended)
> -                continue;
> +                continue;   
> +
> +            /* tell the driver the disconnect */
> +            if (udc->driver) {
> +                spin_unlock(&udc->lock);
> +                memset(&req, 0, sizeof(req));
> +                req.bRequestType = USB_DIR_OUT | USB_TYPE_STANDARD |
> USB_RECIP_DEVICE;    
> +                req.bRequest = USB_REQ_SET_CONFIGURATION;    
> +                status = udc->driver->setup(&udc->gadget, &req);
> +                spin_lock(&udc->lock);
> +            }
> +

doesn't seem like the right way to go about this. Look for a Disconnect
IRQ in your documentation. The way the driver is written now, it appears
to depend on a GPIO notifying VBUS status. Without that, you'll never
receive a disconnect notification.