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

[Marek Vasut]

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.

[...]