[PATCH] usb: host: Allow EHCI_OMAP to be built-in when USB_GADGET is 'm'

[PATCH] usb: host: Allow EHCI_OMAP to be built-in when USB_GADGET is 'm'

[Roger Quadros]

NOP_USB_XCEIV is used not only by gadget drivers but by
host drivers as well e.g. EHCI_OMAP.

commit 5a8d651a2bde ("usb: gadget: move gadget API functions to udc-core")
made it so that NOP_USB_XCEIV can't be built-in if USB_GADGET is 'm'.
But this prevents EHCI_OMAP to be built-in if USB_GADGET is 'm'.

Fix this undesired behaviour by moving usb_gadget_vbus_connect/disconnect()
to usb/gadget.h so that NOP_USB_XCEIV has no build dependency
on USB_GADGET.

Retain the original Kconfig behaviour i.e. NOP_USB_XCEIV is selected
by drivers that need it.

Fixes: 5a8d651a2bde ("usb: gadget: move gadget API functions to udc-core")
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/usb/gadget/udc/core.c | 60 -------------------------------------------
 drivers/usb/host/Kconfig      |  2 +-
 drivers/usb/phy/Kconfig       |  5 ++--
 include/linux/usb/gadget.h    | 44 ++++++++++++++++++++++++++-----
 4 files changed, 41 insertions(+), 70 deletions(-)

diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
index ff8685e..687828d 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -538,37 +538,6 @@ out:
 EXPORT_SYMBOL_GPL(usb_gadget_clear_selfpowered);
 
 /**
- * usb_gadget_vbus_connect - Notify controller that VBUS is powered
- * @gadget:The device which now has VBUS power.
- * Context: can sleep
- *
- * This call is used by a driver for an external transceiver (or GPIO)
- * that detects a VBUS power session starting.  Common responses include
- * resuming the controller, activating the D+ (or D-) pullup to let the
- * host detect that a USB device is attached, and starting to draw power
- * (8mA or possibly more, especially after SET_CONFIGURATION).
- *
- * Returns zero on success, else negative errno.
- */
-int usb_gadget_vbus_connect(struct usb_gadget *gadget)
-{
-	int ret = 0;
-
-	if (!gadget->ops->vbus_session) {
-		ret = -EOPNOTSUPP;
-		goto out;
-	}
-
-	ret = gadget->ops->vbus_session(gadget, 1);
-
-out:
-	trace_usb_gadget_vbus_connect(gadget, ret);
-
-	return ret;
-}
-EXPORT_SYMBOL_GPL(usb_gadget_vbus_connect);
-
-/**
  * usb_gadget_vbus_draw - constrain controller's VBUS power usage
  * @gadget:The device whose VBUS usage is being described
  * @mA:How much current to draw, in milliAmperes.  This should be twice
@@ -601,35 +570,6 @@ out:
 EXPORT_SYMBOL_GPL(usb_gadget_vbus_draw);
 
 /**
- * usb_gadget_vbus_disconnect - notify controller about VBUS session end
- * @gadget:the device whose VBUS supply is being described
- * Context: can sleep
- *
- * This call is used by a driver for an external transceiver (or GPIO)
- * that detects a VBUS power session ending.  Common responses include
- * reversing everything done in usb_gadget_vbus_connect().
- *
- * Returns zero on success, else negative errno.
- */
-int usb_gadget_vbus_disconnect(struct usb_gadget *gadget)
-{
-	int ret = 0;
-
-	if (!gadget->ops->vbus_session) {
-		ret = -EOPNOTSUPP;
-		goto out;
-	}
-
-	ret = gadget->ops->vbus_session(gadget, 0);
-
-out:
-	trace_usb_gadget_vbus_disconnect(gadget, ret);
-
-	return ret;
-}
-EXPORT_SYMBOL_GPL(usb_gadget_vbus_disconnect);
-
-/**
  * usb_gadget_connect - software-controlled connect to USB host
  * @gadget:the peripheral being connected
  *
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 2e710a4..d8f5674 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -180,7 +180,7 @@ config USB_EHCI_MXC
 config USB_EHCI_HCD_OMAP
 	tristate "EHCI support for OMAP3 and later chips"
 	depends on ARCH_OMAP
-	depends on NOP_USB_XCEIV
+	select NOP_USB_XCEIV
 	default y
 	---help---
 	  Enables support for the on-chip EHCI controller on
diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index b9c409a..fe94bbc 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -45,7 +45,7 @@ config ISP1301_OMAP
 config KEYSTONE_USB_PHY
 	tristate "Keystone USB PHY Driver"
 	depends on ARCH_KEYSTONE || COMPILE_TEST
-	depends on NOP_USB_XCEIV
+	select NOP_USB_XCEIV
 	help
 	  Enable this to support Keystone USB phy. This driver provides
 	  interface to interact with USB 2.0 and USB 3.0 PHY that is part
@@ -53,7 +53,6 @@ config KEYSTONE_USB_PHY
 
 config NOP_USB_XCEIV
 	tristate "NOP USB Transceiver Driver"
-	depends on USB_GADGET || !USB_GADGET # if USB_GADGET=m, NOP can't be built-in
 	select USB_PHY
 	help
 	  This driver is to be used by all the usb transceiver which are either
@@ -66,7 +65,7 @@ config AM335X_CONTROL_USB
 config AM335X_PHY_USB
 	tristate "AM335x USB PHY Driver"
 	depends on ARM || COMPILE_TEST
-	depends on NOP_USB_XCEIV
+	select NOP_USB_XCEIV
 	select USB_PHY
 	select AM335X_CONTROL_USB
 	select USB_COMMON
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 612dbdf..305d81e 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -508,14 +508,50 @@ static inline int gadget_is_otg(struct usb_gadget *g)
 
 /*-------------------------------------------------------------------------*/
 
+/**
+ * usb_gadget_vbus_connect - Notify controller that VBUS is powered
+ * @gadget:The device which now has VBUS power.
+ * Context: can sleep
+ *
+ * This call is used by a driver for an external transceiver (or GPIO)
+ * that detects a VBUS power session starting.  Common responses include
+ * resuming the controller, activating the D+ (or D-) pullup to let the
+ * host detect that a USB device is attached, and starting to draw power
+ * (8mA or possibly more, especially after SET_CONFIGURATION).
+ *
+ * Returns zero on success, else negative errno.
+ */
+static inline int usb_gadget_vbus_connect(struct usb_gadget *gadget)
+{
+	if (!gadget->ops->vbus_session)
+		return -EOPNOTSUPP;
+	return gadget->ops->vbus_session(gadget, 1);
+}
+
+/**
+ * usb_gadget_vbus_disconnect - notify controller about VBUS session end
+ * @gadget:the device whose VBUS supply is being described
+ * Context: can sleep
+ *
+ * This call is used by a driver for an external transceiver (or GPIO)
+ * that detects a VBUS power session ending.  Common responses include
+ * reversing everything done in usb_gadget_vbus_connect().
+ *
+ * Returns zero on success, else negative errno.
+ */
+static inline int usb_gadget_vbus_disconnect(struct usb_gadget *gadget)
+{
+	if (!gadget->ops->vbus_session)
+		return -EOPNOTSUPP;
+	return gadget->ops->vbus_session(gadget, 0);
+}
+
 #if IS_ENABLED(CONFIG_USB_GADGET)
 int usb_gadget_frame_number(struct usb_gadget *gadget);
 int usb_gadget_wakeup(struct usb_gadget *gadget);
 int usb_gadget_set_selfpowered(struct usb_gadget *gadget);
 int usb_gadget_clear_selfpowered(struct usb_gadget *gadget);
-int usb_gadget_vbus_connect(struct usb_gadget *gadget);
 int usb_gadget_vbus_draw(struct usb_gadget *gadget, unsigned mA);
-int usb_gadget_vbus_disconnect(struct usb_gadget *gadget);
 int usb_gadget_connect(struct usb_gadget *gadget);
 int usb_gadget_disconnect(struct usb_gadget *gadget);
 int usb_gadget_deactivate(struct usb_gadget *gadget);
@@ -529,12 +565,8 @@ static inline int usb_gadget_set_selfpowered(struct usb_gadget *gadget)
 { return 0; }
 static inline int usb_gadget_clear_selfpowered(struct usb_gadget *gadget)
 { return 0; }
-static inline int usb_gadget_vbus_connect(struct usb_gadget *gadget)
-{ return 0; }
 static inline int usb_gadget_vbus_draw(struct usb_gadget *gadget, unsigned mA)
 { return 0; }
-static inline int usb_gadget_vbus_disconnect(struct usb_gadget *gadget)
-{ return 0; }
 static inline int usb_gadget_connect(struct usb_gadget *gadget)
 { return 0; }
 static inline int usb_gadget_disconnect(struct usb_gadget *gadget)
-- 
2.7.4

[PATCH] usb: host: Allow EHCI_OMAP to be built-in when USB_GADGET is 'm'

[Roger Quadros]

NOP_USB_XCEIV is used not only by gadget drivers but by
host drivers as well e.g. EHCI_OMAP.

commit 5a8d651a2bde ("usb: gadget: move gadget API functions to udc-core")
made it so that NOP_USB_XCEIV can't be built-in if USB_GADGET is 'm'.
But this prevents EHCI_OMAP to be built-in if USB_GADGET is 'm'.

Fix this undesired behaviour by moving usb_gadget_vbus_connect/disconnect()
to usb/gadget.h so that NOP_USB_XCEIV has no build dependency
on USB_GADGET.

Retain the original Kconfig behaviour i.e. NOP_USB_XCEIV is selected
by drivers that need it.

Fixes: 5a8d651a2bde ("usb: gadget: move gadget API functions to udc-core")
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/usb/gadget/udc/core.c | 60 -------------------------------------------
 drivers/usb/host/Kconfig      |  2 +-
 drivers/usb/phy/Kconfig       |  5 ++--
 include/linux/usb/gadget.h    | 44 ++++++++++++++++++++++++++-----
 4 files changed, 41 insertions(+), 70 deletions(-)

diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
index ff8685e..687828d 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -538,37 +538,6 @@ out:
 EXPORT_SYMBOL_GPL(usb_gadget_clear_selfpowered);
 
 /**
- * usb_gadget_vbus_connect - Notify controller that VBUS is powered
- * @gadget:The device which now has VBUS power.
- * Context: can sleep
- *
- * This call is used by a driver for an external transceiver (or GPIO)
- * that detects a VBUS power session starting.  Common responses include
- * resuming the controller, activating the D+ (or D-) pullup to let the
- * host detect that a USB device is attached, and starting to draw power
- * (8mA or possibly more, especially after SET_CONFIGURATION).
- *
- * Returns zero on success, else negative errno.
- */
-int usb_gadget_vbus_connect(struct usb_gadget *gadget)
-{
-	int ret = 0;
-
-	if (!gadget->ops->vbus_session) {
-		ret = -EOPNOTSUPP;
-		goto out;
-	}
-
-	ret = gadget->ops->vbus_session(gadget, 1);
-
-out:
-	trace_usb_gadget_vbus_connect(gadget, ret);
-
-	return ret;
-}
-EXPORT_SYMBOL_GPL(usb_gadget_vbus_connect);
-
-/**
  * usb_gadget_vbus_draw - constrain controller's VBUS power usage
  * @gadget:The device whose VBUS usage is being described
  * @mA:How much current to draw, in milliAmperes.  This should be twice
@@ -601,35 +570,6 @@ out:
 EXPORT_SYMBOL_GPL(usb_gadget_vbus_draw);
 
 /**
- * usb_gadget_vbus_disconnect - notify controller about VBUS session end
- * @gadget:the device whose VBUS supply is being described
- * Context: can sleep
- *
- * This call is used by a driver for an external transceiver (or GPIO)
- * that detects a VBUS power session ending.  Common responses include
- * reversing everything done in usb_gadget_vbus_connect().
- *
- * Returns zero on success, else negative errno.
- */
-int usb_gadget_vbus_disconnect(struct usb_gadget *gadget)
-{
-	int ret = 0;
-
-	if (!gadget->ops->vbus_session) {
-		ret = -EOPNOTSUPP;
-		goto out;
-	}
-
-	ret = gadget->ops->vbus_session(gadget, 0);
-
-out:
-	trace_usb_gadget_vbus_disconnect(gadget, ret);
-
-	return ret;
-}
-EXPORT_SYMBOL_GPL(usb_gadget_vbus_disconnect);
-
-/**
  * usb_gadget_connect - software-controlled connect to USB host
  * @gadget:the peripheral being connected
  *
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 2e710a4..d8f5674 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -180,7 +180,7 @@ config USB_EHCI_MXC
 config USB_EHCI_HCD_OMAP
 	tristate "EHCI support for OMAP3 and later chips"
 	depends on ARCH_OMAP
-	depends on NOP_USB_XCEIV
+	select NOP_USB_XCEIV
 	default y
 	---help---
 	  Enables support for the on-chip EHCI controller on
diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index b9c409a..fe94bbc 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -45,7 +45,7 @@ config ISP1301_OMAP
 config KEYSTONE_USB_PHY
 	tristate "Keystone USB PHY Driver"
 	depends on ARCH_KEYSTONE || COMPILE_TEST
-	depends on NOP_USB_XCEIV
+	select NOP_USB_XCEIV
 	help
 	  Enable this to support Keystone USB phy. This driver provides
 	  interface to interact with USB 2.0 and USB 3.0 PHY that is part
@@ -53,7 +53,6 @@ config KEYSTONE_USB_PHY
 
 config NOP_USB_XCEIV
 	tristate "NOP USB Transceiver Driver"
-	depends on USB_GADGET || !USB_GADGET # if USB_GADGET=m, NOP can't be built-in
 	select USB_PHY
 	help
 	  This driver is to be used by all the usb transceiver which are either
@@ -66,7 +65,7 @@ config AM335X_CONTROL_USB
 config AM335X_PHY_USB
 	tristate "AM335x USB PHY Driver"
 	depends on ARM || COMPILE_TEST
-	depends on NOP_USB_XCEIV
+	select NOP_USB_XCEIV
 	select USB_PHY
 	select AM335X_CONTROL_USB
 	select USB_COMMON
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 612dbdf..305d81e 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -508,14 +508,50 @@ static inline int gadget_is_otg(struct usb_gadget *g)
 
 /*-------------------------------------------------------------------------*/
 
+/**
+ * usb_gadget_vbus_connect - Notify controller that VBUS is powered
+ * @gadget:The device which now has VBUS power.
+ * Context: can sleep
+ *
+ * This call is used by a driver for an external transceiver (or GPIO)
+ * that detects a VBUS power session starting.  Common responses include
+ * resuming the controller, activating the D+ (or D-) pullup to let the
+ * host detect that a USB device is attached, and starting to draw power
+ * (8mA or possibly more, especially after SET_CONFIGURATION).
+ *
+ * Returns zero on success, else negative errno.
+ */
+static inline int usb_gadget_vbus_connect(struct usb_gadget *gadget)
+{
+	if (!gadget->ops->vbus_session)
+		return -EOPNOTSUPP;
+	return gadget->ops->vbus_session(gadget, 1);
+}
+
+/**
+ * usb_gadget_vbus_disconnect - notify controller about VBUS session end
+ * @gadget:the device whose VBUS supply is being described
+ * Context: can sleep
+ *
+ * This call is used by a driver for an external transceiver (or GPIO)
+ * that detects a VBUS power session ending.  Common responses include
+ * reversing everything done in usb_gadget_vbus_connect().
+ *
+ * Returns zero on success, else negative errno.
+ */
+static inline int usb_gadget_vbus_disconnect(struct usb_gadget *gadget)
+{
+	if (!gadget->ops->vbus_session)
+		return -EOPNOTSUPP;
+	return gadget->ops->vbus_session(gadget, 0);
+}
+
 #if IS_ENABLED(CONFIG_USB_GADGET)
 int usb_gadget_frame_number(struct usb_gadget *gadget);
 int usb_gadget_wakeup(struct usb_gadget *gadget);
 int usb_gadget_set_selfpowered(struct usb_gadget *gadget);
 int usb_gadget_clear_selfpowered(struct usb_gadget *gadget);
-int usb_gadget_vbus_connect(struct usb_gadget *gadget);
 int usb_gadget_vbus_draw(struct usb_gadget *gadget, unsigned mA);
-int usb_gadget_vbus_disconnect(struct usb_gadget *gadget);
 int usb_gadget_connect(struct usb_gadget *gadget);
 int usb_gadget_disconnect(struct usb_gadget *gadget);
 int usb_gadget_deactivate(struct usb_gadget *gadget);
@@ -529,12 +565,8 @@ static inline int usb_gadget_set_selfpowered(struct usb_gadget *gadget)
 { return 0; }
 static inline int usb_gadget_clear_selfpowered(struct usb_gadget *gadget)
 { return 0; }
-static inline int usb_gadget_vbus_connect(struct usb_gadget *gadget)
-{ return 0; }
 static inline int usb_gadget_vbus_draw(struct usb_gadget *gadget, unsigned mA)
 { return 0; }
-static inline int usb_gadget_vbus_disconnect(struct usb_gadget *gadget)
-{ return 0; }
 static inline int usb_gadget_connect(struct usb_gadget *gadget)
 { return 0; }
 static inline int usb_gadget_disconnect(struct usb_gadget *gadget)
-- 
2.7.4

Re: [PATCH] usb: host: Allow EHCI_OMAP to be built-in when USB_GADGET is 'm'

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1183 bytes --]


Hi,

Roger Quadros <rogerq@ti.com> writes:
> NOP_USB_XCEIV is used not only by gadget drivers but by
> host drivers as well e.g. EHCI_OMAP.
>
> commit 5a8d651a2bde ("usb: gadget: move gadget API functions to udc-core")
> made it so that NOP_USB_XCEIV can't be built-in if USB_GADGET is 'm'.
> But this prevents EHCI_OMAP to be built-in if USB_GADGET is 'm'.
>
> Fix this undesired behaviour by moving usb_gadget_vbus_connect/disconnect()
> to usb/gadget.h so that NOP_USB_XCEIV has no build dependency
> on USB_GADGET.
>
> Retain the original Kconfig behaviour i.e. NOP_USB_XCEIV is selected
> by drivers that need it.

no, this is the wrong way to fix this. NOP _has_ a dependency on the
Gadget API if it calls Gadget API functions. Dependencies are proper.

Maybe the reason for the problem is that we ended up adding far too much
code to phy-generic.c itself. Maybe it shouldn't know about clks and
interrupts. The original idea of that driver was to simply satisfy a
requirement to have a valid transceiver by some platforms. Maybe we
should fix that instead. Moving functions around to workaround a problem
is not the way to go, sorry.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH] usb: host: Allow EHCI_OMAP to be built-in when USB_GADGET is 'm'

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1183 bytes --]


Hi,

Roger Quadros <rogerq@ti.com> writes:
> NOP_USB_XCEIV is used not only by gadget drivers but by
> host drivers as well e.g. EHCI_OMAP.
>
> commit 5a8d651a2bde ("usb: gadget: move gadget API functions to udc-core")
> made it so that NOP_USB_XCEIV can't be built-in if USB_GADGET is 'm'.
> But this prevents EHCI_OMAP to be built-in if USB_GADGET is 'm'.
>
> Fix this undesired behaviour by moving usb_gadget_vbus_connect/disconnect()
> to usb/gadget.h so that NOP_USB_XCEIV has no build dependency
> on USB_GADGET.
>
> Retain the original Kconfig behaviour i.e. NOP_USB_XCEIV is selected
> by drivers that need it.

no, this is the wrong way to fix this. NOP _has_ a dependency on the
Gadget API if it calls Gadget API functions. Dependencies are proper.

Maybe the reason for the problem is that we ended up adding far too much
code to phy-generic.c itself. Maybe it shouldn't know about clks and
interrupts. The original idea of that driver was to simply satisfy a
requirement to have a valid transceiver by some platforms. Maybe we
should fix that instead. Moving functions around to workaround a problem
is not the way to go, sorry.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH] usb: host: Allow EHCI_OMAP to be built-in when USB_GADGET is 'm'

[Roger Quadros]

Hi Felipe,

On 30/06/16 13:14, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros <rogerq@ti.com> writes:
>> NOP_USB_XCEIV is used not only by gadget drivers but by
>> host drivers as well e.g. EHCI_OMAP.
>>
>> commit 5a8d651a2bde ("usb: gadget: move gadget API functions to udc-core")
>> made it so that NOP_USB_XCEIV can't be built-in if USB_GADGET is 'm'.
>> But this prevents EHCI_OMAP to be built-in if USB_GADGET is 'm'.
>>
>> Fix this undesired behaviour by moving usb_gadget_vbus_connect/disconnect()
>> to usb/gadget.h so that NOP_USB_XCEIV has no build dependency
>> on USB_GADGET.
>>
>> Retain the original Kconfig behaviour i.e. NOP_USB_XCEIV is selected
>> by drivers that need it.
> 
> no, this is the wrong way to fix this. NOP _has_ a dependency on the
> Gadget API if it calls Gadget API functions. Dependencies are proper.
> 
> Maybe the reason for the problem is that we ended up adding far too much
> code to phy-generic.c itself. Maybe it shouldn't know about clks and
> interrupts. The original idea of that driver was to simply satisfy a
> requirement to have a valid transceiver by some platforms. Maybe we
> should fix that instead. Moving functions around to workaround a problem
> is not the way to go, sorry.
> 
OK but something that was working all these years is broken by your patch.
Do you mind fixing it please? Or at least let me know how you want to get it fixed.

cheers,
-roger

Re: [PATCH] usb: host: Allow EHCI_OMAP to be built-in when USB_GADGET is 'm'

[Roger Quadros]

Hi Felipe,

On 30/06/16 13:14, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros <rogerq@ti.com> writes:
>> NOP_USB_XCEIV is used not only by gadget drivers but by
>> host drivers as well e.g. EHCI_OMAP.
>>
>> commit 5a8d651a2bde ("usb: gadget: move gadget API functions to udc-core")
>> made it so that NOP_USB_XCEIV can't be built-in if USB_GADGET is 'm'.
>> But this prevents EHCI_OMAP to be built-in if USB_GADGET is 'm'.
>>
>> Fix this undesired behaviour by moving usb_gadget_vbus_connect/disconnect()
>> to usb/gadget.h so that NOP_USB_XCEIV has no build dependency
>> on USB_GADGET.
>>
>> Retain the original Kconfig behaviour i.e. NOP_USB_XCEIV is selected
>> by drivers that need it.
> 
> no, this is the wrong way to fix this. NOP _has_ a dependency on the
> Gadget API if it calls Gadget API functions. Dependencies are proper.
> 
> Maybe the reason for the problem is that we ended up adding far too much
> code to phy-generic.c itself. Maybe it shouldn't know about clks and
> interrupts. The original idea of that driver was to simply satisfy a
> requirement to have a valid transceiver by some platforms. Maybe we
> should fix that instead. Moving functions around to workaround a problem
> is not the way to go, sorry.
> 
OK but something that was working all these years is broken by your patch.
Do you mind fixing it please? Or at least let me know how you want to get it fixed.

cheers,
-roger

Re: [PATCH] usb: host: Allow EHCI_OMAP to be built-in when USB_GADGET is 'm'

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1910 bytes --]


Hi,

Roger Quadros <rogerq@ti.com> writes:
>>> NOP_USB_XCEIV is used not only by gadget drivers but by
>>> host drivers as well e.g. EHCI_OMAP.
>>>
>>> commit 5a8d651a2bde ("usb: gadget: move gadget API functions to udc-core")
>>> made it so that NOP_USB_XCEIV can't be built-in if USB_GADGET is 'm'.
>>> But this prevents EHCI_OMAP to be built-in if USB_GADGET is 'm'.
>>>
>>> Fix this undesired behaviour by moving usb_gadget_vbus_connect/disconnect()
>>> to usb/gadget.h so that NOP_USB_XCEIV has no build dependency
>>> on USB_GADGET.
>>>
>>> Retain the original Kconfig behaviour i.e. NOP_USB_XCEIV is selected
>>> by drivers that need it.
>> 
>> no, this is the wrong way to fix this. NOP _has_ a dependency on the
>> Gadget API if it calls Gadget API functions. Dependencies are proper.
>> 
>> Maybe the reason for the problem is that we ended up adding far too much
>> code to phy-generic.c itself. Maybe it shouldn't know about clks and
>> interrupts. The original idea of that driver was to simply satisfy a
>> requirement to have a valid transceiver by some platforms. Maybe we
>> should fix that instead. Moving functions around to workaround a problem
>> is not the way to go, sorry.
>> 
> OK but something that was working all these years is broken by your
> patch.
> Do you mind fixing it please? Or at least let me know how you want to
> get it fixed.

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 2e710a4cca52..89fd095ca33d 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -180,7 +180,7 @@ config USB_EHCI_MXC
 config USB_EHCI_HCD_OMAP
        tristate "EHCI support for OMAP3 and later chips"
        depends on ARCH_OMAP
-       depends on NOP_USB_XCEIV
+       depends on USB_PHY
        default y
        ---help---
          Enables support for the on-chip EHCI controller on


-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH] usb: host: Allow EHCI_OMAP to be built-in when USB_GADGET is 'm'

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1932 bytes --]


Hi,

Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org> writes:
>>> NOP_USB_XCEIV is used not only by gadget drivers but by
>>> host drivers as well e.g. EHCI_OMAP.
>>>
>>> commit 5a8d651a2bde ("usb: gadget: move gadget API functions to udc-core")
>>> made it so that NOP_USB_XCEIV can't be built-in if USB_GADGET is 'm'.
>>> But this prevents EHCI_OMAP to be built-in if USB_GADGET is 'm'.
>>>
>>> Fix this undesired behaviour by moving usb_gadget_vbus_connect/disconnect()
>>> to usb/gadget.h so that NOP_USB_XCEIV has no build dependency
>>> on USB_GADGET.
>>>
>>> Retain the original Kconfig behaviour i.e. NOP_USB_XCEIV is selected
>>> by drivers that need it.
>> 
>> no, this is the wrong way to fix this. NOP _has_ a dependency on the
>> Gadget API if it calls Gadget API functions. Dependencies are proper.
>> 
>> Maybe the reason for the problem is that we ended up adding far too much
>> code to phy-generic.c itself. Maybe it shouldn't know about clks and
>> interrupts. The original idea of that driver was to simply satisfy a
>> requirement to have a valid transceiver by some platforms. Maybe we
>> should fix that instead. Moving functions around to workaround a problem
>> is not the way to go, sorry.
>> 
> OK but something that was working all these years is broken by your
> patch.
> Do you mind fixing it please? Or at least let me know how you want to
> get it fixed.

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 2e710a4cca52..89fd095ca33d 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -180,7 +180,7 @@ config USB_EHCI_MXC
 config USB_EHCI_HCD_OMAP
        tristate "EHCI support for OMAP3 and later chips"
        depends on ARCH_OMAP
-       depends on NOP_USB_XCEIV
+       depends on USB_PHY
        default y
        ---help---
          Enables support for the on-chip EHCI controller on


-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH] usb: host: Allow EHCI_OMAP to be built-in when USB_GADGET is 'm'

[Roger Quadros]

On 01/07/16 10:57, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros <rogerq@ti.com> writes:
>>>> NOP_USB_XCEIV is used not only by gadget drivers but by
>>>> host drivers as well e.g. EHCI_OMAP.
>>>>
>>>> commit 5a8d651a2bde ("usb: gadget: move gadget API functions to udc-core")
>>>> made it so that NOP_USB_XCEIV can't be built-in if USB_GADGET is 'm'.
>>>> But this prevents EHCI_OMAP to be built-in if USB_GADGET is 'm'.
>>>>
>>>> Fix this undesired behaviour by moving usb_gadget_vbus_connect/disconnect()
>>>> to usb/gadget.h so that NOP_USB_XCEIV has no build dependency
>>>> on USB_GADGET.
>>>>
>>>> Retain the original Kconfig behaviour i.e. NOP_USB_XCEIV is selected
>>>> by drivers that need it.
>>>
>>> no, this is the wrong way to fix this. NOP _has_ a dependency on the
>>> Gadget API if it calls Gadget API functions. Dependencies are proper.
>>>
>>> Maybe the reason for the problem is that we ended up adding far too much
>>> code to phy-generic.c itself. Maybe it shouldn't know about clks and
>>> interrupts. The original idea of that driver was to simply satisfy a
>>> requirement to have a valid transceiver by some platforms. Maybe we
>>> should fix that instead. Moving functions around to workaround a problem
>>> is not the way to go, sorry.
>>>
>> OK but something that was working all these years is broken by your
>> patch.
>> Do you mind fixing it please? Or at least let me know how you want to
>> get it fixed.
> 
> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index 2e710a4cca52..89fd095ca33d 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -180,7 +180,7 @@ config USB_EHCI_MXC
>  config USB_EHCI_HCD_OMAP
>         tristate "EHCI support for OMAP3 and later chips"
>         depends on ARCH_OMAP
> -       depends on NOP_USB_XCEIV
> +       depends on USB_PHY
>         default y
>         ---help---
>           Enables support for the on-chip EHCI controller on
> 
> 
This doesn't fix the real problem. i.e. we can't have NFS root on pandaboard
and omap5_uevm because network device is on USB EHCI, unless we have
both USB_GADGET and NOP_USB_XCEVI built-in, which is not really that
desirable.

cheers,
-roger

Re: [PATCH] usb: host: Allow EHCI_OMAP to be built-in when USB_GADGET is 'm'

[Roger Quadros]

On 01/07/16 10:57, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros <rogerq@ti.com> writes:
>>>> NOP_USB_XCEIV is used not only by gadget drivers but by
>>>> host drivers as well e.g. EHCI_OMAP.
>>>>
>>>> commit 5a8d651a2bde ("usb: gadget: move gadget API functions to udc-core")
>>>> made it so that NOP_USB_XCEIV can't be built-in if USB_GADGET is 'm'.
>>>> But this prevents EHCI_OMAP to be built-in if USB_GADGET is 'm'.
>>>>
>>>> Fix this undesired behaviour by moving usb_gadget_vbus_connect/disconnect()
>>>> to usb/gadget.h so that NOP_USB_XCEIV has no build dependency
>>>> on USB_GADGET.
>>>>
>>>> Retain the original Kconfig behaviour i.e. NOP_USB_XCEIV is selected
>>>> by drivers that need it.
>>>
>>> no, this is the wrong way to fix this. NOP _has_ a dependency on the
>>> Gadget API if it calls Gadget API functions. Dependencies are proper.
>>>
>>> Maybe the reason for the problem is that we ended up adding far too much
>>> code to phy-generic.c itself. Maybe it shouldn't know about clks and
>>> interrupts. The original idea of that driver was to simply satisfy a
>>> requirement to have a valid transceiver by some platforms. Maybe we
>>> should fix that instead. Moving functions around to workaround a problem
>>> is not the way to go, sorry.
>>>
>> OK but something that was working all these years is broken by your
>> patch.
>> Do you mind fixing it please? Or at least let me know how you want to
>> get it fixed.
> 
> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index 2e710a4cca52..89fd095ca33d 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -180,7 +180,7 @@ config USB_EHCI_MXC
>  config USB_EHCI_HCD_OMAP
>         tristate "EHCI support for OMAP3 and later chips"
>         depends on ARCH_OMAP
> -       depends on NOP_USB_XCEIV
> +       depends on USB_PHY
>         default y
>         ---help---
>           Enables support for the on-chip EHCI controller on
> 
> 
This doesn't fix the real problem. i.e. we can't have NFS root on pandaboard
and omap5_uevm because network device is on USB EHCI, unless we have
both USB_GADGET and NOP_USB_XCEVI built-in, which is not really that
desirable.

cheers,
-roger

Re: [PATCH] usb: host: Allow EHCI_OMAP to be built-in when USB_GADGET is 'm'

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2751 bytes --]


Hi,

Roger Quadros <rogerq@ti.com> writes:
>>>>> NOP_USB_XCEIV is used not only by gadget drivers but by
>>>>> host drivers as well e.g. EHCI_OMAP.
>>>>>
>>>>> commit 5a8d651a2bde ("usb: gadget: move gadget API functions to udc-core")
>>>>> made it so that NOP_USB_XCEIV can't be built-in if USB_GADGET is 'm'.
>>>>> But this prevents EHCI_OMAP to be built-in if USB_GADGET is 'm'.
>>>>>
>>>>> Fix this undesired behaviour by moving usb_gadget_vbus_connect/disconnect()
>>>>> to usb/gadget.h so that NOP_USB_XCEIV has no build dependency
>>>>> on USB_GADGET.
>>>>>
>>>>> Retain the original Kconfig behaviour i.e. NOP_USB_XCEIV is selected
>>>>> by drivers that need it.
>>>>
>>>> no, this is the wrong way to fix this. NOP _has_ a dependency on the
>>>> Gadget API if it calls Gadget API functions. Dependencies are proper.
>>>>
>>>> Maybe the reason for the problem is that we ended up adding far too much
>>>> code to phy-generic.c itself. Maybe it shouldn't know about clks and
>>>> interrupts. The original idea of that driver was to simply satisfy a
>>>> requirement to have a valid transceiver by some platforms. Maybe we
>>>> should fix that instead. Moving functions around to workaround a problem
>>>> is not the way to go, sorry.
>>>>
>>> OK but something that was working all these years is broken by your
>>> patch.
>>> Do you mind fixing it please? Or at least let me know how you want to
>>> get it fixed.
>> 
>> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
>> index 2e710a4cca52..89fd095ca33d 100644
>> --- a/drivers/usb/host/Kconfig
>> +++ b/drivers/usb/host/Kconfig
>> @@ -180,7 +180,7 @@ config USB_EHCI_MXC
>>  config USB_EHCI_HCD_OMAP
>>         tristate "EHCI support for OMAP3 and later chips"
>>         depends on ARCH_OMAP
>> -       depends on NOP_USB_XCEIV
>> +       depends on USB_PHY
>>         default y
>>         ---help---
>>           Enables support for the on-chip EHCI controller on
>> 
>> 
> This doesn't fix the real problem. i.e. we can't have NFS root on pandaboard
> and omap5_uevm because network device is on USB EHCI, unless we have
> both USB_GADGET and NOP_USB_XCEVI built-in, which is not really that
> desirable.

Well, NOP calls a symbol that belongs to the Gadget API. So NOP cannot
be built-in if gadget API is a module. Satisfy the dependencies and
everything will work for you.

Don't get me wrong, I see what you mean; but you _are_ relying in a
driver that calls into the Gadget API. Consider if it was the other way
around: say NOP was calling something from usbcore for whatever reason
and MUSB depended on NOP. You wouldn't have MUSB built-in unless NOP and
usbcore were also built-in, right?

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH] usb: host: Allow EHCI_OMAP to be built-in when USB_GADGET is 'm'

[Roger Quadros]

On 01/07/16 11:15, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros <rogerq@ti.com> writes:
>>>>>> NOP_USB_XCEIV is used not only by gadget drivers but by
>>>>>> host drivers as well e.g. EHCI_OMAP.
>>>>>>
>>>>>> commit 5a8d651a2bde ("usb: gadget: move gadget API functions to udc-core")
>>>>>> made it so that NOP_USB_XCEIV can't be built-in if USB_GADGET is 'm'.
>>>>>> But this prevents EHCI_OMAP to be built-in if USB_GADGET is 'm'.
>>>>>>
>>>>>> Fix this undesired behaviour by moving usb_gadget_vbus_connect/disconnect()
>>>>>> to usb/gadget.h so that NOP_USB_XCEIV has no build dependency
>>>>>> on USB_GADGET.
>>>>>>
>>>>>> Retain the original Kconfig behaviour i.e. NOP_USB_XCEIV is selected
>>>>>> by drivers that need it.
>>>>>
>>>>> no, this is the wrong way to fix this. NOP _has_ a dependency on the
>>>>> Gadget API if it calls Gadget API functions. Dependencies are proper.
>>>>>
>>>>> Maybe the reason for the problem is that we ended up adding far too much
>>>>> code to phy-generic.c itself. Maybe it shouldn't know about clks and
>>>>> interrupts. The original idea of that driver was to simply satisfy a
>>>>> requirement to have a valid transceiver by some platforms. Maybe we
>>>>> should fix that instead. Moving functions around to workaround a problem
>>>>> is not the way to go, sorry.
>>>>>
>>>> OK but something that was working all these years is broken by your
>>>> patch.
>>>> Do you mind fixing it please? Or at least let me know how you want to
>>>> get it fixed.
>>>
>>> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
>>> index 2e710a4cca52..89fd095ca33d 100644
>>> --- a/drivers/usb/host/Kconfig
>>> +++ b/drivers/usb/host/Kconfig
>>> @@ -180,7 +180,7 @@ config USB_EHCI_MXC
>>>  config USB_EHCI_HCD_OMAP
>>>         tristate "EHCI support for OMAP3 and later chips"
>>>         depends on ARCH_OMAP
>>> -       depends on NOP_USB_XCEIV
>>> +       depends on USB_PHY
>>>         default y
>>>         ---help---
>>>           Enables support for the on-chip EHCI controller on
>>>
>>>
>> This doesn't fix the real problem. i.e. we can't have NFS root on pandaboard
>> and omap5_uevm because network device is on USB EHCI, unless we have
>> both USB_GADGET and NOP_USB_XCEVI built-in, which is not really that
>> desirable.
> 
> Well, NOP calls a symbol that belongs to the Gadget API. So NOP cannot
> be built-in if gadget API is a module. Satisfy the dependencies and
> everything will work for you.
> 
> Don't get me wrong, I see what you mean; but you _are_ relying in a
> driver that calls into the Gadget API. Consider if it was the other way
> around: say NOP was calling something from usbcore for whatever reason
> and MUSB depended on NOP. You wouldn't have MUSB built-in unless NOP and
> usbcore were also built-in, right?
> 
I agree with you. As far as EHCI_OMAP is concerned we're only interested
in configuring the PHY clocks and issuing a GPIO reset.
As of now because of dependency issues with GADGET, NOP_USB_XCEIV doesn't
seem to be the right place. I'm OK with using some other driver
that doesn't have such dependency, even if it breaks DT compatibility with
some OMAP boards. Or I'll just let EHCI_OMAP do it all by itself.

Unless Tony has any objections.

cheers,
-roger

Re: [PATCH] usb: host: Allow EHCI_OMAP to be built-in when USB_GADGET is 'm'

[Roger Quadros]

On 01/07/16 11:15, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros <rogerq@ti.com> writes:
>>>>>> NOP_USB_XCEIV is used not only by gadget drivers but by
>>>>>> host drivers as well e.g. EHCI_OMAP.
>>>>>>
>>>>>> commit 5a8d651a2bde ("usb: gadget: move gadget API functions to udc-core")
>>>>>> made it so that NOP_USB_XCEIV can't be built-in if USB_GADGET is 'm'.
>>>>>> But this prevents EHCI_OMAP to be built-in if USB_GADGET is 'm'.
>>>>>>
>>>>>> Fix this undesired behaviour by moving usb_gadget_vbus_connect/disconnect()
>>>>>> to usb/gadget.h so that NOP_USB_XCEIV has no build dependency
>>>>>> on USB_GADGET.
>>>>>>
>>>>>> Retain the original Kconfig behaviour i.e. NOP_USB_XCEIV is selected
>>>>>> by drivers that need it.
>>>>>
>>>>> no, this is the wrong way to fix this. NOP _has_ a dependency on the
>>>>> Gadget API if it calls Gadget API functions. Dependencies are proper.
>>>>>
>>>>> Maybe the reason for the problem is that we ended up adding far too much
>>>>> code to phy-generic.c itself. Maybe it shouldn't know about clks and
>>>>> interrupts. The original idea of that driver was to simply satisfy a
>>>>> requirement to have a valid transceiver by some platforms. Maybe we
>>>>> should fix that instead. Moving functions around to workaround a problem
>>>>> is not the way to go, sorry.
>>>>>
>>>> OK but something that was working all these years is broken by your
>>>> patch.
>>>> Do you mind fixing it please? Or at least let me know how you want to
>>>> get it fixed.
>>>
>>> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
>>> index 2e710a4cca52..89fd095ca33d 100644
>>> --- a/drivers/usb/host/Kconfig
>>> +++ b/drivers/usb/host/Kconfig
>>> @@ -180,7 +180,7 @@ config USB_EHCI_MXC
>>>  config USB_EHCI_HCD_OMAP
>>>         tristate "EHCI support for OMAP3 and later chips"
>>>         depends on ARCH_OMAP
>>> -       depends on NOP_USB_XCEIV
>>> +       depends on USB_PHY
>>>         default y
>>>         ---help---
>>>           Enables support for the on-chip EHCI controller on
>>>
>>>
>> This doesn't fix the real problem. i.e. we can't have NFS root on pandaboard
>> and omap5_uevm because network device is on USB EHCI, unless we have
>> both USB_GADGET and NOP_USB_XCEVI built-in, which is not really that
>> desirable.
> 
> Well, NOP calls a symbol that belongs to the Gadget API. So NOP cannot
> be built-in if gadget API is a module. Satisfy the dependencies and
> everything will work for you.
> 
> Don't get me wrong, I see what you mean; but you _are_ relying in a
> driver that calls into the Gadget API. Consider if it was the other way
> around: say NOP was calling something from usbcore for whatever reason
> and MUSB depended on NOP. You wouldn't have MUSB built-in unless NOP and
> usbcore were also built-in, right?
> 
I agree with you. As far as EHCI_OMAP is concerned we're only interested
in configuring the PHY clocks and issuing a GPIO reset.
As of now because of dependency issues with GADGET, NOP_USB_XCEIV doesn't
seem to be the right place. I'm OK with using some other driver
that doesn't have such dependency, even if it breaks DT compatibility with
some OMAP boards. Or I'll just let EHCI_OMAP do it all by itself.

Unless Tony has any objections.

cheers,
-roger

Re: [PATCH] usb: host: Allow EHCI_OMAP to be built-in when USB_GADGET is 'm'

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 3438 bytes --]


Hi,

Roger Quadros <rogerq@ti.com> writes:
>>>>>>> NOP_USB_XCEIV is used not only by gadget drivers but by
>>>>>>> host drivers as well e.g. EHCI_OMAP.
>>>>>>>
>>>>>>> commit 5a8d651a2bde ("usb: gadget: move gadget API functions to udc-core")
>>>>>>> made it so that NOP_USB_XCEIV can't be built-in if USB_GADGET is 'm'.
>>>>>>> But this prevents EHCI_OMAP to be built-in if USB_GADGET is 'm'.
>>>>>>>
>>>>>>> Fix this undesired behaviour by moving usb_gadget_vbus_connect/disconnect()
>>>>>>> to usb/gadget.h so that NOP_USB_XCEIV has no build dependency
>>>>>>> on USB_GADGET.
>>>>>>>
>>>>>>> Retain the original Kconfig behaviour i.e. NOP_USB_XCEIV is selected
>>>>>>> by drivers that need it.
>>>>>>
>>>>>> no, this is the wrong way to fix this. NOP _has_ a dependency on the
>>>>>> Gadget API if it calls Gadget API functions. Dependencies are proper.
>>>>>>
>>>>>> Maybe the reason for the problem is that we ended up adding far too much
>>>>>> code to phy-generic.c itself. Maybe it shouldn't know about clks and
>>>>>> interrupts. The original idea of that driver was to simply satisfy a
>>>>>> requirement to have a valid transceiver by some platforms. Maybe we
>>>>>> should fix that instead. Moving functions around to workaround a problem
>>>>>> is not the way to go, sorry.
>>>>>>
>>>>> OK but something that was working all these years is broken by your
>>>>> patch.
>>>>> Do you mind fixing it please? Or at least let me know how you want to
>>>>> get it fixed.
>>>>
>>>> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
>>>> index 2e710a4cca52..89fd095ca33d 100644
>>>> --- a/drivers/usb/host/Kconfig
>>>> +++ b/drivers/usb/host/Kconfig
>>>> @@ -180,7 +180,7 @@ config USB_EHCI_MXC
>>>>  config USB_EHCI_HCD_OMAP
>>>>         tristate "EHCI support for OMAP3 and later chips"
>>>>         depends on ARCH_OMAP
>>>> -       depends on NOP_USB_XCEIV
>>>> +       depends on USB_PHY
>>>>         default y
>>>>         ---help---
>>>>           Enables support for the on-chip EHCI controller on
>>>>
>>>>
>>> This doesn't fix the real problem. i.e. we can't have NFS root on pandaboard
>>> and omap5_uevm because network device is on USB EHCI, unless we have
>>> both USB_GADGET and NOP_USB_XCEVI built-in, which is not really that
>>> desirable.
>> 
>> Well, NOP calls a symbol that belongs to the Gadget API. So NOP cannot
>> be built-in if gadget API is a module. Satisfy the dependencies and
>> everything will work for you.
>> 
>> Don't get me wrong, I see what you mean; but you _are_ relying in a
>> driver that calls into the Gadget API. Consider if it was the other way
>> around: say NOP was calling something from usbcore for whatever reason
>> and MUSB depended on NOP. You wouldn't have MUSB built-in unless NOP and
>> usbcore were also built-in, right?
>> 
> I agree with you. As far as EHCI_OMAP is concerned we're only interested
> in configuring the PHY clocks and issuing a GPIO reset.
> As of now because of dependency issues with GADGET, NOP_USB_XCEIV doesn't
> seem to be the right place. I'm OK with using some other driver
> that doesn't have such dependency, even if it breaks DT compatibility with
> some OMAP boards. Or I'll just let EHCI_OMAP do it all by itself.
>
> Unless Tony has any objections.

I don't have memory of why EHCI-omap is using PHY, actually. Is it
really resetting a PHY?

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH] usb: host: Allow EHCI_OMAP to be built-in when USB_GADGET is 'm'

[Roger Quadros]

On 01/07/16 12:41, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros <rogerq@ti.com> writes:
>>>>>>>> NOP_USB_XCEIV is used not only by gadget drivers but by
>>>>>>>> host drivers as well e.g. EHCI_OMAP.
>>>>>>>>
>>>>>>>> commit 5a8d651a2bde ("usb: gadget: move gadget API functions to udc-core")
>>>>>>>> made it so that NOP_USB_XCEIV can't be built-in if USB_GADGET is 'm'.
>>>>>>>> But this prevents EHCI_OMAP to be built-in if USB_GADGET is 'm'.
>>>>>>>>
>>>>>>>> Fix this undesired behaviour by moving usb_gadget_vbus_connect/disconnect()
>>>>>>>> to usb/gadget.h so that NOP_USB_XCEIV has no build dependency
>>>>>>>> on USB_GADGET.
>>>>>>>>
>>>>>>>> Retain the original Kconfig behaviour i.e. NOP_USB_XCEIV is selected
>>>>>>>> by drivers that need it.
>>>>>>>
>>>>>>> no, this is the wrong way to fix this. NOP _has_ a dependency on the
>>>>>>> Gadget API if it calls Gadget API functions. Dependencies are proper.
>>>>>>>
>>>>>>> Maybe the reason for the problem is that we ended up adding far too much
>>>>>>> code to phy-generic.c itself. Maybe it shouldn't know about clks and
>>>>>>> interrupts. The original idea of that driver was to simply satisfy a
>>>>>>> requirement to have a valid transceiver by some platforms. Maybe we
>>>>>>> should fix that instead. Moving functions around to workaround a problem
>>>>>>> is not the way to go, sorry.
>>>>>>>
>>>>>> OK but something that was working all these years is broken by your
>>>>>> patch.
>>>>>> Do you mind fixing it please? Or at least let me know how you want to
>>>>>> get it fixed.
>>>>>
>>>>> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
>>>>> index 2e710a4cca52..89fd095ca33d 100644
>>>>> --- a/drivers/usb/host/Kconfig
>>>>> +++ b/drivers/usb/host/Kconfig
>>>>> @@ -180,7 +180,7 @@ config USB_EHCI_MXC
>>>>>  config USB_EHCI_HCD_OMAP
>>>>>         tristate "EHCI support for OMAP3 and later chips"
>>>>>         depends on ARCH_OMAP
>>>>> -       depends on NOP_USB_XCEIV
>>>>> +       depends on USB_PHY
>>>>>         default y
>>>>>         ---help---
>>>>>           Enables support for the on-chip EHCI controller on
>>>>>
>>>>>
>>>> This doesn't fix the real problem. i.e. we can't have NFS root on pandaboard
>>>> and omap5_uevm because network device is on USB EHCI, unless we have
>>>> both USB_GADGET and NOP_USB_XCEVI built-in, which is not really that
>>>> desirable.
>>>
>>> Well, NOP calls a symbol that belongs to the Gadget API. So NOP cannot
>>> be built-in if gadget API is a module. Satisfy the dependencies and
>>> everything will work for you.
>>>
>>> Don't get me wrong, I see what you mean; but you _are_ relying in a
>>> driver that calls into the Gadget API. Consider if it was the other way
>>> around: say NOP was calling something from usbcore for whatever reason
>>> and MUSB depended on NOP. You wouldn't have MUSB built-in unless NOP and
>>> usbcore were also built-in, right?
>>>
>> I agree with you. As far as EHCI_OMAP is concerned we're only interested
>> in configuring the PHY clocks and issuing a GPIO reset.
>> As of now because of dependency issues with GADGET, NOP_USB_XCEIV doesn't
>> seem to be the right place. I'm OK with using some other driver
>> that doesn't have such dependency, even if it breaks DT compatibility with
>> some OMAP boards. Or I'll just let EHCI_OMAP do it all by itself.
>>
>> Unless Tony has any objections.
> 
> I don't have memory of why EHCI-omap is using PHY, actually. Is it
> really resetting a PHY?
> 
on omap3-beagle, omap4-panda and omap5-uevm, we have an external USB PHY
for the EHCI host. That PHY has it's reset line tied to a GPIO and CLK line
tied to one of the SoC clock pins.

cheers,
-roger

Re: [PATCH] usb: host: Allow EHCI_OMAP to be built-in when USB_GADGET is 'm'

[Roger Quadros]

On 01/07/16 12:41, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros <rogerq@ti.com> writes:
>>>>>>>> NOP_USB_XCEIV is used not only by gadget drivers but by
>>>>>>>> host drivers as well e.g. EHCI_OMAP.
>>>>>>>>
>>>>>>>> commit 5a8d651a2bde ("usb: gadget: move gadget API functions to udc-core")
>>>>>>>> made it so that NOP_USB_XCEIV can't be built-in if USB_GADGET is 'm'.
>>>>>>>> But this prevents EHCI_OMAP to be built-in if USB_GADGET is 'm'.
>>>>>>>>
>>>>>>>> Fix this undesired behaviour by moving usb_gadget_vbus_connect/disconnect()
>>>>>>>> to usb/gadget.h so that NOP_USB_XCEIV has no build dependency
>>>>>>>> on USB_GADGET.
>>>>>>>>
>>>>>>>> Retain the original Kconfig behaviour i.e. NOP_USB_XCEIV is selected
>>>>>>>> by drivers that need it.
>>>>>>>
>>>>>>> no, this is the wrong way to fix this. NOP _has_ a dependency on the
>>>>>>> Gadget API if it calls Gadget API functions. Dependencies are proper.
>>>>>>>
>>>>>>> Maybe the reason for the problem is that we ended up adding far too much
>>>>>>> code to phy-generic.c itself. Maybe it shouldn't know about clks and
>>>>>>> interrupts. The original idea of that driver was to simply satisfy a
>>>>>>> requirement to have a valid transceiver by some platforms. Maybe we
>>>>>>> should fix that instead. Moving functions around to workaround a problem
>>>>>>> is not the way to go, sorry.
>>>>>>>
>>>>>> OK but something that was working all these years is broken by your
>>>>>> patch.
>>>>>> Do you mind fixing it please? Or at least let me know how you want to
>>>>>> get it fixed.
>>>>>
>>>>> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
>>>>> index 2e710a4cca52..89fd095ca33d 100644
>>>>> --- a/drivers/usb/host/Kconfig
>>>>> +++ b/drivers/usb/host/Kconfig
>>>>> @@ -180,7 +180,7 @@ config USB_EHCI_MXC
>>>>>  config USB_EHCI_HCD_OMAP
>>>>>         tristate "EHCI support for OMAP3 and later chips"
>>>>>         depends on ARCH_OMAP
>>>>> -       depends on NOP_USB_XCEIV
>>>>> +       depends on USB_PHY
>>>>>         default y
>>>>>         ---help---
>>>>>           Enables support for the on-chip EHCI controller on
>>>>>
>>>>>
>>>> This doesn't fix the real problem. i.e. we can't have NFS root on pandaboard
>>>> and omap5_uevm because network device is on USB EHCI, unless we have
>>>> both USB_GADGET and NOP_USB_XCEVI built-in, which is not really that
>>>> desirable.
>>>
>>> Well, NOP calls a symbol that belongs to the Gadget API. So NOP cannot
>>> be built-in if gadget API is a module. Satisfy the dependencies and
>>> everything will work for you.
>>>
>>> Don't get me wrong, I see what you mean; but you _are_ relying in a
>>> driver that calls into the Gadget API. Consider if it was the other way
>>> around: say NOP was calling something from usbcore for whatever reason
>>> and MUSB depended on NOP. You wouldn't have MUSB built-in unless NOP and
>>> usbcore were also built-in, right?
>>>
>> I agree with you. As far as EHCI_OMAP is concerned we're only interested
>> in configuring the PHY clocks and issuing a GPIO reset.
>> As of now because of dependency issues with GADGET, NOP_USB_XCEIV doesn't
>> seem to be the right place. I'm OK with using some other driver
>> that doesn't have such dependency, even if it breaks DT compatibility with
>> some OMAP boards. Or I'll just let EHCI_OMAP do it all by itself.
>>
>> Unless Tony has any objections.
> 
> I don't have memory of why EHCI-omap is using PHY, actually. Is it
> really resetting a PHY?
> 
on omap3-beagle, omap4-panda and omap5-uevm, we have an external USB PHY
for the EHCI host. That PHY has it's reset line tied to a GPIO and CLK line
tied to one of the SoC clock pins.

cheers,
-roger