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 linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
