Regression in net2280 locking

Regression in net2280 locking

[D. Ziesche]

Hi,

thanks for the patch.
I applied the patch on linux master and it fixed the issue.


shell output:
$ ./a.out /dev/usb_gadget/aio_simple_test
Event BIND
Event ENABLE
submit: in
submit: out
Event SUSPEND
Event DISABLE
Event ENABLE
ev=in; ret=2112
ev=out; ret=0
submit: in
submit: out
Event SUSPEND
Event DISABLE
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Regression in net2280 locking

[Alan Stern]

On Wed, 8 Aug 2018, D. Ziesche wrote:

> Hi,
> 
> thanks for the patch.
> I applied the patch on linux master and it fixed the issue.
> 
> 
> shell output:
> $ ./a.out /dev/usb_gadget/aio_simple_test
> Event BIND
> Event ENABLE
> submit: in
> submit: out
> Event SUSPEND
> Event DISABLE
> Event ENABLE
> ev=in; ret=2112
> ev=out; ret=0
> submit: in
> submit: out
> Event SUSPEND
> Event DISABLE

Thank you for testing.  I will submit the patch.

Alan Stern
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Regression in net2280 locking

[Alan Stern]

On Tue, 7 Aug 2018, D. Ziesche wrote:

> Hello,
> 
> I use a 'USB controller: PLX Technology, Inc. Device 3380 (rev ab)' for 
> a custom usb gadget.
> With a more recent kernel (4.12+) the cpu hangs and the gadget device is 
> not functioning at all.
> 
> I used git-bisect and a basic test gadget from the kernel tree
> (tools/usb/ffs-aio-example/simple/device_app/aio_simple.c) to find the 
> first
> bad commit: f16443a034c7aa359ddf6f0f9bc40d01ca31faea
> The bug still occurs with 4.18.0-rc7+.
> 
> Steps to reproduce:
> 1. Compile and run the testtool 
> 'tools/usb/ffs-aio-example/simple/device_app/aio_simple.c'
> 2. Connect with a host system
> 3. Disconnect from host system
> 4. Kernel suffers CPU lockups
> 
> Shell output:
> ---
> $ ./a.out /dev/usb_gadget/aio_simple_gadget
> Event Bind
> Event Enable     <- connected to host
> submit: in
> submit: out
> Event Suspend  <- disconnected from host
> ---hangs---         <- Event Disable is missing
> ---

Yes, I see the problem.  Commit f16443a034c7 was wrong in several 
respects.  One was fixed later in commit 7dbd8f4cabd9, but the problem 
in net2280 remained.

I think the patch below (which is mostly a reversion of the relevant
parts of f16443a034c7) should fix it.  Can you please test the patch
and let me know?

Alan Stern
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Index: usb-4.x/drivers/usb/gadget/udc/net2280.c
===================================================================
--- usb-4.x.orig/drivers/usb/gadget/udc/net2280.c
+++ usb-4.x/drivers/usb/gadget/udc/net2280.c
@@ -1545,7 +1545,7 @@ static int net2280_pullup(struct usb_gad
 		writel(tmp | BIT(USB_DETECT_ENABLE), &dev->usb->usbctl);
 	} else {
 		writel(tmp & ~BIT(USB_DETECT_ENABLE), &dev->usb->usbctl);
-		stop_activity(dev, dev->driver);
+		stop_activity(dev, NULL);
 	}
 
 	spin_unlock_irqrestore(&dev->lock, flags);
@@ -2466,8 +2466,11 @@ static void stop_activity(struct net2280
 		nuke(&dev->ep[i]);
 
 	/* report disconnect; the driver is already quiesced */
-	if (driver)
+	if (driver) {
+		spin_unlock(&dev->lock);
 		driver->disconnect(&dev->gadget);
+		spin_lock(&dev->lock);
+	}
 
 	usb_reinit(dev);
 }
@@ -3381,12 +3384,14 @@ static void handle_stat1_irqs(struct net
 			if (disconnect || reset) {
 				stop_activity(dev, dev->driver);
 				ep0_start(dev);
+				spin_unlock(&dev->lock);
 				if (reset)
 					usb_gadget_udc_reset
 						(&dev->gadget, dev->driver);
 				else
 					(dev->driver->disconnect)
 						(&dev->gadget);
+				spin_lock(&dev->lock);
 				return;
 			}
 		}