[U-Boot] [PATCH 02/15][v4] imx: usb: ehci-mx7 add usb driver for i.MX7D

[U-Boot] [PATCH 02/15][v4] imx: usb: ehci-mx7 add usb driver for i.MX7D

[Adrian Alonso]

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>
---
Changes for V2: Resend
Changes for V3: 
- Integrate review obserbations
- Add comments for fucntions board_ehci_hcd_init/board_ehci_power
Changes for V4:
- Follow kernel doc comments style

 drivers/usb/host/Makefile   |   1 +
 drivers/usb/host/ehci-mx7.c | 113 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 114 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..da6b4dc
--- /dev/null
+++ b/drivers/usb/host/ehci-mx7.c
@@ -0,0 +1,113 @@
+/*
+ * 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 */
+
+/* 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;
+	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);
+
+	setbits_le32(ctrl, UCTRL_OVER_CUR_POL);
+	setbits_le32(ctrl, UCTRL_OVER_CUR_DIS | UCTRL_PM);
+}
+
+/*
+ * board_ehci_hcd_init - set usb vbus voltage
+ * @port:	usb otg port
+ *
+ * Setup iomux pad to setup supply vbus voltage for usb otg port.
+ * Machine board file overrides board_ehci_hcd_init
+ */
+int __weak board_ehci_hcd_init(int port)
+{
+	return 0;
+}
+
+/*
+ * board_ehci_power - enables/disables usb vbus voltage
+ * @port:	usb otg port
+ * @on:	on/off vbus voltage
+ *
+ * Enables/disables supply vbus voltage for usb otg port.
+ * Machine board file overrides board_ehci_power
+ *
+ */
+int __weak board_ehci_power(int port, int on)
+{
+	return 0;
+}
+
+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);
+	writel(CONFIG_MXC_USB_PORTSC, &ehci->portsc);
+	setbits_le32(&ehci->portsc, USB_EN);
+
+	mdelay(10);
+
+	return 0;
+}
+
+int ehci_hcd_stop(int index)
+{
+	return 0;
+}
-- 
2.1.4

[U-Boot] [PATCH 02/15][v4] imx: usb: ehci-mx7 add usb driver for i.MX7D

[Marek Vasut]

On Thursday, July 23, 2015 at 09:25:17 PM, 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>

So errr, why exactly can ehci-mx6 not be used here ? The code looks
almost like a copy of ehci-mx6 with minor tweaks here and there.

Best regards,
Marek Vasut

[U-Boot] [PATCH 02/15][v4] imx: usb: ehci-mx7 add usb driver for i.MX7D

[Alonso Adrian]

Hi Marek,

> -----Original Message-----
> From: Marek Vasut [mailto:marex at denx.de]
> Sent: Friday, July 24, 2015 4:44 AM
> 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][v4] imx: usb: ehci-mx7 add usb driver for i.MX7D
> 
> On Thursday, July 23, 2015 at 09:25:17 PM, 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>
> 
> So errr, why exactly can ehci-mx6 not be used here ? The code looks almost like
> a copy of ehci-mx6 with minor tweaks here and there.
> 
> Best regards,
> Marek Vasut

[Adrian] We can definitely have a single driver to support imx7 and imx6, but we mainly decide
to have a driver for each SoC to try to make easier to maintain, there are so many variants of imx6
(Quad/Dual/Solo/SoloX/QuadPlus/UL) and imx7 (Dual/Solo); while there are several similarities in
USB IP for iMX6 and iMX7, USB PHY Control and clock settings differ slightly making code harder to
Understand and maintain in a single driver for imx7 and imx6.

Regards
Adrian

[U-Boot] [PATCH 02/15][v4] imx: usb: ehci-mx7 add usb driver for i.MX7D

[Marek Vasut]

On Friday, July 24, 2015 at 05:34:27 PM, Alonso Adrian wrote:
> Hi Marek,

Hi!

> > -----Original Message-----
> > From: Marek Vasut [mailto:marex at denx.de]
> > Sent: Friday, July 24, 2015 4:44 AM
> > 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][v4] imx: usb: ehci-mx7 add usb driver for
> > i.MX7D
> > 
> > On Thursday, July 23, 2015 at 09:25:17 PM, 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>
> > 
> > So errr, why exactly can ehci-mx6 not be used here ? The code looks
> > almost like a copy of ehci-mx6 with minor tweaks here and there.
> > 
> > Best regards,
> > Marek Vasut
> 
> [Adrian] We can definitely have a single driver to support imx7 and imx6,
> but we mainly decide to have a driver for each SoC to try to make easier
> to maintain, there are so many variants of imx6
> (Quad/Dual/Solo/SoloX/QuadPlus/UL) and imx7 (Dual/Solo);

I just took a look into ehci-mx6.c and I see 1 (one) bogus conditional
which involves a board type. I see zero conditionals there which would
check CPU type.

> while there are
> several similarities in USB IP for iMX6 and iMX7, USB PHY Control and
> clock settings differ slightly making code harder to Understand and
> maintain in a single driver for imx7 and imx6.

The downside of having multiple copies of the code is that if you fix bug
in one copy and not in the other, the bug will remain unfixed on many of
the other machines. I don't like that. I am also not convinced that there
is justification for having separate drivers for MX6 and MX7 EHCI. I would
like to see a patch adding MX7 support into ehci-mx6 and in case it is
really as hairy as you say, I'd pick a separate one.

Best regards,
Marek Vasut

[U-Boot] [PATCH 02/15][v4] imx: usb: ehci-mx7 add usb driver for i.MX7D

[Stefano Babic]

Hi Adrian,

On 24/07/2015 17:34, Alonso Adrian wrote:
> Hi Marek,
> 
>> -----Original Message-----
>> From: Marek Vasut [mailto:marex at denx.de]
>> Sent: Friday, July 24, 2015 4:44 AM
>> 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][v4] imx: usb: ehci-mx7 add usb driver for i.MX7D
>>
>> On Thursday, July 23, 2015 at 09:25:17 PM, 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>
>>
>> So errr, why exactly can ehci-mx6 not be used here ? The code looks almost like
>> a copy of ehci-mx6 with minor tweaks here and there.
>>
>> Best regards,
>> Marek Vasut
> 
> [Adrian] We can definitely have a single driver to support imx7 and imx6, but we mainly decide
> to have a driver for each SoC to try to make easier to maintain, there are so many variants of imx6
> (Quad/Dual/Solo/SoloX/QuadPlus/UL) and imx7 (Dual/Solo);

This is exactly we do not want to have in U-Boot mainline. We try as
much as p?ossible to duplicate code, and when it is possible to have a
single driver across more platforms or event architectures.

> while there are several similarities in
> USB IP for iMX6 and iMX7, USB PHY Control and clock settings differ slightly making code harder to
> Understand and maintain in a single driver for imx7 and imx6.

This could mean that some parts should be extracted and move into a
separate SOC specific file, but it is not a reson for duplicating code.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH 02/15][v4] imx: usb: ehci-mx7 add usb driver for i.MX7D

[Marek Vasut]

On Sunday, July 26, 2015 at 11:29:45 AM, Stefano Babic wrote:

[...]

> > [Adrian] We can definitely have a single driver to support imx7 and imx6,
> > but we mainly decide to have a driver for each SoC to try to make easier
> > to maintain, there are so many variants of imx6
> > (Quad/Dual/Solo/SoloX/QuadPlus/UL) and imx7 (Dual/Solo);
> 
> This is exactly we do not want to have in U-Boot mainline. We try as
> much as p?ossible to duplicate code, and when it is possible to have a
> single driver across more platforms or event architectures.

I think you're missing one "NOT" there ;-)
[...]

Best regards,
Marek Vasut