Re: [rfc patch 3/3] usb: musb: fix problem with usb_function_deactivate()

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

 



On Monday 27 July 2009, Felipe Balbi wrote:
> musb was enabling softconnect flag only after gadget driver's
> bind() was beeing called,

In mainline I see that flag *set* (to the default, "device is
ready to respond after binding _unless_ it says otherwise")
just before the spin_lock_irqsave() at the beginning of this
patch ... i.e. *before* gadget binding.


> meaning that if a gadget driver 
> was calling usb_function_deactivate, we would never be able
> to defer gadget enumeration until usb_function_activate was
> called.

I'd have thought the issue was entirely different.  See this
comment right in the middle of the block of code you were
working with (moving bind from beginning to end of block):

                /* 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.
                 */

In short, making that mechanism work with this driver will
involve first coding it, then debugging it.  Notice how the
musb_gadget_vbus_session() mechanism isn't enabled, and how
it's just stubbed in as another big FIXME...

So, NAK on this too.


> Cc: David Brownell <dbrownell@xxxxxxxxxxxxxxxxxxxxx>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxx>
> Signed-off-by: Felipe Balbi <felipe.balbi@xxxxxxxxx>
> ---
>  drivers/usb/musb/musb_gadget.c |   16 ++++++++--------
>  1 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
> index 8b3c4e2..839f8c3 100644
> --- a/drivers/usb/musb/musb_gadget.c
> +++ b/drivers/usb/musb/musb_gadget.c
> @@ -1723,14 +1723,6 @@ int usb_gadget_register_driver(struct usb_gadget_driver *driver)
>  	spin_unlock_irqrestore(&musb->lock, flags);
>  
>  	if (retval == 0) {
> -		retval = driver->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;
> -		}
> -
>  		spin_lock_irqsave(&musb->lock, flags);
>  
>  		otg_set_peripheral(musb->xceiv, &musb->g);
> @@ -1766,6 +1758,14 @@ int usb_gadget_register_driver(struct usb_gadget_driver *driver)
>  				spin_unlock_irqrestore(&musb->lock, flags);
>  			}
>  		}
> +		retval = driver->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;
> +		}
> +
>  	}
>  
>  	return retval;
> -- 
> 1.6.3.3.385.g60647
> 
> 


--
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