USB OTG/dual-role framework

The OTG core will use struct otg_hcd_ops to interface with the HCD controller.

The main purpose of this interface is to avoid directly calling HCD APIs from the OTG core as they wouldn't be defined in the built-in symbol table if CONFIG_USB is m.

Signed-off-by Roger Quadros <[email protected]>

Acked-by Peter Chen <[email protected]>

---
 include/linux/usb/hcd.h | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

include/linux/usb/hcd.h +0 -0

index 66fc137..7914bed 100644
@@ -400,6 +400,30 @@ struct hc_driver {
 
 };

Hello.

Quoted 2 lines · Roger Quadros

The OTG core will use struct otg_hcd_ops to interface with the HCD controller.

Host controller driver (HCD) controller? Maybe just HC? :-)

Quoted 20 lines

The main purpose of this interface is to avoid directly calling HCD APIs from the OTG core as they wouldn't be defined in the built-in symbol table if CONFIG_USB is m.

Signed-off-by Roger Quadros <[email protected]>

Acked-by Peter Chen <[email protected]>

---
 include/linux/usb/hcd.h | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

include/linux/usb/hcd.h +0 -0

index 66fc137..7914bed 100644
@@ -400,6 +400,30 @@ struct hc_driver {
 
 };
 

include/linux/usb/hcd.h struct hc_driver { · line 411

Quoted 9 lines

include/linux/usb/hcd.h +9 -0

+/**
+ * struct otg_hcd_ops - Interface between OTG core and HCD
+ *
+ * Provided by the HCD core to allow the OTG core to interface with the HCD
+ *
+ * @add: function to add the HCD
+ * @remove: function to remove the HCD
+ * @usb_bus_start_enum: function to immediately start bus enumeration
+ * @usb_control_msg: function to build and send of a control urb

That "of" is not needed. And it's URB.

[...]

Quoted 4 lines

Quoted 2 lines

The OTG core will use struct otg_hcd_ops to interface with the HCD controller.

Host controller driver (HCD) controller? Maybe just HC? :-)

OK.

--
cheers,
-roger

Quoted 6 lines · Roger Quadros

Quoted 4 lines

Quoted 2 lines

The OTG core will use struct otg_hcd_ops to interface with the HCD controller.

Host controller driver (HCD) controller? Maybe just HC? :-)

OK.

OTOH, my googling has shown that HCD may stand for both HC driver and HC device... The host controller device sounds a bit tautological however...

Quoted 8 lines

Quoted 6 lines

Quoted 4 lines

Quoted 2 lines

The OTG core will use struct otg_hcd_ops to interface with the HCD controller.

Host controller driver (HCD) controller? Maybe just HC? :-)

OK.

OTOH, my googling has shown that HCD may stand for both HC driver and HC device... The host controller device sounds a bit tautological however...

Yeah I always meant it to be Host Controller Device :).

--
cheers,
-roger

Hello.

Quoted 5 lines · Roger Quadros

Quoted 5 lines

Quoted 5 lines

Quoted 4 lines

Quoted 2 lines

The OTG core will use struct otg_hcd_ops to interface with the HCD controller.

Host controller driver (HCD) controller? Maybe just HC? :-)

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

OK.

OTOH, my googling has shown that HCD may stand for both HC driver and HC device... The host controller device sounds a bit tautological however...

Yeah I always meant it to be Host Controller Device :).

The question here is more what was meant by the previous Linux USB
authors. :-)

Quoted 9 lines · Sergei Shtylyov

Hello.

Quoted 5 lines · Roger Quadros

Quoted 5 lines

Quoted 5 lines

Quoted 4 lines

Quoted 2 lines

The OTG core will use struct otg_hcd_ops to interface with the HCD controller.

Host controller driver (HCD) controller? Maybe just HC? :-)

Quoted 8 lines

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

OK.

OTOH, my googling has shown that HCD may stand for both HC driver and HC device... The host controller device sounds a bit tautological however...

Yeah I always meant it to be Host Controller Device :).

The question here is more what was meant by the previous Linux USB
authors. :-)

I have always used "HCD" to mean Host Controller Driver, and I believe that's what other people have done too.

If I want to refer to a Host Controller Device, I would write either "HC" or "host controller".

Admittedly, this means that struct usb_hcd has a slightly illogical name.

drivers/usb/common/usb-otg.c new file +7 -0

+int usb_otg_unregister(struct device *dev)
+{
+	struct usb_otg *otg;
+
+	mutex_lock(&otg_list_mutex);
+	otg = usb_otg_get_data(dev);
+	if (!otg) {

drivers/usb/common/usb-otg.c new file +41 -0

+	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);
+

include/linux/usb/otg.h +166 -12

+#include <linux/usb.h>
 #include <linux/usb/hcd.h>
+#include <linux/usb/gadget.h>
+#include <linux/usb/otg-fsm.h>
+#include <linux/usb/phy.h>
+
+/**
+ * struct otg_hcd - host controller state and interface
+ *
+ * @hcd: host controller
+ * @irqnum: irq number
+ * @irqflags: irq flags
+ * @ops: otg to host controller interface
+ * @ops: otg to host controller interface
+ * @otg_dev: otg controller device
+ */
+struct otg_hcd {
+	struct usb_hcd *hcd;
+	unsigned int irqnum;
+	unsigned long irqflags;
+	struct otg_hcd_ops *ops;
+	struct device *otg_dev;
+};
+
+/**
+ * struct usb_otg_caps - describes the otg capabilities of the device
+ * @otg_rev: The OTG revision number the device is compliant with, it's
+ *		in binary-coded decimal (i.e. 2.0 is 0200H).
+ * @hnp_support: Indicates if the device supports HNP.
+ * @srp_support: Indicates if the device supports SRP.
+ * @adp_support: Indicates if the device supports ADP.
+ */
+struct usb_otg_caps {
+	u16 otg_rev;
+	bool hnp_support;
+	bool srp_support;
+	bool adp_support;
+};
 
+/**
+ * struct usb_otg - usb otg controller state
+ *
+ * @default_a: Indicates we are an A device. i.e. Host.
+ * @phy: USB phy interface
+ * @usb_phy: old usb_phy interface
+ * @host: host controller bus
+ * @gadget: gadget device
+ * @state: current otg state
+ * @dev: otg controller device
+ * @caps: otg capabilities revision, hnp, srp, etc
+ * @fsm: otg finite state machine
+ * @hcd_ops: host controller interface
+ * ------- internal use only -------
+ * @primary_hcd: primary host state and interface
+ * @shared_hcd: shared host state and interface
+ * @gadget_ops: gadget controller interface
+ * @list: list of otg controllers
+ * @work: otg state machine work
+ * @wq: otg state machine work queue
+ * @flags: to track if host/gadget is running
+ */
 struct usb_otg {
 	u8			default_a;
 

@@ -24,9 +84,25 @@ struct usb_otg {
 	struct usb_gadget	*gadget;
 
 	enum usb_otg_state	state;
+	struct device *dev;
+	struct usb_otg_caps	caps;
 	struct otg_fsm fsm;
 	struct otg_hcd_ops	*hcd_ops;
 
+	/* internal use only */
+	struct otg_hcd primary_hcd;
+	struct otg_hcd shared_hcd;
+	struct otg_gadget_ops *gadget_ops;
+	bool gadget_ready;
+	struct list_head list;
+	struct work_struct work;
+	struct workqueue_struct *wq;
+	u32 flags;
+#define OTG_FLAG_GADGET_RUNNING (1 << 0)
+#define OTG_FLAG_HOST_RUNNING (1 << 1)
+	/* use otg->fsm.lock for serializing access */
+
+/*------------- deprecated interface -----------------------------*/
 	/* bind/unbind the host controller */
 	int	(*set_host)(struct usb_otg *otg, struct usb_bus *host);
 

@@ -42,26 +118,101 @@ struct usb_otg {
 
 	/* start or continue HNP role switch */
 	int	(*start_hnp)(struct usb_otg *otg);
-
+/*---------------------------------------------------------------*/
 };
 
 /**
- * struct usb_otg_caps - describes the otg capabilities of the device
- * @otg_rev: The OTG revision number the device is compliant with, it's
- *		in binary-coded decimal (i.e. 2.0 is 0200H).
- * @hnp_support: Indicates if the device supports HNP.
- * @srp_support: Indicates if the device supports SRP.
- * @adp_support: Indicates if the device supports ADP.
+ * struct usb_otg_config - otg controller configuration
+ * @caps: otg capabilities of the controller
+ * @ops: otg fsm operations
+ * @otg_work: optional custom otg state machine work function
  */
-struct usb_otg_caps {
-	u16 otg_rev;
-	bool hnp_support;
-	bool srp_support;
-	bool adp_support;
+struct usb_otg_config {
+	struct usb_otg_caps *otg_caps;
+	struct otg_fsm_ops *fsm_ops;
+	void (*otg_work)(struct work_struct *work);
 };
 
 extern const char *usb_otg_state_string(enum usb_otg_state state);
 
+#if IS_ENABLED(CONFIG_USB_OTG)
+struct usb_otg *usb_otg_register(struct device *dev,
+				 struct usb_otg_config *config);
+int usb_otg_unregister(struct device *dev);
+int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
+			 unsigned long irqflags, struct otg_hcd_ops *ops);
+int usb_otg_unregister_hcd(struct usb_hcd *hcd);
+int usb_otg_register_gadget(struct usb_gadget *gadget,
+			    struct otg_gadget_ops *ops);
+int usb_otg_unregister_gadget(struct usb_gadget *gadget);
+void usb_otg_sync_inputs(struct usb_otg *otg);
+int usb_otg_kick_fsm(struct device *otg_dev);
+int usb_otg_start_host(struct usb_otg *otg, int on);
+int usb_otg_start_gadget(struct usb_otg *otg, int on);
+int usb_otg_gadget_ready(struct usb_gadget *gadget, bool ready);
+
+#else /* CONFIG_USB_OTG */
+
+static inline struct usb_otg *usb_otg_register(struct device *dev,
+					       struct usb_otg_config *config)
+{
+	return ERR_PTR(-ENOTSUPP);
+}
+
+static inline int usb_otg_unregister(struct device *dev)
+{
+	return -ENOTSUPP;
+}
+
+static inline int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
+				       unsigned long irqflags,
+				       struct otg_hcd_ops *ops)
+{
+	return -ENOTSUPP;
+}
+
+static inline int usb_otg_unregister_hcd(struct usb_hcd *hcd)
+{
+	return -ENOTSUPP;
+}
+
+static inline int usb_otg_register_gadget(struct usb_gadget *gadget,
+					  struct otg_gadget_ops *ops)
+{
+	return -ENOTSUPP;
+}
+
+static inline int usb_otg_unregister_gadget(struct usb_gadget *gadget)
+{
+	return -ENOTSUPP;
+}
+
+static inline void usb_otg_sync_inputs(struct usb_otg *otg)
+{
+}
+
+static inline int usb_otg_kick_fsm(struct device *otg_dev)
+{
+	return -ENOTSUPP;
+}
+
+static inline int usb_otg_start_host(struct usb_otg *otg, int on)
+{
+	return -ENOTSUPP;
+}
+
+static inline int usb_otg_start_gadget(struct usb_otg *otg, int on)
+{
+	return -ENOTSUPP;
+}
+
+static inline int usb_otg_gadget_ready(struct usb_gadget *gadget, bool ready)
+{
+	return -ENOTSUPP;
+}
+#endif /* CONFIG_USB_OTG */
+
+/*------------- deprecated interface -----------------------------*/
 /* Context: can sleep */
 static inline int
 otg_start_hnp(struct usb_otg *otg)

@@ -113,6 +264,8 @@ otg_start_srp(struct usb_otg *otg)
 	return -ENOTSUPP;
 }
 
+/*---------------------------------------------------------------*/
+
 /* for OTG controller drivers (and maybe other stuff) */
 extern int usb_bus_start_enum(struct usb_bus *bus, unsigned port_num);
 

@@ -237,4 +390,6 @@ static inline int otg_start_gadget(struct usb_otg *otg, int on)
 	return otg->fsm.ops->start_gadget(otg, on);
 }
 
+int drd_statemachine(struct usb_otg *otg);
+
 #endif /* __LINUX_USB_OTG_H */

Signature · 6 lines

2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Quoted 11 lines · Roger Quadros

drivers/usb/common/usb-otg.c new file +11 -0

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

drivers/usb/common/usb-otg.c line 403

Quoted 6 lines

drivers/usb/common/usb-otg.c new file +6 -0

+	}
+
+	/* 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

drivers/usb/common/usb-otg.c line 790

Quoted 4 lines

drivers/usb/common/usb-otg.c new file +3 -0

+
+int usb_otg_gadget_ready(struct usb_gadget *gadget, bool ready)
+{

What this API is for? Why need it in this version?

Quoted 47 lines

drivers/usb/common/usb-otg.c new file +41 -0

+	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);
+

include/linux/usb/otg.h +0 -0

@@ -10,10 +10,70 @@
 #define __LINUX_USB_OTG_H
 

include/linux/usb/otg.h line 15

Quoted 6 lines

include/linux/usb/otg.h +3 -2

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

Hi,

Quoted 12 lines · Peter Chen

Quoted 11 lines · Roger Quadros

drivers/usb/common/usb-otg.c new file +11 -0

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

Quoted 8 lines

drivers/usb/common/usb-otg.c line 403

Quoted 6 lines

drivers/usb/common/usb-otg.c new file +6 -0

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

Quoted 6 lines

drivers/usb/common/usb-otg.c line 790

Quoted 4 lines

drivers/usb/common/usb-otg.c new file +3 -0

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

Quoted 36 lines · Roger Quadros

Hi,

Quoted 12 lines · Peter Chen

Quoted 11 lines · Roger Quadros

drivers/usb/common/usb-otg.c new file +11 -0

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

Quoted 8 lines

drivers/usb/common/usb-otg.c line 403

Quoted 6 lines

drivers/usb/common/usb-otg.c new file +6 -0

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

Quoted 6 lines

drivers/usb/common/usb-otg.c line 790

Quoted 4 lines

drivers/usb/common/usb-otg.c new file +3 -0

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

Quoted 3 lines

usb_otg_gadget_ready() is used by gadget core to notify the otg core when the function driver is ready or not-ready.

Why you need to move this from gadget's probe to udc's probe? Currently, the sequence of gadget and udc's probe is random, but udc_bind_to_driver is called when udc is ready.

Quoted 41 lines · Peter Chen

Quoted 39 lines · Roger Quadros

Hi,

Quoted 12 lines · Peter Chen

Quoted 11 lines · Roger Quadros

drivers/usb/common/usb-otg.c new file +11 -0

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

Quoted 8 lines

drivers/usb/common/usb-otg.c line 403

Quoted 6 lines

drivers/usb/common/usb-otg.c new file +6 -0

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

Quoted 6 lines

drivers/usb/common/usb-otg.c line 790

Quoted 4 lines

drivers/usb/common/usb-otg.c new file +3 -0

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

Quoted 4 lines

Why you need to move this from gadget's probe to udc's probe? Currently, the sequence of gadget and udc's probe is random, but udc_bind_to_driver is called when udc is ready.

Because we need to support -EPROBE_DEFER at udc probe. At udc_bind_to_driver there is no scope for -EPROBE_DEFER.

Another issue is that we need to register with otg and have otg->caps available before driver->bind() is called in udc_bind_to_driver(). Else gadget function will not have the correct OTG descriptor.

Quoted 42 lines · Roger Quadros

Quoted 41 lines · Peter Chen

Quoted 39 lines · Roger Quadros

Hi,

Quoted 12 lines · Peter Chen

Quoted 11 lines · Roger Quadros

drivers/usb/common/usb-otg.c new file +11 -0

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

Quoted 8 lines

drivers/usb/common/usb-otg.c line 403

Quoted 6 lines

drivers/usb/common/usb-otg.c new file +6 -0

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

Quoted 6 lines

drivers/usb/common/usb-otg.c line 790

Quoted 4 lines

drivers/usb/common/usb-otg.c new file +3 -0

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

Quoted 7 lines

Quoted 4 lines

Why you need to move this from gadget's probe to udc's probe? Currently, the sequence of gadget and udc's probe is random, but udc_bind_to_driver is called when udc is ready.

Because we need to support -EPROBE_DEFER at udc probe. At udc_bind_to_driver there is no scope for -EPROBE_DEFER.

Why udc_bind_to_driver can't return -EPROBE_DEFER if register to OTG is failed?

Quoted 4 lines

Another issue is that we need to register with otg and have otg->caps available before driver->bind() is called in udc_bind_to_driver(). Else gadget function will not have the correct OTG descriptor.

You can do register before calling driver->bind.

It provides APIs for the following tasks

- Registering an OTG/dual-role capable controller
- Registering Host and Gadget controllers to OTG core
- Providing inputs to and kicking the OTG state machine

Provide a dual-role device (DRD) state machine. DRD mode is a reduced functionality OTG mode. In this mode we don't support SRP, HNP and dynamic role-swap.

In DRD operation, the controller mode (Host or Peripheral) is decided based on the ID pin status. Once a cable plug (Type-A or Type-B) is attached the controller selects the state and doesn't change till the cable in unplugged and a different cable type is inserted.

As we don't need most of the complex OTG states and OTG timers we implement a lean DRD state machine in usb-otg.c. The DRD state machine is only interested in 2 hardware inputs 'id' and 'b_sess_vld'.

Signed-off-by Roger Quadros <[email protected]>

drivers/usb/common/usb-otg.c new file +0 -0

new file mode 100644
index 0000000..baebe5c
@@ -0,0 +1,833 @@

+/**
+ * Change USB protocol when there is a protocol change.
+ * fsm->lock must be held.
+ */

+static int drd_set_protocol(struct otg_fsm *fsm, int protocol)

+/**
+ * Called when entering a DRD state.
+ * fsm->lock must be held.
+ */

+static void drd_set_state(struct otg_fsm *fsm, enum usb_otg_state new_state)

+/**
+ * DRD state change judgement
+ *
+ * For DRD we're only interested in some of the OTG states
+ * i.e. OTG_STATE_B_IDLE: both peripheral and host are stopped
+ * OTG_STATE_B_PERIPHERAL: peripheral active
+ * OTG_STATE_A_HOST: host active
+ * we're only interested in the following inputs
+ * fsm->id, fsm->b_sess_vld
+ */

+/**
+ * Dual-role device (DRD) work function
+ */

+/**
+ * start/kick the OTG FSM if we can
+ * fsm->lock must be held
+ */

+/**
+ * stop the OTG FSM. Stops Host & Gadget controllers as well.
+ * fsm->lock must be held
+ */

+/**
+ * usb_otg_sync_inputs - Sync OTG inputs with the OTG state machine
+ * @fsm: OTG FSM instance

+ *
+ * Used by the OTG driver to update the inputs to the OTG
+ * state machine.
+ *
+ * Can be called in IRQ context.
+ */
+void usb_otg_sync_inputs(struct usb_otg *otg)

+/**
+ * usb_otg_register_gadget - Register the gadget controller to OTG core
+ * @gadget: gadget controller

include/linux/usb/otg.h +2 -2

index 85b8fb5..5d4850a 100644
@@ -10,10 +10,68 @@
 #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/usb.h>
 #include <linux/usb/hcd.h>

include/linux/usb/otg.h +10 -0

+ * @hcd_ops: host controller interface
+ * ------- internal use only -------
+ * @primary_hcd: primary host state and interface
+ * @shared_hcd: shared host state and interface
+ * @gadget_ops: gadget controller interface
+ * @list: list of otg controllers
+ * @work: otg state machine work
+ * @wq: otg state machine work queue
+ * @flags: to track if host/gadget is running
+ */
 struct usb_otg {
 	u8			default_a;
 

include/linux/usb/otg.h +1 -1

@@ -42,26 +116,101 @@ struct usb_otg {
 
 	/* start or continue HNP role switch */
 	int	(*start_hnp)(struct usb_otg *otg);
-
+/*---------------------------------------------------------------*/
 };
 

Quoted 22 lines · Roger Quadros

It provides APIs for the following tasks

- Registering an OTG/dual-role capable controller
- Registering Host and Gadget controllers to OTG core
- Providing inputs to and kicking the OTG state machine

Provide a dual-role device (DRD) state machine. DRD mode is a reduced functionality OTG mode. In this mode we don't support SRP, HNP and dynamic role-swap.

In DRD operation, the controller mode (Host or Peripheral) is decided based on the ID pin status. Once a cable plug (Type-A or Type-B) is attached the controller selects the state and doesn't change till the cable in unplugged and a different cable type is inserted.

As we don't need most of the complex OTG states and OTG timers we implement a lean DRD state machine in usb-otg.c. The DRD state machine is only interested in 2 hardware inputs 'id' and 'b_sess_vld'.

Signed-off-by Roger Quadros <[email protected]>

[...]

Quoted 6 lines

drivers/usb/common/usb-otg.c new file +0 -0

new file mode 100644
index 0000000..baebe5c
@@ -0,0 +1,833 @@

[...]

Quoted 4 lines

+/**
+ * Change USB protocol when there is a protocol change.
+ * fsm->lock must be held.
+ */

If you're using the kernel-doc comment, please follow the rules.

[...]

Phew, that was a long patch... normally I don't review the patches that are such big. :-)

Quoted 22 lines · Roger Quadros

It provides APIs for the following tasks

- Registering an OTG/dual-role capable controller
- Registering Host and Gadget controllers to OTG core
- Providing inputs to and kicking the OTG state machine

Provide a dual-role device (DRD) state machine. DRD mode is a reduced functionality OTG mode. In this mode we don't support SRP, HNP and dynamic role-swap.

In DRD operation, the controller mode (Host or Peripheral) is decided based on the ID pin status. Once a cable plug (Type-A or Type-B) is attached the controller selects the state and doesn't change till the cable in unplugged and a different cable type is inserted.

As we don't need most of the complex OTG states and OTG timers we implement a lean DRD state machine in usb-otg.c. The DRD state machine is only interested in 2 hardware inputs 'id' and 'b_sess_vld'.

Signed-off-by Roger Quadros <[email protected]>

[...]

Quoted 3 lines

+/**
+ * usb_otg_register_gadget - Register the gadget controller to OTG core
+ * @gadget: gadget controller

We call that USB device controller (UDC). I'm not sure what you meant here... And what about the 2nd arg, 'ops'?

Quoted 23 lines

Quoted 22 lines

It provides APIs for the following tasks

- Registering an OTG/dual-role capable controller
- Registering Host and Gadget controllers to OTG core
- Providing inputs to and kicking the OTG state machine

Provide a dual-role device (DRD) state machine. DRD mode is a reduced functionality OTG mode. In this mode we don't support SRP, HNP and dynamic role-swap.

In DRD operation, the controller mode (Host or Peripheral) is decided based on the ID pin status. Once a cable plug (Type-A or Type-B) is attached the controller selects the state and doesn't change till the cable in unplugged and a different cable type is inserted.

As we don't need most of the complex OTG states and OTG timers we implement a lean DRD state machine in usb-otg.c. The DRD state machine is only interested in 2 hardware inputs 'id' and 'b_sess_vld'.

Signed-off-by Roger Quadros <[email protected]>

<snip>

Quoted 7 lines

[...]

Quoted 3 lines

+/**
+ * usb_otg_register_gadget - Register the gadget controller to OTG core
+ * @gadget: gadget controller

We call that USB device controller (UDC). I'm not sure what you meant here... And what about the 2nd arg, 'ops'?

There are 2 data structures representing the Device controller.

struct usb_gadget - represents a usb slave device
struct usb_udc -struct usb_udc - describes one usb device controller

usb_udc is for private use only. usb_otg_register_gadget() takes struct usb_gadget as argument.

Do you want me to refer to struct usb_gadget as UDC?

What is wrong with calling it gadget controller?

--
cheers,
-roger

Quoted 42 lines · Roger Quadros

Quoted 23 lines

Quoted 22 lines

It provides APIs for the following tasks

- Registering an OTG/dual-role capable controller
- Registering Host and Gadget controllers to OTG core
- Providing inputs to and kicking the OTG state machine

Provide a dual-role device (DRD) state machine. DRD mode is a reduced functionality OTG mode. In this mode we don't support SRP, HNP and dynamic role-swap.

In DRD operation, the controller mode (Host or Peripheral) is decided based on the ID pin status. Once a cable plug (Type-A or Type-B) is attached the controller selects the state and doesn't change till the cable in unplugged and a different cable type is inserted.

As we don't need most of the complex OTG states and OTG timers we implement a lean DRD state machine in usb-otg.c. The DRD state machine is only interested in 2 hardware inputs 'id' and 'b_sess_vld'.

Signed-off-by Roger Quadros <[email protected]>

<snip>

Quoted 7 lines

[...]

Quoted 3 lines

+/**
+ * usb_otg_register_gadget - Register the gadget controller to OTG core
+ * @gadget: gadget controller

We call that USB device controller (UDC). I'm not sure what you meant here... And what about the 2nd arg, 'ops'?

There are 2 data structures representing the Device controller.

struct usb_gadget - represents a usb slave device
struct usb_udc -struct usb_udc - describes one usb device controller

usb_udc is for private use only. usb_otg_register_gadget() takes struct usb_gadget as argument.

Quoted 1 line

Do you want me to refer to struct usb_gadget as UDC?

   No.

Quoted 1 line

What is wrong with calling it gadget controller?

It's not a controller, it's a piece of software AFAIU. Or is my English just too weak? :-)

Quoted 3 lines

Quoted 1 line

What is wrong with calling it gadget controller?

It's not a controller, it's a piece of software AFAIU. Or is my English just too weak? :-)

Everything at this point is a piece of software :).

struct usb_gadget, represents the gadget controller device not the driver (or software).

struct usb_gadget_driver represents the gadget function driver. struct usb_gadget_ops represents the UDC driver ops.

Now that we have a device reference in struct usb_otg let's use dev_dbg() for debug messages.

Quoted 2 lines · Roger Quadros

Now that we have a device reference in struct usb_otg let's use dev_dbg() for debug messages.

dev_vdbg vs dev_dbg

The patch subject and commit message don't match the code changes.

drivers/usb/common/usb-otg-fsm.c

Quoted 2 lines

drivers/usb/common/usb-otg-fsm.c +0 -0

[]

Quoted 3 lines

drivers/usb/common/usb-otg-fsm.c +0 -0

@@ -44,8 +37,9 @@ static int otg_set_protocol(struct otg_fsm *fsm, int protocol)
 	int ret = 0;
 

drivers/usb/common/usb-otg-fsm.c static int otg_set_protocol(struct otg_fsm *fsm, int protocol) · line 42

Quoted 6 lines

drivers/usb/common/usb-otg-fsm.c +3 -2

 	if (fsm->protocol != protocol) {
-		VDBG("Changing role fsm->protocol= %d; new protocol= %d\n",
-			fsm->protocol, protocol);
+		dev_vdbg(otg->dev,
+			 "Changing role fsm->protocol= %d; new protocol= %d\n",
+			 fsm->protocol, protocol);

etc...

Now that we have a device reference in struct usb_otg let's use dev_vdbg() for debug messages instead of VDBG().

Signed-off-by Roger Quadros <[email protected]>

Acked-by Peter Chen <[email protected]>

---
v10:
- in the commit message say dev_vdbg() instead of dev_dbg()

Quoted 3 lines · Roger Quadros

Now that we have a device reference in struct usb_otg let's use dev_vdbg() for debug messages instead of VDBG().

Quoted 5 lines

Signed-off-by Roger Quadros <[email protected]>

Acked-by Peter Chen <[email protected]>

---
v10:
- in the commit message say dev_vdbg() instead of dev_dbg()

50% - partial credit, see: patch/email subject.

Introduce usb_otg_add/remove_hcd() for use by host controllers that are part of OTG/dual-role port.

Non Device tree platforms can use the otg_dev argument to specify the OTG controller device. If otg_dev is NULL then the device tree node's otg-controller property is used to get the otg_dev device.

Signed-off-by Roger Quadros <[email protected]>

Acked-by Peter Chen <[email protected]>

---
 drivers/usb/core/hcd.c  | 55 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/usb/hcd.h |  4 ++++
 2 files changed, 59 insertions(+)

drivers/usb/core/hcd.c +0 -0

index ae6c76d..c6f4155 100644

drivers/usb/core/hcd.c +4 -0

@@ -3025,6 +3030,56 @@ void usb_remove_hcd(struct usb_hcd *hcd)
 }
 EXPORT_SYMBOL_GPL(usb_remove_hcd);
 
+static struct otg_hcd_ops otg_hcd_intf = {
+	.add = usb_add_hcd,
+	.remove = usb_remove_hcd,
+	.usb_bus_start_enum = usb_bus_start_enum,

Quoted 19 lines · Roger Quadros

Introduce usb_otg_add/remove_hcd() for use by host controllers that are part of OTG/dual-role port.

Non Device tree platforms can use the otg_dev argument to specify the OTG controller device. If otg_dev is NULL then the device tree node's otg-controller property is used to get the otg_dev device.

Signed-off-by Roger Quadros <[email protected]>

Acked-by Peter Chen <[email protected]>

---
 drivers/usb/core/hcd.c  | 55 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/usb/hcd.h |  4 ++++
 2 files changed, 59 insertions(+)

drivers/usb/core/hcd.c +0 -0

index ae6c76d..c6f4155 100644

[...]

Quoted 4 lines

drivers/usb/core/hcd.c +0 -0

@@ -3025,6 +3030,56 @@ void usb_remove_hcd(struct usb_hcd *hcd)
 }
 EXPORT_SYMBOL_GPL(usb_remove_hcd);
 

Quoted 4 lines

drivers/usb/core/hcd.c +4 -0

+static struct otg_hcd_ops otg_hcd_intf = {
+    .add = usb_add_hcd,
+    .remove = usb_remove_hcd,
+    .usb_bus_start_enum = usb_bus_start_enum,

drivers/usb/core/hcd.c void usb_remove_hcd(struct usb_hcd *hcd) · line 3046

Quoted 10 lines

drivers/usb/core/hcd.c +10 -0

+    .usb_control_msg = usb_control_msg,
+    .usb_hub_find_child = usb_hub_find_child,
+};
+
+/**
+ * usb_otg_add_hcd - Register the HCD with OTG core.
+ * @hcd: the usb_hcd structure to initialize
+ * @irqnum: Interrupt line to allocate
+ * @irqflags: Interrupt type flags
+ * @otg_dev: OTG controller device managing this HCD

Device managing a driver? That's interesting. :-)

Quoted 24 lines

Quoted 19 lines

Introduce usb_otg_add/remove_hcd() for use by host controllers that are part of OTG/dual-role port.

Non Device tree platforms can use the otg_dev argument to specify the OTG controller device. If otg_dev is NULL then the device tree node's otg-controller property is used to get the otg_dev device.

Signed-off-by Roger Quadros <[email protected]>

Acked-by Peter Chen <[email protected]>

---
 drivers/usb/core/hcd.c  | 55 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/usb/hcd.h |  4 ++++
 2 files changed, 59 insertions(+)

drivers/usb/core/hcd.c +0 -0

index ae6c76d..c6f4155 100644

[...]

Quoted 4 lines

drivers/usb/core/hcd.c +0 -0

@@ -3025,6 +3030,56 @@ void usb_remove_hcd(struct usb_hcd *hcd)
 }
 EXPORT_SYMBOL_GPL(usb_remove_hcd);
 

Quoted 16 lines

Quoted 4 lines

drivers/usb/core/hcd.c +4 -0

+static struct otg_hcd_ops otg_hcd_intf = {
+    .add = usb_add_hcd,
+    .remove = usb_remove_hcd,
+    .usb_bus_start_enum = usb_bus_start_enum,

drivers/usb/core/hcd.c void usb_remove_hcd(struct usb_hcd *hcd) · line 3046

Quoted 10 lines

drivers/usb/core/hcd.c +10 -0

+    .usb_control_msg = usb_control_msg,
+    .usb_hub_find_child = usb_hub_find_child,
+};
+
+/**
+ * usb_otg_add_hcd - Register the HCD with OTG core.
+ * @hcd: the usb_hcd structure to initialize
+ * @irqnum: Interrupt line to allocate
+ * @irqflags: Interrupt type flags
+ * @otg_dev: OTG controller device managing this HCD

Device managing a driver? That's interesting. :-)

Well it is the OTG controller instance really.
How else do you want me to write it?

cheers,
-roger

Quoted 41 lines · Roger Quadros

Quoted 40 lines

Quoted 19 lines

Introduce usb_otg_add/remove_hcd() for use by host controllers that are part of OTG/dual-role port.

Non Device tree platforms can use the otg_dev argument to specify the OTG controller device. If otg_dev is NULL then the device tree node's otg-controller property is used to get the otg_dev device.

Signed-off-by Roger Quadros <[email protected]>

Acked-by Peter Chen <[email protected]>

---
 drivers/usb/core/hcd.c  | 55 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/usb/hcd.h |  4 ++++
 2 files changed, 59 insertions(+)

drivers/usb/core/hcd.c +0 -0

index ae6c76d..c6f4155 100644

[...]

Quoted 8 lines

drivers/usb/core/hcd.c +4 -0

@@ -3025,6 +3030,56 @@ void usb_remove_hcd(struct usb_hcd *hcd)
 }
 EXPORT_SYMBOL_GPL(usb_remove_hcd);
 
+static struct otg_hcd_ops otg_hcd_intf = {
+    .add = usb_add_hcd,
+    .remove = usb_remove_hcd,
+    .usb_bus_start_enum = usb_bus_start_enum,

drivers/usb/core/hcd.c void usb_remove_hcd(struct usb_hcd *hcd) · line 3046

Quoted 10 lines

drivers/usb/core/hcd.c +10 -0

+    .usb_control_msg = usb_control_msg,
+    .usb_hub_find_child = usb_hub_find_child,
+};
+
+/**
+ * usb_otg_add_hcd - Register the HCD with OTG core.
+ * @hcd: the usb_hcd structure to initialize
+ * @irqnum: Interrupt line to allocate
+ * @irqflags: Interrupt type flags
+ * @otg_dev: OTG controller device managing this HCD

Device managing a driver? That's interesting. :-)

Quoted 2 lines

Well it is the OTG controller instance really.
How else do you want me to write it?

Just HC again? Or maybe HCI (HC interface)?

From: Roger Quadros
Sent: Wednesday, June 08, 2016 6:04 PM

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 <[email protected]>

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

Hi Roger,

Quoted 25 lines

From: Roger Quadros
Sent: Wednesday, June 08, 2016 6:04 PM

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.

Quoted 5 lines

Signed-off-by Roger Quadros <[email protected]>

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

I tried to apply this patch on the Felipe's usb.git / testing/next branch, but I couldn't apply because the udc-core.c was renamed to core.c.
https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?h=testing/next&id=a22b539c7082b6a3046b3a9cebb356a047a2d81d

I guess this patch needs rebase for that.