Re: [PATCH v11 13/14] usb: gadget: udc: adapt to OTG core

[Roger Quadros]

On 13/06/16 10:56, Peter Chen wrote:
> On Mon, Jun 13, 2016 at 10:55:12AM +0300, Roger Quadros wrote:
>>  err1:
>> -if (ret != -EISNAM)
>> +if ((ret != -EISNAM))
> 
> Since you do not need above change.

Ah :P, will fix it.
> 
> Expect above, I am ok with this patch.
> 
> Acked-by: Peter Chen 

Thanks for the patient review of this entire series :).

cheers,
-roger
--
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

Re: [PATCH v11 13/14] usb: gadget: udc: adapt to OTG core

[Peter Chen]

On Mon, Jun 13, 2016 at 10:55:12AM +0300, Roger Quadros wrote:
>  err1:
> - if (ret != -EISNAM)
> + if ((ret != -EISNAM))

Since you do not need above change.

Expect above, I am ok with this patch.

Acked-by: Peter Chen 

>   dev_err(>dev, "failed to start %s: %d\n",
>   udc->driver->function, ret);
>   udc->driver = NULL;
> @@ -1389,6 +1565,12 @@ static ssize_t usb_udc_softconn_store(struct device 
> *dev,
>   return -EOPNOTSUPP;
>   }
>  
> + /* In OTG/dual-role mode, soft-connect should be handled by OTG core */
> + if (udc->gadget->otg_dev) {
> + dev_err(dev, "soft-connect not supported in OTG mode\n");
> + return -EOPNOTSUPP;
> + }
> +
>   if (sysfs_streq(buf, "connect")) {
>   usb_gadget_udc_start(udc);
>   usb_gadget_connect(udc->gadget);
> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
> index 1d74fb8..8c6880d 100644
> --- a/include/linux/usb/gadget.h
> +++ b/include/linux/usb/gadget.h
> @@ -701,6 +701,10 @@ extern int usb_add_gadget_udc(struct device *parent, 
> struct usb_gadget *gadget);
>  extern void usb_del_gadget_udc(struct usb_gadget *gadget);
>  extern char *usb_get_gadget_udc_name(void);
>  
> +extern int usb_otg_add_gadget_udc(struct device *parent,
> +   struct usb_gadget *gadget,
> +   struct device *otg_dev);
> +
>  /*-*/
>  
>  /* utility to simplify dealing with string descriptors */
> -- 
> 2.7.4
> 
> 
> --
> 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

-- 

Best Regards,
Peter Chen
--
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

[PATCH v11 13/14] usb: gadget: udc: adapt to OTG core

[Roger Quadros]

The OTG state machine needs a mechanism to start and
stop the gadget controller as well as connect/disconnect
from the bus. Add usb_gadget_start(), usb_gadget_stop()
and usb_gadget_connect_control().

Introduce usb_otg_add_gadget_udc() to allow controller drivers
to register a gadget controller that is part of an OTG instance.

Register with OTG core when UDC is added in usb_add_gadget_udc_release()
and unregister on usb_del_gadget_udc().

Notify the OTG core when gadget function driver is available on
udc_bind_to_driver() and when it is removed in usb_gadget_remove_driver().

We need to unlock the usb_lock mutex before calling
usb_otg_register_gadget() else it will cause a circular
locking dependency.

Ignore softconnect sysfs control when we're in OTG
mode as OTG FSM should care of gadget softconnect using
the b_bus_req mechanism.

Signed-off-by: Roger Quadros 
---
v11:
- removed left over -EPROBE_DEFER in udc_bind_to_driver().

 drivers/usb/gadget/udc/core.c | 202 +++---
 include/linux/usb/gadget.h|   4 +
 2 files changed, 196 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
index 42756d7..1290f03 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -28,6 +28,11 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+
+#include 
+#include 
 
 #include "trace.h"
 
@@ -1060,6 +1065,113 @@ static inline void usb_gadget_udc_stop(struct usb_udc 
*udc)
 }
 
 /**
+ * usb_gadget_to_udc - get the UDC owning the gadget
+ *
+ * udc_lock must be held.
+ * Returs NULL if UDC is not found.
+ */
+static struct usb_udc *usb_gadget_to_udc(struct usb_gadget *gadget)
+{
+   struct usb_udc *udc;
+
+   list_for_each_entry(udc, _list, list)
+   if (udc->gadget == gadget)
+   return udc;
+
+   return NULL;
+}
+
+/**
+ * usb_gadget_start - start the usb gadget controller
+ * @gadget: the gadget device to start
+ *
+ * This is external API for use by OTG core.
+ *
+ * Start the usb device controller. Does not connect to the bus.
+ */
+static int usb_gadget_start(struct usb_gadget *gadget)
+{
+   int ret;
+   struct usb_udc *udc;
+
+   mutex_lock(_lock);
+   udc = usb_gadget_to_udc(gadget);
+   if (!udc) {
+   dev_err(gadget->dev.parent, "%s: gadget not registered.\n",
+   __func__);
+   mutex_unlock(_lock);
+   return -EINVAL;
+   }
+
+   ret = usb_gadget_udc_start(udc);
+   if (ret)
+   dev_err(>dev, "USB Device Controller didn't start: %d\n",
+   ret);
+
+   mutex_unlock(_lock);
+
+   return ret;
+}
+
+/**
+ * usb_gadget_stop - stop the usb gadget controller
+ * @gadget: the gadget device we want to stop
+ *
+ * This is external API for use by OTG core.
+ *
+ * Stop the gadget controller. Does not disconnect from the bus.
+ * Caller must ensure that gadget has disconnected from the bus
+ * before calling usb_gadget_stop().
+ */
+static int usb_gadget_stop(struct usb_gadget *gadget)
+{
+   struct usb_udc *udc;
+
+   mutex_lock(_lock);
+   udc = usb_gadget_to_udc(gadget);
+   if (!udc) {
+   dev_err(gadget->dev.parent, "%s: gadget not registered.\n",
+   __func__);
+   mutex_unlock(_lock);
+   return -EINVAL;
+   }
+
+   if (gadget->connected)
+   dev_dbg(gadget->dev.parent,
+   "%s: called while still connected\n", __func__);
+
+   usb_gadget_udc_stop(udc);
+   mutex_unlock(_lock);
+
+   return 0;
+}
+
+static int usb_gadget_connect_control(struct usb_gadget *gadget, bool connect)
+{
+   struct usb_udc *udc;
+
+   mutex_lock(_lock);
+   udc = usb_gadget_to_udc(gadget);
+   if (!udc) {
+   dev_err(gadget->dev.parent, "%s: gadget not registered.\n",
+   __func__);
+   mutex_unlock(_lock);
+   return -EINVAL;
+   }
+
+   if (connect) {
+   usb_gadget_connect(udc->gadget);
+   } else {
+   usb_gadget_disconnect(udc->gadget);
+   udc->driver->disconnect(udc->gadget);
+   }
+
+   mutex_unlock(_lock);
+
+   return 0;
+}
+
+/**
  * usb_udc_release - release the usb_udc struct
  * @dev: the dev member within usb_udc
  *
@@ -1082,6 +1194,12 @@ static void usb_udc_nop_release(struct device *dev)
dev_vdbg(dev, "%s\n", __func__);
 }
 
+struct otg_gadget_ops otg_gadget_intf = {
+   .start = usb_gadget_start,
+   .stop = usb_gadget_stop,
+   .connect_control = usb_gadget_connect_control,
+};
+
 /**
  * usb_add_gadget_udc_release - adds a new gadget to the udc class driver list
  * @parent: the parent device to this udc. Usually the controller driver's
@@ -1137,6 +1255,14 @@ int usb_add_gadget_udc_release(struct device *parent, 
struct usb_gadget *gadget,