All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/7] usb: ehci-mx6: Add powerup_fixup implementation
@ 2020-06-29  2:13 Peng Fan
  2020-06-29  2:13 ` [PATCH 2/7] usb: ehci-mx6: Add i.MX8 OTG controller support Peng Fan
                   ` (7 more replies)
  0 siblings, 8 replies; 15+ messages in thread
From: Peng Fan @ 2020-06-29  2:13 UTC (permalink / raw)
  To: u-boot

From: Ye Li <ye.li@nxp.com>

When doing port reset, the PR bit of PORTSC1 will be automatically
cleared by our IP, but standard EHCI needs explicit clear by software. The
EHCI-HCD driver follow the EHCI specification, so after 50ms wait, it
clear the PR bit by writting to the PORTSC1 register with value loaded before
setting PR.

This sequence is ok for our IP when the delay time is exact. But when the timer
is slower, some bits like PE, PSPD have been set by controller automatically
after the PR is automatically cleared. So the writing to the PORTSC1 will overwrite
these bits set by controller. And eventually the driver gets wrong status.

We implement the powerup_fixup operation which delays 50ms and will check
the PR until it is cleared by controller. And will update the reg value which is written
to PORTSC register by EHCI-HCD driver. This is much safer than depending on the delay
time to be accurate and aligining with controller's behaiver.

Signed-off-by: Ye Li <ye.li@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/usb/host/ehci-mx6.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c
index 5f84c7b91d..e654595683 100644
--- a/drivers/usb/host/ehci-mx6.c
+++ b/drivers/usb/host/ehci-mx6.c
@@ -267,6 +267,25 @@ int usb_phy_mode(int port)
 }
 #endif
 
+static void ehci_mx6_powerup_fixup(struct ehci_ctrl *ctrl, uint32_t *status_reg,
+				   uint32_t *reg)
+{
+	u32 result;
+	int usec = 2000;
+
+	mdelay(50);
+
+	do {
+		result = ehci_readl(status_reg);
+		udelay(5);
+		if (!(result & EHCI_PS_PR))
+			break;
+		usec--;
+	} while (usec > 0);
+
+	*reg = ehci_readl(status_reg);
+}
+
 static void usb_oc_config(int index)
 {
 #if defined(CONFIG_MX6)
@@ -366,6 +385,10 @@ int ehci_mx6_common_init(struct usb_ehci *ehci, int index)
 }
 
 #if !CONFIG_IS_ENABLED(DM_USB)
+static const struct ehci_ops mx6_ehci_ops = {
+	.powerup_fixup		= ehci_mx6_powerup_fixup,
+};
+
 int ehci_hcd_init(int index, enum usb_init_type init,
 		struct ehci_hccr **hccr, struct ehci_hcor **hcor)
 {
@@ -394,6 +417,8 @@ int ehci_hcd_init(int index, enum usb_init_type init,
 	if (ret)
 		return ret;
 
+	ehci_set_controller_priv(index, NULL, &mx6_ehci_ops);
+
 	type = board_usb_phy_mode(index);
 
 	if (hccr && hcor) {
@@ -467,7 +492,8 @@ static int mx6_init_after_reset(struct ehci_ctrl *dev)
 }
 
 static const struct ehci_ops mx6_ehci_ops = {
-	.init_after_reset = mx6_init_after_reset
+	.powerup_fixup		= ehci_mx6_powerup_fixup,
+	.init_after_reset	= mx6_init_after_reset
 };
 
 static int ehci_usb_phy_mode(struct udevice *dev)
-- 
2.16.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/7] usb: ehci-mx6: Add i.MX8 OTG controller support
  2020-06-29  2:13 [PATCH 1/7] usb: ehci-mx6: Add powerup_fixup implementation Peng Fan
@ 2020-06-29  2:13 ` Peng Fan
  2020-06-29  2:26   ` Marek Vasut
  2020-06-29  2:13 ` [PATCH 3/7] usb: ehci-mx6: Turn on the power domain of USB PHY Peng Fan
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Peng Fan @ 2020-06-29  2:13 UTC (permalink / raw)
  To: u-boot

From: Ye Li <ye.li@nxp.com>

The i.MX8 has two USB controllers: USBOH and USB3. The USBOH reuses
previous i.MX6/7. It has same PHY IP as i.MX7ULP but NC registers
are same as i.MX7D. So add its support in ehci-mx6 driver.

Also the driver is updated to remove build warning for 64 bits CPU.

Signed-off-by: Ye Li <ye.li@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/usb/host/Kconfig    |  4 +-
 drivers/usb/host/ehci-mx6.c | 97 ++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 85 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 1c374a7bd8..f48547caa0 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -139,8 +139,8 @@ config USB_EHCI_MX5
 	  Enables support for the on-chip EHCI controller on i.MX5 SoCs.
 
 config USB_EHCI_MX6
-	bool "Support for i.MX6/i.MX7ULP on-chip EHCI USB controller"
-	depends on ARCH_MX6 || ARCH_MX7ULP
+	bool "Support for i.MX6/i.MX7ULP/i.MX8 on-chip EHCI USB controller"
+	depends on ARCH_MX6 || ARCH_MX7ULP || ARCH_IMX8
 	default y
 	---help---
 	  Enables support for the on-chip EHCI controller on i.MX6 SoCs.
diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c
index e654595683..191d619220 100644
--- a/drivers/usb/host/ehci-mx6.c
+++ b/drivers/usb/host/ehci-mx6.c
@@ -62,12 +62,20 @@ DECLARE_GLOBAL_DATA_PTR;
 #define UCTRL_OVER_CUR_POL	(1 << 8) /* OTG Polarity of Overcurrent */
 #define UCTRL_OVER_CUR_DIS	(1 << 7) /* Disable OTG Overcurrent Detection */
 
+#define PLL_USB_EN_USB_CLKS_MASK	(0x01 << 6)
+#define PLL_USB_PWR_MASK		(0x01 << 12)
+#define PLL_USB_ENABLE_MASK		(0x01 << 13)
+#define PLL_USB_BYPASS_MASK		(0x01 << 16)
+#define PLL_USB_REG_ENABLE_MASK		(0x01 << 21)
+#define PLL_USB_DIV_SEL_MASK		(0x07 << 22)
+#define PLL_USB_LOCK_MASK		(0x01 << 31)
+
 /* USBCMD */
 #define UCMD_RUN_STOP           (1 << 0) /* controller run/stop */
 #define UCMD_RESET		(1 << 1) /* controller reset */
 
-#if defined(CONFIG_MX6) || defined(CONFIG_MX7ULP)
-static const unsigned phy_bases[] = {
+#if defined(CONFIG_MX6) || defined(CONFIG_MX7ULP) || defined(CONFIG_IMX8)
+static const ulong phy_bases[] = {
 	USB_PHY0_BASE_ADDR,
 #if defined(USB_PHY1_BASE_ADDR)
 	USB_PHY1_BASE_ADDR,
@@ -101,6 +109,53 @@ static void usb_power_config(int index)
 
 	scg_enable_usb_pll(true);
 
+#elif defined(CONFIG_IMX8)
+	struct usbphy_regs __iomem *usbphy =
+		(struct usbphy_regs __iomem *)USB_PHY0_BASE_ADDR;
+	int timeout = 1000000;
+
+	if (index > 0)
+		return;
+
+	writel(ANADIG_USB2_CHRG_DETECT_EN_B |
+		   ANADIG_USB2_CHRG_DETECT_CHK_CHRG_B,
+		   &usbphy->usb1_chrg_detect);
+
+	if (!(readl(&usbphy->usb1_pll_480_ctrl) & PLL_USB_LOCK_MASK)) {
+
+		/* Enable the regulator first */
+		writel(PLL_USB_REG_ENABLE_MASK,
+		       &usbphy->usb1_pll_480_ctrl_set);
+
+		/* Wait at least 25us */
+		udelay(25);
+
+		/* Enable the power */
+		writel(PLL_USB_PWR_MASK, &usbphy->usb1_pll_480_ctrl_set);
+
+		/* Wait lock */
+		while (timeout--) {
+			if (readl(&usbphy->usb1_pll_480_ctrl) &
+			    PLL_USB_LOCK_MASK)
+				break;
+			udelay(10);
+		}
+
+		if (timeout <= 0) {
+			/* If timeout, we power down the pll */
+			writel(PLL_USB_PWR_MASK,
+			       &usbphy->usb1_pll_480_ctrl_clr);
+			return;
+		}
+	}
+
+	/* Clear the bypass */
+	writel(PLL_USB_BYPASS_MASK, &usbphy->usb1_pll_480_ctrl_clr);
+
+	/* Enable the PLL clock out to USB */
+	writel((PLL_USB_EN_USB_CLKS_MASK | PLL_USB_ENABLE_MASK),
+	       &usbphy->usb1_pll_480_ctrl_set);
+
 #else
 	struct anatop_regs __iomem *anatop =
 		(struct anatop_regs __iomem *)ANATOP_BASE_ADDR;
@@ -212,6 +267,20 @@ struct usbnc_regs {
 	u32 reserve0[2];
 	u32 hsic_ctrl;
 };
+#elif defined(CONFIG_IMX8)
+struct usbnc_regs {
+	u32 ctrl1;
+	u32 ctrl2;
+	u32 reserve1[10];
+	u32 phy_cfg1;
+	u32 phy_cfg2;
+	u32 reserve2;
+	u32 phy_status;
+	u32 reserve3[4];
+	u32 adp_cfg1;
+	u32 adp_cfg2;
+	u32 adp_status;
+};
 #else
 /* Base address for this IP block is 0x02184800 */
 struct usbnc_regs {
@@ -240,7 +309,7 @@ struct usbnc_regs {
 
 static void usb_power_config(int index)
 {
-	struct usbnc_regs *usbnc = (struct usbnc_regs *)(USB_BASE_ADDR +
+	struct usbnc_regs *usbnc = (struct usbnc_regs *)(ulong)(USB_BASE_ADDR +
 			(0x10000 * index) + USBNC_OFFSET);
 	void __iomem *phy_cfg2 = (void __iomem *)(&usbnc->phy_cfg2);
 
@@ -253,7 +322,7 @@ static void usb_power_config(int index)
 
 int usb_phy_mode(int port)
 {
-	struct usbnc_regs *usbnc = (struct usbnc_regs *)(USB_BASE_ADDR +
+	struct usbnc_regs *usbnc = (struct usbnc_regs *)(ulong)(USB_BASE_ADDR +
 			(0x10000 * port) + USBNC_OFFSET);
 	void __iomem *status = (void __iomem *)(&usbnc->phy_status);
 	u32 val;
@@ -292,8 +361,8 @@ static void usb_oc_config(int index)
 	struct usbnc_regs *usbnc = (struct usbnc_regs *)(USB_BASE_ADDR +
 			USB_OTHERREGS_OFFSET);
 	void __iomem *ctrl = (void __iomem *)(&usbnc->ctrl[index]);
-#elif defined(CONFIG_MX7) || defined(CONFIG_MX7ULP)
-	struct usbnc_regs *usbnc = (struct usbnc_regs *)(USB_BASE_ADDR +
+#elif defined(CONFIG_MX7) || defined(CONFIG_MX7ULP) || defined(CONFIG_IMX8)
+	struct usbnc_regs *usbnc = (struct usbnc_regs *)(ulong)(USB_BASE_ADDR +
 			(0x10000 * index) + USBNC_OFFSET);
 	void __iomem *ctrl = (void __iomem *)(&usbnc->ctrl1);
 #endif
@@ -376,7 +445,7 @@ int ehci_mx6_common_init(struct usb_ehci *ehci, int index)
 	usb_power_config(index);
 	usb_oc_config(index);
 
-#if defined(CONFIG_MX6) || defined(CONFIG_MX7ULP)
+#if defined(CONFIG_MX6) || defined(CONFIG_MX7ULP) || defined(CONFIG_IMX8)
 	usb_internal_phy_clock_gate(index, 1);
 	usb_phy_enable(index, ehci);
 #endif
@@ -395,10 +464,10 @@ int ehci_hcd_init(int index, enum usb_init_type init,
 	enum usb_init_type type;
 #if defined(CONFIG_MX6)
 	u32 controller_spacing = 0x200;
-#elif defined(CONFIG_MX7) || defined(CONFIG_MX7ULP)
+#elif defined(CONFIG_MX7) || defined(CONFIG_MX7ULP) || defined(CONFIG_IMX8)
 	u32 controller_spacing = 0x10000;
 #endif
-	struct usb_ehci *ehci = (struct usb_ehci *)(USB_BASE_ADDR +
+	struct usb_ehci *ehci = (struct usb_ehci *)(ulong)(USB_BASE_ADDR +
 		(controller_spacing * index));
 	int ret;
 
@@ -422,8 +491,8 @@ int ehci_hcd_init(int index, enum usb_init_type init,
 	type = board_usb_phy_mode(index);
 
 	if (hccr && hcor) {
-		*hccr = (struct ehci_hccr *)((uint32_t)&ehci->caplength);
-		*hcor = (struct ehci_hcor *)((uint32_t)*hccr +
+		*hccr = (struct ehci_hccr *)((ulong)&ehci->caplength);
+		*hcor = (struct ehci_hcor *)((ulong)*hccr +
 				HC_LENGTH(ehci_readl(&(*hccr)->cr_capbase)));
 	}
 
@@ -509,7 +578,7 @@ static int ehci_usb_phy_mode(struct udevice *dev)
 	 * About fsl,usbphy, Refer to
 	 * Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt.
 	 */
-	if (is_mx6() || is_mx7ulp()) {
+	if (is_mx6() || is_mx7ulp() || is_imx8()) {
 		phy_off = fdtdec_lookup_phandle(blob,
 						offset,
 						"fsl,usbphy");
@@ -655,8 +724,8 @@ static int ehci_usb_probe(struct udevice *dev)
 
 	mdelay(10);
 
-	hccr = (struct ehci_hccr *)((uint32_t)&ehci->caplength);
-	hcor = (struct ehci_hcor *)((uint32_t)hccr +
+	hccr = (struct ehci_hccr *)((ulong)&ehci->caplength);
+	hcor = (struct ehci_hcor *)((ulong)hccr +
 			HC_LENGTH(ehci_readl(&(hccr)->cr_capbase)));
 
 	return ehci_register(dev, hccr, hcor, &mx6_ehci_ops, 0, priv->init_type);
-- 
2.16.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/7] usb: ehci-mx6: Turn on the power domain of USB PHY
  2020-06-29  2:13 [PATCH 1/7] usb: ehci-mx6: Add powerup_fixup implementation Peng Fan
  2020-06-29  2:13 ` [PATCH 2/7] usb: ehci-mx6: Add i.MX8 OTG controller support Peng Fan
@ 2020-06-29  2:13 ` Peng Fan
  2020-06-29  2:27   ` Marek Vasut
  2020-06-29  2:13 ` [PATCH 4/7] usb: ehci-mx6: Update driver to support i.MX8MM Peng Fan
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Peng Fan @ 2020-06-29  2:13 UTC (permalink / raw)
  To: u-boot

From: Ye Li <ye.li@nxp.com>

Since there is no uclass for USB PHY. The device won't be setup for the USB PHY
node in DTB. And its associated power domain device won't be turned on neither
by DM framework.

This patch modifies the ehci-mx6 driver to enable the power domain device before
access the USB PHY. This is only for DM driver. For non-DM part, users still
need to power on the USB PHY in boards/SoC codes.

Reviewed-by: Peng Fan <peng.fan@nxp.com>
Signed-off-by: Ye Li <ye.li@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/usb/host/ehci-mx6.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c
index 191d619220..92e8bb91d2 100644
--- a/drivers/usb/host/ehci-mx6.c
+++ b/drivers/usb/host/ehci-mx6.c
@@ -2,6 +2,8 @@
 /*
  * Copyright (c) 2009 Daniel Mack <daniel@caiaq.de>
  * Copyright (C) 2010 Freescale Semiconductor, Inc.
+ * Copyright 2017 NXP
+ *
  */
 
 #include <common.h>
@@ -23,6 +25,9 @@
 #include <linux/usb/otg.h>
 
 #include "ehci.h"
+#if CONFIG_IS_ENABLED(POWER_DOMAIN)
+#include <power-domain.h>
+#endif
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -590,6 +595,18 @@ static int ehci_usb_phy_mode(struct udevice *dev)
 		if ((fdt_addr_t)addr == FDT_ADDR_T_NONE)
 			return -EINVAL;
 
+		/* Need to power on the PHY before access it */
+#if CONFIG_IS_ENABLED(POWER_DOMAIN)
+		struct udevice phy_dev;
+		struct power_domain pd;
+
+		phy_dev.node = offset_to_ofnode(phy_off);
+		if (!power_domain_get(&phy_dev, &pd)) {
+			if (power_domain_on(&pd))
+				return -EINVAL;
+		}
+#endif
+
 		phy_ctrl = (void __iomem *)(addr + USBPHY_CTRL);
 		val = readl(phy_ctrl);
 
-- 
2.16.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 4/7] usb: ehci-mx6: Update driver to support i.MX8MM
  2020-06-29  2:13 [PATCH 1/7] usb: ehci-mx6: Add powerup_fixup implementation Peng Fan
  2020-06-29  2:13 ` [PATCH 2/7] usb: ehci-mx6: Add i.MX8 OTG controller support Peng Fan
  2020-06-29  2:13 ` [PATCH 3/7] usb: ehci-mx6: Turn on the power domain of USB PHY Peng Fan
@ 2020-06-29  2:13 ` Peng Fan
  2020-06-29  2:30   ` Marek Vasut
  2020-06-29  2:13 ` [PATCH 5/7] usb: ehci-mx6: fix controller index for imx8m and imx8 Peng Fan
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Peng Fan @ 2020-06-29  2:13 UTC (permalink / raw)
  To: u-boot

From: Ye Li <ye.li@nxp.com>

Since the i.MX8MM reuses the otg controllers on i.MX7D. We can use
CONFIG_USB_EHCI_MX7 for them.

Due the TCPC and load switch are used on Typec circuit. Add the
board_usb_init and board_usb_cleanup to ehci-mx6 DM driver. So
we can implement the TCPC settings in these board functions.

Signed-off-by: Ye Li <ye.li@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/usb/host/Kconfig    |  2 +-
 drivers/usb/host/ehci-mx6.c | 31 ++++++++++++++++++++++++++-----
 2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index f48547caa0..1e2c5006d4 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -147,7 +147,7 @@ config USB_EHCI_MX6
 
 config USB_EHCI_MX7
 	bool "Support for i.MX7 on-chip EHCI USB controller"
-	depends on ARCH_MX7
+	depends on ARCH_MX7 || IMX8MM
 	default y
 	---help---
 	  Enables support for the on-chip EHCI controller on i.MX7 SoCs.
diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c
index 92e8bb91d2..046c6ab283 100644
--- a/drivers/usb/host/ehci-mx6.c
+++ b/drivers/usb/host/ehci-mx6.c
@@ -297,7 +297,7 @@ struct usbnc_regs {
 };
 #endif
 
-#elif defined(CONFIG_MX7)
+#elif defined(CONFIG_USB_EHCI_MX7)
 struct usbnc_regs {
 	u32 ctrl1;
 	u32 ctrl2;
@@ -366,7 +366,7 @@ static void usb_oc_config(int index)
 	struct usbnc_regs *usbnc = (struct usbnc_regs *)(USB_BASE_ADDR +
 			USB_OTHERREGS_OFFSET);
 	void __iomem *ctrl = (void __iomem *)(&usbnc->ctrl[index]);
-#elif defined(CONFIG_MX7) || defined(CONFIG_MX7ULP) || defined(CONFIG_IMX8)
+#elif defined(CONFIG_USB_EHCI_MX7) || defined(CONFIG_MX7ULP) || defined(CONFIG_IMX8)
 	struct usbnc_regs *usbnc = (struct usbnc_regs *)(ulong)(USB_BASE_ADDR +
 			(0x10000 * index) + USBNC_OFFSET);
 	void __iomem *ctrl = (void __iomem *)(&usbnc->ctrl1);
@@ -469,7 +469,7 @@ int ehci_hcd_init(int index, enum usb_init_type init,
 	enum usb_init_type type;
 #if defined(CONFIG_MX6)
 	u32 controller_spacing = 0x200;
-#elif defined(CONFIG_MX7) || defined(CONFIG_MX7ULP) || defined(CONFIG_IMX8)
+#elif defined(CONFIG_USB_EHCI_MX7) || defined(CONFIG_MX7ULP) || defined(CONFIG_IMX8)
 	u32 controller_spacing = 0x10000;
 #endif
 	struct usb_ehci *ehci = (struct usb_ehci *)(ulong)(USB_BASE_ADDR +
@@ -537,6 +537,12 @@ static int mx6_init_after_reset(struct ehci_ctrl *dev)
 	struct usb_ehci *ehci = priv->ehci;
 	int ret;
 
+	ret = board_usb_init(priv->portnr, priv->init_type);
+	if (ret) {
+		printf("Failed to initialize board for USB\n");
+		return ret;
+	}
+
 	ret = ehci_mx6_common_init(priv->ehci, priv->portnr);
 	if (ret)
 		return ret;
@@ -614,7 +620,7 @@ static int ehci_usb_phy_mode(struct udevice *dev)
 			plat->init_type = USB_INIT_DEVICE;
 		else
 			plat->init_type = USB_INIT_HOST;
-	} else if (is_mx7()) {
+	} else if (is_mx7() || is_imx8mm()) {
 		phy_status = (void __iomem *)(addr +
 					      USBNC_PHY_STATUS_OFFSET);
 		val = readl(phy_status);
@@ -711,6 +717,12 @@ static int ehci_usb_probe(struct udevice *dev)
 	priv->portnr = dev->seq;
 	priv->init_type = type;
 
+	ret = board_usb_init(priv->portnr, priv->init_type);
+	if (ret) {
+		printf("Failed to initialize board for USB\n");
+		return ret;
+	}
+
 #if CONFIG_IS_ENABLED(DM_REGULATOR)
 	ret = device_get_supply_regulator(dev, "vbus-supply",
 					  &priv->vbus_supply);
@@ -748,6 +760,15 @@ static int ehci_usb_probe(struct udevice *dev)
 	return ehci_register(dev, hccr, hcor, &mx6_ehci_ops, 0, priv->init_type);
 }
 
+int ehci_usb_remove(struct udevice *dev)
+{
+	struct ehci_mx6_priv_data *priv = dev_get_priv(dev);
+
+	ehci_deregister(dev);
+
+	return board_usb_cleanup(dev->seq, priv->init_type);
+}
+
 static const struct udevice_id mx6_usb_ids[] = {
 	{ .compatible = "fsl,imx27-usb" },
 	{ }
@@ -760,7 +781,7 @@ U_BOOT_DRIVER(usb_mx6) = {
 	.ofdata_to_platdata = ehci_usb_ofdata_to_platdata,
 	.bind	= ehci_usb_bind,
 	.probe	= ehci_usb_probe,
-	.remove = ehci_deregister,
+	.remove = ehci_usb_remove,
 	.ops	= &ehci_usb_ops,
 	.platdata_auto_alloc_size = sizeof(struct usb_platdata),
 	.priv_auto_alloc_size = sizeof(struct ehci_mx6_priv_data),
-- 
2.16.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 5/7] usb: ehci-mx6: fix controller index for imx8m and imx8
  2020-06-29  2:13 [PATCH 1/7] usb: ehci-mx6: Add powerup_fixup implementation Peng Fan
                   ` (2 preceding siblings ...)
  2020-06-29  2:13 ` [PATCH 4/7] usb: ehci-mx6: Update driver to support i.MX8MM Peng Fan
@ 2020-06-29  2:13 ` Peng Fan
  2020-06-29  2:34   ` Marek Vasut
  2020-06-29  2:13 ` [PATCH 6/7] usb: ehci-mx6: Enable iMX EHCI driver for iMX8M Nano Peng Fan
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Peng Fan @ 2020-06-29  2:13 UTC (permalink / raw)
  To: u-boot

From: Ye Li <ye.li@nxp.com>

The bind codes calculate the address offset for controller index, but
it is wrong for imx8m and imx8. It causes secondary controller fails to
work. So fix this problem, and also change all seq to req_seq.

There is another issue here for imx8, the codes use runtime CPU type check.
One iMX8, this check depends on IPC communication with SCFW, and need
MU driver been probed. But the usbotg is configured with "u-boot,dm_spl"
property for SDP download in SPL, so it will bind in early DM in u-boot
while the MU is not bound at same time, so the CPU type check will hang.
Fix it to use static check.

Signed-off-by: Ye Li <ye.li@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/usb/host/ehci-mx6.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c
index 046c6ab283..8090cb27fe 100644
--- a/drivers/usb/host/ehci-mx6.c
+++ b/drivers/usb/host/ehci-mx6.c
@@ -687,7 +687,11 @@ static int ehci_usb_bind(struct udevice *dev)
 	 * With these changes in place, the ad-hoc indexing goes away and
 	 * the driver is fully converted to DT probing.
 	 */
-	u32 controller_spacing = is_mx7() ? 0x10000 : 0x200;
+	u32 controller_spacing;
+	if (IS_ENABLED(CONFIG_MX6))
+		controller_spacing = 0x200;
+	else
+		controller_spacing = 0x10000;
 	fdt_addr_t addr = devfdt_get_addr_index(dev, 0);
 
 	dev->req_seq = (addr - USB_BASE_ADDR) / controller_spacing;
@@ -714,7 +718,7 @@ static int ehci_usb_probe(struct udevice *dev)
 	}
 
 	priv->ehci = ehci;
-	priv->portnr = dev->seq;
+	priv->portnr = dev->req_seq;
 	priv->init_type = type;
 
 	ret = board_usb_init(priv->portnr, priv->init_type);
@@ -766,7 +770,7 @@ int ehci_usb_remove(struct udevice *dev)
 
 	ehci_deregister(dev);
 
-	return board_usb_cleanup(dev->seq, priv->init_type);
+	return board_usb_cleanup(dev->req_seq, priv->init_type);
 }
 
 static const struct udevice_id mx6_usb_ids[] = {
-- 
2.16.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 6/7] usb: ehci-mx6: Enable iMX EHCI driver for iMX8M Nano
  2020-06-29  2:13 [PATCH 1/7] usb: ehci-mx6: Add powerup_fixup implementation Peng Fan
                   ` (3 preceding siblings ...)
  2020-06-29  2:13 ` [PATCH 5/7] usb: ehci-mx6: fix controller index for imx8m and imx8 Peng Fan
@ 2020-06-29  2:13 ` Peng Fan
  2020-06-29  2:13 ` [PATCH 7/7] usb: ehci-mx6: configure usb out of suspend state Peng Fan
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Peng Fan @ 2020-06-29  2:13 UTC (permalink / raw)
  To: u-boot

From: Ye Li <ye.li@nxp.com>

Add the IMX8MN to the EHCI-MX7 kconfig dependency.

Signed-off-by: Ye Li <ye.li@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/usb/host/Kconfig    | 2 +-
 drivers/usb/host/ehci-mx6.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 1e2c5006d4..36eb1138c4 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -147,7 +147,7 @@ config USB_EHCI_MX6
 
 config USB_EHCI_MX7
 	bool "Support for i.MX7 on-chip EHCI USB controller"
-	depends on ARCH_MX7 || IMX8MM
+	depends on ARCH_MX7 || IMX8MM || IMX8MN
 	default y
 	---help---
 	  Enables support for the on-chip EHCI controller on i.MX7 SoCs.
diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c
index 8090cb27fe..74f6c80d93 100644
--- a/drivers/usb/host/ehci-mx6.c
+++ b/drivers/usb/host/ehci-mx6.c
@@ -620,7 +620,7 @@ static int ehci_usb_phy_mode(struct udevice *dev)
 			plat->init_type = USB_INIT_DEVICE;
 		else
 			plat->init_type = USB_INIT_HOST;
-	} else if (is_mx7() || is_imx8mm()) {
+	} else if (is_mx7() || is_imx8mm() || is_imx8mn()) {
 		phy_status = (void __iomem *)(addr +
 					      USBNC_PHY_STATUS_OFFSET);
 		val = readl(phy_status);
-- 
2.16.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 7/7] usb: ehci-mx6: configure usb out of suspend state
  2020-06-29  2:13 [PATCH 1/7] usb: ehci-mx6: Add powerup_fixup implementation Peng Fan
                   ` (4 preceding siblings ...)
  2020-06-29  2:13 ` [PATCH 6/7] usb: ehci-mx6: Enable iMX EHCI driver for iMX8M Nano Peng Fan
@ 2020-06-29  2:13 ` Peng Fan
  2020-06-29  2:21 ` [PATCH 1/7] usb: ehci-mx6: Add powerup_fixup implementation Marek Vasut
  2020-08-24 10:20 ` Lukasz Majewski
  7 siblings, 0 replies; 15+ messages in thread
From: Peng Fan @ 2020-06-29  2:13 UTC (permalink / raw)
  To: u-boot

When moving to support partition reboot or android auto on XEN,
linux kernel will runs into runtime suspend state, and the usb
will be configured to low power suspend state by Linux.

Then we reboot and runs into U-Boot, however the usb already in
suspended state and uboot not able to lock the phy pll,
after clearing PHCD to out of suspended state, the phy pll could be
locked and fastboot works.

Suggested-by: Li Jun <jun.li@nxp.com>
Reviewed-by: Ye Li <ye.li@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/usb/host/ehci-mx6.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c
index 74f6c80d93..041b3da9ac 100644
--- a/drivers/usb/host/ehci-mx6.c
+++ b/drivers/usb/host/ehci-mx6.c
@@ -438,10 +438,17 @@ int __weak board_ehci_power(int port, int on)
 int ehci_mx6_common_init(struct usb_ehci *ehci, int index)
 {
 	int ret;
+	u32 portsc;
 
 	enable_usboh3_clk(1);
 	mdelay(1);
 
+	portsc = readl(&ehci->portsc);
+	if (portsc & PORT_PTS_PHCD) {
+		debug("suspended: portsc %x, enabled it.\n", portsc);
+		clrbits_le32(&ehci->portsc, PORT_PTS_PHCD);
+	}
+
 	/* Do board specific initialization */
 	ret = board_ehci_hcd_init(index);
 	if (ret)
-- 
2.16.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 1/7] usb: ehci-mx6: Add powerup_fixup implementation
  2020-06-29  2:13 [PATCH 1/7] usb: ehci-mx6: Add powerup_fixup implementation Peng Fan
                   ` (5 preceding siblings ...)
  2020-06-29  2:13 ` [PATCH 7/7] usb: ehci-mx6: configure usb out of suspend state Peng Fan
@ 2020-06-29  2:21 ` Marek Vasut
  2020-08-24 10:20 ` Lukasz Majewski
  7 siblings, 0 replies; 15+ messages in thread
From: Marek Vasut @ 2020-06-29  2:21 UTC (permalink / raw)
  To: u-boot

On 6/29/20 4:13 AM, Peng Fan wrote:

[...]

> +static void ehci_mx6_powerup_fixup(struct ehci_ctrl *ctrl, uint32_t *status_reg,
> +				   uint32_t *reg)
> +{
> +	u32 result;
> +	int usec = 2000;
> +
> +	mdelay(50);
> +
> +	do {
> +		result = ehci_readl(status_reg);
> +		udelay(5);
> +		if (!(result & EHCI_PS_PR))
> +			break;
> +		usec--;
> +	} while (usec > 0);
> +
> +	*reg = ehci_readl(status_reg);
> +}

Please use wait_for_bit*() . Also, is the 50 mS delay upfront required
or can this be wait_for_bit with 60000 uS timeout ?

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 2/7] usb: ehci-mx6: Add i.MX8 OTG controller support
  2020-06-29  2:13 ` [PATCH 2/7] usb: ehci-mx6: Add i.MX8 OTG controller support Peng Fan
@ 2020-06-29  2:26   ` Marek Vasut
  2020-06-29  8:24     ` Peng Fan
  0 siblings, 1 reply; 15+ messages in thread
From: Marek Vasut @ 2020-06-29  2:26 UTC (permalink / raw)
  To: u-boot

On 6/29/20 4:13 AM, Peng Fan wrote:

Hi,

> The i.MX8 has two USB controllers: USBOH and USB3. The USBOH reuses
> previous i.MX6/7. It has same PHY IP as i.MX7ULP but NC registers
> are same as i.MX7D. So add its support in ehci-mx6 driver.
> 
> Also the driver is updated to remove build warning for 64 bits CPU.

Please split the fixes into separate patch.

> --- a/drivers/usb/host/ehci-mx6.c
> +++ b/drivers/usb/host/ehci-mx6.c
> @@ -62,12 +62,20 @@ DECLARE_GLOBAL_DATA_PTR;
>  #define UCTRL_OVER_CUR_POL	(1 << 8) /* OTG Polarity of Overcurrent */
>  #define UCTRL_OVER_CUR_DIS	(1 << 7) /* Disable OTG Overcurrent Detection */
>  
> +#define PLL_USB_EN_USB_CLKS_MASK	(0x01 << 6)
> +#define PLL_USB_PWR_MASK		(0x01 << 12)
> +#define PLL_USB_ENABLE_MASK		(0x01 << 13)
> +#define PLL_USB_BYPASS_MASK		(0x01 << 16)
> +#define PLL_USB_REG_ENABLE_MASK		(0x01 << 21)
> +#define PLL_USB_DIV_SEL_MASK		(0x07 << 22)
> +#define PLL_USB_LOCK_MASK		(0x01 << 31)

Use BIT() macro

>  /* USBCMD */
>  #define UCMD_RUN_STOP           (1 << 0) /* controller run/stop */
>  #define UCMD_RESET		(1 << 1) /* controller reset */
>  
> -#if defined(CONFIG_MX6) || defined(CONFIG_MX7ULP)
> -static const unsigned phy_bases[] = {
> +#if defined(CONFIG_MX6) || defined(CONFIG_MX7ULP) || defined(CONFIG_IMX8)
> +static const ulong phy_bases[] = {
>  	USB_PHY0_BASE_ADDR,
>  #if defined(USB_PHY1_BASE_ADDR)
>  	USB_PHY1_BASE_ADDR,
> @@ -101,6 +109,53 @@ static void usb_power_config(int index)
>  
>  	scg_enable_usb_pll(true);
>  
> +#elif defined(CONFIG_IMX8)

This function should be split into multiple, it's too long, make it a
per-SoC function.

> +	struct usbphy_regs __iomem *usbphy =
> +		(struct usbphy_regs __iomem *)USB_PHY0_BASE_ADDR;
> +	int timeout = 1000000;
> +
> +	if (index > 0)
> +		return;
> +
> +	writel(ANADIG_USB2_CHRG_DETECT_EN_B |
> +		   ANADIG_USB2_CHRG_DETECT_CHK_CHRG_B,
> +		   &usbphy->usb1_chrg_detect);
> +
> +	if (!(readl(&usbphy->usb1_pll_480_ctrl) & PLL_USB_LOCK_MASK)) {
> +
> +		/* Enable the regulator first */
> +		writel(PLL_USB_REG_ENABLE_MASK,
> +		       &usbphy->usb1_pll_480_ctrl_set);
> +
> +		/* Wait at least 25us */
> +		udelay(25);
> +
> +		/* Enable the power */
> +		writel(PLL_USB_PWR_MASK, &usbphy->usb1_pll_480_ctrl_set);
> +
> +		/* Wait lock */
> +		while (timeout--) {
> +			if (readl(&usbphy->usb1_pll_480_ctrl) &
> +			    PLL_USB_LOCK_MASK)
> +				break;
> +			udelay(10);
> +		}

Use wait_for_bit*() here.

[...]

>  static void usb_power_config(int index)
>  {
> -	struct usbnc_regs *usbnc = (struct usbnc_regs *)(USB_BASE_ADDR +
> +	struct usbnc_regs *usbnc = (struct usbnc_regs *)(ulong)(USB_BASE_ADDR +

Is the extra type cast really needed ? If so, then it should be
uintptr_t so it would work with 64bit addresses.

[...]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 3/7] usb: ehci-mx6: Turn on the power domain of USB PHY
  2020-06-29  2:13 ` [PATCH 3/7] usb: ehci-mx6: Turn on the power domain of USB PHY Peng Fan
@ 2020-06-29  2:27   ` Marek Vasut
  0 siblings, 0 replies; 15+ messages in thread
From: Marek Vasut @ 2020-06-29  2:27 UTC (permalink / raw)
  To: u-boot

On 6/29/20 4:13 AM, Peng Fan wrote:
[...]
> @@ -23,6 +25,9 @@
>  #include <linux/usb/otg.h>
>  
>  #include "ehci.h"
> +#if CONFIG_IS_ENABLED(POWER_DOMAIN)
> +#include <power-domain.h>
> +#endif

The ifdef here should not be needed.

>  DECLARE_GLOBAL_DATA_PTR;
>  
> @@ -590,6 +595,18 @@ static int ehci_usb_phy_mode(struct udevice *dev)
>  		if ((fdt_addr_t)addr == FDT_ADDR_T_NONE)
>  			return -EINVAL;
>  
> +		/* Need to power on the PHY before access it */
> +#if CONFIG_IS_ENABLED(POWER_DOMAIN)
> +		struct udevice phy_dev;
> +		struct power_domain pd;
> +
> +		phy_dev.node = offset_to_ofnode(phy_off);
> +		if (!power_domain_get(&phy_dev, &pd)) {
> +			if (power_domain_on(&pd))
> +				return -EINVAL;

Please return the return value of power_domain_on() in case of failure.

[...]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 4/7] usb: ehci-mx6: Update driver to support i.MX8MM
  2020-06-29  2:13 ` [PATCH 4/7] usb: ehci-mx6: Update driver to support i.MX8MM Peng Fan
@ 2020-06-29  2:30   ` Marek Vasut
  0 siblings, 0 replies; 15+ messages in thread
From: Marek Vasut @ 2020-06-29  2:30 UTC (permalink / raw)
  To: u-boot

On 6/29/20 4:13 AM, Peng Fan wrote:

[...]

> @@ -366,7 +366,7 @@ static void usb_oc_config(int index)
>  	struct usbnc_regs *usbnc = (struct usbnc_regs *)(USB_BASE_ADDR +
>  			USB_OTHERREGS_OFFSET);
>  	void __iomem *ctrl = (void __iomem *)(&usbnc->ctrl[index]);
> -#elif defined(CONFIG_MX7) || defined(CONFIG_MX7ULP) || defined(CONFIG_IMX8)
> +#elif defined(CONFIG_USB_EHCI_MX7) || defined(CONFIG_MX7ULP) || defined(CONFIG_IMX8)

Can someone please clean up this growing ifdeffery ? This doesn't work
with the driver model. It's fine to do that in a subsequent patch, but
it really should be done soon.

>  	struct usbnc_regs *usbnc = (struct usbnc_regs *)(ulong)(USB_BASE_ADDR +
>  			(0x10000 * index) + USBNC_OFFSET);
>  	void __iomem *ctrl = (void __iomem *)(&usbnc->ctrl1);
> @@ -469,7 +469,7 @@ int ehci_hcd_init(int index, enum usb_init_type init,
>  	enum usb_init_type type;
>  #if defined(CONFIG_MX6)
>  	u32 controller_spacing = 0x200;
> -#elif defined(CONFIG_MX7) || defined(CONFIG_MX7ULP) || defined(CONFIG_IMX8)
> +#elif defined(CONFIG_USB_EHCI_MX7) || defined(CONFIG_MX7ULP) || defined(CONFIG_IMX8)
>  	u32 controller_spacing = 0x10000;
>  #endif
>  	struct usb_ehci *ehci = (struct usb_ehci *)(ulong)(USB_BASE_ADDR +
> @@ -537,6 +537,12 @@ static int mx6_init_after_reset(struct ehci_ctrl *dev)
>  	struct usb_ehci *ehci = priv->ehci;
>  	int ret;
>  
> +	ret = board_usb_init(priv->portnr, priv->init_type);
> +	if (ret) {
> +		printf("Failed to initialize board for USB\n");
> +		return ret;
> +	}

Are these hooks needed ?

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 5/7] usb: ehci-mx6: fix controller index for imx8m and imx8
  2020-06-29  2:13 ` [PATCH 5/7] usb: ehci-mx6: fix controller index for imx8m and imx8 Peng Fan
@ 2020-06-29  2:34   ` Marek Vasut
  0 siblings, 0 replies; 15+ messages in thread
From: Marek Vasut @ 2020-06-29  2:34 UTC (permalink / raw)
  To: u-boot

On 6/29/20 4:13 AM, Peng Fan wrote:
[...]

> diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c
> index 046c6ab283..8090cb27fe 100644
> --- a/drivers/usb/host/ehci-mx6.c
> +++ b/drivers/usb/host/ehci-mx6.c
> @@ -687,7 +687,11 @@ static int ehci_usb_bind(struct udevice *dev)
>  	 * With these changes in place, the ad-hoc indexing goes away and
>  	 * the driver is fully converted to DT probing.
>  	 */
> -	u32 controller_spacing = is_mx7() ? 0x10000 : 0x200;
> +	u32 controller_spacing;
> +	if (IS_ENABLED(CONFIG_MX6))
> +		controller_spacing = 0x200;
> +	else
> +		controller_spacing = 0x10000;

This looks related to this problem:
501547cec1 ("usb: ehci-mx6: Fix bus enumeration for DM case")
Maybe instead of adding to the problem, it would make sense to convert
the driver to DM fully and then this problem with figuring out offsets
would go away ?

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 2/7] usb: ehci-mx6: Add i.MX8 OTG controller support
  2020-06-29  2:26   ` Marek Vasut
@ 2020-06-29  8:24     ` Peng Fan
  2020-06-29  8:50       ` Marek Vasut
  0 siblings, 1 reply; 15+ messages in thread
From: Peng Fan @ 2020-06-29  8:24 UTC (permalink / raw)
  To: u-boot

> Subject: Re: [PATCH 2/7] usb: ehci-mx6: Add i.MX8 OTG controller support
> 
> On 6/29/20 4:13 AM, Peng Fan wrote:
> 
> Hi,
> 
> > The i.MX8 has two USB controllers: USBOH and USB3. The USBOH reuses
> > previous i.MX6/7. It has same PHY IP as i.MX7ULP but NC registers are
> > same as i.MX7D. So add its support in ehci-mx6 driver.
> >
> > Also the driver is updated to remove build warning for 64 bits CPU.
> 
> Please split the fixes into separate patch.

Sorry for not be clear, but the driver was only built for ARM32 i.MX.
It is after we start supporting i.MX8, there are some warnings, which
was introduced by adding i.MX8 support. It should stay in same patch.

> 
> > --- a/drivers/usb/host/ehci-mx6.c
> > +++ b/drivers/usb/host/ehci-mx6.c
> > @@ -62,12 +62,20 @@ DECLARE_GLOBAL_DATA_PTR;
> >  #define UCTRL_OVER_CUR_POL	(1 << 8) /* OTG Polarity of Overcurrent
> */
> >  #define UCTRL_OVER_CUR_DIS	(1 << 7) /* Disable OTG Overcurrent
> Detection */
> >
> > +#define PLL_USB_EN_USB_CLKS_MASK	(0x01 << 6)
> > +#define PLL_USB_PWR_MASK		(0x01 << 12)
> > +#define PLL_USB_ENABLE_MASK		(0x01 << 13)
> > +#define PLL_USB_BYPASS_MASK		(0x01 << 16)
> > +#define PLL_USB_REG_ENABLE_MASK		(0x01 << 21)
> > +#define PLL_USB_DIV_SEL_MASK		(0x07 << 22)
> > +#define PLL_USB_LOCK_MASK		(0x01 << 31)
> 
> Use BIT() macro

Yes.

> 
> >  /* USBCMD */
> >  #define UCMD_RUN_STOP           (1 << 0) /* controller run/stop */
> >  #define UCMD_RESET		(1 << 1) /* controller reset */
> >
> > -#if defined(CONFIG_MX6) || defined(CONFIG_MX7ULP) -static const
> > unsigned phy_bases[] = {
> > +#if defined(CONFIG_MX6) || defined(CONFIG_MX7ULP) ||
> > +defined(CONFIG_IMX8) static const ulong phy_bases[] = {
> >  	USB_PHY0_BASE_ADDR,
> >  #if defined(USB_PHY1_BASE_ADDR)
> >  	USB_PHY1_BASE_ADDR,
> > @@ -101,6 +109,53 @@ static void usb_power_config(int index)
> >
> >  	scg_enable_usb_pll(true);
> >
> > +#elif defined(CONFIG_IMX8)
> 
> This function should be split into multiple, it's too long, make it a per-SoC
> function.

Ok.

> 
> > +	struct usbphy_regs __iomem *usbphy =
> > +		(struct usbphy_regs __iomem *)USB_PHY0_BASE_ADDR;
> > +	int timeout = 1000000;
> > +
> > +	if (index > 0)
> > +		return;
> > +
> > +	writel(ANADIG_USB2_CHRG_DETECT_EN_B |
> > +		   ANADIG_USB2_CHRG_DETECT_CHK_CHRG_B,
> > +		   &usbphy->usb1_chrg_detect);
> > +
> > +	if (!(readl(&usbphy->usb1_pll_480_ctrl) & PLL_USB_LOCK_MASK)) {
> > +
> > +		/* Enable the regulator first */
> > +		writel(PLL_USB_REG_ENABLE_MASK,
> > +		       &usbphy->usb1_pll_480_ctrl_set);
> > +
> > +		/* Wait at least 25us */
> > +		udelay(25);
> > +
> > +		/* Enable the power */
> > +		writel(PLL_USB_PWR_MASK, &usbphy->usb1_pll_480_ctrl_set);
> > +
> > +		/* Wait lock */
> > +		while (timeout--) {
> > +			if (readl(&usbphy->usb1_pll_480_ctrl) &
> > +			    PLL_USB_LOCK_MASK)
> > +				break;
> > +			udelay(10);
> > +		}
> 
> Use wait_for_bit*() here.

ok.

> 
> [...]
> 
> >  static void usb_power_config(int index)  {
> > -	struct usbnc_regs *usbnc = (struct usbnc_regs *)(USB_BASE_ADDR +
> > +	struct usbnc_regs *usbnc = (struct usbnc_regs
> > +*)(ulong)(USB_BASE_ADDR +
> 
> Is the extra type cast really needed ? If so, then it should be uintptr_t so it
> would work with 64bit addresses.

Will use uintptr_t. there is warning if not.

"warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]"

Thanks,
Peng.
>  
> [...]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 2/7] usb: ehci-mx6: Add i.MX8 OTG controller support
  2020-06-29  8:24     ` Peng Fan
@ 2020-06-29  8:50       ` Marek Vasut
  0 siblings, 0 replies; 15+ messages in thread
From: Marek Vasut @ 2020-06-29  8:50 UTC (permalink / raw)
  To: u-boot

On 6/29/20 10:24 AM, Peng Fan wrote:

[...]

>>> The i.MX8 has two USB controllers: USBOH and USB3. The USBOH reuses
>>> previous i.MX6/7. It has same PHY IP as i.MX7ULP but NC registers are
>>> same as i.MX7D. So add its support in ehci-mx6 driver.
>>>
>>> Also the driver is updated to remove build warning for 64 bits CPU.
>>
>> Please split the fixes into separate patch.
> 
> Sorry for not be clear, but the driver was only built for ARM32 i.MX.
> It is after we start supporting i.MX8, there are some warnings, which
> was introduced by adding i.MX8 support. It should stay in same patch.

I understand that, but the 64bit fixes can be pulled into separate patch
to make it easier to review just the OTG support without having the
fixes mixed in the same patch.

[...]

>>>  static void usb_power_config(int index)  {
>>> -	struct usbnc_regs *usbnc = (struct usbnc_regs *)(USB_BASE_ADDR +
>>> +	struct usbnc_regs *usbnc = (struct usbnc_regs
>>> +*)(ulong)(USB_BASE_ADDR +
>>
>> Is the extra type cast really needed ? If so, then it should be uintptr_t so it
>> would work with 64bit addresses.
> 
> Will use uintptr_t. there is warning if not.
> 
> "warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]"

[...]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/7] usb: ehci-mx6: Add powerup_fixup implementation
  2020-06-29  2:13 [PATCH 1/7] usb: ehci-mx6: Add powerup_fixup implementation Peng Fan
                   ` (6 preceding siblings ...)
  2020-06-29  2:21 ` [PATCH 1/7] usb: ehci-mx6: Add powerup_fixup implementation Marek Vasut
@ 2020-08-24 10:20 ` Lukasz Majewski
  7 siblings, 0 replies; 15+ messages in thread
From: Lukasz Majewski @ 2020-08-24 10:20 UTC (permalink / raw)
  To: u-boot

Hi Peng, Marek

> From: Ye Li <ye.li@nxp.com>
> 
> When doing port reset, the PR bit of PORTSC1 will be automatically
> cleared by our IP, but standard EHCI needs explicit clear by
> software. The EHCI-HCD driver follow the EHCI specification, so after
> 50ms wait, it clear the PR bit by writting to the PORTSC1 register
> with value loaded before setting PR.
> 
> This sequence is ok for our IP when the delay time is exact. But when
> the timer is slower, some bits like PE, PSPD have been set by
> controller automatically after the PR is automatically cleared. So
> the writing to the PORTSC1 will overwrite these bits set by
> controller. And eventually the driver gets wrong status.
> 
> We implement the powerup_fixup operation which delays 50ms and will
> check the PR until it is cleared by controller. And will update the
> reg value which is written to PORTSC register by EHCI-HCD driver.
> This is much safer than depending on the delay time to be accurate
> and aligining with controller's behaiver.

Is there any plan for a follow up for this patch set?

On my imx6q - kp_tpc70 board I can observe that there are some issues
when trying to read data from USB [*]:

Booting update from usb ...
starting USB...
Bus usb at 2184200: USB EHCI 1.00
scanning bus usb at 2184200 for devices... 2 USB Device(s) found
       scanning usb for storage devices... 1 Storage Device(s) found
EHCI timed out on TD - token=0x1f8c80
EHCI timed out on TD - token=0x1f8c80


Peng do you see similar issue when you test it on your iMX8?

The test is very simple - just put on a pendrive a fitImage and
initramfs and try to boot with it. In my case it will hang at least
once per ten attempts.

Note:

[*] - this is a similar issue to one, which I had on the iMX53
controller. However, Marek's patch fixed it on imx53:

"usb: Keep async schedule running only across mass storage xfers"
SHA1: 31232de07ef2bd97ff67625976eecd97eeb1b



> 
> Signed-off-by: Ye Li <ye.li@nxp.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/usb/host/ehci-mx6.c | 28 +++++++++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c
> index 5f84c7b91d..e654595683 100644
> --- a/drivers/usb/host/ehci-mx6.c
> +++ b/drivers/usb/host/ehci-mx6.c
> @@ -267,6 +267,25 @@ int usb_phy_mode(int port)
>  }
>  #endif
>  
> +static void ehci_mx6_powerup_fixup(struct ehci_ctrl *ctrl, uint32_t
> *status_reg,
> +				   uint32_t *reg)
> +{
> +	u32 result;
> +	int usec = 2000;
> +
> +	mdelay(50);
> +
> +	do {
> +		result = ehci_readl(status_reg);
> +		udelay(5);
> +		if (!(result & EHCI_PS_PR))
> +			break;
> +		usec--;
> +	} while (usec > 0);
> +
> +	*reg = ehci_readl(status_reg);
> +}
> +
>  static void usb_oc_config(int index)
>  {
>  #if defined(CONFIG_MX6)
> @@ -366,6 +385,10 @@ int ehci_mx6_common_init(struct usb_ehci *ehci,
> int index) }
>  
>  #if !CONFIG_IS_ENABLED(DM_USB)
> +static const struct ehci_ops mx6_ehci_ops = {
> +	.powerup_fixup		= ehci_mx6_powerup_fixup,
> +};
> +
>  int ehci_hcd_init(int index, enum usb_init_type init,
>  		struct ehci_hccr **hccr, struct ehci_hcor **hcor)
>  {
> @@ -394,6 +417,8 @@ int ehci_hcd_init(int index, enum usb_init_type
> init, if (ret)
>  		return ret;
>  
> +	ehci_set_controller_priv(index, NULL, &mx6_ehci_ops);
> +
>  	type = board_usb_phy_mode(index);
>  
>  	if (hccr && hcor) {
> @@ -467,7 +492,8 @@ static int mx6_init_after_reset(struct ehci_ctrl
> *dev) }
>  
>  static const struct ehci_ops mx6_ehci_ops = {
> -	.init_after_reset = mx6_init_after_reset
> +	.powerup_fixup		= ehci_mx6_powerup_fixup,
> +	.init_after_reset	= mx6_init_after_reset
>  };
>  
>  static int ehci_usb_phy_mode(struct udevice *dev)




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200824/822a56aa/attachment.sig>

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2020-08-24 10:20 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-29  2:13 [PATCH 1/7] usb: ehci-mx6: Add powerup_fixup implementation Peng Fan
2020-06-29  2:13 ` [PATCH 2/7] usb: ehci-mx6: Add i.MX8 OTG controller support Peng Fan
2020-06-29  2:26   ` Marek Vasut
2020-06-29  8:24     ` Peng Fan
2020-06-29  8:50       ` Marek Vasut
2020-06-29  2:13 ` [PATCH 3/7] usb: ehci-mx6: Turn on the power domain of USB PHY Peng Fan
2020-06-29  2:27   ` Marek Vasut
2020-06-29  2:13 ` [PATCH 4/7] usb: ehci-mx6: Update driver to support i.MX8MM Peng Fan
2020-06-29  2:30   ` Marek Vasut
2020-06-29  2:13 ` [PATCH 5/7] usb: ehci-mx6: fix controller index for imx8m and imx8 Peng Fan
2020-06-29  2:34   ` Marek Vasut
2020-06-29  2:13 ` [PATCH 6/7] usb: ehci-mx6: Enable iMX EHCI driver for iMX8M Nano Peng Fan
2020-06-29  2:13 ` [PATCH 7/7] usb: ehci-mx6: configure usb out of suspend state Peng Fan
2020-06-29  2:21 ` [PATCH 1/7] usb: ehci-mx6: Add powerup_fixup implementation Marek Vasut
2020-08-24 10:20 ` Lukasz Majewski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.