diff mbox series

[2/7] usb: ehci-mx6: Add i.MX8 OTG controller support

Message ID 20200629021350.21262-2-peng.fan@nxp.com
State New
Delegated to: Stefano Babic
Headers show
Series [1/7] usb: ehci-mx6: Add powerup_fixup implementation | expand

Commit Message

Peng Fan June 29, 2020, 2:13 a.m. UTC
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(-)

Comments

Marek Vasut June 29, 2020, 2:26 a.m. UTC | #1
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.

[...]
Peng Fan June 29, 2020, 8:24 a.m. UTC | #2
> 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.
>  
> [...]
Marek Vasut June 29, 2020, 8:50 a.m. UTC | #3
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]"

[...]
diff mbox series

Patch

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