Hi, On 08/06/16 12:53, Peter Chen wrote: > On Wed, Jun 08, 2016 at 12:03:40PM +0300, Roger Quadros wrote: >> +int usb_otg_unregister(struct device *dev) >> +{ >> + struct usb_otg *otg; >> + >> + mutex_lock(&otg_list_mutex); >> + otg = usb_otg_get_data(dev); >> + if (!otg) { >> + dev_err(dev, "otg: %s: device not in otg list\n", >> + __func__); >> + mutex_unlock(&otg_list_mutex); >> + return -EINVAL; >> + } >> + >> + /* prevent unregister till both host & gadget have unregistered */ >> + if (otg->host || otg->gadget) { >> + dev_err(dev, "otg: %s: host/gadget still registered\n", >> + __func__); > > You need to call mutex_unlock here Indeed. good catch. > >> + >> +int usb_otg_gadget_ready(struct usb_gadget *gadget, bool ready) >> +{ > > What this API is for? Why need it in this version? we moved gadget to otg registration from udc_bind_to_driver() to usb_add_gadget_udc_release(). This means there is a window when gadget function driver (e.g. g_zero) is not loaded. We don't want to start the gadget controller in that window. usb_otg_gadget_ready() is used by gadget core to notify the otg core when the function driver is ready or not-ready. > >> + struct usb_otg *otg; >> + struct device *gadget_dev = &gadget->dev; >> + struct device *otg_dev = gadget->otg_dev; >> + >> + if (!otg_dev) >> + return -EINVAL; >> + >> + mutex_lock(&otg_list_mutex); >> + otg = usb_otg_get_data(otg_dev); >> + mutex_unlock(&otg_list_mutex); >> + if (!otg) { >> + dev_err(gadget_dev, >> + "otg: gadget %s wasn't registered with otg\n", >> + dev_name(&gadget->dev)); >> + return -EINVAL; >> + } >> + >> + mutex_lock(&otg->fsm.lock); >> + if (otg->gadget != gadget) { >> + mutex_unlock(&otg->fsm.lock); >> + dev_err(otg_dev, "otg: gadget %s wasn't registered with otg\n", >> + dev_name(&gadget->dev)); >> + return -EINVAL; >> + } >> + >> + /* Start/stop FSM & gadget */ >> + otg->gadget_ready = ready; >> + if (ready) >> + usb_otg_start_fsm(otg); >> + else >> + usb_otg_stop_fsm(otg); >> + >> + dev_dbg(otg_dev, "otg: gadget %s %sready\n", dev_name(&gadget->dev), >> + ready ? "" : "not "); >> + >> + mutex_unlock(&otg->fsm.lock); >> + >> + return 0; >> +} >> +EXPORT_SYMBOL_GPL(usb_otg_gadget_ready); >> + >> --- a/include/linux/usb/otg.h >> +++ b/include/linux/usb/otg.h >> @@ -10,10 +10,70 @@ >> #define __LINUX_USB_OTG_H >> >> #include <linux/phy/phy.h> >> -#include <linux/usb/phy.h> >> -#include <linux/usb/otg-fsm.h> >> +#include <linux/device.h> >> +#include <linux/hrtimer.h> >> +#include <linux/ktime.h> > > The above two timer header files are not needed. You had pointed this out earlier, but I missed it. Sorry. -- cheers, -roger -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html