[RFC PATCH for v3.5 1/2] usb: gadget: at91_udc: fix oops regression when connecting usb cable

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Revert "usb: gadget: at91_udc: convert to new style start/stop interface"

This reverts commit f3d8bf34c2c925867322197096ed501ceab8085a.

Tested on the Atmel AT91SAM9260EK board.
The oops occur every time i connect the usb cable.

kernel console output:
pgd = c3aec000
[206d6365] *pgd=00000000
Internal error: Oops: 1 [#1] ARM
Modules linked in:
CPU: 0    Not tainted  (3.5.0-rc7 #22)
PC is at __dev_printk+0x20/0x198
LR is at dev_printk+0x2c/0x3c
pc : [<c017aa20>]    lr : [<c017ae10>]    psr: 20000093
sp : c0365d48  ip : c0309a8c  fp : 00000001
r10: c0374c78  r9 : c038b900  r8 : c0365e04
r7 : c02e8440  r6 : 00000004  r5 : c03877d8  r4 : c030997c
r3 : 206d6365  r2 : c0365e04  r1 : c030997c  r0 : c02e8440
Flags: nzCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
Control: 0005317f  Table: 23aec000  DAC: 00000017
Process swapper (pid: 0, stack limit = 0xc0364270)
Stack: (0xc0365d48 to 0xc0366000)
...
[<c017aa20>] (__dev_printk+0x20/0x198) from [<c017ae10>]
	(dev_printk+0x2c/0x3c)
[<c017ae10>] (dev_printk+0x2c/0x3c) from [<c01e0d5c>]
	(composite_suspend+0x28/0xc8)
[<c01e0d5c>] (composite_suspend+0x28/0xc8) from [<c01dee18>]
	(at91_udc_irq+0xbc/0x878)
[<c01dee18>] (at91_udc_irq+0xbc/0x878) from [<c0054c74>]
	(handle_irq_event_percpu+0x50/0x1cc)
[<c0054c74>] (handle_irq_event_percpu+0x50/0x1cc) from [<c0054e18>]
	(handle_irq_event+0x28/0x38)
[<c0054e18>] (handle_irq_event+0x28/0x38) from [<c0056f40>]
	(handle_level_irq+0x80/0xdc)
[<c0056f40>] (handle_level_irq+0x80/0xdc) from [<c00545f0>]
	(generic_handle_irq+0x28/0x3c)

Mario Jorge Isidoro:
"I've tracked the error to line 1099 of the file
drivers/usb/gadget/composite.c
This line is in the function "static int composite_setup(struct
usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)"
line 1099: req->zero = 0; // req is NULL, this is where it blows up

The function is called from the function "static void
handle_setup(struct at91_udc *udc, struct at91_ep *ep, u32 csr)" in
drivers/usb/gadget/at91_udc.c on line 1246
line 1246: status = udc->driver->setup(&udc->gadget, &pkt.r);

So, from at91_udc_irq to composite_setup the path seems to be:
at91_udc_irq (line 1490) -> handle_ep0 (line 1289) -> handle_setup (line
1264) -> composite_setup"

Signed-off-by: Fabio Porcedda <fabio.porcedda@xxxxxxxxx>
Reported-And-Tested-by: Fabio Porcedda <fabio.porcedda@xxxxxxxxx>
Reported-And-Tested-by: Mario Jorge Isidoro <Mario.Isidoro@xxxxxxxxx>
Cc: linux-usb@xxxxxxxxxxxxxxx
Cc: Sebastian Andrzej Siewior <sebastian@xxxxxxxxxxxxx>
Cc: Felipe Balbi <balbi@xxxxxx>
Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
Cc: Nicolas Ferre <nicolas.ferre@xxxxxxxxx>
Cc: Ido Shayevitz <idos@xxxxxxxxxxxxxx>
Cc: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@xxxxxxxxxxxx>
---
 drivers/usb/gadget/at91_udc.c | 60 +++++++++++++++++++++++++++++++++----------
 1 file changed, 46 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/gadget/at91_udc.c b/drivers/usb/gadget/at91_udc.c
index 1a4430f..7687ccd 100644
--- a/drivers/usb/gadget/at91_udc.c
+++ b/drivers/usb/gadget/at91_udc.c
@@ -977,18 +977,18 @@ static int at91_set_selfpowered(struct usb_gadget *gadget, int is_on)
 	return 0;
 }
 
-static int at91_start(struct usb_gadget *gadget,
-		struct usb_gadget_driver *driver);
-static int at91_stop(struct usb_gadget *gadget,
-		struct usb_gadget_driver *driver);
+static int at91_start(struct usb_gadget_driver *driver,
+		int (*bind)(struct usb_gadget *));
+static int at91_stop(struct usb_gadget_driver *driver);
+
 static const struct usb_gadget_ops at91_udc_ops = {
 	.get_frame		= at91_get_frame,
 	.wakeup			= at91_wakeup,
 	.set_selfpowered	= at91_set_selfpowered,
 	.vbus_session		= at91_vbus_session,
 	.pullup			= at91_pullup,
-	.udc_start		= at91_start,
-	.udc_stop		= at91_stop,
+	.start			= at91_start,
+	.stop			= at91_stop,
 
 	/*
 	 * VBUS-powered devices may also also want to support bigger
@@ -1626,34 +1626,66 @@ static void at91_vbus_timer(unsigned long data)
 		schedule_work(&udc->vbus_timer_work);
 }
 
-static int at91_start(struct usb_gadget *gadget,
-		struct usb_gadget_driver *driver)
+static int at91_start(struct usb_gadget_driver *driver,
+		int (*bind)(struct usb_gadget *))
 {
-	struct at91_udc	*udc;
+	struct at91_udc	*udc = &controller;
+	int		retval;
+	unsigned long	flags;
+
+	if (!driver
+			|| driver->max_speed < USB_SPEED_FULL
+			|| !bind
+			|| !driver->setup) {
+		DBG("bad parameter.\n");
+		return -EINVAL;
+	}
+
+	if (udc->driver) {
+		DBG("UDC already has a gadget driver\n");
+		return -EBUSY;
+	}
 
-	udc = container_of(gadget, struct at91_udc, gadget);
 	udc->driver = driver;
 	udc->gadget.dev.driver = &driver->driver;
 	dev_set_drvdata(&udc->gadget.dev, &driver->driver);
 	udc->enabled = 1;
 	udc->selfpowered = 1;
 
+	retval = bind(&udc->gadget);
+	if (retval) {
+		DBG("bind() returned %d\n", retval);
+		udc->driver = NULL;
+		udc->gadget.dev.driver = NULL;
+		dev_set_drvdata(&udc->gadget.dev, NULL);
+		udc->enabled = 0;
+		udc->selfpowered = 0;
+		return retval;
+	}
+
+	spin_lock_irqsave(&udc->lock, flags);
+	pullup(udc, 1);
+	spin_unlock_irqrestore(&udc->lock, flags);
+
 	DBG("bound to %s\n", driver->driver.name);
 	return 0;
 }
 
-static int at91_stop(struct usb_gadget *gadget,
-		struct usb_gadget_driver *driver)
+static int at91_stop(struct usb_gadget_driver *driver)
 {
-	struct at91_udc *udc;
+	struct at91_udc *udc = &controller;
 	unsigned long	flags;
 
-	udc = container_of(gadget, struct at91_udc, gadget);
+	if (!driver || driver != udc->driver || !driver->unbind)
+		return -EINVAL;
+
 	spin_lock_irqsave(&udc->lock, flags);
 	udc->enabled = 0;
 	at91_udp_write(udc, AT91_UDP_IDR, ~0);
+	pullup(udc, 0);
 	spin_unlock_irqrestore(&udc->lock, flags);
 
+	driver->unbind(&udc->gadget);
 	udc->gadget.dev.driver = NULL;
 	dev_set_drvdata(&udc->gadget.dev, NULL);
 	udc->driver = NULL;
-- 
1.7.11.1

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


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux