From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marek Vasut <marex@denx.de>
Date: Thu, 23 Jul 2015 08:00:17 +0200
Subject: [U-Boot] [PATCH 02/15][v2] imx: usb: ehci-mx7 add usb driver
	for i.MX7D
In-Reply-To: <BN3PR03MB15099CDC956F82CBDD4ED80CA8830@BN3PR03MB1509.namprd03.prod.outlook.com>
References: <1437437826-3171-1-git-send-email-aalonso@freescale.com>
 <201507210356.03975.marex@denx.de>
 <BN3PR03MB15099CDC956F82CBDD4ED80CA8830@BN3PR03MB1509.namprd03.prod.outlook.com>
Message-ID: <201507230800.18058.marex@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On Wednesday, July 22, 2015 at 11:15:31 PM, Alonso Adrian wrote:
> Hi Marek,
> 
> Comments inline.
> 
> > -----Original Message-----
> > From: Marek Vasut [mailto:marex at denx.de]
> > Sent: Monday, July 20, 2015 8:56 PM
> > To: Alonso Lazcano Adrian-B38018
> > Cc: u-boot at lists.denx.de; sbabic at denx.de; otavio at ossystems.com.br;
> > Estevam Fabio-R49496; Li Frank-B20596; Garg Nitin-B37173
> > Subject: Re: [PATCH 02/15][v2] imx: usb: ehci-mx7 add usb driver for
> > i.MX7D
> > 
> > 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 <aalonso@freescale.com>
> > > Signed-off-by: Ye.Li <B37916@freescale.com>
> > > Signed-off-by: Peng Fan <Peng.Fan@freescale.com>
> > 
> > Hi!
> > 
> > Do we really need yet-another-driver for i.MX ?
> 
> [Adrian] USB support for iMX could be re-implemented following DM model,
> At the moment ehci-mx7 handles the SoC specific settings for USB support.

But do we really need yet another driver for i.MX ? We already have one for
MX23/28, MX5, MX6 and one for Vybrid .

> > > ---
> > > 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 <daniel@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 ?
> 
> [Adrian] They are iMX7 specific

OK

[...]

> > > +}
> > > +
> > > +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 ?
> 
> [Adrian] All change suggestion to be addressed in a new patch version.

Thanks!