On Tuesday, July 21, 2015 at 02:17:06 AM, Adrian Alonso wrote:
> * Add support for usb driver for i.MX7D SoC
> 
> Signed-off-by: Adrian Alonso <aalo...@freescale.com>
> Signed-off-by: Ye.Li <b37...@freescale.com>
> Signed-off-by: Peng Fan <peng....@freescale.com>

Hi!

Do we really need yet-another-driver for i.MX ?

> ---
> Changes for V2: Resend
> 
>  drivers/usb/host/Makefile   |   1 +
>  drivers/usb/host/ehci-mx7.c | 103
> ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 104
> insertions(+)
>  create mode 100644 drivers/usb/host/ehci-mx7.c
> 
> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
> index 4d35d3e..7267160 100644
> --- a/drivers/usb/host/Makefile
> +++ b/drivers/usb/host/Makefile
> @@ -36,6 +36,7 @@ obj-$(CONFIG_USB_EHCI_MXC) += ehci-mxc.o
>  obj-$(CONFIG_USB_EHCI_MXS) += ehci-mxs.o
>  obj-$(CONFIG_USB_EHCI_MX5) += ehci-mx5.o
>  obj-$(CONFIG_USB_EHCI_MX6) += ehci-mx6.o
> +obj-$(CONFIG_USB_EHCI_MX7) += ehci-mx7.o
>  obj-$(CONFIG_USB_EHCI_OMAP) += ehci-omap.o
>  obj-$(CONFIG_USB_EHCI_PPC4XX) += ehci-ppc4xx.o
>  obj-$(CONFIG_USB_EHCI_MARVELL) += ehci-marvell.o
> diff --git a/drivers/usb/host/ehci-mx7.c b/drivers/usb/host/ehci-mx7.c
> new file mode 100644
> index 0000000..7429d62
> --- /dev/null
> +++ b/drivers/usb/host/ehci-mx7.c
> @@ -0,0 +1,103 @@
> +/*
> + * Copyright (c) 2009 Daniel Mack <dan...@caiaq.de>
> + * Copyright (C) 2010-2015 Freescale Semiconductor, Inc.
> + *
> + * SPDX-License-Identifier:  GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <usb.h>
> +#include <errno.h>
> +#include <linux/compiler.h>
> +#include <usb/ehci-fsl.h>
> +#include <asm/io.h>
> +#include <asm/arch/crm_regs.h>
> +#include <asm/arch/clock.h>
> +#include <asm/imx-common/iomux-v3.h>
> +
> +#include "ehci.h"
> +
> +#define USB_NC_OFFSET        0x200
> +
> +#define UCTRL_PM             (1 << 9)        /* OTG Power Mask */
> +#define UCTRL_OVER_CUR_POL   (1 << 8) /* OTG Polarity of Overcurrent */
> +#define UCTRL_OVER_CUR_DIS   (1 << 7) /* Disable OTG Overcurrent Detection
> */ +

Are these bits EHCI spiecific or MX7 Chipidea controller specific ?

> +/* USBCMD */
> +#define UCMD_RUN_STOP           (1 << 0) /* controller run/stop */
> +#define UCMD_RESET           (1 << 1) /* controller reset */
> +
> +/* Base address for this IP block is 0x02184800 */
> +struct usbnc_regs {
> +     u32     ctrl1;
> +     u32 ctrl2;

Please stop mixing tab and space, use tab :)

> +     u32 reserve1[11];
> +     u32 phy_ctrl2;
> +     u32 reserve2[6];
> +     u32 adp_cfg1;
> +     u32 reserve3;
> +     u32 adp_status;
> +};
> +
> +static void usb_oc_config(int index)
> +{
> +     struct usbnc_regs *usbnc = (struct usbnc_regs *)(USB_BASE_ADDR +
> +                     (0x10000 * index) + USB_NC_OFFSET);
> +     void __iomem *ctrl = (void __iomem *)(&usbnc->ctrl1);
> +     u32 val;
> +
> +     val = __raw_readl(ctrl);
> +     val |= UCTRL_OVER_CUR_POL;
> +     __raw_writel(val, ctrl);

setbits_le32() here.

> +     val = __raw_readl(ctrl);
> +     val |= (UCTRL_OVER_CUR_DIS | UCTRL_PM);
> +     __raw_writel(val, ctrl);

Here as well.

> +}
> +
> +int __weak board_ehci_hcd_init(int port)
> +{
> +     return 0;
> +}
> +
> +int __weak board_ehci_power(int port, int on)
> +{
> +     return 0;
> +}

Can you add kerneldoc style comments to the functions please ?

> +int ehci_hcd_init(int index, enum usb_init_type init,
> +             struct ehci_hccr **hccr, struct ehci_hcor **hcor)
> +{
> +     struct usb_ehci *ehci = (struct usb_ehci *)(USB_BASE_ADDR +
> +             (0x10000 * index));
> +
> +     if (index > 3)
> +             return -EINVAL;
> +     enable_usboh3_clk(1);
> +     mdelay(1);
> +
> +     /* Do board specific initialization */
> +     board_ehci_hcd_init(index);
> +
> +     usb_oc_config(index);
> +
> +     *hccr = (struct ehci_hccr *)((uint32_t)&ehci->caplength);
> +     *hcor = (struct ehci_hcor *)((uint32_t)*hccr +
> +                     HC_LENGTH(ehci_readl(&(*hccr)->cr_capbase)));
> +
> +     board_ehci_power(index, (init == USB_INIT_DEVICE) ? 0 : 1);
> +     if (init == USB_INIT_DEVICE)
> +             return 0;
> +     setbits_le32(&ehci->usbmode, CM_HOST);
> +     __raw_writel(CONFIG_MXC_USB_PORTSC, &ehci->portsc);

Just use writel().

> +     setbits_le32(&ehci->portsc, USB_EN);
> +
> +     mdelay(10);
> +
> +     return 0;
> +}
> +
> +int ehci_hcd_stop(int index)
> +{

You cannot stop the controller at all once you start it ? That looks
like yet another hardware flub :-(

> +     return 0;
> +}

Best regards,
Marek Vasut
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to