Hi Marek,
>
> On 8/16/19 8:10 AM, Sherry Sun wrote:
>
> [...]
>
> > diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index
> > ac68aa2d27..cc1dfe463b 100644
> > --- a/drivers/usb/host/Kconfig
> > +++ b/drivers/usb/host/Kconfig
> > @@ -95,6 +95,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"
>
> i.MX8 I guess ?
Yes, you are right, I have changed these to i.MX8 instead of imx8.
>
> [...]
>
> > diff --git a/drivers/usb/host/xhci-imx8.c
> > b/drivers/usb/host/xhci-imx8.c new file mode 100644 index
> > 0000000000..0669a05c17
> > --- /dev/null
> > +++ b/drivers/usb/host/xhci-imx8.c
> > @@ -0,0 +1,189 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2019 NXP
> > + *
> > + * NXP i.MX8 USB HOST xHCI Controller (Cadence IP)
> > + *
> > + * Author: Peter Chen <peter.c...@nxp.com> */
> > +
> > +#include <common.h>
> > +#include <usb.h>
> > +#include <malloc.h>
> > +#include <dm.h>
> > +#include <dm/device-internal.h>
> > +#include <clk.h>
> > +#include <wait_bit.h>
> > +#include <power-domain.h>
> > +#include <generic-phy.h>
> > +#include <asm/arch/clock.h>
> > +#include <linux/compat.h>
>
> Keep the list sorted
Okay, I have done this.
>
> > +#include "xhci.h"
> > +
> > +/* Declare global data pointer */
> > +DECLARE_GLOBAL_DATA_PTR;
> > +
> > +/* Host registers */
> > +#define HCIVERSION_CAPLENGTH 0x10000
> > +#define USBSTS 0x10084
> > +
> > +/* None-core registers */
> > +#define USB3_CORE_CTRL1 0x00
> > +#define USB3_CORE_STATUS 0x0c
> > +#define USB3_SSPHY_STATUS 0x4c
> > +
> > +struct xhci_imx8_data {
> > + void __iomem *usb3_ctrl_base;
> > + void __iomem *usb3_core_base;
> > + struct clk_bulk clks;
> > + struct phy phy;
> > +};
> > +
> > +static struct xhci_imx8_data imx8_data;
> > +
> > +static void imx8_xhci_init(void)
> > +{
> > + u32 tmp_data;
> > + int ret;
> > +
> > + tmp_data = readl(imx8_data.usb3_ctrl_base + USB3_SSPHY_STATUS);
> > + writel(tmp_data, imx8_data.usb3_ctrl_base + USB3_SSPHY_STATUS);
>
> Is this read-write really needed ?
I reviewed this register definition. And found the read is not needed, but the
write is needed.
I have changed this read-write already.
>
> > + tmp_data = readl(imx8_data.usb3_ctrl_base + USB3_SSPHY_STATUS);
> > + ret = wait_for_bit_le32(imx8_data.usb3_ctrl_base + USB3_SSPHY_STATUS,
> > + 0xf0000000, true, 100, false);
> > + if (ret)
> > + printf("clkvld is incorrect\n");
>
> Shouldn't this return ret ?
Okay, have done this.
>
> > + clrsetbits_le32(imx8_data.usb3_ctrl_base + USB3_CORE_CTRL1,
> > + 0x7, 0x202);
> > + clrbits_le32(imx8_data.usb3_ctrl_base + USB3_CORE_CTRL1, 1 << 26);
> > + generic_phy_init(&imx8_data.phy);
> > +
> > + /* clear all sw_rst */
> > + clrbits_le32(imx8_data.usb3_ctrl_base + USB3_CORE_CTRL1, 0xFC <<
> > +24);
> > +
> > + debug("wait xhci_power_on_ready\n");
> > + ret = wait_for_bit_le32(imx8_data.usb3_ctrl_base + USB3_CORE_STATUS,
> > + 0x1000, true, 100, false);
> > + if (ret)
> > + printf("wait xhci_power_on_ready timeout\n");
>
> return ret on failure ?
Okay, have done this.
>
> > + debug("xhci_power_on_ready\n");
> > +
> > + debug("waiting CNR 0x%x\n", tmp_data);
> > + ret = wait_for_bit_le32(imx8_data.usb3_core_base + USBSTS,
> > + 0x800, false, 100, false);
> > + if (ret)
> > + printf("wait CNR timeout\n");
>
> return ret on failure ?
Okay, have done this.
>
> > + debug("check CNR has finished\n");
> > +}
> > +
> > +static void imx8_xhci_reset(void)
> > +{
> > + /* Set CORE ctrl to default value, that all rst are hold */
> > + writel(0xfc000001, imx8_data.usb3_ctrl_base + USB3_CORE_CTRL1);
>
> What are these magic values ? Add macros for them.
Okay, have done this.
>
> > +}
> > +
> > +static int xhci_imx8_clk_init(struct udevice *dev) {
> > + int ret;
> > +
> > + ret = clk_get_bulk(dev, &imx8_data.clks);
> > + if (ret)
> > + return ret;
> > +
> > + ret = clk_enable_bulk(&imx8_data.clks);
> > + if (ret)
> > + return ret;
> > +
> > + return 0;
> > +}
> > +
> > +static int xhci_imx8_get_reg_addr(struct udevice *dev) {
> > + imx8_data.usb3_ctrl_base =
> > + (void __iomem *)devfdt_get_addr_index(dev, 0);
> > + imx8_data.usb3_core_base =
> > + (void __iomem *)devfdt_get_addr_index(dev, 4);
> > +
> > + return 0;
> > +}
> > +
> > +static int xhci_imx8_probe(struct udevice *dev) {
> > + struct xhci_hccr *hccr;
> > + struct xhci_hcor *hcor;
> > + struct udevice usbotg_dev;
> > + struct power_domain pd;
> > + int usbotg_off;
> > + int ret = 0;
> > + int len;
> > +
> > + usbotg_off = fdtdec_lookup_phandle(gd->fdt_blob,
> > + dev_of_offset(dev),
> > + "cdns3,usb");
> > + if (usbotg_off < 0)
> > + return -EINVAL;
> > + usbotg_dev.node = offset_to_ofnode(usbotg_off);
> > + usbotg_dev.parent = dev->parent;
> > + xhci_imx8_get_reg_addr(&usbotg_dev);
> > +
> > +#if CONFIG_IS_ENABLED(POWER_DOMAIN)
> > + if (!power_domain_get(&usbotg_dev, &pd)) {
> > + if (power_domain_on(&pd))
> > + return -EINVAL;
>
> ret = power_domain_on()
> if (ret)
> return ret;
>
Okay, I have done this.
Bets regards
Sherry sun
>
> [...]
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
