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.porce...@gmail.com>
Reported-And-Tested-by: Fabio Porcedda <fabio.porce...@gmail.com>
Reported-And-Tested-by: Mario Jorge Isidoro <mario.isid...@tecmic.pt>
Cc: linux-usb@vger.kernel.org
Cc: Sebastian Andrzej Siewior <sebast...@breakpoint.cc>
Cc: Felipe Balbi <ba...@ti.com>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Cc: Nicolas Ferre <nicolas.fe...@atmel.com>
Cc: Ido Shayevitz <i...@codeaurora.org>
Cc: Jean-Christophe PLAGNIOL-VILLARD <plagn...@jcrosoft.com>
---
 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to