Skip to contentSkip to conversation

imx: usb: ehci-mx7 add usb driver for i.MX7D

PATCH New Adrian Alonso 3 participants 6 messages Jul 23, 2015 – Jul 26, 2015

Subsystems (2)
USB USB ehci driver
lore
Share & tools
lore

Adrian Alonso

author
#0 lore raw

PATCH v4 2/15 imx: usb: ehci-mx7 add usb driver for i.MX7D

Add support for usb driver for i.MX7D SoC

Signed-off-by Adrian Alonso <[email protected]>
Signed-off-by Ye.Li <[email protected]>
Signed-off-by Peng Fan <[email protected]>

---
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
drivers/usb/host/Makefile +1 -0
index 4d35d3e..7267160 100644
@@ -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
drivers/usb/host/ehci-mx7.c new file +113 -0

113 lines not on this page · raw message

Signature · 1 line
2.1.4
author
#2 lore raw

PATCH v4 2/15 imx: usb: ehci-mx7 add usb driver for i.MX7D

Hi Marek,

Quoted 15 lines

-----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

Quoted 5 lines · Adrian Alonso

Add support for usb driver for i.MX7D SoC

Signed-off-by Adrian Alonso <[email protected]>
Signed-off-by Ye.Li <[email protected]>
Signed-off-by Peng Fan <[email protected]>
Quoted 5 lines

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

#3 lore raw

PATCH v4 2/15 imx: usb: ehci-mx7 add usb driver for i.MX7D

Quoted 1 line · Alonso Adrian

Hi Marek,

Hi!

Quoted 19 lines
Quoted 19 lines

-----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

Quoted 5 lines · Adrian Alonso

Add support for usb driver for i.MX7D SoC

Signed-off-by Adrian Alonso <[email protected]>
Signed-off-by Ye.Li <[email protected]>
Signed-off-by Peng Fan <[email protected]>

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.

Quoted 7 lines
Quoted 2 lines

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.

Quoted 4 lines

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

#4 lore raw

PATCH v4 2/15 imx: usb: ehci-mx7 add usb driver for i.MX7D

In reply to Alonso Adrian

Hi Adrian,

Quoted 20 lines · Alonso Adrian

Hi Marek,

Quoted 18 lines

-----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

Quoted 5 lines · Adrian Alonso

Add support for usb driver for i.MX7D SoC

Signed-off-by Adrian Alonso <[email protected]>
Signed-off-by Ye.Li <[email protected]>
Signed-off-by Peng Fan <[email protected]>

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.

Quoted 6 lines
Quoted 2 lines

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.

Quoted 3 lines

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

Signature · 5 lines
=====================================================================
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
=====================================================================
#5 lore raw

PATCH v4 2/15 imx: usb: ehci-mx7 add usb driver for i.MX7D

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

[...]

Quoted 8 lines
Quoted 4 lines

[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