[patch v2.6.39 0/3] Patches for next merge window

[patch v2.6.39 0/3] Patches for next merge window

[Felipe Balbi]

Hi all,

This is what I have for next merge window. If you have
patches that should go in, please re-send them now as I
don't have anything in my queue.

I dropped Ming's DMA rework because I'm not sure that's
the best approach. IMO, it's better to simplify DMA
programming by having "one programming model fits all"
rather than have all these flags floating around. So
that'll have to wait another cycle.

I'm also looking into Hema's HWMOD + pm_runtime patches.

I guess those will have to go through Tony's tree since
most of the changes are in arch/arm/mach-omap2/.

Felipe Balbi (3):
  usb: musb: do not error out if Kconfig doesn't match board mode
  usb: musb: gadget: beautify
    usb_gadget_probe_driver()/usb_gadget_unregister_driver
  usb: musb: gadget: do not poke with gadget's list_head

 drivers/usb/musb/musb_core.c       |   25 -----
 drivers/usb/musb/musb_core.h       |    4 +-
 drivers/usb/musb/musb_gadget.c     |  187 +++++++++++++++++++-----------------
 drivers/usb/musb/musb_gadget.h     |    7 +-
 drivers/usb/musb/musb_gadget_ep0.c |   24 +++--
 5 files changed, 120 insertions(+), 127 deletions(-)

-- 
1.7.4.rc2

[patch v2.6.39 1/3] usb: musb: do not error out if Kconfig doesn't match board mode

[Felipe Balbi]

During development, even though board is wired
to e.g. OTG, we might want to compile host-only
or peripheral-only configurations.

Let's allow that to happen.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/usb/musb/musb_core.c |   25 -------------------------
 1 files changed, 0 insertions(+), 25 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 54a8bd1..bc29655 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1949,31 +1949,6 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
 		goto fail0;
 	}
 
-	switch (plat->mode) {
-	case MUSB_HOST:
-#ifdef CONFIG_USB_MUSB_HDRC_HCD
-		break;
-#else
-		goto bad_config;
-#endif
-	case MUSB_PERIPHERAL:
-#ifdef CONFIG_USB_GADGET_MUSB_HDRC
-		break;
-#else
-		goto bad_config;
-#endif
-	case MUSB_OTG:
-#ifdef CONFIG_USB_MUSB_OTG
-		break;
-#else
-bad_config:
-#endif
-	default:
-		dev_err(dev, "incompatible Kconfig role setting\n");
-		status = -EINVAL;
-		goto fail0;
-	}
-
 	/* allocate */
 	musb = allocate_instance(dev, plat->config, ctrl);
 	if (!musb) {
-- 
1.7.4.rc2

[patch v2.6.39 2/3] usb: musb: gadget: beautify usb_gadget_probe_driver()/usb_gadget_unregister_driver

[Felipe Balbi]

Just a few cosmetic fixes to usb_gadget_probe_driver()
and usb_gadget_unregister_driver().

Decreased a few indentation levels with goto statements.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/usb/musb/musb_gadget.c |  150 +++++++++++++++++++++-------------------
 1 files changed, 79 insertions(+), 71 deletions(-)

diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index 2fe3046..86decba 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -1801,90 +1801,95 @@ void musb_gadget_cleanup(struct musb *musb)
 int usb_gadget_probe_driver(struct usb_gadget_driver *driver,
 		int (*bind)(struct usb_gadget *))
 {
-	int retval;
-	unsigned long flags;
-	struct musb *musb = the_gadget;
+	struct musb		*musb = the_gadget;
+	unsigned long		flags;
+	int			retval = -EINVAL;
 
 	if (!driver
 			|| driver->speed != USB_SPEED_HIGH
 			|| !bind || !driver->setup)
-		return -EINVAL;
+		goto err0;
 
 	/* driver must be initialized to support peripheral mode */
 	if (!musb) {
 		DBG(1, "%s, no dev??\n", __func__);
-		return -ENODEV;
+		retval = -ENODEV;
+		goto err0;
 	}
 
 	DBG(3, "registering driver %s\n", driver->function);
-	spin_lock_irqsave(&musb->lock, flags);
 
 	if (musb->gadget_driver) {
 		DBG(1, "%s is already bound to %s\n",
 				musb_driver_name,
 				musb->gadget_driver->driver.name);
 		retval = -EBUSY;
-	} else {
-		musb->gadget_driver = driver;
-		musb->g.dev.driver = &driver->driver;
-		driver->driver.bus = NULL;
-		musb->softconnect = 1;
-		retval = 0;
+		goto err0;
 	}
 
+	spin_lock_irqsave(&musb->lock, flags);
+	musb->gadget_driver = driver;
+	musb->g.dev.driver = &driver->driver;
+	driver->driver.bus = NULL;
+	musb->softconnect = 1;
 	spin_unlock_irqrestore(&musb->lock, flags);
 
-	if (retval == 0) {
-		retval = bind(&musb->g);
-		if (retval != 0) {
-			DBG(3, "bind to driver %s failed --> %d\n",
-					driver->driver.name, retval);
-			musb->gadget_driver = NULL;
-			musb->g.dev.driver = NULL;
-		}
+	retval = bind(&musb->g);
+	if (retval) {
+		DBG(3, "bind to driver %s failed --> %d\n",
+				driver->driver.name, retval);
+		goto err1;
+	}
 
-		spin_lock_irqsave(&musb->lock, flags);
+	spin_lock_irqsave(&musb->lock, flags);
 
-		otg_set_peripheral(musb->xceiv, &musb->g);
-		musb->xceiv->state = OTG_STATE_B_IDLE;
-		musb->is_active = 1;
+	otg_set_peripheral(musb->xceiv, &musb->g);
+	musb->xceiv->state = OTG_STATE_B_IDLE;
+	musb->is_active = 1;
 
-		/* FIXME this ignores the softconnect flag.  Drivers are
-		 * allowed hold the peripheral inactive until for example
-		 * userspace hooks up printer hardware or DSP codecs, so
-		 * hosts only see fully functional devices.
-		 */
+	/*
+	 * FIXME this ignores the softconnect flag.  Drivers are
+	 * allowed hold the peripheral inactive until for example
+	 * userspace hooks up printer hardware or DSP codecs, so
+	 * hosts only see fully functional devices.
+	 */
 
-		if (!is_otg_enabled(musb))
-			musb_start(musb);
+	if (!is_otg_enabled(musb))
+		musb_start(musb);
 
-		otg_set_peripheral(musb->xceiv, &musb->g);
+	otg_set_peripheral(musb->xceiv, &musb->g);
 
-		spin_unlock_irqrestore(&musb->lock, flags);
+	spin_unlock_irqrestore(&musb->lock, flags);
 
-		if (is_otg_enabled(musb)) {
-			struct usb_hcd	*hcd = musb_to_hcd(musb);
+	if (is_otg_enabled(musb)) {
+		struct usb_hcd	*hcd = musb_to_hcd(musb);
 
-			DBG(3, "OTG startup...\n");
+		DBG(3, "OTG startup...\n");
 
-			/* REVISIT:  funcall to other code, which also
-			 * handles power budgeting ... this way also
-			 * ensures HdrcStart is indirectly called.
-			 */
-			retval = usb_add_hcd(musb_to_hcd(musb), -1, 0);
-			if (retval < 0) {
-				DBG(1, "add_hcd failed, %d\n", retval);
-				spin_lock_irqsave(&musb->lock, flags);
-				otg_set_peripheral(musb->xceiv, NULL);
-				musb->gadget_driver = NULL;
-				musb->g.dev.driver = NULL;
-				spin_unlock_irqrestore(&musb->lock, flags);
-			} else {
-				hcd->self.uses_pio_for_control = 1;
-			}
+		/* REVISIT:  funcall to other code, which also
+		 * handles power budgeting ... this way also
+		 * ensures HdrcStart is indirectly called.
+		 */
+		retval = usb_add_hcd(musb_to_hcd(musb), -1, 0);
+		if (retval < 0) {
+			DBG(1, "add_hcd failed, %d\n", retval);
+			goto err2;
 		}
+
+		hcd->self.uses_pio_for_control = 1;
 	}
 
+	return 0;
+
+err2:
+	if (!is_otg_enabled(musb))
+		musb_stop(musb);
+
+err1:
+	musb->gadget_driver = NULL;
+	musb->g.dev.driver = NULL;
+
+err0:
 	return retval;
 }
 EXPORT_SYMBOL(usb_gadget_probe_driver);
@@ -1939,14 +1944,17 @@ static void stop_activity(struct musb *musb, struct usb_gadget_driver *driver)
  */
 int usb_gadget_unregister_driver(struct usb_gadget_driver *driver)
 {
-	unsigned long	flags;
-	int		retval = 0;
 	struct musb	*musb = the_gadget;
+	unsigned long	flags;
 
 	if (!driver || !driver->unbind || !musb)
 		return -EINVAL;
 
-	/* REVISIT always use otg_set_peripheral() here too;
+	if (!musb->gadget_driver)
+		return -EINVAL;
+
+	/*
+	 * REVISIT always use otg_set_peripheral() here too;
 	 * this needs to shut down the OTG engine.
 	 */
 
@@ -1956,29 +1964,26 @@ int usb_gadget_unregister_driver(struct usb_gadget_driver *driver)
 	musb_hnp_stop(musb);
 #endif
 
-	if (musb->gadget_driver == driver) {
+	(void) musb_gadget_vbus_draw(&musb->g, 0);
 
-		(void) musb_gadget_vbus_draw(&musb->g, 0);
+	musb->xceiv->state = OTG_STATE_UNDEFINED;
+	stop_activity(musb, driver);
+	otg_set_peripheral(musb->xceiv, NULL);
 
-		musb->xceiv->state = OTG_STATE_UNDEFINED;
-		stop_activity(musb, driver);
-		otg_set_peripheral(musb->xceiv, NULL);
+	DBG(3, "unregistering driver %s\n", driver->function);
 
-		DBG(3, "unregistering driver %s\n", driver->function);
-		spin_unlock_irqrestore(&musb->lock, flags);
-		driver->unbind(&musb->g);
-		spin_lock_irqsave(&musb->lock, flags);
+	spin_unlock_irqrestore(&musb->lock, flags);
+	driver->unbind(&musb->g);
+	spin_lock_irqsave(&musb->lock, flags);
 
-		musb->gadget_driver = NULL;
-		musb->g.dev.driver = NULL;
+	musb->gadget_driver = NULL;
+	musb->g.dev.driver = NULL;
 
-		musb->is_active = 0;
-		musb_platform_try_idle(musb, 0);
-	} else
-		retval = -EINVAL;
+	musb->is_active = 0;
+	musb_platform_try_idle(musb, 0);
 	spin_unlock_irqrestore(&musb->lock, flags);
 
-	if (is_otg_enabled(musb) && retval == 0) {
+	if (is_otg_enabled(musb)) {
 		usb_remove_hcd(musb_to_hcd(musb));
 		/* FIXME we need to be able to register another
 		 * gadget driver here and have everything work;
@@ -1986,7 +1991,10 @@ int usb_gadget_unregister_driver(struct usb_gadget_driver *driver)
 		 */
 	}
 
-	return retval;
+	if (!is_otg_enabled(musb))
+		musb_stop(musb);
+
+	return 0;
 }
 EXPORT_SYMBOL(usb_gadget_unregister_driver);
 
-- 
1.7.4.rc2

Re: [patch v2.6.39 2/3] usb: musb: gadget: beautify usb_gadget_probe_driver()/usb_gadget_unregister_driver

[Sergei Shtylyov]

Hello.

On 17-02-2011 14:30, Felipe Balbi wrote:

> Just a few cosmetic fixes to usb_gadget_probe_driver()
> and usb_gadget_unregister_driver().

> Decreased a few indentation levels with goto statements.

> Signed-off-by: Felipe Balbi<balbi-l0cyMroinI0@public.gmane.org>
[...]

> diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
> index 2fe3046..86decba 100644
> --- a/drivers/usb/musb/musb_gadget.c
> +++ b/drivers/usb/musb/musb_gadget.c
> @@ -1801,90 +1801,95 @@ void musb_gadget_cleanup(struct musb *musb)
>   int usb_gadget_probe_driver(struct usb_gadget_driver *driver,
>   		int (*bind)(struct usb_gadget *))
>   {
> -	int retval;
> -	unsigned long flags;
> -	struct musb *musb = the_gadget;
> +	struct musb		*musb = the_gadget;
> +	unsigned long		flags;
> +	int			retval = -EINVAL;
>
>   	if (!driver
>   			|| driver->speed != USB_SPEED_HIGH
>   			|| !bind || !driver->setup)
> -		return -EINVAL;
> +		goto err0;

    Not sure what this gains...

>
>   	/* driver must be initialized to support peripheral mode */
>   	if (!musb) {
>   		DBG(1, "%s, no dev??\n", __func__);
> -		return -ENODEV;
> +		retval = -ENODEV;
> +		goto err0;

    This too...

>   	}
>
>   	DBG(3, "registering driver %s\n", driver->function);
> -	spin_lock_irqsave(&musb->lock, flags);

    Is it really correct to remove spinlocm protection here?

>   	if (musb->gadget_driver) {
>   		DBG(1, "%s is already bound to %s\n",
>   				musb_driver_name,
>   				musb->gadget_driver->driver.name);
>   		retval = -EBUSY;
> -	} else {
> -		musb->gadget_driver = driver;
> -		musb->g.dev.driver =&driver->driver;
> -		driver->driver.bus = NULL;
> -		musb->softconnect = 1;
> -		retval = 0;
> +		goto err0;
>   	}
>
> +	spin_lock_irqsave(&musb->lock, flags);
> +	musb->gadget_driver = driver;
> +	musb->g.dev.driver =&driver->driver;
> +	driver->driver.bus = NULL;
> +	musb->softconnect = 1;
>   	spin_unlock_irqrestore(&musb->lock, flags);
>
> -	if (retval == 0) {

    Yeah, I have noticed this too and wanted to change it -- but forgot... :-)

[...]

> -		if (is_otg_enabled(musb)) {
> -			struct usb_hcd	*hcd = musb_to_hcd(musb);
> +	if (is_otg_enabled(musb)) {
> +		struct usb_hcd	*hcd = musb_to_hcd(musb);
>
> -			DBG(3, "OTG startup...\n");
> +		DBG(3, "OTG startup...\n");
>
> -			/* REVISIT:  funcall to other code, which also
> -			 * handles power budgeting ... this way also
> -			 * ensures HdrcStart is indirectly called.
> -			 */
> -			retval = usb_add_hcd(musb_to_hcd(musb), -1, 0);
> -			if (retval<  0) {
> -				DBG(1, "add_hcd failed, %d\n", retval);
> -				spin_lock_irqsave(&musb->lock, flags);
> -				otg_set_peripheral(musb->xceiv, NULL);

    You're removing this call -- is it intentional?

> -				musb->gadget_driver = NULL;
> -				musb->g.dev.driver = NULL;
> -				spin_unlock_irqrestore(&musb->lock, flags);
> -			} else {
> -				hcd->self.uses_pio_for_control = 1;
> -			}
> +		/* REVISIT:  funcall to other code, which also
> +		 * handles power budgeting ... this way also
> +		 * ensures HdrcStart is indirectly called.
> +		 */
> +		retval = usb_add_hcd(musb_to_hcd(musb), -1, 0);
> +		if (retval < 0) {
> +			DBG(1, "add_hcd failed, %d\n", retval);
> +			goto err2;
>   		}
> +
> +		hcd->self.uses_pio_for_control = 1;
>   	}
>
> +	return 0;
> +
> +err2:
> +	if (!is_otg_enabled(musb))
> +		musb_stop(musb);

    Hm, this wasn't called before -- you don't describe why it's needed.

> +
> +err1:
> +	musb->gadget_driver = NULL;
> +	musb->g.dev.driver = NULL;
> +
> +err0:
>   	return retval;
>   }
>   EXPORT_SYMBOL(usb_gadget_probe_driver);
> @@ -1939,14 +1944,17 @@ static void stop_activity(struct musb *musb, struct usb_gadget_driver *driver)
>    */
>   int usb_gadget_unregister_driver(struct usb_gadget_driver *driver)
>   {
> -	unsigned long	flags;
> -	int		retval = 0;
>   	struct musb	*musb = the_gadget;
> +	unsigned long	flags;
>
>   	if (!driver || !driver->unbind || !musb)
>   		return -EINVAL;
>
> -	/* REVISIT always use otg_set_peripheral() here too;
> +	if (!musb->gadget_driver)
> +		return -EINVAL;
> +
> +	/*
> +	 * REVISIT always use otg_set_peripheral() here too;
>   	 * this needs to shut down the OTG engine.
>   	 */
>
> @@ -1956,29 +1964,26 @@ int usb_gadget_unregister_driver(struct usb_gadget_driver *driver)
>   	musb_hnp_stop(musb);
>   #endif
>
> -	if (musb->gadget_driver == driver) {
> +	(void) musb_gadget_vbus_draw(&musb->g, 0);
>
> -		(void) musb_gadget_vbus_draw(&musb->g, 0);
> +	musb->xceiv->state = OTG_STATE_UNDEFINED;
> +	stop_activity(musb, driver);
> +	otg_set_peripheral(musb->xceiv, NULL);
>
> -		musb->xceiv->state = OTG_STATE_UNDEFINED;
> -		stop_activity(musb, driver);
> -		otg_set_peripheral(musb->xceiv, NULL);
> +	DBG(3, "unregistering driver %s\n", driver->function);
>
> -		DBG(3, "unregistering driver %s\n", driver->function);
> -		spin_unlock_irqrestore(&musb->lock, flags);
> -		driver->unbind(&musb->g);
> -		spin_lock_irqsave(&musb->lock, flags);
> +	spin_unlock_irqrestore(&musb->lock, flags);
> +	driver->unbind(&musb->g);
> +	spin_lock_irqsave(&musb->lock, flags);
>
> -		musb->gadget_driver = NULL;
> -		musb->g.dev.driver = NULL;
> +	musb->gadget_driver = NULL;
> +	musb->g.dev.driver = NULL;
>
> -		musb->is_active = 0;
> -		musb_platform_try_idle(musb, 0);
> -	} else
> -		retval = -EINVAL;
> +	musb->is_active = 0;
> +	musb_platform_try_idle(musb, 0);
>   	spin_unlock_irqrestore(&musb->lock, flags);
>
> -	if (is_otg_enabled(musb)&&  retval == 0) {
> +	if (is_otg_enabled(musb)) {
>   		usb_remove_hcd(musb_to_hcd(musb));
>   		/* FIXME we need to be able to register another
>   		 * gadget driver here and have everything work;
> @@ -1986,7 +1991,10 @@ int usb_gadget_unregister_driver(struct usb_gadget_driver *driver)
>   		 */
>   	}
>
> -	return retval;
> +	if (!is_otg_enabled(musb))
> +		musb_stop(musb);

    Wasn't called before either -- you don't describe why it's needed...

> +
> +	return 0;
>   }
>   EXPORT_SYMBOL(usb_gadget_unregister_driver);

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

Re: [patch v2.6.39 2/3] usb: musb: gadget: beautify usb_gadget_probe_driver()/usb_gadget_unregister_driver

[Felipe Balbi]

Hi,

On Thu, Feb 17, 2011 at 04:00:30PM +0300, Sergei Shtylyov wrote:
> >diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
> >index 2fe3046..86decba 100644
> >--- a/drivers/usb/musb/musb_gadget.c
> >+++ b/drivers/usb/musb/musb_gadget.c
> >@@ -1801,90 +1801,95 @@ void musb_gadget_cleanup(struct musb *musb)
> >  int usb_gadget_probe_driver(struct usb_gadget_driver *driver,
> >  		int (*bind)(struct usb_gadget *))
> >  {
> >-	int retval;
> >-	unsigned long flags;
> >-	struct musb *musb = the_gadget;
> >+	struct musb		*musb = the_gadget;
> >+	unsigned long		flags;
> >+	int			retval = -EINVAL;
> >
> >  	if (!driver
> >  			|| driver->speed != USB_SPEED_HIGH
> >  			|| !bind || !driver->setup)
> >-		return -EINVAL;
> >+		goto err0;
> 
>    Not sure what this gains...
> 
> >
> >  	/* driver must be initialized to support peripheral mode */
> >  	if (!musb) {
> >  		DBG(1, "%s, no dev??\n", __func__);
> >-		return -ENODEV;
> >+		retval = -ENODEV;
> >+		goto err0;
> 
>    This too...

I don't like the way it is today, and IMO it looks cleaner to have a
error path with goto statements so that we have only one path for error
handling.

> >  	DBG(3, "registering driver %s\n", driver->function);
> >-	spin_lock_irqsave(&musb->lock, flags);
> 
>    Is it really correct to remove spinlocm protection here?

I'm guessing it won't have any problem. I'm only reading
musb->gadget_driver...

> 
> >  	if (musb->gadget_driver) {
> >  		DBG(1, "%s is already bound to %s\n",
> >  				musb_driver_name,
> >  				musb->gadget_driver->driver.name);
> >  		retval = -EBUSY;
> >-	} else {
> >-		musb->gadget_driver = driver;
> >-		musb->g.dev.driver =&driver->driver;
> >-		driver->driver.bus = NULL;
> >-		musb->softconnect = 1;
> >-		retval = 0;
> >+		goto err0;
> >  	}
> >
> >+	spin_lock_irqsave(&musb->lock, flags);
> >+	musb->gadget_driver = driver;
> >+	musb->g.dev.driver =&driver->driver;
> >+	driver->driver.bus = NULL;
> >+	musb->softconnect = 1;
> >  	spin_unlock_irqrestore(&musb->lock, flags);
> >
> >-	if (retval == 0) {
> 
>    Yeah, I have noticed this too and wanted to change it -- but forgot... :-)

tell me about it. Has been on my TODO list for ages :-p

> >-		if (is_otg_enabled(musb)) {
> >-			struct usb_hcd	*hcd = musb_to_hcd(musb);
> >+	if (is_otg_enabled(musb)) {
> >+		struct usb_hcd	*hcd = musb_to_hcd(musb);
> >
> >-			DBG(3, "OTG startup...\n");
> >+		DBG(3, "OTG startup...\n");
> >
> >-			/* REVISIT:  funcall to other code, which also
> >-			 * handles power budgeting ... this way also
> >-			 * ensures HdrcStart is indirectly called.
> >-			 */
> >-			retval = usb_add_hcd(musb_to_hcd(musb), -1, 0);
> >-			if (retval<  0) {
> >-				DBG(1, "add_hcd failed, %d\n", retval);
> >-				spin_lock_irqsave(&musb->lock, flags);
> >-				otg_set_peripheral(musb->xceiv, NULL);
> 
>    You're removing this call -- is it intentional?

Default state is already peripheral anyway, so there's no point in
having that.

> 
> >-				musb->gadget_driver = NULL;
> >-				musb->g.dev.driver = NULL;
> >-				spin_unlock_irqrestore(&musb->lock, flags);
> >-			} else {
> >-				hcd->self.uses_pio_for_control = 1;
> >-			}
> >+		/* REVISIT:  funcall to other code, which also
> >+		 * handles power budgeting ... this way also
> >+		 * ensures HdrcStart is indirectly called.
> >+		 */
> >+		retval = usb_add_hcd(musb_to_hcd(musb), -1, 0);
> >+		if (retval < 0) {
> >+			DBG(1, "add_hcd failed, %d\n", retval);
> >+			goto err2;
> >  		}
> >+
> >+		hcd->self.uses_pio_for_control = 1;
> >  	}
> >
> >+	return 0;
> >+
> >+err2:
> >+	if (!is_otg_enabled(musb))
> >+		musb_stop(musb);
> 
>    Hm, this wasn't called before -- you don't describe why it's needed.

I can update the commit log, but basically it's the counterpart of
musb_start(). See that we call:

if (!is_otg_enabled(musb))
	musb_start(musb);

so we need to undo that on the error path.

-- 
balbi

Re: [patch v2.6.39 2/3] usb: musb: gadget: beautify usb_gadget_probe_driver()/usb_gadget_unregister_driver

[Felipe Balbi]

hi,

On Thu, Feb 17, 2011 at 03:15:10PM +0200, Felipe Balbi wrote:
> I can update the commit log, but basically it's the counterpart of
> musb_start(). See that we call:
> 
> if (!is_otg_enabled(musb))
> 	musb_start(musb);
> 
> so we need to undo that on the error path.

Better ?

usb: musb: gadget: beautify usb_gadget_probe_driver()/usb_gadget_unregister_driver

Just a few cosmetic fixes to usb_gadget_probe_driver()
and usb_gadget_unregister_driver().

Decreased a few indentation levels with goto statements.

While at that, also add the missing call to musb_stop().
If we don't have OTG, there's no point of leaving
MUSB prepared for operation if a gadget driver fails
to probe. The same is valid for usb_gadget_unregister_driver(),
since we are removing the gadget driver and we don't have
OTG, we can completely unconfigure MUSB.

Signed-off-by: Felipe Balbi <balbi@ti.com>


-- 
balbi

[patch v2.6.39 3/3] usb: musb: gadget: do not poke with gadget's list_head

[Felipe Balbi]

struct usb_request's list_head is supposed to be
used only by gadget drivers, but musb is abusing
that. Give struct musb_request its own list_head
and prevent musb from poking into other driver's
business.

Signed-off-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
---
 drivers/usb/musb/musb_core.h       |    4 +-
 drivers/usb/musb/musb_gadget.c     |   37 +++++++++++++++++++----------------
 drivers/usb/musb/musb_gadget.h     |    7 ++++-
 drivers/usb/musb/musb_gadget_ep0.c |   24 +++++++++++++---------
 4 files changed, 41 insertions(+), 31 deletions(-)

diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index d74a811..2c8dde6 100644
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -328,7 +328,7 @@ struct musb_hw_ep {
 #endif
 };
 
-static inline struct usb_request *next_in_request(struct musb_hw_ep *hw_ep)
+static inline struct musb_request *next_in_request(struct musb_hw_ep *hw_ep)
 {
 #ifdef CONFIG_USB_GADGET_MUSB_HDRC
 	return next_request(&hw_ep->ep_in);
@@ -337,7 +337,7 @@ static inline struct usb_request *next_in_request(struct musb_hw_ep *hw_ep)
 #endif
 }
 
-static inline struct usb_request *next_out_request(struct musb_hw_ep *hw_ep)
+static inline struct musb_request *next_out_request(struct musb_hw_ep *hw_ep)
 {
 #ifdef CONFIG_USB_GADGET_MUSB_HDRC
 	return next_request(&hw_ep->ep_out);
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index 86decba..0f59bf9 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -189,7 +189,7 @@ __acquires(ep->musb->lock)
 
 	req = to_musb_request(request);
 
-	list_del(&request->list);
+	list_del(&req->list);
 	if (req->request.status == -EINPROGRESS)
 		req->request.status = status;
 	musb = req->musb;
@@ -251,9 +251,8 @@ static void nuke(struct musb_ep *ep, const int status)
 		ep->dma = NULL;
 	}
 
-	while (!list_empty(&(ep->req_list))) {
-		req = container_of(ep->req_list.next, struct musb_request,
-				request.list);
+	while (!list_empty(&ep->req_list)) {
+		req = list_first_entry(&ep->req_list, struct musb_request, list);
 		musb_g_giveback(ep, &req->request, status);
 	}
 }
@@ -485,6 +484,7 @@ static void txstate(struct musb *musb, struct musb_request *req)
 void musb_g_tx(struct musb *musb, u8 epnum)
 {
 	u16			csr;
+	struct musb_request	*req;
 	struct usb_request	*request;
 	u8 __iomem		*mbase = musb->mregs;
 	struct musb_ep		*musb_ep = &musb->endpoints[epnum].ep_in;
@@ -492,7 +492,8 @@ void musb_g_tx(struct musb *musb, u8 epnum)
 	struct dma_channel	*dma;
 
 	musb_ep_select(mbase, epnum);
-	request = next_request(musb_ep);
+	req = next_request(musb_ep);
+	request = &req->request;
 
 	csr = musb_readw(epio, MUSB_TXCSR);
 	DBG(4, "<== %s, txcsr %04x\n", musb_ep->end_point.name, csr);
@@ -571,15 +572,15 @@ void musb_g_tx(struct musb *musb, u8 epnum)
 
 		if (request->actual == request->length) {
 			musb_g_giveback(musb_ep, request, 0);
-			request = musb_ep->desc ? next_request(musb_ep) : NULL;
-			if (!request) {
+			req = musb_ep->desc ? next_request(musb_ep) : NULL;
+			if (!req) {
 				DBG(4, "%s idle now\n",
 					musb_ep->end_point.name);
 				return;
 			}
 		}
 
-		txstate(musb, to_musb_request(request));
+		txstate(musb, req);
 	}
 }
 
@@ -821,6 +822,7 @@ static void rxstate(struct musb *musb, struct musb_request *req)
 void musb_g_rx(struct musb *musb, u8 epnum)
 {
 	u16			csr;
+	struct musb_request	*req;
 	struct usb_request	*request;
 	void __iomem		*mbase = musb->mregs;
 	struct musb_ep		*musb_ep;
@@ -835,10 +837,12 @@ void musb_g_rx(struct musb *musb, u8 epnum)
 
 	musb_ep_select(mbase, epnum);
 
-	request = next_request(musb_ep);
-	if (!request)
+	req = next_request(musb_ep);
+	if (!req)
 		return;
 
+	request = &req->request;
+
 	csr = musb_readw(epio, MUSB_RXCSR);
 	dma = is_dma_capable() ? musb_ep->dma : NULL;
 
@@ -914,15 +918,15 @@ void musb_g_rx(struct musb *musb, u8 epnum)
 #endif
 		musb_g_giveback(musb_ep, request, 0);
 
-		request = next_request(musb_ep);
-		if (!request)
+		req = next_request(musb_ep);
+		if (!req)
 			return;
 	}
 #if defined(CONFIG_USB_INVENTRA_DMA) || defined(CONFIG_USB_TUSB_OMAP_DMA)
 exit:
 #endif
 	/* Analyze request */
-	rxstate(musb, to_musb_request(request));
+	rxstate(musb, req);
 }
 
 /* ------------------------------------------------------------ */
@@ -1171,7 +1175,6 @@ struct usb_request *musb_alloc_request(struct usb_ep *ep, gfp_t gfp_flags)
 		return NULL;
 	}
 
-	INIT_LIST_HEAD(&request->request.list);
 	request->request.dma = DMA_ADDR_INVALID;
 	request->epnum = musb_ep->current_epnum;
 	request->ep = musb_ep;
@@ -1257,10 +1260,10 @@ static int musb_gadget_queue(struct usb_ep *ep, struct usb_request *req,
 	}
 
 	/* add request to the list */
-	list_add_tail(&(request->request.list), &(musb_ep->req_list));
+	list_add_tail(&request->list, &musb_ep->req_list);
 
 	/* it this is the head of the queue, start i/o ... */
-	if (!musb_ep->busy && &request->request.list == musb_ep->req_list.next)
+	if (!musb_ep->busy && &request->list == musb_ep->req_list.next)
 		musb_ep_restart(musb, request);
 
 cleanup:
@@ -1349,7 +1352,7 @@ static int musb_gadget_set_halt(struct usb_ep *ep, int value)
 
 	musb_ep_select(mbase, epnum);
 
-	request = to_musb_request(next_request(musb_ep));
+	request = next_request(musb_ep);
 	if (value) {
 		if (request) {
 			DBG(3, "request in progress, cannot halt %s\n",
diff --git a/drivers/usb/musb/musb_gadget.h b/drivers/usb/musb/musb_gadget.h
index a55354f..66b7c5e 100644
--- a/drivers/usb/musb/musb_gadget.h
+++ b/drivers/usb/musb/musb_gadget.h
@@ -35,6 +35,8 @@
 #ifndef __MUSB_GADGET_H
 #define __MUSB_GADGET_H
 
+#include <linux/list.h>
+
 enum buffer_map_state {
 	UN_MAPPED = 0,
 	PRE_MAPPED,
@@ -43,6 +45,7 @@ enum buffer_map_state {
 
 struct musb_request {
 	struct usb_request	request;
+	struct list_head	list;
 	struct musb_ep		*ep;
 	struct musb		*musb;
 	u8 tx;			/* endpoint direction */
@@ -94,13 +97,13 @@ static inline struct musb_ep *to_musb_ep(struct usb_ep *ep)
 	return ep ? container_of(ep, struct musb_ep, end_point) : NULL;
 }
 
-static inline struct usb_request *next_request(struct musb_ep *ep)
+static inline struct musb_request *next_request(struct musb_ep *ep)
 {
 	struct list_head	*queue = &ep->req_list;
 
 	if (list_empty(queue))
 		return NULL;
-	return container_of(queue->next, struct usb_request, list);
+	return container_of(queue->next, struct musb_request, list);
 }
 
 extern void musb_g_tx(struct musb *musb, u8 epnum);
diff --git a/drivers/usb/musb/musb_gadget_ep0.c b/drivers/usb/musb/musb_gadget_ep0.c
index 6dd03f4..75a542e 100644
--- a/drivers/usb/musb/musb_gadget_ep0.c
+++ b/drivers/usb/musb/musb_gadget_ep0.c
@@ -304,8 +304,7 @@ __acquires(musb->lock)
 				}
 
 				/* Maybe start the first request in the queue */
-				request = to_musb_request(
-						next_request(musb_ep));
+				request = next_request(musb_ep);
 				if (!musb_ep->busy && request) {
 					DBG(3, "restarting the request\n");
 					musb_ep_restart(musb, request);
@@ -491,10 +490,12 @@ stall:
 static void ep0_rxstate(struct musb *musb)
 {
 	void __iomem		*regs = musb->control_ep->regs;
+	struct musb_request	*request;
 	struct usb_request	*req;
 	u16			count, csr;
 
-	req = next_ep0_request(musb);
+	request = next_ep0_request(musb);
+	req = &request->request;
 
 	/* read packet and ack; or stall because of gadget driver bug:
 	 * should have provided the rx buffer before setup() returned.
@@ -544,17 +545,20 @@ static void ep0_rxstate(struct musb *musb)
 static void ep0_txstate(struct musb *musb)
 {
 	void __iomem		*regs = musb->control_ep->regs;
-	struct usb_request	*request = next_ep0_request(musb);
+	struct musb_request	*req = next_ep0_request(musb);
+	struct usb_request	*request;
 	u16			csr = MUSB_CSR0_TXPKTRDY;
 	u8			*fifo_src;
 	u8			fifo_count;
 
-	if (!request) {
+	if (!req) {
 		/* WARN_ON(1); */
 		DBG(2, "odd; csr0 %04x\n", musb_readw(regs, MUSB_CSR0));
 		return;
 	}
 
+	request = &req->request;
+
 	/* load the data */
 	fifo_src = (u8 *) request->buf + request->actual;
 	fifo_count = min((unsigned) MUSB_EP0_FIFOSIZE,
@@ -598,7 +602,7 @@ static void ep0_txstate(struct musb *musb)
 static void
 musb_read_setup(struct musb *musb, struct usb_ctrlrequest *req)
 {
-	struct usb_request	*r;
+	struct musb_request	*r;
 	void __iomem		*regs = musb->control_ep->regs;
 
 	musb_read_fifo(&musb->endpoints[0], sizeof *req, (u8 *)req);
@@ -616,7 +620,7 @@ musb_read_setup(struct musb *musb, struct usb_ctrlrequest *req)
 	/* clean up any leftover transfers */
 	r = next_ep0_request(musb);
 	if (r)
-		musb_g_ep0_giveback(musb, r);
+		musb_g_ep0_giveback(musb, &r->request);
 
 	/* For zero-data requests we want to delay the STATUS stage to
 	 * avoid SETUPEND errors.  If we read data (OUT), delay accepting
@@ -758,11 +762,11 @@ irqreturn_t musb_g_ep0_irq(struct musb *musb)
 	case MUSB_EP0_STAGE_STATUSOUT:
 		/* end of sequence #1: write to host (TX state) */
 		{
-			struct usb_request	*req;
+			struct musb_request	*req;
 
 			req = next_ep0_request(musb);
 			if (req)
-				musb_g_ep0_giveback(musb, req);
+				musb_g_ep0_giveback(musb, &req->request);
 		}
 
 		/*
@@ -961,7 +965,7 @@ musb_g_ep0_queue(struct usb_ep *e, struct usb_request *r, gfp_t gfp_flags)
 	}
 
 	/* add request to the list */
-	list_add_tail(&(req->request.list), &(ep->req_list));
+	list_add_tail(&req->list, &ep->req_list);
 
 	DBG(3, "queue to %s (%s), length=%d\n",
 			ep->name, ep->is_in ? "IN/TX" : "OUT/RX",
-- 
1.7.4.rc2

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