[PATCH 0/7][v4] Add OTG support for FSL socs

[PATCH 0/7][v4] Add OTG support for FSL socs

[Ramneek Mehresh]

Add support for otg for all freescale socs having internal
usb phy.

Ramneek Mehresh (7):
  usb:fsl:otg: Make fsl otg driver as tristate
  usb:fsl:otg: Add controller version based ULPI and UTMI phy
  usb:fsl:otg: Add support to add/remove usb host driver
  usb:fsl:otg: Signal host drv when host is otg
  usb:fsl:otg: Modify otg_event to start host drv
  usb:fsl:otg: Combine host/gadget start/resume for ID change
  usb:fsl:otg: Add host-gadget drv sync delay

 drivers/usb/host/ehci-fsl.c   | 73 +++++++++++++++++++++++++++++++------------
 drivers/usb/host/ehci-fsl.h   | 16 ++++++++++
 drivers/usb/phy/Kconfig       |  2 +-
 drivers/usb/phy/phy-fsl-usb.c | 58 +++++++++++++++++++++++++---------
 drivers/usb/phy/phy-fsl-usb.h |  7 +++++
 include/linux/usb.h           |  1 +
 6 files changed, 122 insertions(+), 35 deletions(-)

--
Changes for v4:
	- corrected reference to struct usb_bus inside
	  ehci_fsl_drv_resume() and ehci_fsl_drv_suspend().
	  Took pointer instead of structure.
        - removed CONFIG_PM from inside struct ehci_fsl
	  in ehci-fsl.h
	- added code to remove host drv upon otg init
          by making have_hcd = 1 in probe() from patch 7/8 of
	  patch-set[v3] to 3/7 in patch-set[v4]
1.8.3.1

[PATCH 1/7][v4]usb:fsl:otg: Make fsl otg driver as tristate

[Ramneek Mehresh]

Provide option to load fsl otg driver as loadable module.

Signed-off-by: Ramneek Mehresh <ramneek.mehresh@freescale.com>
---
 drivers/usb/phy/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index 869c0cfcad..8a09267 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -19,7 +19,7 @@ config AB8500_USB
 	  in host mode, low speed.
 
 config FSL_USB2_OTG
-	bool "Freescale USB OTG Transceiver Driver"
+	tristate "Freescale USB OTG Transceiver Driver"
 	depends on USB_EHCI_FSL && USB_FSL_USB2 && USB_OTG_FSM && PM
 	select USB_OTG
 	select USB_PHY
-- 
1.8.3.1

[PATCH 2/7][v4]usb:fsl:otg: Add controller version based ULPI and UTMI phy

[Ramneek Mehresh]

Add controller version based ULPI and UTMI phy
initialization for otg driver.

Signed-off-by: Shengzhou Liu <Shengzhou.Liu@freescale.com>
Signed-off-by: Ramneek Mehresh <ramneek.mehresh@freescale.com>
Reviewed-by: Fleming Andrew-AFLEMING <AFLEMING@freescale.com>
Tested-by: Fleming Andrew-AFLEMING <AFLEMING@freescale.com>
---
 drivers/usb/phy/phy-fsl-usb.c | 20 ++++++++++++++++++++
 drivers/usb/phy/phy-fsl-usb.h |  7 +++++++
 2 files changed, 27 insertions(+)

diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
index 94eb292..4e58aee 100644
--- a/drivers/usb/phy/phy-fsl-usb.c
+++ b/drivers/usb/phy/phy-fsl-usb.c
@@ -923,12 +923,32 @@ int usb_otg_start(struct platform_device *pdev)
 	temp &= ~(PORTSC_PHY_TYPE_SEL | PORTSC_PTW);
 	switch (pdata->phy_mode) {
 	case FSL_USB2_PHY_ULPI:
+		if (pdata->controller_ver) {
+			/* controller version 1.6 or above */
+			setbits32(&p_otg->dr_mem_map->control,
+				USB_CTRL_ULPI_PHY_CLK_SEL);
+			/*
+			 * Due to controller issue of PHY_CLK_VALID in ULPI
+			 * mode, set USB_CTRL_USB_EN before checking
+			 * PHY_CLK_VALID, otherwise PHY_CLK_VALID doesn't work
+			 */
+			clrsetbits_be32(&p_otg->dr_mem_map->control,
+				USB_CTRL_UTMI_PHY_EN, USB_CTRL_IOENB);
+		}
 		temp |= PORTSC_PTS_ULPI;
 		break;
 	case FSL_USB2_PHY_UTMI_WIDE:
 		temp |= PORTSC_PTW_16BIT;
 		/* fall through */
 	case FSL_USB2_PHY_UTMI:
+		if (pdata->controller_ver) {
+			/* controller version 1.6 or above */
+			setbits32(&p_otg->dr_mem_map->control,
+				USB_CTRL_UTMI_PHY_EN);
+			/* Delay for UTMI PHY CLK to become stable - 10ms */
+			mdelay(FSL_UTMI_PHY_DLY);
+		}
+		setbits32(&p_otg->dr_mem_map->control, USB_CTRL_UTMI_PHY_EN);
 		temp |= PORTSC_PTS_UTMI;
 		/* fall through */
 	default:
diff --git a/drivers/usb/phy/phy-fsl-usb.h b/drivers/usb/phy/phy-fsl-usb.h
index 2314995..4a78fb3 100644
--- a/drivers/usb/phy/phy-fsl-usb.h
+++ b/drivers/usb/phy/phy-fsl-usb.h
@@ -199,6 +199,13 @@
 /* control Register Bit Masks */
 #define  USB_CTRL_IOENB			(0x1<<2)
 #define  USB_CTRL_ULPI_INT0EN		(0x1<<0)
+#define  USB_CTRL_WU_INT_EN		(0x1<<1)
+#define  USB_CTRL_LINE_STATE_FILTER__EN	(0x1<<3)
+#define  USB_CTRL_KEEP_OTG_ON		(0x1<<4)
+#define  USB_CTRL_OTG_PORT		(0x1<<5)
+#define  USB_CTRL_PLL_RESET		(0x1<<8)
+#define  USB_CTRL_UTMI_PHY_EN		(0x1<<9)
+#define  USB_CTRL_ULPI_PHY_CLK_SEL	(0x1<<10)
 
 /* BCSR5 */
 #define BCSR5_INT_USB			(0x02)
-- 
1.8.3.1

[PATCH 3/7][v4]usb:fsl:otg: Add support to add/remove usb host driver

[Ramneek Mehresh]

Add workqueue to add/remove host driver (outside
interrupt context) upon each id change, and to
remove host driver upon otg initialization by changing
have_hcd variable.

Signed-off-by: Li Yang <leoli@freescale.com>
Signed-off-by: Ramneek Mehresh <ramneek.mehresh@freescale.com>
---
Changes for v4:
	- corrected reference to struct usb_bus inside
	  ehci_fsl_drv_resume() and ehci_fsl_drv_suspend().
	  Took pointer instead of structure.
	- removed CONFIG_PM from inside struct ehci_fsl
	  in ehci-fsl.h
	- added code to remove host drv upon otg init
	  by making have_hcd = 1 in probe()

 drivers/usb/host/ehci-fsl.c | 73 ++++++++++++++++++++++++++++++++-------------
 drivers/usb/host/ehci-fsl.h | 16 ++++++++++
 2 files changed, 69 insertions(+), 20 deletions(-)

diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c
index 202dafb..8870ff5 100644
--- a/drivers/usb/host/ehci-fsl.c
+++ b/drivers/usb/host/ehci-fsl.c
@@ -44,6 +44,31 @@
 
 static struct hc_driver __read_mostly fsl_ehci_hc_driver;
 
+static struct ehci_fsl *hcd_to_ehci_fsl(struct usb_hcd *hcd)
+{
+	return (struct ehci_fsl *)hcd_to_ehci(hcd)->priv;
+}
+
+static void do_change_hcd(struct work_struct *work)
+{
+	struct ehci_fsl *ehci_fsl = container_of(work, struct ehci_fsl,
+					change_hcd_work);
+	struct usb_hcd *hcd = ehci_fsl->hcd;
+	void __iomem *non_ehci = hcd->regs;
+	int retval;
+
+	if (ehci_fsl->hcd_add && !ehci_fsl->have_hcd) {
+		writel(USBMODE_CM_HOST, non_ehci + FSL_SOC_USB_USBMODE);
+		/* host, gadget and otg share same int line */
+		retval = usb_add_hcd(hcd, hcd->irq, IRQF_SHARED);
+		if (retval == 0)
+			ehci_fsl->have_hcd = 1;
+	} else if (!ehci_fsl->hcd_add && ehci_fsl->have_hcd) {
+		usb_remove_hcd(hcd);
+		ehci_fsl->have_hcd = 0;
+	}
+}
+
 /* configure so an HC device and id are always provided */
 /* always called with process context; sleeping is OK */
 
@@ -147,11 +172,15 @@ static int fsl_ehci_drv_probe(struct platform_device *pdev)
 		goto err2;
 	device_wakeup_enable(hcd->self.controller);
 
-#ifdef CONFIG_USB_OTG
 	if (pdata->operating_mode == FSL_USB2_DR_OTG) {
 		struct ehci_hcd *ehci = hcd_to_ehci(hcd);
+		struct ehci_fsl *ehci_fsl = hcd_to_ehci_fsl(hcd);
 
+		ehci_fsl->hcd = hcd;
 		hcd->usb_phy = usb_get_phy(USB_PHY_TYPE_USB2);
+
+		INIT_WORK(&ehci_fsl->change_hcd_work, do_change_hcd);
+
 		dev_dbg(&pdev->dev, "hcd=0x%p  ehci=0x%p, phy=0x%p\n",
 			hcd, ehci, hcd->usb_phy);
 
@@ -167,8 +196,10 @@ static int fsl_ehci_drv_probe(struct platform_device *pdev)
 			retval = -ENODEV;
 			goto err2;
 		}
+
+		ehci_fsl->have_hcd = 1;
 	}
-#endif
+
 	return retval;
 
       err2:
@@ -371,15 +402,6 @@ static int ehci_fsl_setup(struct usb_hcd *hcd)
 	return retval;
 }
 
-struct ehci_fsl {
-	struct ehci_hcd	ehci;
-
-#ifdef CONFIG_PM
-	/* Saved USB PHY settings, need to restore after deep sleep. */
-	u32 usb_ctrl;
-#endif
-};
-
 #ifdef CONFIG_PM
 
 #ifdef CONFIG_PPC_MPC512x
@@ -527,24 +549,26 @@ static inline int ehci_fsl_mpc512x_drv_resume(struct device *dev)
 }
 #endif /* CONFIG_PPC_MPC512x */
 
-static struct ehci_fsl *hcd_to_ehci_fsl(struct usb_hcd *hcd)
-{
-	struct ehci_hcd *ehci = hcd_to_ehci(hcd);
-
-	return container_of(ehci, struct ehci_fsl, ehci);
-}
-
 static int ehci_fsl_drv_suspend(struct device *dev)
 {
 	struct usb_hcd *hcd = dev_get_drvdata(dev);
-	struct ehci_fsl *ehci_fsl = hcd_to_ehci_fsl(hcd);
 	void __iomem *non_ehci = hcd->regs;
+	struct ehci_fsl *ehci_fsl = hcd_to_ehci_fsl(hcd);
+	struct usb_bus *host = &hcd->self;
 
 	if (of_device_is_compatible(dev->parent->of_node,
 				    "fsl,mpc5121-usb2-dr")) {
 		return ehci_fsl_mpc512x_drv_suspend(dev);
 	}
 
+	if (host->is_otg) {
+		/* remove hcd */
+		ehci_fsl->hcd_add = 0;
+		schedule_work(&ehci_fsl->change_hcd_work);
+		host->is_otg = 0;
+		return 0;
+	}
+
 	ehci_prepare_ports_for_controller_suspend(hcd_to_ehci(hcd),
 			device_may_wakeup(dev));
 	if (!fsl_deep_sleep())
@@ -557,15 +581,24 @@ static int ehci_fsl_drv_suspend(struct device *dev)
 static int ehci_fsl_drv_resume(struct device *dev)
 {
 	struct usb_hcd *hcd = dev_get_drvdata(dev);
-	struct ehci_fsl *ehci_fsl = hcd_to_ehci_fsl(hcd);
 	struct ehci_hcd *ehci = hcd_to_ehci(hcd);
 	void __iomem *non_ehci = hcd->regs;
+	struct ehci_fsl *ehci_fsl = hcd_to_ehci_fsl(hcd);
+	struct usb_bus *host = &hcd->self;
 
 	if (of_device_is_compatible(dev->parent->of_node,
 				    "fsl,mpc5121-usb2-dr")) {
 		return ehci_fsl_mpc512x_drv_resume(dev);
 	}
 
+	if (host->is_otg) {
+		/* add hcd */
+		ehci_fsl->hcd_add = 1;
+		schedule_work(&ehci_fsl->change_hcd_work);
+		host->is_otg = 0;
+		return 0;
+	}
+
 	ehci_prepare_ports_for_controller_resume(ehci);
 	if (!fsl_deep_sleep())
 		return 0;
diff --git a/drivers/usb/host/ehci-fsl.h b/drivers/usb/host/ehci-fsl.h
index 1a8a60a..64538eb 100644
--- a/drivers/usb/host/ehci-fsl.h
+++ b/drivers/usb/host/ehci-fsl.h
@@ -63,4 +63,20 @@
 #define UTMI_PHY_EN             (1<<9)
 #define ULPI_PHY_CLK_SEL        (1<<10)
 #define PHY_CLK_VALID		(1<<17)
+
+struct ehci_fsl {
+	/* Save USB CTRL, need to restore after deep sleep. */
+	u32 usb_ctrl;
+	struct usb_hcd *hcd;
+	struct work_struct change_hcd_work;
+	/*
+	 * store current hcd state for otg;
+	 * have_hcd is true when host drv al already part of otg framework,
+	 * otherwise false;
+	 * hcd_add is true when otg framework wants to add host
+	 * drv as part of otg;flase when it wants to remove it
+	 */
+	unsigned have_hcd:1;
+	unsigned hcd_add:1;
+};
 #endif				/* _EHCI_FSL_H */
-- 
1.8.3.1

[PATCH 4/7][v4]usb:fsl:otg: Signal host drv when host is otg

[Ramneek Mehresh]

Set is_otg boolean flag to signal host driver when host
is running in context of otg host suspend/resume.

Signed-off-by: Ramneek Mehresh <ramneek.mehresh@freescale.com>
---
 drivers/usb/phy/phy-fsl-usb.c | 7 ++++++-
 include/linux/usb.h           | 1 +
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
index 4e58aee..815c22c 100644
--- a/drivers/usb/phy/phy-fsl-usb.c
+++ b/drivers/usb/phy/phy-fsl-usb.c
@@ -463,6 +463,7 @@ void otg_reset_controller(void)
 int fsl_otg_start_host(struct otg_fsm *fsm, int on)
 {
 	struct usb_otg *otg = fsm->otg;
+	struct usb_bus *host = otg->host;
 	struct device *dev;
 	struct fsl_otg *otg_dev =
 		container_of(otg->usb_phy, struct fsl_otg, phy);
@@ -485,6 +486,7 @@ int fsl_otg_start_host(struct otg_fsm *fsm, int on)
 		else {
 			otg_reset_controller();
 			VDBG("host on......\n");
+			host->is_otg = 1;
 			if (dev->driver->pm && dev->driver->pm->resume) {
 				retval = dev->driver->pm->resume(dev);
 				if (fsm->id) {
@@ -510,8 +512,11 @@ int fsl_otg_start_host(struct otg_fsm *fsm, int on)
 		else {
 			VDBG("host off......\n");
 			if (dev && dev->driver) {
-				if (dev->driver->pm && dev->driver->pm->suspend)
+				if (dev->driver->pm &&
+					dev->driver->pm->suspend) {
+					host->is_otg = 1;
 					retval = dev->driver->pm->suspend(dev);
+				}
 				if (fsm->id)
 					/* default-b */
 					fsl_otg_drv_vbus(fsm, 0);
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 447fe29..2208822 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -355,6 +355,7 @@ struct usb_bus {
 					 * for control transfers?
 					 */
 	u8 otg_port;			/* 0, or number of OTG/HNP port */
+	unsigned is_otg:1;		/* true when host is also otg */
 	unsigned is_b_host:1;		/* true during some HNP roleswitches */
 	unsigned b_hnp_enable:1;	/* OTG: did A-Host enable HNP? */
 	unsigned no_stop_on_short:1;    /*
-- 
1.8.3.1

[PATCH 5/7][v4]usb:fsl:otg: Modify otg_event to start host drv

[Ramneek Mehresh]

Add mechanism to start host driver from inside fsl_otg_even upon
each id change interrupt.

Signed-off-by: Ramneek Mehresh <ramneek.mehresh@freescale.com>
Reviewed-by: Fleming Andrew-AFLEMING <AFLEMING@freescale.com>
Tested-by: Fleming Andrew-AFLEMING <AFLEMING@freescale.com>
---
 drivers/usb/phy/phy-fsl-usb.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
index 815c22c..3b8a9e5 100644
--- a/drivers/usb/phy/phy-fsl-usb.c
+++ b/drivers/usb/phy/phy-fsl-usb.c
@@ -677,6 +677,10 @@ static void fsl_otg_event(struct work_struct *work)
 		fsl_otg_start_host(fsm, 0);
 		otg_drv_vbus(fsm, 0);
 		fsl_otg_start_gadget(fsm, 1);
+	} else {
+		fsl_otg_start_gadget(fsm, 0);
+		otg_drv_vbus(fsm, 1);
+		fsl_otg_start_host(fsm, 1);
 	}
 }
 
-- 
1.8.3.1

[PATCH 6/7][v4]usb:fsl:otg: Combine host/gadget start/resume for ID change

[Ramneek Mehresh]

Make call to fsl_otg_event for each id change even.

Signed-off-by: Ramneek Mehresh <ramneek.mehresh@freescale.com>
Reviewed-by: Fleming Andrew-AFLEMING <AFLEMING@freescale.com>
Tested-by: Fleming Andrew-AFLEMING <AFLEMING@freescale.com>
---
 drivers/usb/phy/phy-fsl-usb.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
index 3b8a9e5..689516d 100644
--- a/drivers/usb/phy/phy-fsl-usb.c
+++ b/drivers/usb/phy/phy-fsl-usb.c
@@ -733,6 +733,7 @@ irqreturn_t fsl_otg_isr(int irq, void *dev_id)
 {
 	struct otg_fsm *fsm = &((struct fsl_otg *)dev_id)->fsm;
 	struct usb_otg *otg = ((struct fsl_otg *)dev_id)->phy.otg;
+	struct fsl_otg *otg_dev = dev_id;
 	u32 otg_int_src, otg_sc;
 
 	otg_sc = fsl_readl(&usb_dr_regs->otgsc);
@@ -762,18 +763,8 @@ irqreturn_t fsl_otg_isr(int irq, void *dev_id)
 				otg->gadget->is_a_peripheral = !fsm->id;
 			VDBG("ID int (ID is %d)\n", fsm->id);
 
-			if (fsm->id) {	/* switch to gadget */
-				schedule_delayed_work(
-					&((struct fsl_otg *)dev_id)->otg_event,
-					100);
-			} else {	/* switch to host */
-				cancel_delayed_work(&
-						    ((struct fsl_otg *)dev_id)->
-						    otg_event);
-				fsl_otg_start_gadget(fsm, 0);
-				otg_drv_vbus(fsm, 1);
-				fsl_otg_start_host(fsm, 1);
-			}
+			schedule_delayed_work(&otg_dev->otg_event, 100);
+
 			return IRQ_HANDLED;
 		}
 	}
-- 
1.8.3.1

[PATCH 7/7][v4]usb:fsl:otg: Add host-gadget drv sync delay

[Ramneek Mehresh]

Resolve synchronization issue between host
and gadget drivers upon role-reversal.

Signed-off-by: Ramneek Mehresh <ramneek.mehresh@freescale.com>
Reviewed-by: Li Yang-R58472 <LeoLi@freescale.com>
Reviewed-by: Fleming Andrew-AFLEMING <AFLEMING@freescale.com>
Tested-by: Fleming Andrew-AFLEMING <AFLEMING@freescale.com>
---
 drivers/usb/phy/phy-fsl-usb.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
index 689516d..5d678ea 100644
--- a/drivers/usb/phy/phy-fsl-usb.c
+++ b/drivers/usb/phy/phy-fsl-usb.c
@@ -544,8 +544,18 @@ int fsl_otg_start_gadget(struct otg_fsm *fsm, int on)
 	dev = otg->gadget->dev.parent;
 
 	if (on) {
-		if (dev->driver->resume)
+		/*
+		 * Delay gadget resume to synchronize between host and gadget
+		 * drivers. Upon role-reversal host drv is shutdown by kernel
+		 * worker thread. By the time host drv shuts down, controller
+		 * gets programmed for gadget role. Shutting host drv after
+		 * this results in controller getting reset, and it stops
+		 * responding to otg events
+		 */
+		if (dev->driver->resume) {
+			msleep(1000);
 			dev->driver->resume(dev);
+		}
 	} else {
 		if (dev->driver->suspend)
 			dev->driver->suspend(dev, otg_suspend_state);
-- 
1.8.3.1

Re: [PATCH 0/7][v4] Add OTG support for FSL socs

[Felipe Balbi]

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

Ramneek Mehresh <ramneek.mehresh@freescale.com> writes:

> Add support for otg for all freescale socs having internal
> usb phy.
>
> Ramneek Mehresh (7):
>   usb:fsl:otg: Make fsl otg driver as tristate
>   usb:fsl:otg: Add controller version based ULPI and UTMI phy
>   usb:fsl:otg: Add support to add/remove usb host driver
>   usb:fsl:otg: Signal host drv when host is otg
>   usb:fsl:otg: Modify otg_event to start host drv
>   usb:fsl:otg: Combine host/gadget start/resume for ID change
>   usb:fsl:otg: Add host-gadget drv sync delay
>
>  drivers/usb/host/ehci-fsl.c   | 73 +++++++++++++++++++++++++++++++------------
>  drivers/usb/host/ehci-fsl.h   | 16 ++++++++++

Alan, how do you want to handle these patches ? Do I get your Acked-by
or do we split everything up ?

-- 
balbi

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

Re: [PATCH 0/7][v4] Add OTG support for FSL socs

[Alan Stern]

On Mon, 5 Oct 2015, Felipe Balbi wrote:

> Ramneek Mehresh <ramneek.mehresh@freescale.com> writes:
> 
> > Add support for otg for all freescale socs having internal
> > usb phy.
> >
> > Ramneek Mehresh (7):
> >   usb:fsl:otg: Make fsl otg driver as tristate
> >   usb:fsl:otg: Add controller version based ULPI and UTMI phy
> >   usb:fsl:otg: Add support to add/remove usb host driver
> >   usb:fsl:otg: Signal host drv when host is otg
> >   usb:fsl:otg: Modify otg_event to start host drv
> >   usb:fsl:otg: Combine host/gadget start/resume for ID change
> >   usb:fsl:otg: Add host-gadget drv sync delay
> >
> >  drivers/usb/host/ehci-fsl.c   | 73 +++++++++++++++++++++++++++++++------------
> >  drivers/usb/host/ehci-fsl.h   | 16 ++++++++++
> 
> Alan, how do you want to handle these patches ? Do I get your Acked-by
> or do we split everything up ?

I'll send an Acked-by after looking carefully at the parts that affect 
ehci-fsl.

Alan Stern

Re: [PATCH 0/7][v4] Add OTG support for FSL socs

[Alan Stern]

On Mon, 5 Oct 2015, Felipe Balbi wrote:

> Ramneek Mehresh <ramneek.mehresh@freescale.com> writes:
> 
> > Add support for otg for all freescale socs having internal
> > usb phy.
> >
> > Ramneek Mehresh (7):
> >   usb:fsl:otg: Make fsl otg driver as tristate
> >   usb:fsl:otg: Add controller version based ULPI and UTMI phy
> >   usb:fsl:otg: Add support to add/remove usb host driver
> >   usb:fsl:otg: Signal host drv when host is otg
> >   usb:fsl:otg: Modify otg_event to start host drv
> >   usb:fsl:otg: Combine host/gadget start/resume for ID change
> >   usb:fsl:otg: Add host-gadget drv sync delay
> >
> >  drivers/usb/host/ehci-fsl.c   | 73 +++++++++++++++++++++++++++++++------------
> >  drivers/usb/host/ehci-fsl.h   | 16 ++++++++++
> 
> Alan, how do you want to handle these patches ? Do I get your Acked-by
> or do we split everything up ?

Patch 3/7 is the only one touching the ehci-fsl files.  Its changes 
look okay to me.

Acked-by: Alan Stern <stern@rowland.harvard.edu>

Alan Stern

Re: [PATCH 0/7][v4] Add OTG support for FSL socs

[Fabio Estevam]

On Thu, Aug 27, 2015 at 1:43 PM, Ramneek Mehresh
<ramneek.mehresh@freescale.com> wrote:
> Add support for otg for all freescale socs having internal
> usb phy.
>
> Ramneek Mehresh (7):
>   usb:fsl:otg: Make fsl otg driver as tristate
>   usb:fsl:otg: Add controller version based ULPI and UTMI phy
>   usb:fsl:otg: Add support to add/remove usb host driver
>   usb:fsl:otg: Signal host drv when host is otg
>   usb:fsl:otg: Modify otg_event to start host drv
>   usb:fsl:otg: Combine host/gadget start/resume for ID change
>   usb:fsl:otg: Add host-gadget drv sync delay
>
>  drivers/usb/host/ehci-fsl.c   | 73 +++++++++++++++++++++++++++++++------------
>  drivers/usb/host/ehci-fsl.h   | 16 ++++++++++
>  drivers/usb/phy/Kconfig       |  2 +-
>  drivers/usb/phy/phy-fsl-usb.c | 58 +++++++++++++++++++++++++---------
>  drivers/usb/phy/phy-fsl-usb.h |  7 +++++
>  include/linux/usb.h           |  1 +
>  6 files changed, 122 insertions(+), 35 deletions(-)

Isn't this all Chipidea IP? If so, why don't you use the existing
chipidea driver instead?

Re: [PATCH 0/7][v4] Add OTG support for FSL socs

[Felipe Balbi]

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


Hi,

Ramneek Mehresh <ramneek.mehresh@freescale.com> writes:
> Add support for otg for all freescale socs having internal
> usb phy.
>
> Ramneek Mehresh (7):
>   usb:fsl:otg: Make fsl otg driver as tristate
>   usb:fsl:otg: Add controller version based ULPI and UTMI phy
>   usb:fsl:otg: Add support to add/remove usb host driver
>   usb:fsl:otg: Signal host drv when host is otg
>   usb:fsl:otg: Modify otg_event to start host drv
>   usb:fsl:otg: Combine host/gadget start/resume for ID change
>   usb:fsl:otg: Add host-gadget drv sync delay

Unless Alan's okay with the host side changes, I can't accept any of
these. However, I must say some of the flags you add here already exist
in some way, shape or form. For example, look at is_b_host flag.

-- 
balbi

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

Re: [PATCH 0/7][v4] Add OTG support for FSL socs

[Felipe Balbi]

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

Felipe Balbi <balbi@ti.com> writes:

> Hi,
>
> Ramneek Mehresh <ramneek.mehresh@freescale.com> writes:
>> Add support for otg for all freescale socs having internal
>> usb phy.
>>
>> Ramneek Mehresh (7):
>>   usb:fsl:otg: Make fsl otg driver as tristate
>>   usb:fsl:otg: Add controller version based ULPI and UTMI phy
>>   usb:fsl:otg: Add support to add/remove usb host driver
>>   usb:fsl:otg: Signal host drv when host is otg
>>   usb:fsl:otg: Modify otg_event to start host drv
>>   usb:fsl:otg: Combine host/gadget start/resume for ID change
>>   usb:fsl:otg: Add host-gadget drv sync delay
>
> Unless Alan's okay with the host side changes, I can't accept any of
> these. However, I must say some of the flags you add here already exist
> in some way, shape or form. For example, look at is_b_host flag.

Just saw Alan's reply, still, please have a look at these other flags
which already exist in usbcore today.

-- 
balbi

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

RE: [PATCH 0/7][v4] Add OTG support for FSL socs

[Ramneek Mehresh]

> -----Original Message-----
> From: Felipe Balbi [mailto:balbi@ti.com]
> Sent: Saturday, October 10, 2015 3:04 AM
> To: Mehresh Ramneek-B31383 <ramneek.mehresh@freescale.com>; linux-
> kernel@vger.kernel.org
> Cc: stern@rowland.harvard.edu; gregkh@linuxfoundation.org; linux-
> usb@vger.kernel.org; Mehresh Ramneek-B31383
> <ramneek.mehresh@freescale.com>
> Subject: Re: [PATCH 0/7][v4] Add OTG support for FSL socs
> 
> Felipe Balbi <balbi@ti.com> writes:
> 
> > Hi,
> >
> > Ramneek Mehresh <ramneek.mehresh@freescale.com> writes:
> >> Add support for otg for all freescale socs having internal usb phy.
> >>
> >> Ramneek Mehresh (7):
> >>   usb:fsl:otg: Make fsl otg driver as tristate
> >>   usb:fsl:otg: Add controller version based ULPI and UTMI phy
> >>   usb:fsl:otg: Add support to add/remove usb host driver
> >>   usb:fsl:otg: Signal host drv when host is otg
> >>   usb:fsl:otg: Modify otg_event to start host drv
> >>   usb:fsl:otg: Combine host/gadget start/resume for ID change
> >>   usb:fsl:otg: Add host-gadget drv sync delay
> >
> > Unless Alan's okay with the host side changes, I can't accept any of
> > these. However, I must say some of the flags you add here already
> > exist in some way, shape or form. For example, look at is_b_host flag.
> 
Could you please be more specific...which flag you think that I should remove/I'm
re-defining. The flags I'm defining are:
have_hcd : defined in fsl specific structure for fsl specific use-case
had_hcd: defined in fsl specific structure for fsl specific use-case
is_otg : defined in include/linux/usb.h
Are you suggesting using otg_port or is_b_host instead of is_otg?
As I understand, is_b_host is specifically to check if an otg B device is in host mode...correct?
I just need a flag to check if a controller is capable of otg operations? That's why defined
"is_otg" flag. Please suggest.

> Just saw Alan's reply, still, please have a look at these other flags which
> already exist in usbcore today.
> 
> --
> balbi

RE: [PATCH 0/7][v4] Add OTG support for FSL socs

[Felipe Balbi]

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


Hi,

Ramneek Mehresh <ramneek.mehresh@freescale.com> writes:
>> -----Original Message-----
>> From: Felipe Balbi [mailto:balbi@ti.com]
>> Sent: Saturday, October 10, 2015 3:04 AM
>> To: Mehresh Ramneek-B31383 <ramneek.mehresh@freescale.com>; linux-
>> kernel@vger.kernel.org
>> Cc: stern@rowland.harvard.edu; gregkh@linuxfoundation.org; linux-
>> usb@vger.kernel.org; Mehresh Ramneek-B31383
>> <ramneek.mehresh@freescale.com>
>> Subject: Re: [PATCH 0/7][v4] Add OTG support for FSL socs
>> 
>> Felipe Balbi <balbi@ti.com> writes:
>> 
>> > Hi,
>> >
>> > Ramneek Mehresh <ramneek.mehresh@freescale.com> writes:
>> >> Add support for otg for all freescale socs having internal usb phy.
>> >>
>> >> Ramneek Mehresh (7):
>> >>   usb:fsl:otg: Make fsl otg driver as tristate
>> >>   usb:fsl:otg: Add controller version based ULPI and UTMI phy
>> >>   usb:fsl:otg: Add support to add/remove usb host driver
>> >>   usb:fsl:otg: Signal host drv when host is otg
>> >>   usb:fsl:otg: Modify otg_event to start host drv
>> >>   usb:fsl:otg: Combine host/gadget start/resume for ID change
>> >>   usb:fsl:otg: Add host-gadget drv sync delay
>> >
>> > Unless Alan's okay with the host side changes, I can't accept any of
>> > these. However, I must say some of the flags you add here already
>> > exist in some way, shape or form. For example, look at is_b_host flag.
>>
>
> Could you please be more specific...which flag you think that I should
>remove/I'm re-defining. The flags I'm defining are:
>
> have_hcd : defined in fsl specific structure for fsl specific use-case
>
> had_hcd: defined in fsl specific structure for fsl specific use-case
>
> is_otg : defined in include/linux/usb.h
>
> Are you suggesting using otg_port or is_b_host instead of is_otg?
>
> As I understand, is_b_host is specifically to check if an otg B device
> is in host mode...correct?  I just need a flag to check if a
> controller is capable of otg operations? That's why defined "is_otg"
> flag. Please suggest.

no, I don't know why I made that comment. You could use otg_port, but
that wouldn't look very clean. Can you resend with Alan's ack, then I'll
move this series into testing/next.

-- 
balbi

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

RE: [PATCH 0/7][v4] Add OTG support for FSL socs

[Jun Li]

Hi

> -----Original Message-----
> From: linux-usb-owner@vger.kernel.org [mailto:linux-usb-
> owner@vger.kernel.org] On Behalf Of Felipe Balbi
> Sent: Wednesday, December 23, 2015 2:21 AM
> To: Ramneek Mehresh <ramneek.mehresh@freescale.com>; linux-
> kernel@vger.kernel.org
> Cc: stern@rowland.harvard.edu; gregkh@linuxfoundation.org; linux-
> usb@vger.kernel.org
> Subject: RE: [PATCH 0/7][v4] Add OTG support for FSL socs
> 
> 
> Hi,
> 
> Ramneek Mehresh <ramneek.mehresh@freescale.com> writes:
> >> -----Original Message-----
> >> From: Felipe Balbi [mailto:balbi@ti.com]
> >> Sent: Saturday, October 10, 2015 3:04 AM
> >> To: Mehresh Ramneek-B31383 <ramneek.mehresh@freescale.com>; linux-
> >> kernel@vger.kernel.org
> >> Cc: stern@rowland.harvard.edu; gregkh@linuxfoundation.org; linux-
> >> usb@vger.kernel.org; Mehresh Ramneek-B31383
> >> <ramneek.mehresh@freescale.com>
> >> Subject: Re: [PATCH 0/7][v4] Add OTG support for FSL socs
> >>
> >> Felipe Balbi <balbi@ti.com> writes:
> >>
> >> > Hi,
> >> >
> >> > Ramneek Mehresh <ramneek.mehresh@freescale.com> writes:
> >> >> Add support for otg for all freescale socs having internal usb phy.
> >> >>
> >> >> Ramneek Mehresh (7):
> >> >>   usb:fsl:otg: Make fsl otg driver as tristate
> >> >>   usb:fsl:otg: Add controller version based ULPI and UTMI phy
> >> >>   usb:fsl:otg: Add support to add/remove usb host driver
> >> >>   usb:fsl:otg: Signal host drv when host is otg
> >> >>   usb:fsl:otg: Modify otg_event to start host drv
> >> >>   usb:fsl:otg: Combine host/gadget start/resume for ID change
> >> >>   usb:fsl:otg: Add host-gadget drv sync delay
> >> >
> >> > Unless Alan's okay with the host side changes, I can't accept any
> >> > of these. However, I must say some of the flags you add here
> >> > already exist in some way, shape or form. For example, look at
> is_b_host flag.
> >>
> >
> > Could you please be more specific...which flag you think that I should
> >remove/I'm re-defining. The flags I'm defining are:
> >
> > have_hcd : defined in fsl specific structure for fsl specific use-case
> >
> > had_hcd: defined in fsl specific structure for fsl specific use-case
> >
> > is_otg : defined in include/linux/usb.h
> >
> > Are you suggesting using otg_port or is_b_host instead of is_otg?
> >
> > As I understand, is_b_host is specifically to check if an otg B device
> > is in host mode...correct?  I just need a flag to check if a
> > controller is capable of otg operations? That's why defined "is_otg"
> > flag. Please suggest.
> 
> no, I don't know why I made that comment. You could use otg_port, but that
> wouldn't look very clean. Can you resend with Alan's ack, then I'll move
> this series into testing/next.
> 
> --
> balbi

Can you directly put the change_hcd_work in its phy driver(phy-fsl-usb.c)?
Then add/remove hcd will not through ehci_fsl_drv_suspend/resume,
With this, you can make it work without a new flag "is_otg".

Li Jun 

RE: [PATCH 0/7][v4] Add OTG support for FSL socs

[Ramneek Mehresh]

> -----Original Message-----
> From: Jun Li [mailto:jun.li@nxp.com]
> Sent: Wednesday, December 23, 2015 10:36 AM
> To: Felipe Balbi <balbi@ti.com>; Mehresh Ramneek-B31383
> <ramneek.mehresh@freescale.com>; linux-kernel@vger.kernel.org
> Cc: stern@rowland.harvard.edu; gregkh@linuxfoundation.org; linux-
> usb@vger.kernel.org
> Subject: RE: [PATCH 0/7][v4] Add OTG support for FSL socs
> 
> Hi
> 
> > -----Original Message-----
> > From: linux-usb-owner@vger.kernel.org [mailto:linux-usb-
> > owner@vger.kernel.org] On Behalf Of Felipe Balbi
> > Sent: Wednesday, December 23, 2015 2:21 AM
> > To: Ramneek Mehresh <ramneek.mehresh@freescale.com>; linux-
> > kernel@vger.kernel.org
> > Cc: stern@rowland.harvard.edu; gregkh@linuxfoundation.org; linux-
> > usb@vger.kernel.org
> > Subject: RE: [PATCH 0/7][v4] Add OTG support for FSL socs
> >
> >
> > Hi,
> >
> > Ramneek Mehresh <ramneek.mehresh@freescale.com> writes:
> > >> -----Original Message-----
> > >> From: Felipe Balbi [mailto:balbi@ti.com]
> > >> Sent: Saturday, October 10, 2015 3:04 AM
> > >> To: Mehresh Ramneek-B31383 <ramneek.mehresh@freescale.com>;
> linux-
> > >> kernel@vger.kernel.org
> > >> Cc: stern@rowland.harvard.edu; gregkh@linuxfoundation.org; linux-
> > >> usb@vger.kernel.org; Mehresh Ramneek-B31383
> > >> <ramneek.mehresh@freescale.com>
> > >> Subject: Re: [PATCH 0/7][v4] Add OTG support for FSL socs
> > >>
> > >> Felipe Balbi <balbi@ti.com> writes:
> > >>
> > >> > Hi,
> > >> >
> > >> > Ramneek Mehresh <ramneek.mehresh@freescale.com> writes:
> > >> >> Add support for otg for all freescale socs having internal usb phy.
> > >> >>
> > >> >> Ramneek Mehresh (7):
> > >> >>   usb:fsl:otg: Make fsl otg driver as tristate
> > >> >>   usb:fsl:otg: Add controller version based ULPI and UTMI phy
> > >> >>   usb:fsl:otg: Add support to add/remove usb host driver
> > >> >>   usb:fsl:otg: Signal host drv when host is otg
> > >> >>   usb:fsl:otg: Modify otg_event to start host drv
> > >> >>   usb:fsl:otg: Combine host/gadget start/resume for ID change
> > >> >>   usb:fsl:otg: Add host-gadget drv sync delay
> > >> >
> > >> > Unless Alan's okay with the host side changes, I can't accept any
> > >> > of these. However, I must say some of the flags you add here
> > >> > already exist in some way, shape or form. For example, look at
> > is_b_host flag.
> > >>
> > >
> > > Could you please be more specific...which flag you think that I
> > >should remove/I'm re-defining. The flags I'm defining are:
> > >
> > > have_hcd : defined in fsl specific structure for fsl specific
> > > use-case
> > >
> > > had_hcd: defined in fsl specific structure for fsl specific use-case
> > >
> > > is_otg : defined in include/linux/usb.h
> > >
> > > Are you suggesting using otg_port or is_b_host instead of is_otg?
> > >
> > > As I understand, is_b_host is specifically to check if an otg B
> > > device is in host mode...correct?  I just need a flag to check if a
> > > controller is capable of otg operations? That's why defined "is_otg"
> > > flag. Please suggest.
> >
> > no, I don't know why I made that comment. You could use otg_port, but
> > that wouldn't look very clean. Can you resend with Alan's ack, then
> > I'll move this series into testing/next.
> >
> > --
> > balbi
> 
> Can you directly put the change_hcd_work in its phy driver(phy-fsl-usb.c)?
> Then add/remove hcd will not through ehci_fsl_drv_suspend/resume, With
> this, you can make it work without a new flag "is_otg".
> 
change_hcd_work() changes host controller mode, and hence, should be in host controller
driver. To move this to phy driver (just to avoid usage of a single flag) won't be a good idea.
In this case, I'll also have to move some host controller specific flags to phy drv which won't be correct.

> Li Jun

RE: [PATCH 0/7][v4] Add OTG support for FSL socs

[Jun Li]

> -----Original Message-----
> From: Ramneek Mehresh [mailto:ramneek.mehresh@freescale.com]
> Sent: Wednesday, December 23, 2015 8:20 PM
> To: Jun Li <jun.li@nxp.com>; Felipe Balbi <balbi@ti.com>; linux-
> kernel@vger.kernel.org
> Cc: stern@rowland.harvard.edu; gregkh@linuxfoundation.org; linux-
> usb@vger.kernel.org
> Subject: RE: [PATCH 0/7][v4] Add OTG support for FSL socs
> 
> 
> 
> > -----Original Message-----
> > From: Jun Li [mailto:jun.li@nxp.com]
> > Sent: Wednesday, December 23, 2015 10:36 AM
> > To: Felipe Balbi <balbi@ti.com>; Mehresh Ramneek-B31383
> > <ramneek.mehresh@freescale.com>; linux-kernel@vger.kernel.org
> > Cc: stern@rowland.harvard.edu; gregkh@linuxfoundation.org; linux-
> > usb@vger.kernel.org
> > Subject: RE: [PATCH 0/7][v4] Add OTG support for FSL socs
> >
> > Hi
> >
> > > -----Original Message-----
> > > From: linux-usb-owner@vger.kernel.org [mailto:linux-usb-
> > > owner@vger.kernel.org] On Behalf Of Felipe Balbi
> > > Sent: Wednesday, December 23, 2015 2:21 AM
> > > To: Ramneek Mehresh <ramneek.mehresh@freescale.com>; linux-
> > > kernel@vger.kernel.org
> > > Cc: stern@rowland.harvard.edu; gregkh@linuxfoundation.org; linux-
> > > usb@vger.kernel.org
> > > Subject: RE: [PATCH 0/7][v4] Add OTG support for FSL socs
> > >
> > >
> > > Hi,
> > >
> > > Ramneek Mehresh <ramneek.mehresh@freescale.com> writes:
> > > >> -----Original Message-----
> > > >> From: Felipe Balbi [mailto:balbi@ti.com]
> > > >> Sent: Saturday, October 10, 2015 3:04 AM
> > > >> To: Mehresh Ramneek-B31383 <ramneek.mehresh@freescale.com>;
> > linux-
> > > >> kernel@vger.kernel.org
> > > >> Cc: stern@rowland.harvard.edu; gregkh@linuxfoundation.org; linux-
> > > >> usb@vger.kernel.org; Mehresh Ramneek-B31383
> > > >> <ramneek.mehresh@freescale.com>
> > > >> Subject: Re: [PATCH 0/7][v4] Add OTG support for FSL socs
> > > >>
> > > >> Felipe Balbi <balbi@ti.com> writes:
> > > >>
> > > >> > Hi,
> > > >> >
> > > >> > Ramneek Mehresh <ramneek.mehresh@freescale.com> writes:
> > > >> >> Add support for otg for all freescale socs having internal usb
> phy.
> > > >> >>
> > > >> >> Ramneek Mehresh (7):
> > > >> >>   usb:fsl:otg: Make fsl otg driver as tristate
> > > >> >>   usb:fsl:otg: Add controller version based ULPI and UTMI phy
> > > >> >>   usb:fsl:otg: Add support to add/remove usb host driver
> > > >> >>   usb:fsl:otg: Signal host drv when host is otg
> > > >> >>   usb:fsl:otg: Modify otg_event to start host drv
> > > >> >>   usb:fsl:otg: Combine host/gadget start/resume for ID change
> > > >> >>   usb:fsl:otg: Add host-gadget drv sync delay
> > > >> >
> > > >> > Unless Alan's okay with the host side changes, I can't accept
> > > >> > any of these. However, I must say some of the flags you add
> > > >> > here already exist in some way, shape or form. For example,
> > > >> > look at
> > > is_b_host flag.
> > > >>
> > > >
> > > > Could you please be more specific...which flag you think that I
> > > >should remove/I'm re-defining. The flags I'm defining are:
> > > >
> > > > have_hcd : defined in fsl specific structure for fsl specific
> > > > use-case
> > > >
> > > > had_hcd: defined in fsl specific structure for fsl specific
> > > > use-case
> > > >
> > > > is_otg : defined in include/linux/usb.h
> > > >
> > > > Are you suggesting using otg_port or is_b_host instead of is_otg?
> > > >
> > > > As I understand, is_b_host is specifically to check if an otg B
> > > > device is in host mode...correct?  I just need a flag to check if
> > > > a controller is capable of otg operations? That's why defined
> "is_otg"
> > > > flag. Please suggest.
> > >
> > > no, I don't know why I made that comment. You could use otg_port,
> > > but that wouldn't look very clean. Can you resend with Alan's ack,
> > > then I'll move this series into testing/next.
> > >
> > > --
> > > balbi
> >
> > Can you directly put the change_hcd_work in its phy driver(phy-fsl-
> usb.c)?
> > Then add/remove hcd will not through ehci_fsl_drv_suspend/resume, With
> > this, you can make it work without a new flag "is_otg".
> >
> change_hcd_work() changes host controller mode, and hence, should be in
> host controller driver. 

Reasonable, but most of OTG stuff not belong to phy actully,
I see phy-msm-usb.c directly do hcd add/remove for OTG. 

> To move this to phy driver (just to avoid usage of
> a single flag) won't be a good idea.

I just saw you simply still use the suspend/resume method, but the
change_hcd_work actually has nothing to do with it now, then add a
flag to return for it, also the flag in your usage is not to indicate
if the host capable/enabled otg, but something like
is_host_role_to_be_changed_for_otg(test and then clear).

Meanwhile you add another 2 flags in ehci_fsl(have_hcd, add_hcd) also for
OTG host add/remove, so I am thinking if all those can be simplified. 

> In this case, I'll also have to move some host controller specific flags
> to phy drv which won't be correct.

> 
> > Li Jun

RE: [PATCH 0/7][v4] Add OTG support for FSL socs

[Ramneek Mehresh]

> -----Original Message-----
> From: Jun Li [mailto:jun.li@nxp.com]
> Sent: Wednesday, December 23, 2015 10:02 PM
> To: Mehresh Ramneek-B31383 <ramneek.mehresh@freescale.com>; Felipe
> Balbi <balbi@ti.com>; linux-kernel@vger.kernel.org
> Cc: stern@rowland.harvard.edu; gregkh@linuxfoundation.org; linux-
> usb@vger.kernel.org
> Subject: RE: [PATCH 0/7][v4] Add OTG support for FSL socs
> 
> 
> 
> > -----Original Message-----
> > From: Ramneek Mehresh [mailto:ramneek.mehresh@freescale.com]
> > Sent: Wednesday, December 23, 2015 8:20 PM
> > To: Jun Li <jun.li@nxp.com>; Felipe Balbi <balbi@ti.com>; linux-
> > kernel@vger.kernel.org
> > Cc: stern@rowland.harvard.edu; gregkh@linuxfoundation.org; linux-
> > usb@vger.kernel.org
> > Subject: RE: [PATCH 0/7][v4] Add OTG support for FSL socs
> >
> >
> >
> > > -----Original Message-----
> > > From: Jun Li [mailto:jun.li@nxp.com]
> > > Sent: Wednesday, December 23, 2015 10:36 AM
> > > To: Felipe Balbi <balbi@ti.com>; Mehresh Ramneek-B31383
> > > <ramneek.mehresh@freescale.com>; linux-kernel@vger.kernel.org
> > > Cc: stern@rowland.harvard.edu; gregkh@linuxfoundation.org; linux-
> > > usb@vger.kernel.org
> > > Subject: RE: [PATCH 0/7][v4] Add OTG support for FSL socs
> > >
> > > Hi
> > >
> > > > -----Original Message-----
> > > > From: linux-usb-owner@vger.kernel.org [mailto:linux-usb-
> > > > owner@vger.kernel.org] On Behalf Of Felipe Balbi
> > > > Sent: Wednesday, December 23, 2015 2:21 AM
> > > > To: Ramneek Mehresh <ramneek.mehresh@freescale.com>; linux-
> > > > kernel@vger.kernel.org
> > > > Cc: stern@rowland.harvard.edu; gregkh@linuxfoundation.org; linux-
> > > > usb@vger.kernel.org
> > > > Subject: RE: [PATCH 0/7][v4] Add OTG support for FSL socs
> > > >
> > > >
> > > > Hi,
> > > >
> > > > Ramneek Mehresh <ramneek.mehresh@freescale.com> writes:
> > > > >> -----Original Message-----
> > > > >> From: Felipe Balbi [mailto:balbi@ti.com]
> > > > >> Sent: Saturday, October 10, 2015 3:04 AM
> > > > >> To: Mehresh Ramneek-B31383
> <ramneek.mehresh@freescale.com>;
> > > linux-
> > > > >> kernel@vger.kernel.org
> > > > >> Cc: stern@rowland.harvard.edu; gregkh@linuxfoundation.org;
> > > > >> linux- usb@vger.kernel.org; Mehresh Ramneek-B31383
> > > > >> <ramneek.mehresh@freescale.com>
> > > > >> Subject: Re: [PATCH 0/7][v4] Add OTG support for FSL socs
> > > > >>
> > > > >> Felipe Balbi <balbi@ti.com> writes:
> > > > >>
> > > > >> > Hi,
> > > > >> >
> > > > >> > Ramneek Mehresh <ramneek.mehresh@freescale.com> writes:
> > > > >> >> Add support for otg for all freescale socs having internal
> > > > >> >> usb
> > phy.
> > > > >> >>
> > > > >> >> Ramneek Mehresh (7):
> > > > >> >>   usb:fsl:otg: Make fsl otg driver as tristate
> > > > >> >>   usb:fsl:otg: Add controller version based ULPI and UTMI phy
> > > > >> >>   usb:fsl:otg: Add support to add/remove usb host driver
> > > > >> >>   usb:fsl:otg: Signal host drv when host is otg
> > > > >> >>   usb:fsl:otg: Modify otg_event to start host drv
> > > > >> >>   usb:fsl:otg: Combine host/gadget start/resume for ID change
> > > > >> >>   usb:fsl:otg: Add host-gadget drv sync delay
> > > > >> >
> > > > >> > Unless Alan's okay with the host side changes, I can't accept
> > > > >> > any of these. However, I must say some of the flags you add
> > > > >> > here already exist in some way, shape or form. For example,
> > > > >> > look at
> > > > is_b_host flag.
> > > > >>
> > > > >
> > > > > Could you please be more specific...which flag you think that I
> > > > >should remove/I'm re-defining. The flags I'm defining are:
> > > > >
> > > > > have_hcd : defined in fsl specific structure for fsl specific
> > > > > use-case
> > > > >
> > > > > had_hcd: defined in fsl specific structure for fsl specific
> > > > > use-case
> > > > >
> > > > > is_otg : defined in include/linux/usb.h
> > > > >
> > > > > Are you suggesting using otg_port or is_b_host instead of is_otg?
> > > > >
> > > > > As I understand, is_b_host is specifically to check if an otg B
> > > > > device is in host mode...correct?  I just need a flag to check
> > > > > if a controller is capable of otg operations? That's why defined
> > "is_otg"
> > > > > flag. Please suggest.
> > > >
> > > > no, I don't know why I made that comment. You could use otg_port,
> > > > but that wouldn't look very clean. Can you resend with Alan's ack,
> > > > then I'll move this series into testing/next.
> > > >
> > > > --
> > > > balbi
> > >
> > > Can you directly put the change_hcd_work in its phy driver(phy-fsl-
> > usb.c)?
> > > Then add/remove hcd will not through ehci_fsl_drv_suspend/resume,
> > > With this, you can make it work without a new flag "is_otg".
> > >
> > change_hcd_work() changes host controller mode, and hence, should be
> > in host controller driver.
> 
> Reasonable, but most of OTG stuff not belong to phy actully, I see phy-msm-
> usb.c directly do hcd add/remove for OTG.
> 
Thanks for your input...but I still strongly believe that controller specific data structure and flags
should remain inside the controller drv...to make things more maintainable. I'm adding and removing
host controller driver on the basis of the id change value for fsl controller drv...
> > To move this to phy driver (just to avoid usage of a single flag)
> > won't be a good idea.
> 
> I just saw you simply still use the suspend/resume method, but the
> change_hcd_work actually has nothing to do with it now, 
change_hcd_work() is a work that is scheduled via suspend/resume to
remove/add host controller drv...
then add a flag to
> return for it, also the flag in your usage is not to indicate if the host
> capable/enabled otg, but something like
> is_host_role_to_be_changed_for_otg(test and then clear).
> 
Actually...you're right in usage of is_host_role_to_be_changed_for_otg(test and then clear)...
this could be a better way to do this...however I also use have_hcd flag to remove host drv for 
the first time. On fsl platforms, gadget is the default mode, and hence, I need to remove host drv 
as soon as it is loaded. In addition, I was thinking that it might be a good idea to have information
inside ehci_fsl struct to know if host drv is loaded currently....may be used for some 
future implementation... 
> Meanwhile you add another 2 flags in ehci_fsl(have_hcd, add_hcd) also for
> OTG host add/remove, so I am thinking if all those can be simplified.
> 
thanks for your input ...have_hcd helps in determining if the host controller drv is already up...so I need to 
remove it in order to load gadget driver, and add_hcd simply tells me if I need to bring-up
host driver again, and remove gadget driver....these are essential mechanisms for adding/removing
host controller drv...again moving them to phy layer is not very convincing.... 
> > In this case, I'll also have to move some host controller specific
> > flags to phy drv which won't be correct.
> 
> >
> > > Li Jun