[RFC PATCH 0/4] Implement USB device/host switch for Vybrid

[RFC PATCH 0/4] Implement USB device/host switch for Vybrid

[Sanchayan Maity]

Hello Peter,

The existing usage of extcon in Chipidea driver relies on OTG
registers. In case of SoC with dual role device but not a true
OTG controller, this does not work. Such SoC's should specify
the existing CI_HDRC_DUAL_ROLE_NOT_OTG flag and do the role
switch without checking any of the OTG registers in my opinion.
This is the case for Vybrid which uses a Chipidea IP but does
not have a true 5 pin OTG implemented.

This patch almost reverts most of commit 3ecb3e09b042e70799ff3a1ff464a5ecaa7547d9
"usb: chipidea: Use extcon framework for VBUS and ID detect".
We do not rely anymore on emulating an OTG capable controller
by faking OTG controller reads.

Observations without the following patchset with the current
implementation.

CONFIG_USB_OTG was kept selected. Just the device tree changes
done in the fourth patch of this patch were done.

1. Booting with the USB device connected and then disconnecting
on boot results in "irq 38: nobody cared (try booting with the
"irqpoll" option)" stack trace.

2. Disconnecting the USB device and reconnecting results in a
timeout error message coming ci_handle_id_switch --> hw_wait_reg
while waiting for OTGSC_BSV register field. Why rely on a read
from OTG register while using extcon?

Also, usb_gadget_vbus_connect seems not to be called, render device
mode useless (usb_gadget_vbus_connect according to my understanding
should be called through ci_vbus_notifier -> ci_irq -> ci_otg_work ->
ci_handle_vbus_change).

3. Once in a while doing the switch from host to device would
result "ci_hdrc.0 Freeing queued request" but the switch won't
be successful as expected and I wont get the following message

[  167.298040] ci_hdrc ci_hdrc.0: USB bus 2 deregistered
[  167.772520] configfs-gadget gadget: high-speed config #1: c

and further our RNDIS client configuration on said USB connection
won't work. Once in a while I also experience a complete freeze
if I try to ping the interface after this.

4. With the current "OTG" implementation, should the ci->lock not
be acquired before calling ci_role_stop or ci_role_start in the
code path ci_id_notifier -> ci_irq -> otg_workqueue?

Looking at the commit message this extcon was introduced for Qualcomm's
410 Dragonboard and while I did not look at Dragonboard's or 410's DS,
requirement seems similar to Vybrid.?

What are your thoughts on this? Or considering the requirement of Qualcomm
would keeping direct role switch in case of DUAL_ROLE_NOT_OTG flag
and keeping existing OTG reads would be acceptable? Currently in our tree
we keep both implementations. The 410 is a 64bit SoC but grepping arch/
arm64/boot/dts/qcom for an extcon entry I didn't find any.

Regards,
Sanchayan.

Sanchayan Maity (4):
  usb: chipidea: Do not rely on OTG while using extcon
  usb: chipidea: ci_hdrc_imx: Introduce CI_HDRC_DUAL_ROLE_NOT_OTG for Vybrid
  ARM: dts: vfxxx: Make Vybrid match only on it's own compatible string
  ARM: dts: vf-colibri: USB device/host switch using extcon gpio

 arch/arm/boot/dts/vf-colibri-eval-v3.dtsi | 12 ++++++
 arch/arm/boot/dts/vf-colibri.dtsi         |  7 ++++
 arch/arm/boot/dts/vfxxx.dtsi              |  2 +-
 drivers/usb/chipidea/ci_hdrc_imx.c        |  5 +++
 drivers/usb/chipidea/core.c               | 64 +++++++++++++++++--------------
 drivers/usb/chipidea/otg.c                | 39 +------------------
 include/linux/usb/chipidea.h              |  2 -
 7 files changed, 61 insertions(+), 70 deletions(-)

-- 
2.7.3

[RFC PATCH 0/4] Implement USB device/host switch for Vybrid

[Sanchayan Maity]

Hello Peter,

The existing usage of extcon in Chipidea driver relies on OTG
registers. In case of SoC with dual role device but not a true
OTG controller, this does not work. Such SoC's should specify
the existing CI_HDRC_DUAL_ROLE_NOT_OTG flag and do the role
switch without checking any of the OTG registers in my opinion.
This is the case for Vybrid which uses a Chipidea IP but does
not have a true 5 pin OTG implemented.

This patch almost reverts most of commit 3ecb3e09b042e70799ff3a1ff464a5ecaa7547d9
"usb: chipidea: Use extcon framework for VBUS and ID detect".
We do not rely anymore on emulating an OTG capable controller
by faking OTG controller reads.

Observations without the following patchset with the current
implementation.

CONFIG_USB_OTG was kept selected. Just the device tree changes
done in the fourth patch of this patch were done.

1. Booting with the USB device connected and then disconnecting
on boot results in "irq 38: nobody cared (try booting with the
"irqpoll" option)" stack trace.

2. Disconnecting the USB device and reconnecting results in a
timeout error message coming ci_handle_id_switch --> hw_wait_reg
while waiting for OTGSC_BSV register field. Why rely on a read
from OTG register while using extcon?

Also, usb_gadget_vbus_connect seems not to be called, render device
mode useless (usb_gadget_vbus_connect according to my understanding
should be called through ci_vbus_notifier -> ci_irq -> ci_otg_work ->
ci_handle_vbus_change).

3. Once in a while doing the switch from host to device would
result "ci_hdrc.0 Freeing queued request" but the switch won't
be successful as expected and I wont get the following message

[  167.298040] ci_hdrc ci_hdrc.0: USB bus 2 deregistered
[  167.772520] configfs-gadget gadget: high-speed config #1: c

and further our RNDIS client configuration on said USB connection
won't work. Once in a while I also experience a complete freeze
if I try to ping the interface after this.

4. With the current "OTG" implementation, should the ci->lock not
be acquired before calling ci_role_stop or ci_role_start in the
code path ci_id_notifier -> ci_irq -> otg_workqueue?

Looking at the commit message this extcon was introduced for Qualcomm's
410 Dragonboard and while I did not look at Dragonboard's or 410's DS,
requirement seems similar to Vybrid.?

What are your thoughts on this? Or considering the requirement of Qualcomm
would keeping direct role switch in case of DUAL_ROLE_NOT_OTG flag
and keeping existing OTG reads would be acceptable? Currently in our tree
we keep both implementations. The 410 is a 64bit SoC but grepping arch/
arm64/boot/dts/qcom for an extcon entry I didn't find any.

Regards,
Sanchayan.

Sanchayan Maity (4):
  usb: chipidea: Do not rely on OTG while using extcon
  usb: chipidea: ci_hdrc_imx: Introduce CI_HDRC_DUAL_ROLE_NOT_OTG for Vybrid
  ARM: dts: vfxxx: Make Vybrid match only on it's own compatible string
  ARM: dts: vf-colibri: USB device/host switch using extcon gpio

 arch/arm/boot/dts/vf-colibri-eval-v3.dtsi | 12 ++++++
 arch/arm/boot/dts/vf-colibri.dtsi         |  7 ++++
 arch/arm/boot/dts/vfxxx.dtsi              |  2 +-
 drivers/usb/chipidea/ci_hdrc_imx.c        |  5 +++
 drivers/usb/chipidea/core.c               | 64 +++++++++++++++++--------------
 drivers/usb/chipidea/otg.c                | 39 +------------------
 include/linux/usb/chipidea.h              |  2 -
 7 files changed, 61 insertions(+), 70 deletions(-)

-- 
2.7.3

[RFC PATCH 0/4] Implement USB device/host switch for Vybrid

[Sanchayan Maity]

Hello Peter,

The existing usage of extcon in Chipidea driver relies on OTG
registers. In case of SoC with dual role device but not a true
OTG controller, this does not work. Such SoC's should specify
the existing CI_HDRC_DUAL_ROLE_NOT_OTG flag and do the role
switch without checking any of the OTG registers in my opinion.
This is the case for Vybrid which uses a Chipidea IP but does
not have a true 5 pin OTG implemented.

This patch almost reverts most of commit 3ecb3e09b042e70799ff3a1ff464a5ecaa7547d9
"usb: chipidea: Use extcon framework for VBUS and ID detect".
We do not rely anymore on emulating an OTG capable controller
by faking OTG controller reads.

Observations without the following patchset with the current
implementation.

CONFIG_USB_OTG was kept selected. Just the device tree changes
done in the fourth patch of this patch were done.

1. Booting with the USB device connected and then disconnecting
on boot results in "irq 38: nobody cared (try booting with the
"irqpoll" option)" stack trace.

2. Disconnecting the USB device and reconnecting results in a
timeout error message coming ci_handle_id_switch --> hw_wait_reg
while waiting for OTGSC_BSV register field. Why rely on a read
from OTG register while using extcon?

Also, usb_gadget_vbus_connect seems not to be called, render device
mode useless (usb_gadget_vbus_connect according to my understanding
should be called through ci_vbus_notifier -> ci_irq -> ci_otg_work ->
ci_handle_vbus_change).

3. Once in a while doing the switch from host to device would
result "ci_hdrc.0 Freeing queued request" but the switch won't
be successful as expected and I wont get the following message

[  167.298040] ci_hdrc ci_hdrc.0: USB bus 2 deregistered
[  167.772520] configfs-gadget gadget: high-speed config #1: c

and further our RNDIS client configuration on said USB connection
won't work. Once in a while I also experience a complete freeze
if I try to ping the interface after this.

4. With the current "OTG" implementation, should the ci->lock not
be acquired before calling ci_role_stop or ci_role_start in the
code path ci_id_notifier -> ci_irq -> otg_workqueue?

Looking at the commit message this extcon was introduced for Qualcomm's
410 Dragonboard and while I did not look at Dragonboard's or 410's DS,
requirement seems similar to Vybrid.?

What are your thoughts on this? Or considering the requirement of Qualcomm
would keeping direct role switch in case of DUAL_ROLE_NOT_OTG flag
and keeping existing OTG reads would be acceptable? Currently in our tree
we keep both implementations. The 410 is a 64bit SoC but grepping arch/
arm64/boot/dts/qcom for an extcon entry I didn't find any.

Regards,
Sanchayan.

Sanchayan Maity (4):
  usb: chipidea: Do not rely on OTG while using extcon
  usb: chipidea: ci_hdrc_imx: Introduce CI_HDRC_DUAL_ROLE_NOT_OTG for Vybrid
  ARM: dts: vfxxx: Make Vybrid match only on it's own compatible string
  ARM: dts: vf-colibri: USB device/host switch using extcon gpio

 arch/arm/boot/dts/vf-colibri-eval-v3.dtsi | 12 ++++++
 arch/arm/boot/dts/vf-colibri.dtsi         |  7 ++++
 arch/arm/boot/dts/vfxxx.dtsi              |  2 +-
 drivers/usb/chipidea/ci_hdrc_imx.c        |  5 +++
 drivers/usb/chipidea/core.c               | 64 +++++++++++++++++--------------
 drivers/usb/chipidea/otg.c                | 39 +------------------
 include/linux/usb/chipidea.h              |  2 -
 7 files changed, 61 insertions(+), 70 deletions(-)

-- 
2.7.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC PATCH 1/4] usb: chipidea: Do not rely on OTG while using extcon

[Sanchayan Maity]

The existing usage of extcon in Chipidea driver relies on OTG
registers. In case of SoC with dual role device but not a true
OTG controller, this does not work. Such SoC's should specify
the existing CI_HDRC_DUAL_ROLE_NOT_OTG flag and do the role
switch without checking any of the OTG registers.

This patch almost reverts most of commit "usb: chipidea: Use
extcon framework for VBUS and ID detect". We do not rely
anymore on emulating an OTG capable controller by faking OTG
controller reads.

Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
---
 drivers/usb/chipidea/core.c  | 64 ++++++++++++++++++++++++--------------------
 drivers/usb/chipidea/otg.c   | 39 +--------------------------
 include/linux/usb/chipidea.h |  2 --
 3 files changed, 36 insertions(+), 69 deletions(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 7404064..d29118d 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -607,14 +607,15 @@ static int ci_vbus_notifier(struct notifier_block *nb, unsigned long event,
 	struct ci_hdrc_cable *vbus = container_of(nb, struct ci_hdrc_cable, nb);
 	struct ci_hdrc *ci = vbus->ci;
 
+	pm_runtime_get_sync(ci->dev);
+
 	if (event)
-		vbus->state = true;
+		usb_gadget_vbus_connect(&ci->gadget);
 	else
-		vbus->state = false;
+		usb_gadget_vbus_disconnect(&ci->gadget);
 
-	vbus->changed = true;
+	pm_runtime_put_sync(ci->dev);
 
-	ci_irq(ci->irq, ci);
 	return NOTIFY_DONE;
 }
 
@@ -624,14 +625,19 @@ static int ci_id_notifier(struct notifier_block *nb, unsigned long event,
 	struct ci_hdrc_cable *id = container_of(nb, struct ci_hdrc_cable, nb);
 	struct ci_hdrc *ci = id->ci;
 
-	if (event)
-		id->state = false;
-	else
-		id->state = true;
+	pm_runtime_get_sync(ci->dev);
+
+	ci_role_stop(ci);
+
+	hw_wait_phy_stable();
+
+	if (ci_role_start(ci, event ? CI_ROLE_HOST : CI_ROLE_GADGET))
+		dev_err(ci->dev,
+			"Can't start %s role\n",
+			event ? "host" : "gadget");
 
-	id->changed = true;
+	pm_runtime_put_sync(ci->dev);
 
-	ci_irq(ci->irq, ci);
 	return NOTIFY_DONE;
 }
 
@@ -738,25 +744,10 @@ static int ci_get_platdata(struct device *dev,
 	cable->nb.notifier_call = ci_vbus_notifier;
 	cable->edev = ext_vbus;
 
-	if (!IS_ERR(ext_vbus)) {
-		ret = extcon_get_cable_state_(cable->edev, EXTCON_USB);
-		if (ret)
-			cable->state = true;
-		else
-			cable->state = false;
-	}
-
 	cable = &platdata->id_extcon;
 	cable->nb.notifier_call = ci_id_notifier;
 	cable->edev = ext_id;
 
-	if (!IS_ERR(ext_id)) {
-		ret = extcon_get_cable_state_(cable->edev, EXTCON_USB_HOST);
-		if (ret)
-			cable->state = false;
-		else
-			cable->state = true;
-	}
 	return 0;
 }
 
@@ -896,6 +887,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 	void __iomem	*base;
 	int		ret;
 	enum usb_dr_mode dr_mode;
+	struct ci_hdrc_cable *cable;
 
 	if (!dev_get_platdata(dev)) {
 		dev_err(dev, "platform data missing\n");
@@ -963,6 +955,12 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 
 	ci_get_otg_capable(ci);
 
+	ret = ci_extcon_register(ci);
+	if (ret) {
+		dev_err(dev, "extcon registration failed\n");
+		goto deinit_phy;
+	}
+
 	dr_mode = ci->platdata->dr_mode;
 	/* initialize role(s) before the interrupt is requested */
 	if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) {
@@ -1003,6 +1001,12 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 			 * user can switch it through debugfs.
 			 */
 			ci->role = CI_ROLE_GADGET;
+			cable = &ci->platdata->id_extcon;
+			if (!IS_ERR(cable->edev)) {
+				if (extcon_get_cable_state(cable->edev,
+						"USB-HOST") == true)
+					ci->role = CI_ROLE_HOST;
+			}
 		}
 	} else {
 		ci->role = ci->roles[CI_ROLE_HOST]
@@ -1021,6 +1025,12 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 						ci_role(ci)->name);
 			goto stop;
 		}
+		cable = &ci->platdata->vbus_extcon;
+		if (!IS_ERR(cable->edev)) {
+			if ((ci->role == CI_ROLE_GADGET) &&
+			(extcon_get_cable_state(cable->edev, "USB") == true))
+				usb_gadget_vbus_connect(&ci->gadget);
+		}
 	}
 
 	platform_set_drvdata(pdev, ci);
@@ -1029,10 +1039,6 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 	if (ret)
 		goto stop;
 
-	ret = ci_extcon_register(ci);
-	if (ret)
-		goto stop;
-
 	if (ci->supports_runtime_pm) {
 		pm_runtime_set_active(&pdev->dev);
 		pm_runtime_enable(&pdev->dev);
diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
index 45f86da..3169c51 100644
--- a/drivers/usb/chipidea/otg.c
+++ b/drivers/usb/chipidea/otg.c
@@ -30,44 +30,7 @@
  */
 u32 hw_read_otgsc(struct ci_hdrc *ci, u32 mask)
 {
-	struct ci_hdrc_cable *cable;
-	u32 val = hw_read(ci, OP_OTGSC, mask);
-
-	/*
-	 * If using extcon framework for VBUS and/or ID signal
-	 * detection overwrite OTGSC register value
-	 */
-	cable = &ci->platdata->vbus_extcon;
-	if (!IS_ERR(cable->edev)) {
-		if (cable->changed)
-			val |= OTGSC_BSVIS;
-		else
-			val &= ~OTGSC_BSVIS;
-
-		cable->changed = false;
-
-		if (cable->state)
-			val |= OTGSC_BSV;
-		else
-			val &= ~OTGSC_BSV;
-	}
-
-	cable = &ci->platdata->id_extcon;
-	if (!IS_ERR(cable->edev)) {
-		if (cable->changed)
-			val |= OTGSC_IDIS;
-		else
-			val &= ~OTGSC_IDIS;
-
-		cable->changed = false;
-
-		if (cable->state)
-			val |= OTGSC_ID;
-		else
-			val &= ~OTGSC_ID;
-	}
-
-	return val;
+	return hw_read(ci, OP_OTGSC, mask);
 }
 
 /**
diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
index 5dd75fa..f70ab21 100644
--- a/include/linux/usb/chipidea.h
+++ b/include/linux/usb/chipidea.h
@@ -20,8 +20,6 @@ struct ci_hdrc;
  * @conn: used for notification registration
  */
 struct ci_hdrc_cable {
-	bool				state;
-	bool				changed;
 	struct extcon_dev		*edev;
 	struct ci_hdrc			*ci;
 	struct notifier_block		nb;
-- 
2.7.3

[RFC PATCH 1/4] usb: chipidea: Do not rely on OTG while using extcon

[Sanchayan Maity]

The existing usage of extcon in Chipidea driver relies on OTG
registers. In case of SoC with dual role device but not a true
OTG controller, this does not work. Such SoC's should specify
the existing CI_HDRC_DUAL_ROLE_NOT_OTG flag and do the role
switch without checking any of the OTG registers.

This patch almost reverts most of commit "usb: chipidea: Use
extcon framework for VBUS and ID detect". We do not rely
anymore on emulating an OTG capable controller by faking OTG
controller reads.

Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
---
 drivers/usb/chipidea/core.c  | 64 ++++++++++++++++++++++++--------------------
 drivers/usb/chipidea/otg.c   | 39 +--------------------------
 include/linux/usb/chipidea.h |  2 --
 3 files changed, 36 insertions(+), 69 deletions(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 7404064..d29118d 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -607,14 +607,15 @@ static int ci_vbus_notifier(struct notifier_block *nb, unsigned long event,
 	struct ci_hdrc_cable *vbus = container_of(nb, struct ci_hdrc_cable, nb);
 	struct ci_hdrc *ci = vbus->ci;
 
+	pm_runtime_get_sync(ci->dev);
+
 	if (event)
-		vbus->state = true;
+		usb_gadget_vbus_connect(&ci->gadget);
 	else
-		vbus->state = false;
+		usb_gadget_vbus_disconnect(&ci->gadget);
 
-	vbus->changed = true;
+	pm_runtime_put_sync(ci->dev);
 
-	ci_irq(ci->irq, ci);
 	return NOTIFY_DONE;
 }
 
@@ -624,14 +625,19 @@ static int ci_id_notifier(struct notifier_block *nb, unsigned long event,
 	struct ci_hdrc_cable *id = container_of(nb, struct ci_hdrc_cable, nb);
 	struct ci_hdrc *ci = id->ci;
 
-	if (event)
-		id->state = false;
-	else
-		id->state = true;
+	pm_runtime_get_sync(ci->dev);
+
+	ci_role_stop(ci);
+
+	hw_wait_phy_stable();
+
+	if (ci_role_start(ci, event ? CI_ROLE_HOST : CI_ROLE_GADGET))
+		dev_err(ci->dev,
+			"Can't start %s role\n",
+			event ? "host" : "gadget");
 
-	id->changed = true;
+	pm_runtime_put_sync(ci->dev);
 
-	ci_irq(ci->irq, ci);
 	return NOTIFY_DONE;
 }
 
@@ -738,25 +744,10 @@ static int ci_get_platdata(struct device *dev,
 	cable->nb.notifier_call = ci_vbus_notifier;
 	cable->edev = ext_vbus;
 
-	if (!IS_ERR(ext_vbus)) {
-		ret = extcon_get_cable_state_(cable->edev, EXTCON_USB);
-		if (ret)
-			cable->state = true;
-		else
-			cable->state = false;
-	}
-
 	cable = &platdata->id_extcon;
 	cable->nb.notifier_call = ci_id_notifier;
 	cable->edev = ext_id;
 
-	if (!IS_ERR(ext_id)) {
-		ret = extcon_get_cable_state_(cable->edev, EXTCON_USB_HOST);
-		if (ret)
-			cable->state = false;
-		else
-			cable->state = true;
-	}
 	return 0;
 }
 
@@ -896,6 +887,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 	void __iomem	*base;
 	int		ret;
 	enum usb_dr_mode dr_mode;
+	struct ci_hdrc_cable *cable;
 
 	if (!dev_get_platdata(dev)) {
 		dev_err(dev, "platform data missing\n");
@@ -963,6 +955,12 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 
 	ci_get_otg_capable(ci);
 
+	ret = ci_extcon_register(ci);
+	if (ret) {
+		dev_err(dev, "extcon registration failed\n");
+		goto deinit_phy;
+	}
+
 	dr_mode = ci->platdata->dr_mode;
 	/* initialize role(s) before the interrupt is requested */
 	if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) {
@@ -1003,6 +1001,12 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 			 * user can switch it through debugfs.
 			 */
 			ci->role = CI_ROLE_GADGET;
+			cable = &ci->platdata->id_extcon;
+			if (!IS_ERR(cable->edev)) {
+				if (extcon_get_cable_state(cable->edev,
+						"USB-HOST") == true)
+					ci->role = CI_ROLE_HOST;
+			}
 		}
 	} else {
 		ci->role = ci->roles[CI_ROLE_HOST]
@@ -1021,6 +1025,12 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 						ci_role(ci)->name);
 			goto stop;
 		}
+		cable = &ci->platdata->vbus_extcon;
+		if (!IS_ERR(cable->edev)) {
+			if ((ci->role == CI_ROLE_GADGET) &&
+			(extcon_get_cable_state(cable->edev, "USB") == true))
+				usb_gadget_vbus_connect(&ci->gadget);
+		}
 	}
 
 	platform_set_drvdata(pdev, ci);
@@ -1029,10 +1039,6 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 	if (ret)
 		goto stop;
 
-	ret = ci_extcon_register(ci);
-	if (ret)
-		goto stop;
-
 	if (ci->supports_runtime_pm) {
 		pm_runtime_set_active(&pdev->dev);
 		pm_runtime_enable(&pdev->dev);
diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
index 45f86da..3169c51 100644
--- a/drivers/usb/chipidea/otg.c
+++ b/drivers/usb/chipidea/otg.c
@@ -30,44 +30,7 @@
  */
 u32 hw_read_otgsc(struct ci_hdrc *ci, u32 mask)
 {
-	struct ci_hdrc_cable *cable;
-	u32 val = hw_read(ci, OP_OTGSC, mask);
-
-	/*
-	 * If using extcon framework for VBUS and/or ID signal
-	 * detection overwrite OTGSC register value
-	 */
-	cable = &ci->platdata->vbus_extcon;
-	if (!IS_ERR(cable->edev)) {
-		if (cable->changed)
-			val |= OTGSC_BSVIS;
-		else
-			val &= ~OTGSC_BSVIS;
-
-		cable->changed = false;
-
-		if (cable->state)
-			val |= OTGSC_BSV;
-		else
-			val &= ~OTGSC_BSV;
-	}
-
-	cable = &ci->platdata->id_extcon;
-	if (!IS_ERR(cable->edev)) {
-		if (cable->changed)
-			val |= OTGSC_IDIS;
-		else
-			val &= ~OTGSC_IDIS;
-
-		cable->changed = false;
-
-		if (cable->state)
-			val |= OTGSC_ID;
-		else
-			val &= ~OTGSC_ID;
-	}
-
-	return val;
+	return hw_read(ci, OP_OTGSC, mask);
 }
 
 /**
diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
index 5dd75fa..f70ab21 100644
--- a/include/linux/usb/chipidea.h
+++ b/include/linux/usb/chipidea.h
@@ -20,8 +20,6 @@ struct ci_hdrc;
  * @conn: used for notification registration
  */
 struct ci_hdrc_cable {
-	bool				state;
-	bool				changed;
 	struct extcon_dev		*edev;
 	struct ci_hdrc			*ci;
 	struct notifier_block		nb;
-- 
2.7.3

[RFC PATCH 1/4] usb: chipidea: Do not rely on OTG while using extcon

[Sanchayan Maity]

The existing usage of extcon in Chipidea driver relies on OTG
registers. In case of SoC with dual role device but not a true
OTG controller, this does not work. Such SoC's should specify
the existing CI_HDRC_DUAL_ROLE_NOT_OTG flag and do the role
switch without checking any of the OTG registers.

This patch almost reverts most of commit "usb: chipidea: Use
extcon framework for VBUS and ID detect". We do not rely
anymore on emulating an OTG capable controller by faking OTG
controller reads.

Signed-off-by: Sanchayan Maity <maitysanchayan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/usb/chipidea/core.c  | 64 ++++++++++++++++++++++++--------------------
 drivers/usb/chipidea/otg.c   | 39 +--------------------------
 include/linux/usb/chipidea.h |  2 --
 3 files changed, 36 insertions(+), 69 deletions(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 7404064..d29118d 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -607,14 +607,15 @@ static int ci_vbus_notifier(struct notifier_block *nb, unsigned long event,
 	struct ci_hdrc_cable *vbus = container_of(nb, struct ci_hdrc_cable, nb);
 	struct ci_hdrc *ci = vbus->ci;
 
+	pm_runtime_get_sync(ci->dev);
+
 	if (event)
-		vbus->state = true;
+		usb_gadget_vbus_connect(&ci->gadget);
 	else
-		vbus->state = false;
+		usb_gadget_vbus_disconnect(&ci->gadget);
 
-	vbus->changed = true;
+	pm_runtime_put_sync(ci->dev);
 
-	ci_irq(ci->irq, ci);
 	return NOTIFY_DONE;
 }
 
@@ -624,14 +625,19 @@ static int ci_id_notifier(struct notifier_block *nb, unsigned long event,
 	struct ci_hdrc_cable *id = container_of(nb, struct ci_hdrc_cable, nb);
 	struct ci_hdrc *ci = id->ci;
 
-	if (event)
-		id->state = false;
-	else
-		id->state = true;
+	pm_runtime_get_sync(ci->dev);
+
+	ci_role_stop(ci);
+
+	hw_wait_phy_stable();
+
+	if (ci_role_start(ci, event ? CI_ROLE_HOST : CI_ROLE_GADGET))
+		dev_err(ci->dev,
+			"Can't start %s role\n",
+			event ? "host" : "gadget");
 
-	id->changed = true;
+	pm_runtime_put_sync(ci->dev);
 
-	ci_irq(ci->irq, ci);
 	return NOTIFY_DONE;
 }
 
@@ -738,25 +744,10 @@ static int ci_get_platdata(struct device *dev,
 	cable->nb.notifier_call = ci_vbus_notifier;
 	cable->edev = ext_vbus;
 
-	if (!IS_ERR(ext_vbus)) {
-		ret = extcon_get_cable_state_(cable->edev, EXTCON_USB);
-		if (ret)
-			cable->state = true;
-		else
-			cable->state = false;
-	}
-
 	cable = &platdata->id_extcon;
 	cable->nb.notifier_call = ci_id_notifier;
 	cable->edev = ext_id;
 
-	if (!IS_ERR(ext_id)) {
-		ret = extcon_get_cable_state_(cable->edev, EXTCON_USB_HOST);
-		if (ret)
-			cable->state = false;
-		else
-			cable->state = true;
-	}
 	return 0;
 }
 
@@ -896,6 +887,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 	void __iomem	*base;
 	int		ret;
 	enum usb_dr_mode dr_mode;
+	struct ci_hdrc_cable *cable;
 
 	if (!dev_get_platdata(dev)) {
 		dev_err(dev, "platform data missing\n");
@@ -963,6 +955,12 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 
 	ci_get_otg_capable(ci);
 
+	ret = ci_extcon_register(ci);
+	if (ret) {
+		dev_err(dev, "extcon registration failed\n");
+		goto deinit_phy;
+	}
+
 	dr_mode = ci->platdata->dr_mode;
 	/* initialize role(s) before the interrupt is requested */
 	if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) {
@@ -1003,6 +1001,12 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 			 * user can switch it through debugfs.
 			 */
 			ci->role = CI_ROLE_GADGET;
+			cable = &ci->platdata->id_extcon;
+			if (!IS_ERR(cable->edev)) {
+				if (extcon_get_cable_state(cable->edev,
+						"USB-HOST") == true)
+					ci->role = CI_ROLE_HOST;
+			}
 		}
 	} else {
 		ci->role = ci->roles[CI_ROLE_HOST]
@@ -1021,6 +1025,12 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 						ci_role(ci)->name);
 			goto stop;
 		}
+		cable = &ci->platdata->vbus_extcon;
+		if (!IS_ERR(cable->edev)) {
+			if ((ci->role == CI_ROLE_GADGET) &&
+			(extcon_get_cable_state(cable->edev, "USB") == true))
+				usb_gadget_vbus_connect(&ci->gadget);
+		}
 	}
 
 	platform_set_drvdata(pdev, ci);
@@ -1029,10 +1039,6 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 	if (ret)
 		goto stop;
 
-	ret = ci_extcon_register(ci);
-	if (ret)
-		goto stop;
-
 	if (ci->supports_runtime_pm) {
 		pm_runtime_set_active(&pdev->dev);
 		pm_runtime_enable(&pdev->dev);
diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
index 45f86da..3169c51 100644
--- a/drivers/usb/chipidea/otg.c
+++ b/drivers/usb/chipidea/otg.c
@@ -30,44 +30,7 @@
  */
 u32 hw_read_otgsc(struct ci_hdrc *ci, u32 mask)
 {
-	struct ci_hdrc_cable *cable;
-	u32 val = hw_read(ci, OP_OTGSC, mask);
-
-	/*
-	 * If using extcon framework for VBUS and/or ID signal
-	 * detection overwrite OTGSC register value
-	 */
-	cable = &ci->platdata->vbus_extcon;
-	if (!IS_ERR(cable->edev)) {
-		if (cable->changed)
-			val |= OTGSC_BSVIS;
-		else
-			val &= ~OTGSC_BSVIS;
-
-		cable->changed = false;
-
-		if (cable->state)
-			val |= OTGSC_BSV;
-		else
-			val &= ~OTGSC_BSV;
-	}
-
-	cable = &ci->platdata->id_extcon;
-	if (!IS_ERR(cable->edev)) {
-		if (cable->changed)
-			val |= OTGSC_IDIS;
-		else
-			val &= ~OTGSC_IDIS;
-
-		cable->changed = false;
-
-		if (cable->state)
-			val |= OTGSC_ID;
-		else
-			val &= ~OTGSC_ID;
-	}

Re: [RFC PATCH 1/4] usb: chipidea: Do not rely on OTG while using extcon

[Chanwoo Choi]

Hi Sanchayan,

I recommend that you use the unique id (ex. EXTCON_USB, EXTCON_USB_HOST)
when getting/setting the state of external connector with extcon functions
- extcon_get_cable_state() is deprecated -> extcon_get_cable_state_()
- extcon_set_cable_state() is deprecated -> extcon_set_cable_state_()

You can refer to usage for new function with unique id on patch[1]
[1] 5960387a2fb83 (usb: dwc3: omap: Replace deprecated API of extcon)

I'll remove the extcon_[get/set]_cable_state() functions using the string type
to identify the external connector.

On 2016년 03월 15일 17:38, Sanchayan Maity wrote:
> The existing usage of extcon in Chipidea driver relies on OTG
> registers. In case of SoC with dual role device but not a true
> OTG controller, this does not work. Such SoC's should specify
> the existing CI_HDRC_DUAL_ROLE_NOT_OTG flag and do the role
> switch without checking any of the OTG registers.
> 
> This patch almost reverts most of commit "usb: chipidea: Use
> extcon framework for VBUS and ID detect". We do not rely
> anymore on emulating an OTG capable controller by faking OTG
> controller reads.
> 
> Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
> ---
>  drivers/usb/chipidea/core.c  | 64 ++++++++++++++++++++++++--------------------
>  drivers/usb/chipidea/otg.c   | 39 +--------------------------
>  include/linux/usb/chipidea.h |  2 --
>  3 files changed, 36 insertions(+), 69 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 7404064..d29118d 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -607,14 +607,15 @@ static int ci_vbus_notifier(struct notifier_block *nb, unsigned long event,
>  	struct ci_hdrc_cable *vbus = container_of(nb, struct ci_hdrc_cable, nb);
>  	struct ci_hdrc *ci = vbus->ci;
>  
> +	pm_runtime_get_sync(ci->dev);
> +
>  	if (event)
> -		vbus->state = true;
> +		usb_gadget_vbus_connect(&ci->gadget);
>  	else
> -		vbus->state = false;
> +		usb_gadget_vbus_disconnect(&ci->gadget);
>  
> -	vbus->changed = true;
> +	pm_runtime_put_sync(ci->dev);
>  
> -	ci_irq(ci->irq, ci);
>  	return NOTIFY_DONE;
>  }
>  
> @@ -624,14 +625,19 @@ static int ci_id_notifier(struct notifier_block *nb, unsigned long event,
>  	struct ci_hdrc_cable *id = container_of(nb, struct ci_hdrc_cable, nb);
>  	struct ci_hdrc *ci = id->ci;
>  
> -	if (event)
> -		id->state = false;
> -	else
> -		id->state = true;
> +	pm_runtime_get_sync(ci->dev);
> +
> +	ci_role_stop(ci);
> +
> +	hw_wait_phy_stable();
> +
> +	if (ci_role_start(ci, event ? CI_ROLE_HOST : CI_ROLE_GADGET))
> +		dev_err(ci->dev,
> +			"Can't start %s role\n",
> +			event ? "host" : "gadget");
>  
> -	id->changed = true;
> +	pm_runtime_put_sync(ci->dev);
>  
> -	ci_irq(ci->irq, ci);
>  	return NOTIFY_DONE;
>  }
>  
> @@ -738,25 +744,10 @@ static int ci_get_platdata(struct device *dev,
>  	cable->nb.notifier_call = ci_vbus_notifier;
>  	cable->edev = ext_vbus;
>  
> -	if (!IS_ERR(ext_vbus)) {
> -		ret = extcon_get_cable_state_(cable->edev, EXTCON_USB);
> -		if (ret)
> -			cable->state = true;
> -		else
> -			cable->state = false;
> -	}
> -
>  	cable = &platdata->id_extcon;
>  	cable->nb.notifier_call = ci_id_notifier;
>  	cable->edev = ext_id;
>  
> -	if (!IS_ERR(ext_id)) {
> -		ret = extcon_get_cable_state_(cable->edev, EXTCON_USB_HOST);
> -		if (ret)
> -			cable->state = false;
> -		else
> -			cable->state = true;
> -	}
>  	return 0;
>  }
>  
> @@ -896,6 +887,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>  	void __iomem	*base;
>  	int		ret;
>  	enum usb_dr_mode dr_mode;
> +	struct ci_hdrc_cable *cable;
>  
>  	if (!dev_get_platdata(dev)) {
>  		dev_err(dev, "platform data missing\n");
> @@ -963,6 +955,12 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>  
>  	ci_get_otg_capable(ci);
>  
> +	ret = ci_extcon_register(ci);
> +	if (ret) {
> +		dev_err(dev, "extcon registration failed\n");
> +		goto deinit_phy;
> +	}
> +
>  	dr_mode = ci->platdata->dr_mode;
>  	/* initialize role(s) before the interrupt is requested */
>  	if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) {
> @@ -1003,6 +1001,12 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>  			 * user can switch it through debugfs.
>  			 */
>  			ci->role = CI_ROLE_GADGET;
> +			cable = &ci->platdata->id_extcon;
> +			if (!IS_ERR(cable->edev)) {
> +				if (extcon_get_cable_state(cable->edev,

Use extcon_get_cable_state_() to use the EXTCON_USB_HOST id instead of using string type directly ("USB-HOST")

> +						"USB-HOST") == true)
> +					ci->role = CI_ROLE_HOST;
> +			}
>  		}
>  	} else {
>  		ci->role = ci->roles[CI_ROLE_HOST]
> @@ -1021,6 +1025,12 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>  						ci_role(ci)->name);
>  			goto stop;
>  		}
> +		cable = &ci->platdata->vbus_extcon;
> +		if (!IS_ERR(cable->edev)) {
> +			if ((ci->role == CI_ROLE_GADGET) &&
> +			(extcon_get_cable_state(cable->edev, "USB") == true))

Use extcon_get_cable_state_(cable->edev, EXTCON_USB)

[snip]

Best Regards,
Chanwoo Choi

[RFC PATCH 1/4] usb: chipidea: Do not rely on OTG while using extcon

[Chanwoo Choi]

Hi Sanchayan,

I recommend that you use the unique id (ex. EXTCON_USB, EXTCON_USB_HOST)
when getting/setting the state of external connector with extcon functions
- extcon_get_cable_state() is deprecated -> extcon_get_cable_state_()
- extcon_set_cable_state() is deprecated -> extcon_set_cable_state_()

You can refer to usage for new function with unique id on patch[1]
[1] 5960387a2fb83 (usb: dwc3: omap: Replace deprecated API of extcon)

I'll remove the extcon_[get/set]_cable_state() functions using the string type
to identify the external connector.

On 2016? 03? 15? 17:38, Sanchayan Maity wrote:
> The existing usage of extcon in Chipidea driver relies on OTG
> registers. In case of SoC with dual role device but not a true
> OTG controller, this does not work. Such SoC's should specify
> the existing CI_HDRC_DUAL_ROLE_NOT_OTG flag and do the role
> switch without checking any of the OTG registers.
> 
> This patch almost reverts most of commit "usb: chipidea: Use
> extcon framework for VBUS and ID detect". We do not rely
> anymore on emulating an OTG capable controller by faking OTG
> controller reads.
> 
> Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
> ---
>  drivers/usb/chipidea/core.c  | 64 ++++++++++++++++++++++++--------------------
>  drivers/usb/chipidea/otg.c   | 39 +--------------------------
>  include/linux/usb/chipidea.h |  2 --
>  3 files changed, 36 insertions(+), 69 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 7404064..d29118d 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -607,14 +607,15 @@ static int ci_vbus_notifier(struct notifier_block *nb, unsigned long event,
>  	struct ci_hdrc_cable *vbus = container_of(nb, struct ci_hdrc_cable, nb);
>  	struct ci_hdrc *ci = vbus->ci;
>  
> +	pm_runtime_get_sync(ci->dev);
> +
>  	if (event)
> -		vbus->state = true;
> +		usb_gadget_vbus_connect(&ci->gadget);
>  	else
> -		vbus->state = false;
> +		usb_gadget_vbus_disconnect(&ci->gadget);
>  
> -	vbus->changed = true;
> +	pm_runtime_put_sync(ci->dev);
>  
> -	ci_irq(ci->irq, ci);
>  	return NOTIFY_DONE;
>  }
>  
> @@ -624,14 +625,19 @@ static int ci_id_notifier(struct notifier_block *nb, unsigned long event,
>  	struct ci_hdrc_cable *id = container_of(nb, struct ci_hdrc_cable, nb);
>  	struct ci_hdrc *ci = id->ci;
>  
> -	if (event)
> -		id->state = false;
> -	else
> -		id->state = true;
> +	pm_runtime_get_sync(ci->dev);
> +
> +	ci_role_stop(ci);
> +
> +	hw_wait_phy_stable();
> +
> +	if (ci_role_start(ci, event ? CI_ROLE_HOST : CI_ROLE_GADGET))
> +		dev_err(ci->dev,
> +			"Can't start %s role\n",
> +			event ? "host" : "gadget");
>  
> -	id->changed = true;
> +	pm_runtime_put_sync(ci->dev);
>  
> -	ci_irq(ci->irq, ci);
>  	return NOTIFY_DONE;
>  }
>  
> @@ -738,25 +744,10 @@ static int ci_get_platdata(struct device *dev,
>  	cable->nb.notifier_call = ci_vbus_notifier;
>  	cable->edev = ext_vbus;
>  
> -	if (!IS_ERR(ext_vbus)) {
> -		ret = extcon_get_cable_state_(cable->edev, EXTCON_USB);
> -		if (ret)
> -			cable->state = true;
> -		else
> -			cable->state = false;
> -	}
> -
>  	cable = &platdata->id_extcon;
>  	cable->nb.notifier_call = ci_id_notifier;
>  	cable->edev = ext_id;
>  
> -	if (!IS_ERR(ext_id)) {
> -		ret = extcon_get_cable_state_(cable->edev, EXTCON_USB_HOST);
> -		if (ret)
> -			cable->state = false;
> -		else
> -			cable->state = true;
> -	}
>  	return 0;
>  }
>  
> @@ -896,6 +887,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>  	void __iomem	*base;
>  	int		ret;
>  	enum usb_dr_mode dr_mode;
> +	struct ci_hdrc_cable *cable;
>  
>  	if (!dev_get_platdata(dev)) {
>  		dev_err(dev, "platform data missing\n");
> @@ -963,6 +955,12 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>  
>  	ci_get_otg_capable(ci);
>  
> +	ret = ci_extcon_register(ci);
> +	if (ret) {
> +		dev_err(dev, "extcon registration failed\n");
> +		goto deinit_phy;
> +	}
> +
>  	dr_mode = ci->platdata->dr_mode;
>  	/* initialize role(s) before the interrupt is requested */
>  	if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) {
> @@ -1003,6 +1001,12 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>  			 * user can switch it through debugfs.
>  			 */
>  			ci->role = CI_ROLE_GADGET;
> +			cable = &ci->platdata->id_extcon;
> +			if (!IS_ERR(cable->edev)) {
> +				if (extcon_get_cable_state(cable->edev,

Use extcon_get_cable_state_() to use the EXTCON_USB_HOST id instead of using string type directly ("USB-HOST")

> +						"USB-HOST") == true)
> +					ci->role = CI_ROLE_HOST;
> +			}
>  		}
>  	} else {
>  		ci->role = ci->roles[CI_ROLE_HOST]
> @@ -1021,6 +1025,12 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>  						ci_role(ci)->name);
>  			goto stop;
>  		}
> +		cable = &ci->platdata->vbus_extcon;
> +		if (!IS_ERR(cable->edev)) {
> +			if ((ci->role == CI_ROLE_GADGET) &&
> +			(extcon_get_cable_state(cable->edev, "USB") == true))

Use extcon_get_cable_state_(cable->edev, EXTCON_USB)

[snip]

Best Regards,
Chanwoo Choi

Re: [RFC PATCH 1/4] usb: chipidea: Do not rely on OTG while using extcon

[Chanwoo Choi]

Hi Sanchayan,

I recommend that you use the unique id (ex. EXTCON_USB, EXTCON_USB_HOST)
when getting/setting the state of external connector with extcon functions
- extcon_get_cable_state() is deprecated -> extcon_get_cable_state_()
- extcon_set_cable_state() is deprecated -> extcon_set_cable_state_()

You can refer to usage for new function with unique id on patch[1]
[1] 5960387a2fb83 (usb: dwc3: omap: Replace deprecated API of extcon)

I'll remove the extcon_[get/set]_cable_state() functions using the string type
to identify the external connector.

On 2016년 03월 15일 17:38, Sanchayan Maity wrote:
> The existing usage of extcon in Chipidea driver relies on OTG
> registers. In case of SoC with dual role device but not a true
> OTG controller, this does not work. Such SoC's should specify
> the existing CI_HDRC_DUAL_ROLE_NOT_OTG flag and do the role
> switch without checking any of the OTG registers.
> 
> This patch almost reverts most of commit "usb: chipidea: Use
> extcon framework for VBUS and ID detect". We do not rely
> anymore on emulating an OTG capable controller by faking OTG
> controller reads.
> 
> Signed-off-by: Sanchayan Maity <maitysanchayan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/usb/chipidea/core.c  | 64 ++++++++++++++++++++++++--------------------
>  drivers/usb/chipidea/otg.c   | 39 +--------------------------
>  include/linux/usb/chipidea.h |  2 --
>  3 files changed, 36 insertions(+), 69 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 7404064..d29118d 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -607,14 +607,15 @@ static int ci_vbus_notifier(struct notifier_block *nb, unsigned long event,
>  	struct ci_hdrc_cable *vbus = container_of(nb, struct ci_hdrc_cable, nb);
>  	struct ci_hdrc *ci = vbus->ci;
>  
> +	pm_runtime_get_sync(ci->dev);
> +
>  	if (event)
> -		vbus->state = true;
> +		usb_gadget_vbus_connect(&ci->gadget);
>  	else
> -		vbus->state = false;
> +		usb_gadget_vbus_disconnect(&ci->gadget);
>  
> -	vbus->changed = true;
> +	pm_runtime_put_sync(ci->dev);
>  
> -	ci_irq(ci->irq, ci);
>  	return NOTIFY_DONE;
>  }
>  
> @@ -624,14 +625,19 @@ static int ci_id_notifier(struct notifier_block *nb, unsigned long event,
>  	struct ci_hdrc_cable *id = container_of(nb, struct ci_hdrc_cable, nb);
>  	struct ci_hdrc *ci = id->ci;
>  
> -	if (event)
> -		id->state = false;
> -	else
> -		id->state = true;
> +	pm_runtime_get_sync(ci->dev);
> +
> +	ci_role_stop(ci);
> +
> +	hw_wait_phy_stable();
> +
> +	if (ci_role_start(ci, event ? CI_ROLE_HOST : CI_ROLE_GADGET))
> +		dev_err(ci->dev,
> +			"Can't start %s role\n",
> +			event ? "host" : "gadget");
>  
> -	id->changed = true;
> +	pm_runtime_put_sync(ci->dev);
>  
> -	ci_irq(ci->irq, ci);
>  	return NOTIFY_DONE;
>  }
>  
> @@ -738,25 +744,10 @@ static int ci_get_platdata(struct device *dev,
>  	cable->nb.notifier_call = ci_vbus_notifier;
>  	cable->edev = ext_vbus;
>  
> -	if (!IS_ERR(ext_vbus)) {
> -		ret = extcon_get_cable_state_(cable->edev, EXTCON_USB);
> -		if (ret)
> -			cable->state = true;
> -		else
> -			cable->state = false;
> -	}
> -
>  	cable = &platdata->id_extcon;
>  	cable->nb.notifier_call = ci_id_notifier;
>  	cable->edev = ext_id;
>  
> -	if (!IS_ERR(ext_id)) {
> -		ret = extcon_get_cable_state_(cable->edev, EXTCON_USB_HOST);
> -		if (ret)
> -			cable->state = false;
> -		else
> -			cable->state = true;
> -	}
>  	return 0;
>  }
>  
> @@ -896,6 +887,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>  	void __iomem	*base;
>  	int		ret;
>  	enum usb_dr_mode dr_mode;
> +	struct ci_hdrc_cable *cable;
>  
>  	if (!dev_get_platdata(dev)) {
>  		dev_err(dev, "platform data missing\n");
> @@ -963,6 +955,12 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>  
>  	ci_get_otg_capable(ci);
>  
> +	ret = ci_extcon_register(ci);
> +	if (ret) {
> +		dev_err(dev, "extcon registration failed\n");
> +		goto deinit_phy;
> +	}
> +
>  	dr_mode = ci->platdata->dr_mode;
>  	/* initialize role(s) before the interrupt is requested */
>  	if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) {
> @@ -1003,6 +1001,12 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>  			 * user can switch it through debugfs.
>  			 */
>  			ci->role = CI_ROLE_GADGET;
> +			cable = &ci->platdata->id_extcon;
> +			if (!IS_ERR(cable->edev)) {
> +				if (extcon_get_cable_state(cable->edev,

Use extcon_get_cable_state_() to use the EXTCON_USB_HOST id instead of using string type directly ("USB-HOST")

> +						"USB-HOST") == true)
> +					ci->role = CI_ROLE_HOST;
> +			}
>  		}
>  	} else {
>  		ci->role = ci->roles[CI_ROLE_HOST]
> @@ -1021,6 +1025,12 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>  						ci_role(ci)->name);
>  			goto stop;
>  		}
> +		cable = &ci->platdata->vbus_extcon;
> +		if (!IS_ERR(cable->edev)) {
> +			if ((ci->role == CI_ROLE_GADGET) &&
> +			(extcon_get_cable_state(cable->edev, "USB") == true))

Use extcon_get_cable_state_(cable->edev, EXTCON_USB)

[snip]

Best Regards,
Chanwoo Choi

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC PATCH 2/4] usb: chipidea: ci_hdrc_imx: Introduce CI_HDRC_DUAL_ROLE_NOT_OTG for Vybrid

[Sanchayan Maity]

The Vybrid SoC has a dual role device which can be configured to
act as a host or device through firmware interface, but, it is
not a true OTG controller.

Use the CI_HDRC_DUAL_ROLE_NOT_OTG in the platform flag, introduce
a new platform data structure for Vybrid and match on the compatible
string fsl,vf610-usb instead of the earlier fsl,imx27-usb.

Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
---
 drivers/usb/chipidea/ci_hdrc_imx.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
index f14f4ab..d9b8a53 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -65,6 +65,10 @@ static const struct ci_hdrc_imx_platform_flag imx7d_usb_data = {
 	.flags = CI_HDRC_SUPPORTS_RUNTIME_PM,
 };
 
+static const struct ci_hdrc_imx_platform_flag vf610_usb_data = {
+	.flags = CI_HDRC_DUAL_ROLE_NOT_OTG,
+};
+
 static const struct of_device_id ci_hdrc_imx_dt_ids[] = {
 	{ .compatible = "fsl,imx28-usb", .data = &imx28_usb_data},
 	{ .compatible = "fsl,imx27-usb", .data = &imx27_usb_data},
@@ -73,6 +77,7 @@ static const struct of_device_id ci_hdrc_imx_dt_ids[] = {
 	{ .compatible = "fsl,imx6sx-usb", .data = &imx6sx_usb_data},
 	{ .compatible = "fsl,imx6ul-usb", .data = &imx6ul_usb_data},
 	{ .compatible = "fsl,imx7d-usb", .data = &imx7d_usb_data},
+	{ .compatible = "fsl,vf610-usb", .data = &vf610_usb_data},
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, ci_hdrc_imx_dt_ids);
-- 
2.7.3

[RFC PATCH 2/4] usb: chipidea: ci_hdrc_imx: Introduce CI_HDRC_DUAL_ROLE_NOT_OTG for Vybrid

[Sanchayan Maity]

The Vybrid SoC has a dual role device which can be configured to
act as a host or device through firmware interface, but, it is
not a true OTG controller.

Use the CI_HDRC_DUAL_ROLE_NOT_OTG in the platform flag, introduce
a new platform data structure for Vybrid and match on the compatible
string fsl,vf610-usb instead of the earlier fsl,imx27-usb.

Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
---
 drivers/usb/chipidea/ci_hdrc_imx.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
index f14f4ab..d9b8a53 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -65,6 +65,10 @@ static const struct ci_hdrc_imx_platform_flag imx7d_usb_data = {
 	.flags = CI_HDRC_SUPPORTS_RUNTIME_PM,
 };
 
+static const struct ci_hdrc_imx_platform_flag vf610_usb_data = {
+	.flags = CI_HDRC_DUAL_ROLE_NOT_OTG,
+};
+
 static const struct of_device_id ci_hdrc_imx_dt_ids[] = {
 	{ .compatible = "fsl,imx28-usb", .data = &imx28_usb_data},
 	{ .compatible = "fsl,imx27-usb", .data = &imx27_usb_data},
@@ -73,6 +77,7 @@ static const struct of_device_id ci_hdrc_imx_dt_ids[] = {
 	{ .compatible = "fsl,imx6sx-usb", .data = &imx6sx_usb_data},
 	{ .compatible = "fsl,imx6ul-usb", .data = &imx6ul_usb_data},
 	{ .compatible = "fsl,imx7d-usb", .data = &imx7d_usb_data},
+	{ .compatible = "fsl,vf610-usb", .data = &vf610_usb_data},
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, ci_hdrc_imx_dt_ids);
-- 
2.7.3

[RFC PATCH 3/4] ARM: dts: vfxxx: Make Vybrid match only on it's own compatible string

[Sanchayan Maity]

Remove the compatible string "fsl,imx27-usb" from being used to
match on usb device nodes for Vybrid. This is required for specifying
that Vybrid USB controller is a dual role but not a true OTG controller
using the CI_HDRC_DUAL_ROLE_NOT_OTG flag in the corresponding ci_hdrc_imx
driver.

Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
---
 arch/arm/boot/dts/vfxxx.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/vfxxx.dtsi b/arch/arm/boot/dts/vfxxx.dtsi
index 5e49fbd..c2d1b5d 100644
--- a/arch/arm/boot/dts/vfxxx.dtsi
+++ b/arch/arm/boot/dts/vfxxx.dtsi
@@ -452,7 +452,7 @@
 			};
 
 			usbdev0: usb@40034000 {
-				compatible = "fsl,vf610-usb", "fsl,imx27-usb";
+				compatible = "fsl,vf610-usb";
 				reg = <0x40034000 0x800>;
 				interrupts = <75 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks VF610_CLK_USBC0>;
-- 
2.7.3

[RFC PATCH 3/4] ARM: dts: vfxxx: Make Vybrid match only on it's own compatible string

[Sanchayan Maity]

Remove the compatible string "fsl,imx27-usb" from being used to
match on usb device nodes for Vybrid. This is required for specifying
that Vybrid USB controller is a dual role but not a true OTG controller
using the CI_HDRC_DUAL_ROLE_NOT_OTG flag in the corresponding ci_hdrc_imx
driver.

Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
---
 arch/arm/boot/dts/vfxxx.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/vfxxx.dtsi b/arch/arm/boot/dts/vfxxx.dtsi
index 5e49fbd..c2d1b5d 100644
--- a/arch/arm/boot/dts/vfxxx.dtsi
+++ b/arch/arm/boot/dts/vfxxx.dtsi
@@ -452,7 +452,7 @@
 			};
 
 			usbdev0: usb at 40034000 {
-				compatible = "fsl,vf610-usb", "fsl,imx27-usb";
+				compatible = "fsl,vf610-usb";
 				reg = <0x40034000 0x800>;
 				interrupts = <75 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks VF610_CLK_USBC0>;
-- 
2.7.3

[RFC PATCH 4/4] ARM: dts: vf-colibri: USB device/host switch using extcon gpio

[Sanchayan Maity]

Use USBC_DET feature of standard Colibri SODIMM pin 137 for USB
device/host switching using the generic extcon USB gpio implementation.

Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
---
 arch/arm/boot/dts/vf-colibri-eval-v3.dtsi | 12 ++++++++++++
 arch/arm/boot/dts/vf-colibri.dtsi         |  7 +++++++
 2 files changed, 19 insertions(+)

diff --git a/arch/arm/boot/dts/vf-colibri-eval-v3.dtsi b/arch/arm/boot/dts/vf-colibri-eval-v3.dtsi
index 4d8b7f6..794c02e 100644
--- a/arch/arm/boot/dts/vf-colibri-eval-v3.dtsi
+++ b/arch/arm/boot/dts/vf-colibri-eval-v3.dtsi
@@ -50,6 +50,14 @@
 		clock-frequency = <16000000>;
 	};
 
+	extcon_usbc_det: usbc_det {
+		compatible = "linux,extcon-usb-gpio";
+		debounce = <25>;
+		id-gpio = <&gpio3 6 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_usbc_det>;
+	};
+
 	reg_3v3: regulator-3v3 {
 		compatible = "regulator-fixed";
 		regulator-name = "3.3V";
@@ -146,6 +154,10 @@
 	status = "okay";
 };
 
+&usbdev0 {
+	extcon = <&extcon_usbc_det>, <&extcon_usbc_det>;
+};
+
 &usbh1 {
 	vbus-supply = <&reg_usbh_vbus>;
 };
diff --git a/arch/arm/boot/dts/vf-colibri.dtsi b/arch/arm/boot/dts/vf-colibri.dtsi
index fda7f28..551b6a1 100644
--- a/arch/arm/boot/dts/vf-colibri.dtsi
+++ b/arch/arm/boot/dts/vf-colibri.dtsi
@@ -171,6 +171,7 @@
 
 &usbdev0 {
 	disable-over-current;
+	dr_mode = "otg";
 	status = "okay";
 };
 
@@ -326,6 +327,12 @@
 			>;
 		};
 
+		pinctrl_usbc_det: gpio_usbc_det {
+			fsl,pins = <
+				VF610_PAD_PTC29__GPIO_102		0x22ed
+			>;
+		};
+
 		pinctrl_usbh1_reg: gpio_usb_vbus {
 			fsl,pins = <
 				VF610_PAD_PTD4__GPIO_83			0x22ed
-- 
2.7.3

[RFC PATCH 4/4] ARM: dts: vf-colibri: USB device/host switch using extcon gpio

[Sanchayan Maity]

Use USBC_DET feature of standard Colibri SODIMM pin 137 for USB
device/host switching using the generic extcon USB gpio implementation.

Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
---
 arch/arm/boot/dts/vf-colibri-eval-v3.dtsi | 12 ++++++++++++
 arch/arm/boot/dts/vf-colibri.dtsi         |  7 +++++++
 2 files changed, 19 insertions(+)

diff --git a/arch/arm/boot/dts/vf-colibri-eval-v3.dtsi b/arch/arm/boot/dts/vf-colibri-eval-v3.dtsi
index 4d8b7f6..794c02e 100644
--- a/arch/arm/boot/dts/vf-colibri-eval-v3.dtsi
+++ b/arch/arm/boot/dts/vf-colibri-eval-v3.dtsi
@@ -50,6 +50,14 @@
 		clock-frequency = <16000000>;
 	};
 
+	extcon_usbc_det: usbc_det {
+		compatible = "linux,extcon-usb-gpio";
+		debounce = <25>;
+		id-gpio = <&gpio3 6 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_usbc_det>;
+	};
+
 	reg_3v3: regulator-3v3 {
 		compatible = "regulator-fixed";
 		regulator-name = "3.3V";
@@ -146,6 +154,10 @@
 	status = "okay";
 };
 
+&usbdev0 {
+	extcon = <&extcon_usbc_det>, <&extcon_usbc_det>;
+};
+
 &usbh1 {
 	vbus-supply = <&reg_usbh_vbus>;
 };
diff --git a/arch/arm/boot/dts/vf-colibri.dtsi b/arch/arm/boot/dts/vf-colibri.dtsi
index fda7f28..551b6a1 100644
--- a/arch/arm/boot/dts/vf-colibri.dtsi
+++ b/arch/arm/boot/dts/vf-colibri.dtsi
@@ -171,6 +171,7 @@
 
 &usbdev0 {
 	disable-over-current;
+	dr_mode = "otg";
 	status = "okay";
 };
 
@@ -326,6 +327,12 @@
 			>;
 		};
 
+		pinctrl_usbc_det: gpio_usbc_det {
+			fsl,pins = <
+				VF610_PAD_PTC29__GPIO_102		0x22ed
+			>;
+		};
+
 		pinctrl_usbh1_reg: gpio_usb_vbus {
 			fsl,pins = <
 				VF610_PAD_PTD4__GPIO_83			0x22ed
-- 
2.7.3

[RFC PATCH 4/4] ARM: dts: vf-colibri: USB device/host switch using extcon gpio

[Sanchayan Maity]

Use USBC_DET feature of standard Colibri SODIMM pin 137 for USB
device/host switching using the generic extcon USB gpio implementation.

Signed-off-by: Sanchayan Maity <maitysanchayan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 arch/arm/boot/dts/vf-colibri-eval-v3.dtsi | 12 ++++++++++++
 arch/arm/boot/dts/vf-colibri.dtsi         |  7 +++++++
 2 files changed, 19 insertions(+)

diff --git a/arch/arm/boot/dts/vf-colibri-eval-v3.dtsi b/arch/arm/boot/dts/vf-colibri-eval-v3.dtsi
index 4d8b7f6..794c02e 100644
--- a/arch/arm/boot/dts/vf-colibri-eval-v3.dtsi
+++ b/arch/arm/boot/dts/vf-colibri-eval-v3.dtsi
@@ -50,6 +50,14 @@
 		clock-frequency = <16000000>;
 	};
 
+	extcon_usbc_det: usbc_det {
+		compatible = "linux,extcon-usb-gpio";
+		debounce = <25>;
+		id-gpio = <&gpio3 6 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_usbc_det>;
+	};
+
 	reg_3v3: regulator-3v3 {
 		compatible = "regulator-fixed";
 		regulator-name = "3.3V";
@@ -146,6 +154,10 @@
 	status = "okay";
 };
 
+&usbdev0 {
+	extcon = <&extcon_usbc_det>, <&extcon_usbc_det>;
+};
+
 &usbh1 {
 	vbus-supply = <&reg_usbh_vbus>;
 };
diff --git a/arch/arm/boot/dts/vf-colibri.dtsi b/arch/arm/boot/dts/vf-colibri.dtsi
index fda7f28..551b6a1 100644
--- a/arch/arm/boot/dts/vf-colibri.dtsi
+++ b/arch/arm/boot/dts/vf-colibri.dtsi
@@ -171,6 +171,7 @@
 
 &usbdev0 {
 	disable-over-current;
+	dr_mode = "otg";
 	status = "okay";
 };
 
@@ -326,6 +327,12 @@
 			>;
 		};
 
+		pinctrl_usbc_det: gpio_usbc_det {
+			fsl,pins = <
+				VF610_PAD_PTC29__GPIO_102		0x22ed
+			>;
+		};
+
 		pinctrl_usbh1_reg: gpio_usb_vbus {
 			fsl,pins = <
 				VF610_PAD_PTD4__GPIO_83			0x22ed
-- 
2.7.3

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 0/4] Implement USB device/host switch for Vybrid

[Peter Chen]

On Tue, Mar 15, 2016 at 02:08:26PM +0530, Sanchayan Maity wrote:
> Hello Peter,
> 
> The existing usage of extcon in Chipidea driver relies on OTG
> registers. In case of SoC with dual role device but not a true
> OTG controller, this does not work. Such SoC's should specify
> the existing CI_HDRC_DUAL_ROLE_NOT_OTG flag and do the role
> switch without checking any of the OTG registers in my opinion.
> This is the case for Vybrid which uses a Chipidea IP but does
> not have a true 5 pin OTG implemented.

Sorry to reply you late due to my new born baby.

Are you sure Vybrid is NOT OTG core? Afaik, it is uses the same
IP base with other Freescale SoCs, just the IP core is 2.40a.
When working at device mode, can you read vbus status through
OTGSC? And if there is an ID pin (input pin) for Vybrid? I mean
SoC, not the board.

-- 
Best Regards,
Peter Chen

[RFC PATCH 0/4] Implement USB device/host switch for Vybrid

[Peter Chen]

On Tue, Mar 15, 2016 at 02:08:26PM +0530, Sanchayan Maity wrote:
> Hello Peter,
> 
> The existing usage of extcon in Chipidea driver relies on OTG
> registers. In case of SoC with dual role device but not a true
> OTG controller, this does not work. Such SoC's should specify
> the existing CI_HDRC_DUAL_ROLE_NOT_OTG flag and do the role
> switch without checking any of the OTG registers in my opinion.
> This is the case for Vybrid which uses a Chipidea IP but does
> not have a true 5 pin OTG implemented.

Sorry to reply you late due to my new born baby.

Are you sure Vybrid is NOT OTG core? Afaik, it is uses the same
IP base with other Freescale SoCs, just the IP core is 2.40a.
When working at device mode, can you read vbus status through
OTGSC? And if there is an ID pin (input pin) for Vybrid? I mean
SoC, not the board.

-- 
Best Regards,
Peter Chen

Re: [RFC PATCH 0/4] Implement USB device/host switch for Vybrid

[Stefan Agner]

On 2016-03-25 00:40, Peter Chen wrote:
> On Tue, Mar 15, 2016 at 02:08:26PM +0530, Sanchayan Maity wrote:
>> Hello Peter,
>>
>> The existing usage of extcon in Chipidea driver relies on OTG
>> registers. In case of SoC with dual role device but not a true
>> OTG controller, this does not work. Such SoC's should specify
>> the existing CI_HDRC_DUAL_ROLE_NOT_OTG flag and do the role
>> switch without checking any of the OTG registers in my opinion.
>> This is the case for Vybrid which uses a Chipidea IP but does
>> not have a true 5 pin OTG implemented.
> 
> Sorry to reply you late due to my new born baby.
> 
> Are you sure Vybrid is NOT OTG core? Afaik, it is uses the same
> IP base with other Freescale SoCs, just the IP core is 2.40a.
> When working at device mode, can you read vbus status through
> OTGSC? And if there is an ID pin (input pin) for Vybrid? I mean
> SoC, not the board.

I think the IP is actually OTG capable, the registers are there, but the
signals seem not to be available on the SoC package. That is also what
the RM says...

Quotes from the RM:

"OTG controller should be treated as
Dual role controller that allows the
controller to act as either a Host or a
device with no support for HNP/SRP."

And later, in Chapter 11.1:

"The USB is not a true OTG. It can be configured by software to function
either as
peripheral or as host. The ID pin, which is unique for OTG operation, is
not present in
this implementation. There are no five pin interface. The user will get
four pin host/
device interface."

--
Stefan

[RFC PATCH 0/4] Implement USB device/host switch for Vybrid

[Stefan Agner]

On 2016-03-25 00:40, Peter Chen wrote:
> On Tue, Mar 15, 2016 at 02:08:26PM +0530, Sanchayan Maity wrote:
>> Hello Peter,
>>
>> The existing usage of extcon in Chipidea driver relies on OTG
>> registers. In case of SoC with dual role device but not a true
>> OTG controller, this does not work. Such SoC's should specify
>> the existing CI_HDRC_DUAL_ROLE_NOT_OTG flag and do the role
>> switch without checking any of the OTG registers in my opinion.
>> This is the case for Vybrid which uses a Chipidea IP but does
>> not have a true 5 pin OTG implemented.
> 
> Sorry to reply you late due to my new born baby.
> 
> Are you sure Vybrid is NOT OTG core? Afaik, it is uses the same
> IP base with other Freescale SoCs, just the IP core is 2.40a.
> When working at device mode, can you read vbus status through
> OTGSC? And if there is an ID pin (input pin) for Vybrid? I mean
> SoC, not the board.

I think the IP is actually OTG capable, the registers are there, but the
signals seem not to be available on the SoC package. That is also what
the RM says...

Quotes from the RM:

"OTG controller should be treated as
Dual role controller that allows the
controller to act as either a Host or a
device with no support for HNP/SRP."

And later, in Chapter 11.1:

"The USB is not a true OTG. It can be configured by software to function
either as
peripheral or as host. The ID pin, which is unique for OTG operation, is
not present in
this implementation. There are no five pin interface. The user will get
four pin host/
device interface."

--
Stefan

Re: [RFC PATCH 0/4] Implement USB device/host switch for Vybrid

[Stefan Agner]

On 2016-03-25 00:40, Peter Chen wrote:
> On Tue, Mar 15, 2016 at 02:08:26PM +0530, Sanchayan Maity wrote:
>> Hello Peter,
>>
>> The existing usage of extcon in Chipidea driver relies on OTG
>> registers. In case of SoC with dual role device but not a true
>> OTG controller, this does not work. Such SoC's should specify
>> the existing CI_HDRC_DUAL_ROLE_NOT_OTG flag and do the role
>> switch without checking any of the OTG registers in my opinion.
>> This is the case for Vybrid which uses a Chipidea IP but does
>> not have a true 5 pin OTG implemented.
> 
> Sorry to reply you late due to my new born baby.
> 
> Are you sure Vybrid is NOT OTG core? Afaik, it is uses the same
> IP base with other Freescale SoCs, just the IP core is 2.40a.
> When working at device mode, can you read vbus status through
> OTGSC? And if there is an ID pin (input pin) for Vybrid? I mean
> SoC, not the board.

I think the IP is actually OTG capable, the registers are there, but the
signals seem not to be available on the SoC package. That is also what
the RM says...

Quotes from the RM:

"OTG controller should be treated as
Dual role controller that allows the
controller to act as either a Host or a
device with no support for HNP/SRP."

And later, in Chapter 11.1:

"The USB is not a true OTG. It can be configured by software to function
either as
peripheral or as host. The ID pin, which is unique for OTG operation, is
not present in
this implementation. There are no five pin interface. The user will get
four pin host/
device interface."

RE: [RFC PATCH 0/4] Implement USB device/host switch for Vybrid

[Peter Chen]

 
> 
> On 2016-03-25 00:40, Peter Chen wrote:
> > On Tue, Mar 15, 2016 at 02:08:26PM +0530, Sanchayan Maity wrote:
> >> Hello Peter,
> >>
> >> The existing usage of extcon in Chipidea driver relies on OTG
> >> registers. In case of SoC with dual role device but not a true OTG
> >> controller, this does not work. Such SoC's should specify the
> >> existing CI_HDRC_DUAL_ROLE_NOT_OTG flag and do the role switch
> >> without checking any of the OTG registers in my opinion.
> >> This is the case for Vybrid which uses a Chipidea IP but does not
> >> have a true 5 pin OTG implemented.
> >
> > Sorry to reply you late due to my new born baby.
> >
> > Are you sure Vybrid is NOT OTG core? Afaik, it is uses the same IP
> > base with other Freescale SoCs, just the IP core is 2.40a.
> > When working at device mode, can you read vbus status through OTGSC?
> > And if there is an ID pin (input pin) for Vybrid? I mean SoC, not the
> > board.
> 
> I think the IP is actually OTG capable, the registers are there, but the signals
> seem not to be available on the SoC package. That is also what the RM says...
> 
> Quotes from the RM:
> 
> "OTG controller should be treated as
> Dual role controller that allows the
> controller to act as either a Host or a
> device with no support for HNP/SRP."
> 
> And later, in Chapter 11.1:
> 
> "The USB is not a true OTG. It can be configured by software to function either
> as peripheral or as host. The ID pin, which is unique for OTG operation, is not
> present in this implementation. There are no five pin interface. The user will
> get four pin host/ device interface."
> 

Get it, thanks. I am doing a patch for covering this case and vbus always-on case.

Peter

[RFC PATCH 0/4] Implement USB device/host switch for Vybrid

[Peter Chen]

 
> 
> On 2016-03-25 00:40, Peter Chen wrote:
> > On Tue, Mar 15, 2016 at 02:08:26PM +0530, Sanchayan Maity wrote:
> >> Hello Peter,
> >>
> >> The existing usage of extcon in Chipidea driver relies on OTG
> >> registers. In case of SoC with dual role device but not a true OTG
> >> controller, this does not work. Such SoC's should specify the
> >> existing CI_HDRC_DUAL_ROLE_NOT_OTG flag and do the role switch
> >> without checking any of the OTG registers in my opinion.
> >> This is the case for Vybrid which uses a Chipidea IP but does not
> >> have a true 5 pin OTG implemented.
> >
> > Sorry to reply you late due to my new born baby.
> >
> > Are you sure Vybrid is NOT OTG core? Afaik, it is uses the same IP
> > base with other Freescale SoCs, just the IP core is 2.40a.
> > When working at device mode, can you read vbus status through OTGSC?
> > And if there is an ID pin (input pin) for Vybrid? I mean SoC, not the
> > board.
> 
> I think the IP is actually OTG capable, the registers are there, but the signals
> seem not to be available on the SoC package. That is also what the RM says...
> 
> Quotes from the RM:
> 
> "OTG controller should be treated as
> Dual role controller that allows the
> controller to act as either a Host or a
> device with no support for HNP/SRP."
> 
> And later, in Chapter 11.1:
> 
> "The USB is not a true OTG. It can be configured by software to function either
> as peripheral or as host. The ID pin, which is unique for OTG operation, is not
> present in this implementation. There are no five pin interface. The user will
> get four pin host/ device interface."
> 

Get it, thanks. I am doing a patch for covering this case and vbus always-on case.

Peter

RE: [RFC PATCH 0/4] Implement USB device/host switch for Vybrid

[Peter Chen]

 
> 
> On 2016-03-25 00:40, Peter Chen wrote:
> > On Tue, Mar 15, 2016 at 02:08:26PM +0530, Sanchayan Maity wrote:
> >> Hello Peter,
> >>
> >> The existing usage of extcon in Chipidea driver relies on OTG
> >> registers. In case of SoC with dual role device but not a true OTG
> >> controller, this does not work. Such SoC's should specify the
> >> existing CI_HDRC_DUAL_ROLE_NOT_OTG flag and do the role switch
> >> without checking any of the OTG registers in my opinion.
> >> This is the case for Vybrid which uses a Chipidea IP but does not
> >> have a true 5 pin OTG implemented.
> >
> > Sorry to reply you late due to my new born baby.
> >
> > Are you sure Vybrid is NOT OTG core? Afaik, it is uses the same IP
> > base with other Freescale SoCs, just the IP core is 2.40a.
> > When working at device mode, can you read vbus status through OTGSC?
> > And if there is an ID pin (input pin) for Vybrid? I mean SoC, not the
> > board.
> 
> I think the IP is actually OTG capable, the registers are there, but the signals
> seem not to be available on the SoC package. That is also what the RM says...
> 
> Quotes from the RM:
> 
> "OTG controller should be treated as
> Dual role controller that allows the
> controller to act as either a Host or a
> device with no support for HNP/SRP."
> 
> And later, in Chapter 11.1:
> 
> "The USB is not a true OTG. It can be configured by software to function either
> as peripheral or as host. The ID pin, which is unique for OTG operation, is not
> present in this implementation. There are no five pin interface. The user will
> get four pin host/ device interface."
> 

Get it, thanks. I am doing a patch for covering this case and vbus always-on case.

Peter
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 0/4] Implement USB device/host switch for Vybrid

[maitysanchayan]

Hello Peter,

On 16-03-29 00:24:46, Peter Chen wrote:
>  
> > 
> > On 2016-03-25 00:40, Peter Chen wrote:
> > > On Tue, Mar 15, 2016 at 02:08:26PM +0530, Sanchayan Maity wrote:
> > >> Hello Peter,
> > >>
> > >> The existing usage of extcon in Chipidea driver relies on OTG
> > >> registers. In case of SoC with dual role device but not a true OTG
> > >> controller, this does not work. Such SoC's should specify the
> > >> existing CI_HDRC_DUAL_ROLE_NOT_OTG flag and do the role switch
> > >> without checking any of the OTG registers in my opinion.
> > >> This is the case for Vybrid which uses a Chipidea IP but does not
> > >> have a true 5 pin OTG implemented.
> > >
> > > Sorry to reply you late due to my new born baby.
> > >
> > > Are you sure Vybrid is NOT OTG core? Afaik, it is uses the same IP
> > > base with other Freescale SoCs, just the IP core is 2.40a.
> > > When working at device mode, can you read vbus status through OTGSC?
> > > And if there is an ID pin (input pin) for Vybrid? I mean SoC, not the
> > > board.
> > 
> > I think the IP is actually OTG capable, the registers are there, but the signals
> > seem not to be available on the SoC package. That is also what the RM says...
> > 
> > Quotes from the RM:
> > 
> > "OTG controller should be treated as
> > Dual role controller that allows the
> > controller to act as either a Host or a
> > device with no support for HNP/SRP."
> > 
> > And later, in Chapter 11.1:
> > 
> > "The USB is not a true OTG. It can be configured by software to function either
> > as peripheral or as host. The ID pin, which is unique for OTG operation, is not
> > present in this implementation. There are no five pin interface. The user will
> > get four pin host/ device interface."
> > 
> 
> Get it, thanks. I am doing a patch for covering this case and vbus always-on case.

If I may ask at this point, how would your implementation be covering this case
for Vybrid?

- Sanchayan.

[RFC PATCH 0/4] Implement USB device/host switch for Vybrid

[maitysanchayan at gmail.com]

Hello Peter,

On 16-03-29 00:24:46, Peter Chen wrote:
>  
> > 
> > On 2016-03-25 00:40, Peter Chen wrote:
> > > On Tue, Mar 15, 2016 at 02:08:26PM +0530, Sanchayan Maity wrote:
> > >> Hello Peter,
> > >>
> > >> The existing usage of extcon in Chipidea driver relies on OTG
> > >> registers. In case of SoC with dual role device but not a true OTG
> > >> controller, this does not work. Such SoC's should specify the
> > >> existing CI_HDRC_DUAL_ROLE_NOT_OTG flag and do the role switch
> > >> without checking any of the OTG registers in my opinion.
> > >> This is the case for Vybrid which uses a Chipidea IP but does not
> > >> have a true 5 pin OTG implemented.
> > >
> > > Sorry to reply you late due to my new born baby.
> > >
> > > Are you sure Vybrid is NOT OTG core? Afaik, it is uses the same IP
> > > base with other Freescale SoCs, just the IP core is 2.40a.
> > > When working at device mode, can you read vbus status through OTGSC?
> > > And if there is an ID pin (input pin) for Vybrid? I mean SoC, not the
> > > board.
> > 
> > I think the IP is actually OTG capable, the registers are there, but the signals
> > seem not to be available on the SoC package. That is also what the RM says...
> > 
> > Quotes from the RM:
> > 
> > "OTG controller should be treated as
> > Dual role controller that allows the
> > controller to act as either a Host or a
> > device with no support for HNP/SRP."
> > 
> > And later, in Chapter 11.1:
> > 
> > "The USB is not a true OTG. It can be configured by software to function either
> > as peripheral or as host. The ID pin, which is unique for OTG operation, is not
> > present in this implementation. There are no five pin interface. The user will
> > get four pin host/ device interface."
> > 
> 
> Get it, thanks. I am doing a patch for covering this case and vbus always-on case.

If I may ask at this point, how would your implementation be covering this case
for Vybrid?

- Sanchayan.

Re: [RFC PATCH 0/4] Implement USB device/host switch for Vybrid

[maitysanchayan-Re5JQEeQqe8AvxtiuMwx3w]

Hello Peter,

On 16-03-29 00:24:46, Peter Chen wrote:
>  
> > 
> > On 2016-03-25 00:40, Peter Chen wrote:
> > > On Tue, Mar 15, 2016 at 02:08:26PM +0530, Sanchayan Maity wrote:
> > >> Hello Peter,
> > >>
> > >> The existing usage of extcon in Chipidea driver relies on OTG
> > >> registers. In case of SoC with dual role device but not a true OTG
> > >> controller, this does not work. Such SoC's should specify the
> > >> existing CI_HDRC_DUAL_ROLE_NOT_OTG flag and do the role switch
> > >> without checking any of the OTG registers in my opinion.
> > >> This is the case for Vybrid which uses a Chipidea IP but does not
> > >> have a true 5 pin OTG implemented.
> > >
> > > Sorry to reply you late due to my new born baby.
> > >
> > > Are you sure Vybrid is NOT OTG core? Afaik, it is uses the same IP
> > > base with other Freescale SoCs, just the IP core is 2.40a.
> > > When working at device mode, can you read vbus status through OTGSC?
> > > And if there is an ID pin (input pin) for Vybrid? I mean SoC, not the
> > > board.
> > 
> > I think the IP is actually OTG capable, the registers are there, but the signals
> > seem not to be available on the SoC package. That is also what the RM says...
> > 
> > Quotes from the RM:
> > 
> > "OTG controller should be treated as
> > Dual role controller that allows the
> > controller to act as either a Host or a
> > device with no support for HNP/SRP."
> > 
> > And later, in Chapter 11.1:
> > 
> > "The USB is not a true OTG. It can be configured by software to function either
> > as peripheral or as host. The ID pin, which is unique for OTG operation, is not
> > present in this implementation. There are no five pin interface. The user will
> > get four pin host/ device interface."
> > 
> 
> Get it, thanks. I am doing a patch for covering this case and vbus always-on case.

If I may ask at this point, how would your implementation be covering this case
for Vybrid?

- Sanchayan.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [RFC PATCH 0/4] Implement USB device/host switch for Vybrid

[Peter Chen]

 
> 
> On 16-03-29 00:24:46, Peter Chen wrote:
> >
> > >
> > > On 2016-03-25 00:40, Peter Chen wrote:
> > > > On Tue, Mar 15, 2016 at 02:08:26PM +0530, Sanchayan Maity wrote:
> > > >> Hello Peter,
> > > >>
> > > >> The existing usage of extcon in Chipidea driver relies on OTG
> > > >> registers. In case of SoC with dual role device but not a true
> > > >> OTG controller, this does not work. Such SoC's should specify the
> > > >> existing CI_HDRC_DUAL_ROLE_NOT_OTG flag and do the role switch
> > > >> without checking any of the OTG registers in my opinion.
> > > >> This is the case for Vybrid which uses a Chipidea IP but does not
> > > >> have a true 5 pin OTG implemented.
> > > >
> > > > Sorry to reply you late due to my new born baby.
> > > >
> > > > Are you sure Vybrid is NOT OTG core? Afaik, it is uses the same IP
> > > > base with other Freescale SoCs, just the IP core is 2.40a.
> > > > When working at device mode, can you read vbus status through OTGSC?
> > > > And if there is an ID pin (input pin) for Vybrid? I mean SoC, not
> > > > the board.
> > >
> > > I think the IP is actually OTG capable, the registers are there, but
> > > the signals seem not to be available on the SoC package. That is also what
> the RM says...
> > >
> > > Quotes from the RM:
> > >
> > > "OTG controller should be treated as Dual role controller that
> > > allows the controller to act as either a Host or a device with no
> > > support for HNP/SRP."
> > >
> > > And later, in Chapter 11.1:
> > >
> > > "The USB is not a true OTG. It can be configured by software to
> > > function either as peripheral or as host. The ID pin, which is
> > > unique for OTG operation, is not present in this implementation.
> > > There are no five pin interface. The user will get four pin host/ device
> interface."
> > >
> >
> > Get it, thanks. I am doing a patch for covering this case and vbus always-on
> case.
> 
> If I may ask at this point, how would your implementation be covering this case
> for Vybrid?
> 

Yes, if the vbus and id status are from the extcon, it will not read register OTGSC.

Peter

[RFC PATCH 0/4] Implement USB device/host switch for Vybrid

[Peter Chen]

 
> 
> On 16-03-29 00:24:46, Peter Chen wrote:
> >
> > >
> > > On 2016-03-25 00:40, Peter Chen wrote:
> > > > On Tue, Mar 15, 2016 at 02:08:26PM +0530, Sanchayan Maity wrote:
> > > >> Hello Peter,
> > > >>
> > > >> The existing usage of extcon in Chipidea driver relies on OTG
> > > >> registers. In case of SoC with dual role device but not a true
> > > >> OTG controller, this does not work. Such SoC's should specify the
> > > >> existing CI_HDRC_DUAL_ROLE_NOT_OTG flag and do the role switch
> > > >> without checking any of the OTG registers in my opinion.
> > > >> This is the case for Vybrid which uses a Chipidea IP but does not
> > > >> have a true 5 pin OTG implemented.
> > > >
> > > > Sorry to reply you late due to my new born baby.
> > > >
> > > > Are you sure Vybrid is NOT OTG core? Afaik, it is uses the same IP
> > > > base with other Freescale SoCs, just the IP core is 2.40a.
> > > > When working at device mode, can you read vbus status through OTGSC?
> > > > And if there is an ID pin (input pin) for Vybrid? I mean SoC, not
> > > > the board.
> > >
> > > I think the IP is actually OTG capable, the registers are there, but
> > > the signals seem not to be available on the SoC package. That is also what
> the RM says...
> > >
> > > Quotes from the RM:
> > >
> > > "OTG controller should be treated as Dual role controller that
> > > allows the controller to act as either a Host or a device with no
> > > support for HNP/SRP."
> > >
> > > And later, in Chapter 11.1:
> > >
> > > "The USB is not a true OTG. It can be configured by software to
> > > function either as peripheral or as host. The ID pin, which is
> > > unique for OTG operation, is not present in this implementation.
> > > There are no five pin interface. The user will get four pin host/ device
> interface."
> > >
> >
> > Get it, thanks. I am doing a patch for covering this case and vbus always-on
> case.
> 
> If I may ask at this point, how would your implementation be covering this case
> for Vybrid?
> 

Yes, if the vbus and id status are from the extcon, it will not read register OTGSC.

Peter

RE: [RFC PATCH 0/4] Implement USB device/host switch for Vybrid

[Peter Chen]

 
> 
> On 16-03-29 00:24:46, Peter Chen wrote:
> >
> > >
> > > On 2016-03-25 00:40, Peter Chen wrote:
> > > > On Tue, Mar 15, 2016 at 02:08:26PM +0530, Sanchayan Maity wrote:
> > > >> Hello Peter,
> > > >>
> > > >> The existing usage of extcon in Chipidea driver relies on OTG
> > > >> registers. In case of SoC with dual role device but not a true
> > > >> OTG controller, this does not work. Such SoC's should specify the
> > > >> existing CI_HDRC_DUAL_ROLE_NOT_OTG flag and do the role switch
> > > >> without checking any of the OTG registers in my opinion.
> > > >> This is the case for Vybrid which uses a Chipidea IP but does not
> > > >> have a true 5 pin OTG implemented.
> > > >
> > > > Sorry to reply you late due to my new born baby.
> > > >
> > > > Are you sure Vybrid is NOT OTG core? Afaik, it is uses the same IP
> > > > base with other Freescale SoCs, just the IP core is 2.40a.
> > > > When working at device mode, can you read vbus status through OTGSC?
> > > > And if there is an ID pin (input pin) for Vybrid? I mean SoC, not
> > > > the board.
> > >
> > > I think the IP is actually OTG capable, the registers are there, but
> > > the signals seem not to be available on the SoC package. That is also what
> the RM says...
> > >
> > > Quotes from the RM:
> > >
> > > "OTG controller should be treated as Dual role controller that
> > > allows the controller to act as either a Host or a device with no
> > > support for HNP/SRP."
> > >
> > > And later, in Chapter 11.1:
> > >
> > > "The USB is not a true OTG. It can be configured by software to
> > > function either as peripheral or as host. The ID pin, which is
> > > unique for OTG operation, is not present in this implementation.
> > > There are no five pin interface. The user will get four pin host/ device
> interface."
> > >
> >
> > Get it, thanks. I am doing a patch for covering this case and vbus always-on
> case.
> 
> If I may ask at this point, how would your implementation be covering this case
> for Vybrid?
> 

Yes, if the vbus and id status are from the extcon, it will not read register OTGSC.

Peter