[U-Boot] 答复: [PATCH 2/3] USB: host: Add the USB3 host driver

[Sherry Sun <sherry.sun@nxp.com>]

Hi, Marek

I'm so sorry to check the e-mail and reply you so late.

> On 7/16/19 2:08 PM, sherry sun wrote:
> > From: Sherry Sun <sherry.sun@nxp.com>
> >
> > This driver is ported from NXP i.MX U-Boot version imx_v2019.04 and
> > some changes have also been made to adapt to U-Boot.
> 
> I don't this this line above is particularly useful.

Ok, I will remove this from the commit messages.

> 
> > Add the USB3 host driver for NXP imx8 platform, and the cadence IP is
> > in it.
> >
> > The USB3 host driver support DM mode. It will probe USB3 host node,
> > enable the power and clk of both USB3 controller and USB3 PHY.
> >
> > Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> > ---
> >  drivers/usb/host/Kconfig        |   9 +
> >  drivers/usb/host/Makefile       |   1 +
> >  drivers/usb/host/xhci-imx8.c    | 311 ++++++++++++++++++++++
> >  include/usb/imx8_usb3_reg_def.h | 455
> > ++++++++++++++++++++++++++++++++
> >  4 files changed, 776 insertions(+)
> >  create mode 100644 drivers/usb/host/xhci-imx8.c  create mode 100644
> > include/usb/imx8_usb3_reg_def.h
> >
> > diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index
> > b1188bcbf5..73c98e0cf4 100644
> > --- a/drivers/usb/host/Kconfig
> > +++ b/drivers/usb/host/Kconfig
> > @@ -94,6 +94,15 @@ config USB_XHCI_FSL
> >  	depends on !SPL_NO_USB
> >  	help
> >  	  Enables support for the on-chip xHCI controller on NXP Layerscape
> SoCs.
> > +
> > +config USB_XHCI_IMX8
> > +	bool "XHCI support for imx8"
> > +	depends on ARCH_IMX8
> > +	default y
> > +	help
> > +	  Enables support for the on-chip xHCI controller on imx8qm and
> > +	  imx8qxp SoCs.
> > +
> >  endif # USB_XHCI_HCD
> >
> >  config USB_EHCI_HCD
> > diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
> > index 6aa574f6f7..e5a0a4ea5a 100644
> > --- a/drivers/usb/host/Makefile
> > +++ b/drivers/usb/host/Makefile
> > @@ -55,6 +55,7 @@ obj-$(CONFIG_USB_XHCI_OMAP) += xhci-omap.o
> >  obj-$(CONFIG_USB_XHCI_PCI) += xhci-pci.o
> >  obj-$(CONFIG_USB_XHCI_RCAR) += xhci-rcar.o
> >  obj-$(CONFIG_USB_XHCI_STI) += dwc3-sti-glue.o
> > +obj-$(CONFIG_USB_XHCI_IMX8) += xhci-imx8.o
> >
> >  # designware
> >  obj-$(CONFIG_USB_DWC2) += dwc2.o
> > diff --git a/drivers/usb/host/xhci-imx8.c
> > b/drivers/usb/host/xhci-imx8.c new file mode 100644 index
> > 0000000000..abac37d446
> > --- /dev/null
> > +++ b/drivers/usb/host/xhci-imx8.c
> > @@ -0,0 +1,311 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2019 NXP
> > + *
> > + * NXP i.MX8 USB HOST xHCI Controller (Cadence IP)
> > + *
> > + * Author: Peter Chen <peter.chen@nxp.com>  */
> > +
> > +#include <common.h>
> > +#include <usb.h>
> > +#include <malloc.h>
> > +#include <usb/imx8_usb3_reg_def.h>
> > +#include <dm.h>
> > +#include <clk.h>
> > +#include <power-domain.h>
> > +#include <asm/arch/clock.h>
> > +#include <linux/compat.h>
> > +#include "xhci.h"
> > +
> > +/* Declare global data pointer */
> > +DECLARE_GLOBAL_DATA_PTR;
> 
> Is this needed ?
> 

Yes, the global_data pointer will be used in later functions.
So the declare is needed here.

> > +
> > +/* According to UG CH 3.1.1 Bring-up Sequence */ static void
> > +imx_usb3_phy_init(void) {
> 
> Shouldn't this be a separate PHY driver ?

Okay, I will try to add a new PHY driver to replace the code below.

> 
> > +	writel(0x0830, PHY_PMA_CMN_CTRL1);
> > +	writel(0x10, TB_ADDR_CMN_DIAG_HSCLK_SEL);
> > +	writel(0x00F0, TB_ADDR_CMN_PLL0_VCOCAL_INIT_TMR);
> > +	writel(0x0018, TB_ADDR_CMN_PLL0_VCOCAL_ITER_TMR);
> > +	writel(0x00D0, TB_ADDR_CMN_PLL0_INTDIV);
> > +	writel(0x4aaa, TB_ADDR_CMN_PLL0_FRACDIV);
> > +	writel(0x0034, TB_ADDR_CMN_PLL0_HIGH_THR);
> > +	writel(0x1ee, TB_ADDR_CMN_PLL0_SS_CTRL1);
> > +	writel(0x7F03, TB_ADDR_CMN_PLL0_SS_CTRL2);
> > +	writel(0x0020, TB_ADDR_CMN_PLL0_DSM_DIAG);
> > +	writel(0x0000, TB_ADDR_CMN_DIAG_PLL0_OVRD);
> > +	writel(0x0000, TB_ADDR_CMN_DIAG_PLL0_FBH_OVRD);
> > +	writel(0x0000, TB_ADDR_CMN_DIAG_PLL0_FBL_OVRD);
> > +	writel(0x0007, TB_ADDR_CMN_DIAG_PLL0_V2I_TUNE);
> > +	writel(0x0027, TB_ADDR_CMN_DIAG_PLL0_CP_TUNE);
> > +	writel(0x0008, TB_ADDR_CMN_DIAG_PLL0_LF_PROG);
> > +	writel(0x0022, TB_ADDR_CMN_DIAG_PLL0_TEST_MODE);
> > +	writel(0x000a, TB_ADDR_CMN_PSM_CLK_CTRL);
> > +	writel(0x139, TB_ADDR_XCVR_DIAG_RX_LANE_CAL_RST_TMR);
> > +	writel(0xbefc, TB_ADDR_XCVR_PSM_RCTRL);
> > +
> > +	writel(0x7799, TB_ADDR_TX_PSC_A0);
> > +	writel(0x7798, TB_ADDR_TX_PSC_A1);
> > +	writel(0x509b, TB_ADDR_TX_PSC_A2);
> > +	writel(0x3, TB_ADDR_TX_DIAG_ECTRL_OVRD);
> > +	writel(0x5098, TB_ADDR_TX_PSC_A3);
> > +	writel(0x2090, TB_ADDR_TX_PSC_CAL);
> > +	writel(0x2090, TB_ADDR_TX_PSC_RDY);
> > +
> > +	writel(0xA6FD, TB_ADDR_RX_PSC_A0);
> > +	writel(0xA6FD, TB_ADDR_RX_PSC_A1);
> > +	writel(0xA410, TB_ADDR_RX_PSC_A2);
> > +	writel(0x2410, TB_ADDR_RX_PSC_A3);
> > +
> > +	writel(0x23FF, TB_ADDR_RX_PSC_CAL);
> > +	writel(0x2010, TB_ADDR_RX_PSC_RDY);
> > +
> > +	writel(0x0020, TB_ADDR_TX_TXCC_MGNLS_MULT_000);
> > +	writel(0x00ff, TB_ADDR_TX_DIAG_BGREF_PREDRV_DELAY);
> > +	writel(0x0002, TB_ADDR_RX_SLC_CU_ITER_TMR);
> > +	writel(0x0013, TB_ADDR_RX_SIGDET_HL_FILT_TMR);
> > +	writel(0x0000, TB_ADDR_RX_SAMP_DAC_CTRL);
> > +	writel(0x1004, TB_ADDR_RX_DIAG_SIGDET_TUNE);
> > +	writel(0x4041, TB_ADDR_RX_DIAG_LFPSDET_TUNE2);
> > +	writel(0x0480, TB_ADDR_RX_DIAG_BS_TM);
> > +	writel(0x8006, TB_ADDR_RX_DIAG_DFE_CTRL1);
> > +	writel(0x003f, TB_ADDR_RX_DIAG_ILL_IQE_TRIM4);
> > +	writel(0x543f, TB_ADDR_RX_DIAG_ILL_E_TRIM0);
> > +	writel(0x543f, TB_ADDR_RX_DIAG_ILL_IQ_TRIM0);
> > +	writel(0x0000, TB_ADDR_RX_DIAG_ILL_IQE_TRIM6);
> > +	writel(0x8000, TB_ADDR_RX_DIAG_RXFE_TM3);
> > +	writel(0x0003, TB_ADDR_RX_DIAG_RXFE_TM4);
> > +	writel(0x2408, TB_ADDR_RX_DIAG_LFPSDET_TUNE);
> > +	writel(0x05ca, TB_ADDR_RX_DIAG_DFE_CTRL3);
> > +	writel(0x0258, TB_ADDR_RX_DIAG_SC2C_DELAY);
> > +	writel(0x1fff, TB_ADDR_RX_REE_VGA_GAIN_NODFE);
> > +
> > +	writel(0x02c6, TB_ADDR_XCVR_PSM_CAL_TMR);
> > +	writel(0x0002, TB_ADDR_XCVR_PSM_A0BYP_TMR);
> > +	writel(0x02c6, TB_ADDR_XCVR_PSM_A0IN_TMR);
> > +	writel(0x0010, TB_ADDR_XCVR_PSM_A1IN_TMR);
> > +	writel(0x0010, TB_ADDR_XCVR_PSM_A2IN_TMR);
> > +	writel(0x0010, TB_ADDR_XCVR_PSM_A3IN_TMR);
> > +	writel(0x0010, TB_ADDR_XCVR_PSM_A4IN_TMR);
> > +	writel(0x0010, TB_ADDR_XCVR_PSM_A5IN_TMR);
> > +
> > +	writel(0x0002, TB_ADDR_XCVR_PSM_A0OUT_TMR);
> > +	writel(0x0002, TB_ADDR_XCVR_PSM_A1OUT_TMR);
> > +	writel(0x0002, TB_ADDR_XCVR_PSM_A2OUT_TMR);
> > +	writel(0x0002, TB_ADDR_XCVR_PSM_A3OUT_TMR);
> > +	writel(0x0002, TB_ADDR_XCVR_PSM_A4OUT_TMR);
> > +	writel(0x0002, TB_ADDR_XCVR_PSM_A5OUT_TMR);
> > +
> > +	/* Change rx detect parameter */
> > +	writel(0x960, TB_ADDR_TX_RCVDET_EN_TMR);
> > +	writel(0x01e0, TB_ADDR_TX_RCVDET_ST_TMR);
> > +	writel(0x0090, TB_ADDR_XCVR_DIAG_LANE_FCM_EN_MGN_TMR);
> > +
> > +	udelay(10);
> > +
> > +	/* force rx detect */
> > +/*	writel(0xc000, TB_ADDR_TX_RCVDET_OVRD); */
> > +}
> > +
> > +void imx8_xhci_init(void)
> > +{
> > +	u32 tmp_data;
> > +
> > +	tmp_data = readl(USB3_SSPHY_STATUS);
> > +	writel(tmp_data, USB3_SSPHY_STATUS);
> > +	tmp_data = readl(USB3_SSPHY_STATUS);
> > +	while ((tmp_data & 0xf0000000) != 0xf0000000) {
> > +		printf("clkvld is incorrect = 0x%x\n", tmp_data);
> > +		udelay(10);
> > +		tmp_data = readl(USB3_SSPHY_STATUS);
> > +	}
> > +
> > +	tmp_data = readl(USB3_CORE_CTRL1);
> > +	tmp_data = (tmp_data & 0xfffffff8) | 0x202;
> > +	writel(tmp_data, USB3_CORE_CTRL1);
> > +	tmp_data &= ~0x04000000; /* clear PHY apb reset */
> > +	writel(tmp_data, USB3_CORE_CTRL1);
> > +	imx_usb3_phy_init();
> > +
> > +	tmp_data = readl(USB3_CORE_CTRL1);
> > +	tmp_data &= ~0xfc000000; /* clear all sw_rst */
> > +	writel(tmp_data, USB3_CORE_CTRL1);
> > +
> 
> clrbits_le32() , fix globally.

I will do it, thanks.

> 
> [...]
> 
> > diff --git a/include/usb/imx8_usb3_reg_def.h
> > b/include/usb/imx8_usb3_reg_def.h new file mode 100644 index
> > 0000000000..dfa16e47e3
> > --- /dev/null
> > +++ b/include/usb/imx8_usb3_reg_def.h
> > @@ -0,0 +1,455 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * Copyright 2019 NXP
> > + *
> > + * NXP i.MX8 USB HOST xHCI Controller (Cadence IP) Register
> > +Definition  */
> > +
> > +#ifndef _IMX8_USB3_H_
> > +#define _IMX8_USB3_H_
> > +
> > +#define REG32(m)	(m)
> 
> Remove this macro.

I will do it, thanks.

> 
> > +#define USB3_REGS_BASE	0x5B110000
> 
> This should come from DT

Ok,I will change this, thanks.

> 
> > +#define USB3_CTRL_BASE (USB3_REGS_BASE + 0x00000) #define
> > +USB3_CORE_BASE (USB3_REGS_BASE + 0x10000) #define USB3_PHY_BASE
> > +(USB3_REGS_BASE + 0x50000)
> > +/* PHY registers */
> > +#define PHY_PMA_CMN_CTRL1			(USB3_PHY_BASE + 0xC800 *
> 4)
> > +#define TB_ADDR_CMN_DIAG_HSCLK_SEL		(USB3_PHY_BASE +
> 0x01e0 * 4)
> > +#define TB_ADDR_CMN_PLL0_VCOCAL_INIT_TMR	(USB3_PHY_BASE +
> 0x0084 * 4)
> > +#define TB_ADDR_CMN_PLL0_VCOCAL_ITER_TMR	(USB3_PHY_BASE +
> 0x0085 * 4)
> > +#define TB_ADDR_CMN_PLL0_INTDIV
> (USB3_PHY_BASE + 0x0094 * 4)
> > +#define TB_ADDR_CMN_PLL0_FRACDIV		(USB3_PHY_BASE + 0x0095 *
> 4)
> > +#define TB_ADDR_CMN_PLL0_HIGH_THR		(USB3_PHY_BASE + 0x0096 *
> 4)
> > +#define TB_ADDR_CMN_PLL0_SS_CTRL1		(USB3_PHY_BASE + 0x0098 *
> 4)
> > +#define TB_ADDR_CMN_PLL0_SS_CTRL2		(USB3_PHY_BASE + 0x0099 *
> 4)
> > +#define TB_ADDR_CMN_PLL0_DSM_DIAG		(USB3_PHY_BASE +
> 0x0097 * 4)
> > +#define TB_ADDR_CMN_DIAG_PLL0_OVRD		(USB3_PHY_BASE +
> 0x01c2 * 4)
> > +#define TB_ADDR_CMN_DIAG_PLL0_FBH_OVRD		(USB3_PHY_BASE +
> 0x01c0 * 4)
> > +#define TB_ADDR_CMN_DIAG_PLL0_FBL_OVRD		(USB3_PHY_BASE +
> 0x01c1 * 4)
> > +#define TB_ADDR_CMN_DIAG_PLL0_V2I_TUNE          (USB3_PHY_BASE
> + 0x01C5 * 4)
> > +#define TB_ADDR_CMN_DIAG_PLL0_CP_TUNE
> (USB3_PHY_BASE + 0x01C6 * 4)
> > +#define TB_ADDR_CMN_DIAG_PLL0_LF_PROG
> (USB3_PHY_BASE + 0x01C7 * 4)
> > +#define TB_ADDR_CMN_DIAG_PLL0_TEST_MODE		(USB3_PHY_BASE +
> 0x01c4 * 4)
> > +#define TB_ADDR_CMN_PSM_CLK_CTRL		(USB3_PHY_BASE + 0x0061 *
> 4)
> > +#define TB_ADDR_XCVR_DIAG_RX_LANE_CAL_RST_TMR	(USB3_PHY_BASE +
> 0x40ea * 4)
> > +#define TB_ADDR_XCVR_PSM_RCTRL
> (USB3_PHY_BASE + 0x4001 * 4)
> 
> Are all these macros needed ?

Most of them are needed, I will remove the unnecessary parts.

Thanks for your advice.

Best regards
Sherry sun

> 
> [...]