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