Skip to contentSkip to conversation

preliminary work for adding imx6q_sabrelite usb support

PATCH New Richard Zhao 8 participants 34 messages May 15, 2012 – May 17, 2012

Subsystems (6)
lore
Share & tools
lore
#0 lore raw

cover preliminary work for adding imx6q_sabrelite usb support

The work is based on usb-next which includes ci13xxx rework by Alexander Shishkin.

Status:
- this version only tested host
- usbotg controler works at host role

Todo:
- usbh1 has strange system error when enumerate devices for on-board hub.
- high speed usb connecton detect failed for the second time. It needs hub code hack.

Thanks Peter Chen for greate help.

Richard Zhao (7):
usb: chipidea: permit user select USB_EHCI_ROOT_HUB_TT
usb: chipidea: remove zero check of hw_ep_max
usb: chipidea: add imx on-soc utmi phy driver
usb: chipidea: add imx driver binding
ARM: imx6q: correct device name of usbphy and usboh3 clock export
ARM: imx6q: add anatop initialization for usb controllers
ARM: dts: imx6q-sabrelite: add usb devices

 arch/arm/boot/dts/imx6q-sabrelite.dts |   19 ++++-
 arch/arm/boot/dts/imx6q.dtsi          |   44 ++++++++-
 arch/arm/mach-imx/clk-imx6q.c         |    7 +-
 arch/arm/mach-imx/mach-imx6q.c        |   60 +++++++++++
 drivers/usb/chipidea/Makefile         |    4 +
 drivers/usb/chipidea/ci13xxx_imx.c    |  177 +++++++++++++++++++++++++++++++++
 drivers/usb/chipidea/core.c           |    2 +-
 drivers/usb/chipidea/phy-imx-utmi.c   |  140 ++++++++++++++++++++++++++
 drivers/usb/host/Kconfig              |    2 +-
 9 files changed, 449 insertions(+), 6 deletions(-)
 create mode 100644 drivers/usb/chipidea/ci13xxx_imx.c
 create mode 100644 drivers/usb/chipidea/phy-imx-utmi.c
Signature · 1 line
1.7.5.4
#1 lore raw

PATCH 1/7 usb: chipidea: permit user select USB_EHCI_ROOT_HUB_TT

The ci13xxx hcd require USB_EHCI_ROOT_HUB_TT.

---
 drivers/usb/host/Kconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/usb/host/Kconfig +1 -1
index 684a7bb..89d0371 100644
@@ -65,7 +65,7 @@ config USB_EHCI_HCD
 
 config USB_EHCI_ROOT_HUB_TT
 	bool "Root Hub Transaction Translators"
-	depends on USB_EHCI_HCD
+	depends on USB_EHCI_HCD || USB_CHIPIDEA
 	---help---
 	  Some EHCI chips have vendor-specific extensions to integrate
 	  transaction translators, so that no OHCI or UHCI companion
Signature · 1 line
1.7.5.4
#2 lore raw

PATCH 1/7 usb: chipidea: permit user select USB_EHCI_ROOT_HUB_TT

Dear Richard Zhao,

Quoted 3 lines

The ci13xxx hcd require USB_EHCI_ROOT_HUB_TT.

I already sent similar patch a few hours ago.

Quoted 9 lines
---
 drivers/usb/host/Kconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/usb/host/Kconfig +0 -0
index 684a7bb..89d0371 100644

drivers/usb/host/Kconfig config USB_EHCI_HCD · line 71

Quoted 8 lines
drivers/usb/host/Kconfig +1 -1
@@ -65,7 +65,7 @@ config USB_EHCI_HCD
 
 config USB_EHCI_ROOT_HUB_TT
 	bool "Root Hub Transaction Translators"
-	depends on USB_EHCI_HCD
+	depends on USB_EHCI_HCD || USB_CHIPIDEA
 	---help---
 	  Some EHCI chips have vendor-specific extensions to integrate
 	  transaction translators, so that no OHCI or UHCI companion

Best regards,
Marek Vasut

maintainer
#3 lore raw

PATCH 1/7 usb: chipidea: permit user select USB_EHCI_ROOT_HUB_TT

Quoted 7 lines · Marek Vasut

Dear Richard Zhao,

Quoted 3 lines

The ci13xxx hcd require USB_EHCI_ROOT_HUB_TT.

I already sent similar patch a few hours ago.

But yours was more "complete" :)

I'll queue up yours in a bit, thanks.

greg k-h

#4 lore raw

PATCH 1/7 usb: chipidea: permit user select USB_EHCI_ROOT_HUB_TT

In reply to Richard Zhao

Dear Richard Zhao,

Quoted 3 lines

The ci13xxx hcd require USB_EHCI_ROOT_HUB_TT.

Ok, this has been sorted out, that makes it 6 patches to go ;-)

Quoted 9 lines
---
 drivers/usb/host/Kconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/usb/host/Kconfig +0 -0
index 684a7bb..89d0371 100644

drivers/usb/host/Kconfig config USB_EHCI_HCD · line 71

Quoted 8 lines
drivers/usb/host/Kconfig +1 -1
@@ -65,7 +65,7 @@ config USB_EHCI_HCD
 
 config USB_EHCI_ROOT_HUB_TT
 	bool "Root Hub Transaction Translators"
-	depends on USB_EHCI_HCD
+	depends on USB_EHCI_HCD || USB_CHIPIDEA
 	---help---
 	  Some EHCI chips have vendor-specific extensions to integrate
 	  transaction translators, so that no OHCI or UHCI companion

Best regards,
Marek Vasut

#5 lore raw

PATCH 2/7 usb: chipidea: remove zero check of hw_ep_max

In reply to Richard Zhao

It's 0 for host only device.

---
 drivers/usb/chipidea/core.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/usb/chipidea/core.c +1 -1
index f568b8e..15e03b3 100644
@@ -195,7 +195,7 @@ static int hw_device_init(struct ci13xxx *ci, void __iomem *base)
 		ffs_nr(DCCPARAMS_DEN);
 	ci->hw_ep_max = reg * 2;   /* cache hw ENDPT_MAX */
 
-	if (ci->hw_ep_max == 0 || ci->hw_ep_max > ENDPT_MAX)
+	if (ci->hw_ep_max > ENDPT_MAX)
 		return -ENODEV;
 
 	dev_dbg(ci->dev, "ChipIdea HDRC found, lpm: %d; cap: %p op: %p\n",
Signature · 1 line
1.7.5.4
#6 lore raw

PATCH 2/7 usb: chipidea: remove zero check of hw_ep_max

Dear Richard Zhao,

Quoted 12 lines

It's 0 for host only device.

---
 drivers/usb/chipidea/core.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/usb/chipidea/core.c +0 -0
index f568b8e..15e03b3 100644

drivers/usb/chipidea/core.c static int hw_device_init(struct ci13xxx *ci, void · line 198

Quoted 5 lines
drivers/usb/chipidea/core.c +1 -1
@@ -195,7 +195,7 @@ static int hw_device_init(struct ci13xxx *ci, void
 __iomem *base) ffs_nr(DCCPARAMS_DEN);
 	ci->hw_ep_max = reg * 2;   /* cache hw ENDPT_MAX */
 
-	if (ci->hw_ep_max == 0 || ci->hw_ep_max > ENDPT_MAX)
+	if (ci->hw_ep_max > ENDPT_MAX)

Well, maybe this check should be extended a bit? Let me send a subsequent patch in reply to this, see what you think about it?

drivers/usb/chipidea/core.c static int hw_device_init(struct ci13xxx *ci, void __iomem *base) · line 201

Quoted 3 lines
drivers/usb/chipidea/core.c +0 -0
 		return -ENODEV;
 
 	dev_dbg(ci->dev, "ChipIdea HDRC found, lpm: %d; cap: %p op: %p\n",

Best regards,
Marek Vasut

#7 lore raw

PATCH 1/1 usb: chipidea: improve the validation of endpoint count

In reply to Richard Zhao

The endpoint count is zero for host-only controller. On the other hand, the endpoint count is non-zero for OTG capable controller.

Cc Richard Zhao <[email protected]>
Cc alexander.shishkin at linux.intel.com
Cc B20596 at freescale.com
Cc B29397 at freescale.com
Cc dong.aisheng at linaro.org
Cc fabio.estevam at freescale.com
Cc gregkh at linuxfoundation.org
Cc kernel at pengutronix.de
Cc linux-arm-kernel at lists.infradead.org
Cc marex at denx.de
Cc shawn.guo at linaro.org

To: linux-usb at vger.kernel.org

---
 drivers/usb/chipidea/core.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/usb/chipidea/core.c +7 -1
index f568b8e..b29a204 100644
@@ -174,6 +174,7 @@ u8 hw_port_test_get(struct ci13xxx *ci)
 static int hw_device_init(struct ci13xxx *ci, void __iomem *base)
 {
 	u32 reg;
+	int dc;
 
 	/* bank is a module variable */
 	ci->hw_bank.abs = base;
@@ -195,7 +196,12 @@ static int hw_device_init(struct ci13xxx *ci, void __iomem *base)
 		ffs_nr(DCCPARAMS_DEN);
 	ci->hw_ep_max = reg * 2;   /* cache hw ENDPT_MAX */
 
-	if (ci->hw_ep_max == 0 || ci->hw_ep_max > ENDPT_MAX)
+	dc = hw_read(ci, CAP_DCCPARAMS, DCCPARAMS_DC);
+
+	if (dc && (ci->hw_ep_max == 0 || ci->hw_ep_max > ENDPT_MAX))
+		return -ENODEV;
+
+	if (!dc && (ci->hw_ep_max != 0))
 		return -ENODEV;
 
 	dev_dbg(ci->dev, "ChipIdea HDRC found, lpm: %d; cap: %p op: %p\n",
Signature · 1 line
1.7.10
#8 lore raw

PATCH 1/1 usb: chipidea: improve the validation of endpoint count

Quoted 34 lines · Marek Vasut

The endpoint count is zero for host-only controller. On the other hand, the endpoint count is non-zero for OTG capable controller.

Cc Richard Zhao <[email protected]>
Cc alexander.shishkin at linux.intel.com
Cc B20596 at freescale.com
Cc B29397 at freescale.com
Cc dong.aisheng at linaro.org
Cc fabio.estevam at freescale.com
Cc gregkh at linuxfoundation.org
Cc kernel at pengutronix.de
Cc linux-arm-kernel at lists.infradead.org
Cc marex at denx.de
Cc shawn.guo at linaro.org

To: linux-usb at vger.kernel.org

---
 drivers/usb/chipidea/core.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/usb/chipidea/core.c +1 -0
index f568b8e..b29a204 100644
@@ -174,6 +174,7 @@ u8 hw_port_test_get(struct ci13xxx *ci)
 static int hw_device_init(struct ci13xxx *ci, void __iomem *base)
 {
 	u32 reg;
+	int dc;
 
 	/* bank is a module variable */
 	ci->hw_bank.abs = base;
@@ -195,7 +196,12 @@ static int hw_device_init(struct ci13xxx *ci, void __iomem *base)
 		ffs_nr(DCCPARAMS_DEN);

drivers/usb/chipidea/core.c static int hw_device_init(struct ci13xxx *ci, void __iomem *base) · line 205

Quoted 10 lines
drivers/usb/chipidea/core.c +6 -1
 	ci->hw_ep_max = reg * 2;   /* cache hw ENDPT_MAX */
 
-	if (ci->hw_ep_max == 0 || ci->hw_ep_max > ENDPT_MAX)
+	dc = hw_read(ci, CAP_DCCPARAMS, DCCPARAMS_DC);
+
+	if (dc && (ci->hw_ep_max == 0 || ci->hw_ep_max > ENDPT_MAX))
+		return -ENODEV;
+
+	if (!dc && (ci->hw_ep_max != 0))
 		return -ENODEV;

According to the spec, this doesn't happen. And taking into account the size of the bitfield encoding the number of endpoints, the (hw_ep_max > ENDPT_MAX) check doesn't make much sense either. Again, according to the spec, this field really encodes the number of hardware endpoints (meaning both OUT and IN is one hw endpoint) and the valid values for this field are 0-16, so even comparing it against ENDPT_MAX makes little sense.

Having said that, there is already a check in udc.c for the DC bit and it should be enough. If any particular controller reports wrong number of endpoints, it needs to be considered separately.

Regards,
--
Alex

#9 lore raw

PATCH 2/7 usb: chipidea: remove zero check of hw_ep_max

In reply to Richard Zhao
Quoted 12 lines · Richard Zhao

It's 0 for host only device.

---
 drivers/usb/chipidea/core.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/usb/chipidea/core.c +0 -0
index f568b8e..15e03b3 100644

drivers/usb/chipidea/core.c static int hw_device_init(struct ci13xxx *ci, void __iomem *base) · line 199

Quoted 6 lines
drivers/usb/chipidea/core.c +1 -1
@@ -195,7 +195,7 @@ static int hw_device_init(struct ci13xxx *ci, void __iomem *base)
 		ffs_nr(DCCPARAMS_DEN);
 	ci->hw_ep_max = reg * 2;   /* cache hw ENDPT_MAX */
 
-	if (ci->hw_ep_max == 0 || ci->hw_ep_max > ENDPT_MAX)
+	if (ci->hw_ep_max > ENDPT_MAX)
 		return -ENODEV;

Can you move this get hw_ep_max code to udc.c? As hw_ep_max is only used for device driver. What do you think, Alex?

Quoted 5 lines
drivers/usb/chipidea/core.c +0 -0
 	dev_dbg(ci->dev, "ChipIdea HDRC found, lpm: %d; cap: %p op: %p\n",
Signature · 1 line
1.7.5.4
Signature · 2 lines
Best Regards,
Peter Chen
#10 lore raw

PATCH 2/7 usb: chipidea: remove zero check of hw_ep_max

Quoted 16 lines · Peter Chen
Quoted 15 lines · Richard Zhao

It's 0 for host only device.

---
 drivers/usb/chipidea/core.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/usb/chipidea/core.c +0 -0
index f568b8e..15e03b3 100644
@@ -195,7 +195,7 @@ static int hw_device_init(struct ci13xxx *ci, void __iomem *base)
 		ffs_nr(DCCPARAMS_DEN);
 	ci->hw_ep_max = reg * 2;   /* cache hw ENDPT_MAX */
 
Quoted 5 lines

drivers/usb/chipidea/core.c static int hw_device_init(struct ci13xxx *ci, void __iomem *base) · line 199

Quoted 3 lines
drivers/usb/chipidea/core.c +1 -1
-	if (ci->hw_ep_max == 0 || ci->hw_ep_max > ENDPT_MAX)
+	if (ci->hw_ep_max > ENDPT_MAX)
 		return -ENODEV;

Can you move this get hw_ep_max code to udc.c? As hw_ep_max is only used for device driver. What do you think, Alex?

Yes, But since Greg has applied the patch, one may need to rebase or let it be as it is.

Thanks
Richard

Quoted 10 lines
Quoted 5 lines
drivers/usb/chipidea/core.c +0 -0
 	dev_dbg(ci->dev, "ChipIdea HDRC found, lpm: %d; cap: %p op: %p\n",
Signature · 1 line
1.7.5.4
Signature · 2 lines
Best Regards,
Peter Chen
#11 lore raw

PATCH 2/7 usb: chipidea: remove zero check of hw_ep_max

In reply to Peter Chen
Quoted 16 lines · Peter Chen
Quoted 15 lines · Richard Zhao

It's 0 for host only device.

---
 drivers/usb/chipidea/core.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/usb/chipidea/core.c +0 -0
index f568b8e..15e03b3 100644
@@ -195,7 +195,7 @@ static int hw_device_init(struct ci13xxx *ci, void __iomem *base)
 		ffs_nr(DCCPARAMS_DEN);
 	ci->hw_ep_max = reg * 2;   /* cache hw ENDPT_MAX */
 
Quoted 5 lines

drivers/usb/chipidea/core.c static int hw_device_init(struct ci13xxx *ci, void __iomem *base) · line 199

Quoted 3 lines
drivers/usb/chipidea/core.c +1 -1
-	if (ci->hw_ep_max == 0 || ci->hw_ep_max > ENDPT_MAX)
+	if (ci->hw_ep_max > ENDPT_MAX)
 		return -ENODEV;

Can you move this get hw_ep_max code to udc.c? As hw_ep_max is only used for device driver. What do you think, Alex?

Yes, I think this needs to go to udc.c in the future.

Regards,
--
Alex

#12 lore raw

PATCH 3/7 usb: chipidea: add imx on-soc utmi phy driver

In reply to Richard Zhao
---
 drivers/usb/chipidea/Makefile       |    4 +
 drivers/usb/chipidea/phy-imx-utmi.c |  140 +++++++++++++++++++++++++++++++++++
 2 files changed, 144 insertions(+), 0 deletions(-)
 create mode 100644 drivers/usb/chipidea/phy-imx-utmi.c
drivers/usb/chipidea/Makefile +4 -0
index cc34937..0f34c0c 100644
@@ -12,3 +12,7 @@ endif
 ifneq ($(CONFIG_ARCH_MSM),)
 	obj-$(CONFIG_USB_CHIPIDEA)	+= ci13xxx_msm.o
 endif
+
+ifneq ($(CONFIG_ARCH_MXC),)
+	obj-$(CONFIG_USB_CHIPIDEA)	+= phy-imx-utmi.o
+endif
drivers/usb/chipidea/phy-imx-utmi.c new file +140 -0

140 lines not on this page · raw message

Signature · 1 line
1.7.5.4
#13 lore raw

PATCH 3/7 usb: chipidea: add imx on-soc utmi phy driver

Dear Richard Zhao,

Quoted 39 lines
---
 drivers/usb/chipidea/Makefile       |    4 +
 drivers/usb/chipidea/phy-imx-utmi.c |  140
+++++++++++++++++++++++++++++++++++ 2 files changed, 144 insertions(+), 0
deletions(-)
 create mode 100644 drivers/usb/chipidea/phy-imx-utmi.c
drivers/usb/chipidea/Makefile +4 -0
index cc34937..0f34c0c 100644
@@ -12,3 +12,7 @@ endif
 ifneq ($(CONFIG_ARCH_MSM),)
 	obj-$(CONFIG_USB_CHIPIDEA)	+= ci13xxx_msm.o
 endif
+
+ifneq ($(CONFIG_ARCH_MXC),)
+	obj-$(CONFIG_USB_CHIPIDEA)	+= phy-imx-utmi.o
+endif
(quoted diff) +0 -0

b/drivers/usb/chipidea/phy-imx-utmi.c new file mode 100644 index 0000000..f7a0fe7

drivers/usb/chipidea/phy-imx-utmi.c new file +13 -0
@@ -0,0 +1,140 @@
+/*
+ * Copyright 2012 Freescale Semiconductor, Inc.
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>

drivers/usb/chipidea/phy-imx-utmi.c line 23

Quoted 10 lines
drivers/usb/chipidea/phy-imx-utmi.c new file +10 -0
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <linux/usb/otg.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/io.h>
+
+#define DRIVER_NAME "imx_utmi"
+
+#define HW_USBPHY_PWD	(0x00000000)

Drop the braces here

Quoted 28 lines
drivers/usb/chipidea/phy-imx-utmi.c new file +28 -0
+#define HW_USBPHY_CTRL	0x00000030
+
+#define BM_USBPHY_CTRL_SFTRST 0x80000000
+#define BM_USBPHY_CTRL_CLKGATE 0x40000000
+#define BM_USBPHY_CTRL_ENUTMILEVEL3 0x00008000
+#define BM_USBPHY_CTRL_ENUTMILEVEL2 0x00004000
+
+struct imx_utmi_phy {
+	struct usb_phy phy;
+	struct clk *clk;
+};
+
+#define to_imx_phy(p) container_of((p), struct imx_utmi_phy, phy)
+
+static int imx_utmi_phy_init(struct usb_phy *phy)
+{
+	struct imx_utmi_phy *imx_phy;
+	u32 reg;
+
+	imx_phy = to_imx_phy(phy);
+
+	clk_prepare_enable(imx_phy->clk);
+
+	reg = readl_relaxed(phy->io_priv + HW_USBPHY_CTRL);
+	reg &= ~BM_USBPHY_CTRL_CLKGATE;
+	writel_relaxed(reg, phy->io_priv + HW_USBPHY_CTRL);
+
+	/* Reset USBPHY module */

drivers/usb/chipidea/phy-imx-utmi.c line 61

Quoted 10 lines
drivers/usb/chipidea/phy-imx-utmi.c new file +10 -0
+	reg = readl_relaxed(phy->io_priv + HW_USBPHY_CTRL);
+	reg |= BM_USBPHY_CTRL_SFTRST;
+	writel_relaxed(reg, phy->io_priv + HW_USBPHY_CTRL);
+	udelay(10);
+
+	/* Remove CLKGATE and SFTRST */
+	reg = readl_relaxed(phy->io_priv + HW_USBPHY_CTRL);
+	reg &= ~(BM_USBPHY_CTRL_CLKGATE | BM_USBPHY_CTRL_SFTRST);
+	writel_relaxed(reg, phy->io_priv + HW_USBPHY_CTRL);
+	udelay(10);

You might want to check my PHY driver for the proper init sequence ... it uses mxs_reset_block() (or maybe there's already stmp_reset_block() ? )

drivers/usb/chipidea/phy-imx-utmi.c line 68

Quoted 7 lines
drivers/usb/chipidea/phy-imx-utmi.c new file +7 -0
+
+	/* Power up the PHY */
+	writel_relaxed(0, phy->io_priv + HW_USBPHY_PWD);
+	/* enable FS/LS device */
+	reg = readl_relaxed(phy->io_priv + HW_USBPHY_CTRL);
+	reg |= BM_USBPHY_CTRL_ENUTMILEVEL2 | BM_USBPHY_CTRL_ENUTMILEVEL3;
+	writel_relaxed(reg, phy->io_priv + HW_USBPHY_CTRL);

Well, you can retrieve this from my PHY driver ;-)

drivers/usb/chipidea/phy-imx-utmi.c line 78

Quoted 10 lines
drivers/usb/chipidea/phy-imx-utmi.c new file +10 -0
+
+	return 0;
+}
+
+static void imx_utmi_phy_shutdown(struct usb_phy *phy)
+{
+	struct imx_utmi_phy *imx_phy;
+
+	imx_phy = to_imx_phy(phy);
+	clk_disable_unprepare(imx_phy->clk);

You forgot to disable the PHY here ... pull it out of my patchset and be done with it ;-)

Quoted 51 lines
drivers/usb/chipidea/phy-imx-utmi.c new file +51 -0
+}
+
+static int imx_utmi_probe(struct platform_device *pdev)
+{
+	struct resource *res;
+	void __iomem *base;
+	struct clk *clk;
+	struct imx_utmi_phy *imx_phy;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "can't get device resources\n");
+		return -ENOENT;
+	}
+
+	base = devm_request_and_ioremap(&pdev->dev, res);
+	if (!base)
+		return -EBUSY;
+
+	clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	imx_phy = devm_kzalloc(&pdev->dev, sizeof(*imx_phy), GFP_KERNEL);
+	if (!imx_phy)
+		return -ENOMEM;
+
+	imx_phy->phy.io_priv = base;
+	imx_phy->phy.dev = &pdev->dev;
+	imx_phy->phy.label = DRIVER_NAME;
+	imx_phy->phy.init = imx_utmi_phy_init;
+	imx_phy->phy.shutdown = imx_utmi_phy_shutdown;
+	imx_phy->clk = clk;
+	platform_set_drvdata(pdev, &imx_phy->phy);
+
+	return 0;
+}
+
+static const struct of_device_id imx_utmi_dt_ids[] = {
+	{ .compatible = "fsl,imx6q-usbphy", },
+	{ /* sentinel */ }
+};
+
+static struct platform_driver imx_utmi_driver = {
+	.probe = imx_utmi_probe,
+	.driver = {
+		.name = DRIVER_NAME,
+		.of_match_table = imx_utmi_dt_ids,
+	 },
+};
+

drivers/usb/chipidea/phy-imx-utmi.c line 139

Quoted 10 lines
drivers/usb/chipidea/phy-imx-utmi.c new file +10 -0
+static int __init imx_utmi_init(void)
+{
+	return platform_driver_register(&imx_utmi_driver);
+}
+postcore_initcall(imx_utmi_init);
+
+static void __exit imx_utmi_exit(void)
+{
+	platform_driver_unregister(&imx_utmi_driver);
+}

module_platform_driver() will make this clearer ... check my ci13xxx_mxs.c binding glue for usage.

Quoted 1 line
drivers/usb/chipidea/phy-imx-utmi.c new file +1 -0
+module_exit(imx_utmi_exit);
#14 lore raw

PATCH 4/7 usb: chipidea: add imx driver binding

In reply to Richard Zhao
---
 drivers/usb/chipidea/Makefile      |    2 +-
 drivers/usb/chipidea/ci13xxx_imx.c |  177 ++++++++++++++++++++++++++++++++++++
 2 files changed, 178 insertions(+), 1 deletions(-)
 create mode 100644 drivers/usb/chipidea/ci13xxx_imx.c
drivers/usb/chipidea/Makefile +1 -1
index 0f34c0c..6821385 100644
@@ -14,5 +14,5 @@ ifneq ($(CONFIG_ARCH_MSM),)
 endif
 
 ifneq ($(CONFIG_ARCH_MXC),)
-	obj-$(CONFIG_USB_CHIPIDEA)	+= phy-imx-utmi.o
+	obj-$(CONFIG_USB_CHIPIDEA)	+= phy-imx-utmi.o ci13xxx_imx.o
 endif
drivers/usb/chipidea/ci13xxx_imx.c new file +177 -0

177 lines not on this page · raw message

Signature · 1 line
1.7.5.4
#15 lore raw

PATCH 4/7 usb: chipidea: add imx driver binding

Quoted 18 lines · Richard Zhao
drivers/usb/chipidea/ci13xxx_imx.c new file +18 -0
+	plat_ci = platform_device_alloc("ci_hdrc", (int)res->start);
+	if (!plat_ci) {
+		dev_err(&pdev->dev, "can't allocate ci_hdrc platform device\n");
+		return -ENOMEM;
+	}
+
+	ret = platform_device_add_resources(plat_ci, pdev->resource,
+					    pdev->num_resources);
+	if (ret) {
+		dev_err(&pdev->dev, "can't add resources to platform device\n");
+		goto put_platform;
+	}
+
+	ret = platform_device_add_data(plat_ci, &ci13xxx_imx_udc_driver,
+				       sizeof(ci13xxx_imx_udc_driver));
+	if (ret)
+		goto put_platform;
+

drivers/usb/chipidea/ci13xxx_imx.c line 138

Quoted 10 lines
drivers/usb/chipidea/ci13xxx_imx.c new file +10 -0
+	plat_ci->dev.dma_mask =
+		kmalloc(sizeof(*pdev->dev.dma_mask), GFP_KERNEL);
+	if (!plat_ci->dev.dma_mask) {
+		ret = -ENOMEM;
+		goto put_platform;
+	}
+	*plat_ci->dev.dma_mask = DMA_BIT_MASK(32);
+	plat_ci->dev.coherent_dma_mask = DMA_BIT_MASK(32);
+
+	ret = platform_device_add(plat_ci);

Is there a reason not to use platform_device_register_full() ?

Also, you don't set the parent device - is there a dependency between 'pdev' and this new platform device you're creating? If so, it should be a child of 'pdev'.

#16 lore raw

PATCH 4/7 usb: chipidea: add imx driver binding

Quoted 32 lines · Russell King - ARM Linux
Quoted 18 lines · Richard Zhao
drivers/usb/chipidea/ci13xxx_imx.c new file +18 -0
+	plat_ci = platform_device_alloc("ci_hdrc", (int)res->start);
+	if (!plat_ci) {
+		dev_err(&pdev->dev, "can't allocate ci_hdrc platform device\n");
+		return -ENOMEM;
+	}
+
+	ret = platform_device_add_resources(plat_ci, pdev->resource,
+					    pdev->num_resources);
+	if (ret) {
+		dev_err(&pdev->dev, "can't add resources to platform device\n");
+		goto put_platform;
+	}
+
+	ret = platform_device_add_data(plat_ci, &ci13xxx_imx_udc_driver,
+				       sizeof(ci13xxx_imx_udc_driver));
+	if (ret)
+		goto put_platform;
+

drivers/usb/chipidea/ci13xxx_imx.c line 138

Quoted 10 lines
drivers/usb/chipidea/ci13xxx_imx.c new file +10 -0
+	plat_ci->dev.dma_mask =
+		kmalloc(sizeof(*pdev->dev.dma_mask), GFP_KERNEL);
+	if (!plat_ci->dev.dma_mask) {
+		ret = -ENOMEM;
+		goto put_platform;
+	}
+	*plat_ci->dev.dma_mask = DMA_BIT_MASK(32);
+	plat_ci->dev.coherent_dma_mask = DMA_BIT_MASK(32);
+
+	ret = platform_device_add(plat_ci);

Is there a reason not to use platform_device_register_full() ?

Quoted 3 lines

Also, you don't set the parent device - is there a dependency between 'pdev' and this new platform device you're creating? If so, it should be a child of 'pdev'.

Thanks for your comment. All taken.

Richard

#17 lore raw

PATCH 4/7 usb: chipidea: add imx driver binding

In reply to Russell King - ARM Linux
Quoted 23 lines · Russell King - ARM Linux
Quoted 22 lines · Richard Zhao
drivers/usb/chipidea/ci13xxx_imx.c new file +22 -0
+	plat_ci = platform_device_alloc("ci_hdrc", (int)res->start);
+	if (!plat_ci) {
+		dev_err(&pdev->dev, "can't allocate ci_hdrc platform device\n");
+		return -ENOMEM;
+	}
+
+	ret = platform_device_add_resources(plat_ci, pdev->resource,
+					    pdev->num_resources);
+	if (ret) {
+		dev_err(&pdev->dev, "can't add resources to platform device\n");
+		goto put_platform;
+	}
+
+	ret = platform_device_add_data(plat_ci, &ci13xxx_imx_udc_driver,
+				       sizeof(ci13xxx_imx_udc_driver));
+	if (ret)
+		goto put_platform;
+
+	plat_ci->dev.dma_mask =
+		kmalloc(sizeof(*pdev->dev.dma_mask), GFP_KERNEL);
+	if (!plat_ci->dev.dma_mask) {
+		ret = -ENOMEM;
Quoted 8 lines

drivers/usb/chipidea/ci13xxx_imx.c line 138

Quoted 6 lines
drivers/usb/chipidea/ci13xxx_imx.c new file +6 -0
+		goto put_platform;
+	}
+	*plat_ci->dev.dma_mask = DMA_BIT_MASK(32);
+	plat_ci->dev.coherent_dma_mask = DMA_BIT_MASK(32);
+
+	ret = platform_device_add(plat_ci);

Is there a reason not to use platform_device_register_full() ?

platform_device_register_full() allocate the dma_mask but platform_device_unregister() didn't free it. I have to free it manually? Sounds strange.

Thanks
Richard

Quoted 5 lines

Also, you don't set the parent device - is there a dependency between 'pdev' and this new platform device you're creating? If so, it should be a child of 'pdev'.

#18 lore raw

PATCH 4/7 usb: chipidea: add imx driver binding

In reply to Richard Zhao

Dear Richard Zhao,

Quoted 13 lines
---
 drivers/usb/chipidea/Makefile      |    2 +-
 drivers/usb/chipidea/ci13xxx_imx.c |  177
++++++++++++++++++++++++++++++++++++ 2 files changed, 178 insertions(+), 1
deletions(-)
 create mode 100644 drivers/usb/chipidea/ci13xxx_imx.c
drivers/usb/chipidea/Makefile +0 -0
index 0f34c0c..6821385 100644

drivers/usb/chipidea/Makefile ifneq ($(CONFIG_ARCH_MSM),) · line 17

Quoted 5 lines
drivers/usb/chipidea/Makefile +1 -1
@@ -14,5 +14,5 @@ ifneq ($(CONFIG_ARCH_MSM),)
 endif
 
 ifneq ($(CONFIG_ARCH_MXC),)
-	obj-$(CONFIG_USB_CHIPIDEA)	+= phy-imx-utmi.o
+	obj-$(CONFIG_USB_CHIPIDEA)	+= phy-imx-utmi.o ci13xxx_imx.o

You're not actually supposed to use the UTMI PHY always. The PHY is separate from the USB stuff, so maybe put it into drivers/usb/otg/ like I had my driver placed?

Quoted 27 lines
drivers/usb/chipidea/Makefile +0 -0
 endif
(quoted diff) +0 -0

b/drivers/usb/chipidea/ci13xxx_imx.c new file mode 100644 index 0000000..5f36f07

drivers/usb/chipidea/ci13xxx_imx.c new file +20 -0
@@ -0,0 +1,177 @@
+/*
+ * Copyright 2012 Freescale Semiconductor, Inc.
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/of_platform.h>
+#include <linux/of_gpio.h>
+#include <linux/pm_runtime.h>
+#include <linux/usb/ulpi.h>
+#include <linux/usb/gadget.h>
+#include <linux/usb/chipidea.h>
+#include <linux/usb/otg.h>

drivers/usb/chipidea/ci13xxx_imx.c line 30

Quoted 10 lines
drivers/usb/chipidea/ci13xxx_imx.c new file +10 -0
+#include <linux/dma-mapping.h>
+
+#include "ci.h"
+
+#define pdev_to_phy(pdev) \
+	((struct usb_phy *)platform_get_drvdata(pdev))
+
+static void ci13xxx_imx_notify_event(struct ci13xxx *udc, unsigned event)
+{
+#if 0

Woooo, what a big slab of dead code :-)

Quoted 90 lines
drivers/usb/chipidea/ci13xxx_imx.c new file +90 -0
+	struct device *dev = udc->gadget.dev.parent;
+	int val;
+
+	switch (event) {
+	case CI13XXX_CONTROLLER_RESET_EVENT:
+		dev_dbg(dev, "CI13XXX_CONTROLLER_RESET_EVENT received\n");
+		writel(0, USB_AHBBURST);
+		writel(0, USB_AHBMODE);
+		break;
+	case CI13XXX_CONTROLLER_STOPPED_EVENT:
+		dev_dbg(dev, "CI13XXX_CONTROLLER_STOPPED_EVENT received\n");
+		/*
+		 * Put the transceiver in non-driving mode. Otherwise host
+		 * may not detect soft-disconnection.
+		 */
+		val = usb_phy_io_read(udc->transceiver, ULPI_FUNC_CTRL);
+		val &= ~ULPI_FUNC_CTRL_OPMODE_MASK;
+		val |= ULPI_FUNC_CTRL_OPMODE_NONDRIVING;
+		usb_phy_io_write(udc->transceiver, val, ULPI_FUNC_CTRL);
+		break;
+	default:
+		dev_dbg(dev, "unknown ci13xxx_udc event\n");
+		break;
+	}
+#endif
+}
+
+static struct ci13xxx_udc_driver ci13xxx_imx_udc_driver = {
+	.name			= "ci13xxx_imx",
+	.flags			= CI13XXX_REGS_SHARED |
+				  CI13XXX_PULLUP_ON_VBUS |
+				  CI13XXX_DISABLE_STREAMING,
+	.capoffset		= 0x100,
+
+	.notify_event		= ci13xxx_imx_notify_event,
+};
+
+static int ci13xxx_imx_probe(struct platform_device *pdev)
+{
+	struct platform_device *plat_ci, *phy_pdev;
+	struct device_node *phy_np;
+	struct resource *res;
+	int hub_reset, vbus_pwr;
+	int ret;
+
+	dev_dbg(&pdev->dev, "ci13xxx_imx_probe\n");
+
+	phy_np = of_parse_phandle(pdev->dev.of_node, "fsl,usbphy", 0);
+	if (phy_np) {
+		phy_pdev = of_find_device_by_node(phy_np);
+		if (phy_pdev) {
+			struct usb_phy *phy;
+			phy = pdev_to_phy(phy_pdev);
+			if (phy)
+				usb_phy_init(phy);
+		}
+		of_node_put(phy_np);
+	}
+
+	hub_reset = of_get_named_gpio(pdev->dev.of_node, "fsl,hub-reset", 0);
+	if (hub_reset > 0 &&
+	    devm_gpio_request(&pdev->dev, hub_reset, "hub-reset")) {
+		gpio_direction_output(hub_reset, 0);
+		udelay(10);
+		gpio_direction_output(hub_reset, 1);
+	}
+
+	/* we only support host now, so enable vbus here */
+	vbus_pwr = of_get_named_gpio(pdev->dev.of_node, "fsl,vbus-power", 0);
+	if (vbus_pwr > 0 &&
+	    devm_gpio_request(&pdev->dev, vbus_pwr, "vbus-pwr")) {
+		gpio_direction_output(vbus_pwr, 1);
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "can't get device resources\n");
+		return -ENOENT;
+	}
+
+	plat_ci = platform_device_alloc("ci_hdrc", (int)res->start);
+	if (!plat_ci) {
+		dev_err(&pdev->dev, "can't allocate ci_hdrc platform device\n");
+		return -ENOMEM;
+	}
+
+	ret = platform_device_add_resources(plat_ci, pdev->resource,
+					    pdev->num_resources);
+	if (ret) {
+		dev_err(&pdev->dev, "can't add resources to platform device\n");

drivers/usb/chipidea/ci13xxx_imx.c line 130

Quoted 10 lines
drivers/usb/chipidea/ci13xxx_imx.c new file +10 -0
+		goto put_platform;
+	}
+
+	ret = platform_device_add_data(plat_ci, &ci13xxx_imx_udc_driver,
+				       sizeof(ci13xxx_imx_udc_driver));
+	if (ret)
+		goto put_platform;
+
+	plat_ci->dev.dma_mask =
+		kmalloc(sizeof(*pdev->dev.dma_mask), GFP_KERNEL);

Can this not be devm_kzalloc()ated?

Quoted 25 lines
drivers/usb/chipidea/ci13xxx_imx.c new file +25 -0
+	if (!plat_ci->dev.dma_mask) {
+		ret = -ENOMEM;
+		goto put_platform;
+	}
+	*plat_ci->dev.dma_mask = DMA_BIT_MASK(32);
+	plat_ci->dev.coherent_dma_mask = DMA_BIT_MASK(32);
+
+	ret = platform_device_add(plat_ci);
+	if (ret)
+		goto free_dma_mask;
+
+	pm_runtime_no_callbacks(&pdev->dev);
+	pm_runtime_enable(&pdev->dev);
+
+	return 0;
+
+free_dma_mask:
+	kfree(plat_ci->dev.dma_mask);
+	plat_ci->dev.dma_mask = NULL;
+put_platform:
+	platform_device_put(plat_ci);
+
+	return ret;
+}
+

drivers/usb/chipidea/ci13xxx_imx.c line 165

Quoted 10 lines
drivers/usb/chipidea/ci13xxx_imx.c new file +10 -0
+static const struct of_device_id ci13xxx_imx_dt_ids[] = {
+	{ .compatible = "fsl,imx6q-usboh3", },
+	{ /* sentinel */ }
+};
+
+static struct platform_driver ci13xxx_imx_driver = {
+	.probe = ci13xxx_imx_probe,
+	.driver = {
+		.name = "imx_usboh3",
+		.of_match_table = ci13xxx_imx_dt_ids,

you're missing .remove() call here, check my implementation ;-)

drivers/usb/chipidea/ci13xxx_imx.c line 175

Quoted 10 lines
drivers/usb/chipidea/ci13xxx_imx.c new file +10 -0
+	 },
+};
+
+MODULE_ALIAS("platform:imx_usboh3");
+
+static int __init ci13xxx_imx_init(void)
+{
+	return platform_driver_register(&ci13xxx_imx_driver);
+}
+module_init(ci13xxx_imx_init);

module_platform_driver().

Quoted 2 lines
drivers/usb/chipidea/ci13xxx_imx.c new file +2 -0
+
+MODULE_LICENSE("GPL v2");
#19 lore raw

PATCH 4/7 usb: chipidea: add imx driver binding

In reply to Richard Zhao
Quoted 1 line · Richard Zhao

Besides things that were already pointed out, some minor comments below:

Quoted 76 lines
---
 drivers/usb/chipidea/Makefile      |    2 +-
 drivers/usb/chipidea/ci13xxx_imx.c |  177 ++++++++++++++++++++++++++++++++++++
 2 files changed, 178 insertions(+), 1 deletions(-)
 create mode 100644 drivers/usb/chipidea/ci13xxx_imx.c
drivers/usb/chipidea/Makefile +1 -1
index 0f34c0c..6821385 100644
@@ -14,5 +14,5 @@ ifneq ($(CONFIG_ARCH_MSM),)
 endif
 
 ifneq ($(CONFIG_ARCH_MXC),)
-	obj-$(CONFIG_USB_CHIPIDEA)	+= phy-imx-utmi.o
+	obj-$(CONFIG_USB_CHIPIDEA)	+= phy-imx-utmi.o ci13xxx_imx.o
 endif
drivers/usb/chipidea/ci13xxx_imx.c new file +53 -0
new file mode 100644
index 0000000..5f36f07
@@ -0,0 +1,177 @@
+/*
+ * Copyright 2012 Freescale Semiconductor, Inc.
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/of_platform.h>
+#include <linux/of_gpio.h>
+#include <linux/pm_runtime.h>
+#include <linux/usb/ulpi.h>
+#include <linux/usb/gadget.h>
+#include <linux/usb/chipidea.h>
+#include <linux/usb/otg.h>
+#include <linux/dma-mapping.h>
+
+#include "ci.h"
+
+#define pdev_to_phy(pdev) \
+	((struct usb_phy *)platform_get_drvdata(pdev))
+
+static void ci13xxx_imx_notify_event(struct ci13xxx *udc, unsigned event)
+{
+#if 0
+	struct device *dev = udc->gadget.dev.parent;
+	int val;
+
+	switch (event) {
+	case CI13XXX_CONTROLLER_RESET_EVENT:
+		dev_dbg(dev, "CI13XXX_CONTROLLER_RESET_EVENT received\n");
+		writel(0, USB_AHBBURST);
+		writel(0, USB_AHBMODE);
+		break;
+	case CI13XXX_CONTROLLER_STOPPED_EVENT:
+		dev_dbg(dev, "CI13XXX_CONTROLLER_STOPPED_EVENT received\n");
+		/*
+		 * Put the transceiver in non-driving mode. Otherwise host
+		 * may not detect soft-disconnection.
+		 */
+		val = usb_phy_io_read(udc->transceiver, ULPI_FUNC_CTRL);
+		val &= ~ULPI_FUNC_CTRL_OPMODE_MASK;
+		val |= ULPI_FUNC_CTRL_OPMODE_NONDRIVING;
+		usb_phy_io_write(udc->transceiver, val, ULPI_FUNC_CTRL);
+		break;
+	default:
+		dev_dbg(dev, "unknown ci13xxx_udc event\n");
+		break;

drivers/usb/chipidea/ci13xxx_imx.c line 63

Quoted 10 lines
drivers/usb/chipidea/ci13xxx_imx.c new file +10 -0
+	}
+#endif
+}
+
+static struct ci13xxx_udc_driver ci13xxx_imx_udc_driver = {
+	.name			= "ci13xxx_imx",
+	.flags			= CI13XXX_REGS_SHARED |
+				  CI13XXX_PULLUP_ON_VBUS |
+				  CI13XXX_DISABLE_STREAMING,
+	.capoffset		= 0x100,

There's a DEF_CAPOFFSET for this in <linux/usb/chipidea.h>, because this is really a single most popular offset of the capability registers in chipidea controllers.

drivers/usb/chipidea/ci13xxx_imx.c line 76

Quoted 13 lines
drivers/usb/chipidea/ci13xxx_imx.c new file +13 -0
+
+	.notify_event		= ci13xxx_imx_notify_event,
+};
+
+static int ci13xxx_imx_probe(struct platform_device *pdev)
+{
+	struct platform_device *plat_ci, *phy_pdev;
+	struct device_node *phy_np;
+	struct resource *res;
+	int hub_reset, vbus_pwr;
+	int ret;
+
+	dev_dbg(&pdev->dev, "ci13xxx_imx_probe\n");

Certain people don't take kindly to adding debug prints like this one. :)

Quoted 25 lines
drivers/usb/chipidea/ci13xxx_imx.c new file +25 -0
+
+	phy_np = of_parse_phandle(pdev->dev.of_node, "fsl,usbphy", 0);
+	if (phy_np) {
+		phy_pdev = of_find_device_by_node(phy_np);
+		if (phy_pdev) {
+			struct usb_phy *phy;
+			phy = pdev_to_phy(phy_pdev);
+			if (phy)
+				usb_phy_init(phy);
+		}
+		of_node_put(phy_np);
+	}
+
+	hub_reset = of_get_named_gpio(pdev->dev.of_node, "fsl,hub-reset", 0);
+	if (hub_reset > 0 &&
+	    devm_gpio_request(&pdev->dev, hub_reset, "hub-reset")) {
+		gpio_direction_output(hub_reset, 0);
+		udelay(10);
+		gpio_direction_output(hub_reset, 1);
+	}
+
+	/* we only support host now, so enable vbus here */
+	vbus_pwr = of_get_named_gpio(pdev->dev.of_node, "fsl,vbus-power", 0);
+	if (vbus_pwr > 0 &&
+	    devm_gpio_request(&pdev->dev, vbus_pwr, "vbus-pwr")) {

drivers/usb/chipidea/ci13xxx_imx.c line 111

Quoted 10 lines
drivers/usb/chipidea/ci13xxx_imx.c new file +10 -0
+		gpio_direction_output(vbus_pwr, 1);
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "can't get device resources\n");
+		return -ENOENT;
+	}
+
+	plat_ci = platform_device_alloc("ci_hdrc", (int)res->start);

Regarding the instance id here: you probably want to use something more meaningful than the starting address. I was thinking about stealing id allocator from Felipe's dwc3 (dwc3_get_device_id()), but decided to first deal with more important things.

I was considering using idr for this, but it kind of feels like an overkill for this purpose. So I'm wondering if it makes sense to generalize dwc3_get_device_id() so that other drivers can make use of it?

Quoted 74 lines
drivers/usb/chipidea/ci13xxx_imx.c new file +66 -0
+	if (!plat_ci) {
+		dev_err(&pdev->dev, "can't allocate ci_hdrc platform device\n");
+		return -ENOMEM;
+	}
+
+	ret = platform_device_add_resources(plat_ci, pdev->resource,
+					    pdev->num_resources);
+	if (ret) {
+		dev_err(&pdev->dev, "can't add resources to platform device\n");
+		goto put_platform;
+	}
+
+	ret = platform_device_add_data(plat_ci, &ci13xxx_imx_udc_driver,
+				       sizeof(ci13xxx_imx_udc_driver));
+	if (ret)
+		goto put_platform;
+
+	plat_ci->dev.dma_mask =
+		kmalloc(sizeof(*pdev->dev.dma_mask), GFP_KERNEL);
+	if (!plat_ci->dev.dma_mask) {
+		ret = -ENOMEM;
+		goto put_platform;
+	}
+	*plat_ci->dev.dma_mask = DMA_BIT_MASK(32);
+	plat_ci->dev.coherent_dma_mask = DMA_BIT_MASK(32);
+
+	ret = platform_device_add(plat_ci);
+	if (ret)
+		goto free_dma_mask;
+
+	pm_runtime_no_callbacks(&pdev->dev);
+	pm_runtime_enable(&pdev->dev);
+
+	return 0;
+
+free_dma_mask:
+	kfree(plat_ci->dev.dma_mask);
+	plat_ci->dev.dma_mask = NULL;
+put_platform:
+	platform_device_put(plat_ci);
+
+	return ret;
+}
+
+static const struct of_device_id ci13xxx_imx_dt_ids[] = {
+	{ .compatible = "fsl,imx6q-usboh3", },
+	{ /* sentinel */ }
+};
+
+static struct platform_driver ci13xxx_imx_driver = {
+	.probe = ci13xxx_imx_probe,
+	.driver = {
+		.name = "imx_usboh3",
+		.of_match_table = ci13xxx_imx_dt_ids,
+	 },
+};
+
+MODULE_ALIAS("platform:imx_usboh3");
+
+static int __init ci13xxx_imx_init(void)
+{
+	return platform_driver_register(&ci13xxx_imx_driver);
+}
+module_init(ci13xxx_imx_init);
+
+MODULE_LICENSE("GPL v2");
Signature · 7 lines
1.7.5.4


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#20 lore raw

PATCH 4/7 usb: chipidea: add imx driver binding

Hi,

Quoted 11 lines · Alexander Shishkin

drivers/usb/chipidea/ci13xxx_imx.c line 111

Quoted 1 line
drivers/usb/chipidea/ci13xxx_imx.c new file +1 -0
+	plat_ci = platform_device_alloc("ci_hdrc", (int)res->start);

Regarding the instance id here: you probably want to use something more meaningful than the starting address. I was thinking about stealing id allocator from Felipe's dwc3 (dwc3_get_device_id()), but decided to first deal with more important things.

I was considering using idr for this, but it kind of feels like an overkill for this purpose. So I'm wondering if it makes sense to generalize dwc3_get_device_id() so that other drivers can make use of it?

Yeah, I once considered a situation were the platform_bus implementation would handle that for us. never got to implement it though.

I would be cool to see though. Not sure how Greg would feel about it.

Signature · 8 lines
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120516/227f265c/attachment.sig>
#21 lore raw

PATCH 4/7 usb: chipidea: add imx driver binding

In reply to Alexander Shishkin
Quoted 5 lines · Alexander Shishkin
Quoted 1 line · Richard Zhao

Besides things that were already pointed out, some minor comments below:

Thanks for your review.

Quoted 83 lines
Quoted 83 lines
---
 drivers/usb/chipidea/Makefile      |    2 +-
 drivers/usb/chipidea/ci13xxx_imx.c |  177 ++++++++++++++++++++++++++++++++++++
 2 files changed, 178 insertions(+), 1 deletions(-)
 create mode 100644 drivers/usb/chipidea/ci13xxx_imx.c
drivers/usb/chipidea/Makefile +1 -1
index 0f34c0c..6821385 100644
@@ -14,5 +14,5 @@ ifneq ($(CONFIG_ARCH_MSM),)
 endif
 
 ifneq ($(CONFIG_ARCH_MXC),)
-	obj-$(CONFIG_USB_CHIPIDEA)	+= phy-imx-utmi.o
+	obj-$(CONFIG_USB_CHIPIDEA)	+= phy-imx-utmi.o ci13xxx_imx.o
 endif
drivers/usb/chipidea/ci13xxx_imx.c new file +59 -0
new file mode 100644
index 0000000..5f36f07
@@ -0,0 +1,177 @@
+/*
+ * Copyright 2012 Freescale Semiconductor, Inc.
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/of_platform.h>
+#include <linux/of_gpio.h>
+#include <linux/pm_runtime.h>
+#include <linux/usb/ulpi.h>
+#include <linux/usb/gadget.h>
+#include <linux/usb/chipidea.h>
+#include <linux/usb/otg.h>
+#include <linux/dma-mapping.h>
+
+#include "ci.h"
+
+#define pdev_to_phy(pdev) \
+	((struct usb_phy *)platform_get_drvdata(pdev))
+
+static void ci13xxx_imx_notify_event(struct ci13xxx *udc, unsigned event)
+{
+#if 0
+	struct device *dev = udc->gadget.dev.parent;
+	int val;
+
+	switch (event) {
+	case CI13XXX_CONTROLLER_RESET_EVENT:
+		dev_dbg(dev, "CI13XXX_CONTROLLER_RESET_EVENT received\n");
+		writel(0, USB_AHBBURST);
+		writel(0, USB_AHBMODE);
+		break;
+	case CI13XXX_CONTROLLER_STOPPED_EVENT:
+		dev_dbg(dev, "CI13XXX_CONTROLLER_STOPPED_EVENT received\n");
+		/*
+		 * Put the transceiver in non-driving mode. Otherwise host
+		 * may not detect soft-disconnection.
+		 */
+		val = usb_phy_io_read(udc->transceiver, ULPI_FUNC_CTRL);
+		val &= ~ULPI_FUNC_CTRL_OPMODE_MASK;
+		val |= ULPI_FUNC_CTRL_OPMODE_NONDRIVING;
+		usb_phy_io_write(udc->transceiver, val, ULPI_FUNC_CTRL);
+		break;
+	default:
+		dev_dbg(dev, "unknown ci13xxx_udc event\n");
+		break;
+	}
+#endif
+}
+
+static struct ci13xxx_udc_driver ci13xxx_imx_udc_driver = {
+	.name			= "ci13xxx_imx",
Quoted 8 lines

drivers/usb/chipidea/ci13xxx_imx.c line 63

Quoted 4 lines
drivers/usb/chipidea/ci13xxx_imx.c new file +4 -0
+	.flags			= CI13XXX_REGS_SHARED |
+				  CI13XXX_PULLUP_ON_VBUS |
+				  CI13XXX_DISABLE_STREAMING,
+	.capoffset		= 0x100,

There's a DEF_CAPOFFSET for this in <linux/usb/chipidea.h>, because this is really a single most popular offset of the capability registers in chipidea controllers.

Thanks.

Quoted 16 lines
Quoted 4 lines
drivers/usb/chipidea/ci13xxx_imx.c new file +3 -0
+
+	.notify_event		= ci13xxx_imx_notify_event,
+};

drivers/usb/chipidea/ci13xxx_imx.c line 76

Quoted 10 lines
drivers/usb/chipidea/ci13xxx_imx.c new file +10 -0
+
+static int ci13xxx_imx_probe(struct platform_device *pdev)
+{
+	struct platform_device *plat_ci, *phy_pdev;
+	struct device_node *phy_np;
+	struct resource *res;
+	int hub_reset, vbus_pwr;
+	int ret;
+
+	dev_dbg(&pdev->dev, "ci13xxx_imx_probe\n");

Certain people don't take kindly to adding debug prints like this one. :)

Removed. Thanks.

Quoted 33 lines
Quoted 33 lines
drivers/usb/chipidea/ci13xxx_imx.c new file +32 -0
+
+	phy_np = of_parse_phandle(pdev->dev.of_node, "fsl,usbphy", 0);
+	if (phy_np) {
+		phy_pdev = of_find_device_by_node(phy_np);
+		if (phy_pdev) {
+			struct usb_phy *phy;
+			phy = pdev_to_phy(phy_pdev);
+			if (phy)
+				usb_phy_init(phy);
+		}
+		of_node_put(phy_np);
+	}
+
+	hub_reset = of_get_named_gpio(pdev->dev.of_node, "fsl,hub-reset", 0);
+	if (hub_reset > 0 &&
+	    devm_gpio_request(&pdev->dev, hub_reset, "hub-reset")) {
+		gpio_direction_output(hub_reset, 0);
+		udelay(10);
+		gpio_direction_output(hub_reset, 1);
+	}
+
+	/* we only support host now, so enable vbus here */
+	vbus_pwr = of_get_named_gpio(pdev->dev.of_node, "fsl,vbus-power", 0);
+	if (vbus_pwr > 0 &&
+	    devm_gpio_request(&pdev->dev, vbus_pwr, "vbus-pwr")) {
+		gpio_direction_output(vbus_pwr, 1);
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "can't get device resources\n");
+		return -ENOENT;
Quoted 8 lines

drivers/usb/chipidea/ci13xxx_imx.c line 111

Quoted 3 lines
drivers/usb/chipidea/ci13xxx_imx.c new file +3 -0
+	}
+
+	plat_ci = platform_device_alloc("ci_hdrc", (int)res->start);

Regarding the instance id here: you probably want to use something more meaningful than the starting address. I was thinking about stealing id allocator from Felipe's dwc3 (dwc3_get_device_id()), but decided to first deal with more important things.

hmm... Is dwc3_get_device_id() really meaningful than base address? It just record the sequence number.

Thanks
Richard

Quoted 6 lines

I was considering using idr for this, but it kind of feels like an overkill for this purpose. So I'm wondering if it makes sense to generalize dwc3_get_device_id() so that other drivers can make use of it?

#22 lore raw

PATCH 4/7 usb: chipidea: add imx driver binding

Quoted 9 lines · Richard Zhao
Quoted 6 lines · Alexander Shishkin

drivers/usb/chipidea/ci13xxx_imx.c line 111

Quoted 1 line
drivers/usb/chipidea/ci13xxx_imx.c new file +1 -0
+	plat_ci = platform_device_alloc("ci_hdrc", (int)res->start);

Regarding the instance id here: you probably want to use something more meaningful than the starting address. I was thinking about stealing id allocator from Felipe's dwc3 (dwc3_get_device_id()), but decided to first deal with more important things.

hmm... Is dwc3_get_device_id() really meaningful than base address? It just record the sequence number.

Well, if you indeed have more than one controller, you might want to know, for example, which one registered first. Or, one might simply not like the look of "/sys/bus/platform/devices/ci_hdrc.-5898240".

Regards,
--
Alex

#23 lore raw

PATCH 4/7 usb: chipidea: add imx driver binding

Quoted 9 lines · Alexander Shishkin
Quoted 7 lines · Richard Zhao
Quoted 6 lines · Alexander Shishkin

drivers/usb/chipidea/ci13xxx_imx.c line 111

Quoted 1 line
drivers/usb/chipidea/ci13xxx_imx.c new file +1 -0
+	plat_ci = platform_device_alloc("ci_hdrc", (int)res->start);

Regarding the instance id here: you probably want to use something more meaningful than the starting address. I was thinking about stealing id allocator from Felipe's dwc3 (dwc3_get_device_id()), but decided to first deal with more important things.

Quoted 6 lines
Quoted 2 lines

hmm... Is dwc3_get_device_id() really meaningful than base address? It just record the sequence number.

Well, if you indeed have more than one controller, you might want to know, for example, which one registered first. Or, one might simply not like the look of "/sys/bus/platform/devices/ci_hdrc.-5898240".

ah, it's singed integer. Then I agree dwc3_get_device_id() is a better and simple way, and we can find the physical device by looking at ci_hdrc devicie's parent.

Thanks
Richard

Quoted 5 lines

Regards,
--
Alex

#24 lore raw

PATCH 5/7 ARM: imx6q: correct device name of usbphy and usboh3 clock export

In reply to Richard Zhao
---
 arch/arm/mach-imx/clk-imx6q.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)
arch/arm/mach-imx/clk-imx6q.c +6 -1
index 1618989..f7eb965 100644
@@ -390,7 +390,12 @@ int __init mx6q_clocks_init(void)
 	clk_register_clkdev(clk[gpt_ipg], "ipg", "imx-gpt.0");
 	clk_register_clkdev(clk[gpt_ipg_per], "per", "imx-gpt.0");
 	clk_register_clkdev(clk[twd], NULL, "smp_twd");
-	clk_register_clkdev(clk[usboh3], NULL, "usboh3");
+	clk_register_clkdev(clk[usboh3], NULL, "2184000.usboh3");
+	clk_register_clkdev(clk[usboh3], NULL, "2184200.usboh3");
+	clk_register_clkdev(clk[usboh3], NULL, "2184400.usboh3");
+	clk_register_clkdev(clk[usboh3], NULL, "2184600.usboh3");
+	clk_register_clkdev(clk[pll3_usb_otg], NULL, "20c9000.usbphy");
+	clk_register_clkdev(clk[pll3_usb_otg], NULL, "20ca000.usbphy");
 	clk_register_clkdev(clk[uart_serial], "per", "2020000.serial");
 	clk_register_clkdev(clk[uart_ipg], "ipg", "2020000.serial");
 	clk_register_clkdev(clk[uart_serial], "per", "21e8000.serial");
Signature · 1 line
1.7.5.4
#25 lore raw

PATCH 6/7 ARM: imx6q: add anatop initialization for usb controllers

In reply to Richard Zhao
---
 arch/arm/mach-imx/mach-imx6q.c |   60 ++++++++++++++++++++++++++++++++++++++++
 1 files changed, 60 insertions(+), 0 deletions(-)
arch/arm/mach-imx/mach-imx6q.c +60 -0
index 74e44d3..4530c45 100644
@@ -22,6 +22,7 @@
 #include <linux/pinctrl/machine.h>
 #include <linux/phy.h>
 #include <linux/micrel_phy.h>
+#include <linux/mfd/anatop.h>
 #include <asm/smp_twd.h>
 #include <asm/hardware/cache-l2x0.h>
 #include <asm/hardware/gic.h>
@@ -82,6 +83,63 @@ static void __init imx6q_sabrelite_init(void)
 				ksz9021rn_phy_fixup);
 }
 
+static void __init imx6q_post_populate(void)
+{
+	u32 val;
+
+#define HW_ANADIG_USB2_CHRG_DETECT	(0x00000210)
+#define BM_ANADIG_USB2_CHRG_DETECT_EN_B 0x00100000
+#define BM_ANADIG_USB2_CHRG_DETECT_CHK_CHRG_B 0x00080000
+
+#define HW_ANADIG_USB2_PLL_480_CTRL	(0x00000020)
+#define BM_ANADIG_USB2_PLL_480_CTRL_BYPASS 0x00010000
+#define BM_ANADIG_USB2_PLL_480_CTRL_ENABLE 0x00002000
+#define BM_ANADIG_USB2_PLL_480_CTRL_POWER 0x00001000
+#define BM_ANADIG_USB2_PLL_480_CTRL_EN_USB_CLKS 0x00000040
+
+	/* Some phy and power's special controls for host1
+	 * 1. The external charger detector needs to be disabled
+	 * or the signal at DP will be poor
+	 * 2. The PLL's power and output to usb for host 1
+	 * is totally controlled by IC, so the Software only needs
+	 * to enable them at initializtion.
+	 */
+
+	anatop_write_reg(NULL, HW_ANADIG_USB2_CHRG_DETECT,
+			BM_ANADIG_USB2_CHRG_DETECT_EN_B |
+			BM_ANADIG_USB2_CHRG_DETECT_CHK_CHRG_B,
+			~0);
+	anatop_write_reg(NULL, HW_ANADIG_USB2_PLL_480_CTRL, 0,
+			BM_ANADIG_USB2_PLL_480_CTRL_BYPASS);
+
+	val = BM_ANADIG_USB2_PLL_480_CTRL_ENABLE |
+	      BM_ANADIG_USB2_PLL_480_CTRL_POWER |
+	      BM_ANADIG_USB2_PLL_480_CTRL_EN_USB_CLKS;
+	anatop_write_reg(NULL, HW_ANADIG_USB2_PLL_480_CTRL, val, val);
+
+	/* Some phy and power's special controls for otg
+	 * 1. The external charger detector needs to be disabled
+	 * or the signal at DP will be poor
+	 * 2. The EN_USB_CLKS is always enabled.
+	 * The PLL's power is controlled by usb and others who
+	 * use pll3 too.
+	 */
+#define HW_ANADIG_USB1_CHRG_DETECT      (0x000001b0)
+#define BM_ANADIG_USB1_CHRG_DETECT_EN_B 0x00100000
+#define BM_ANADIG_USB1_CHRG_DETECT_CHK_CHRG_B 0x00080000
+
+#define HW_ANADIG_USB1_PLL_480_CTRL     (0x00000010)
+#define BM_ANADIG_USB1_PLL_480_CTRL_EN_USB_CLKS 0x00000040
+
+	anatop_write_reg(NULL, HW_ANADIG_USB1_CHRG_DETECT,
+			BM_ANADIG_USB1_CHRG_DETECT_EN_B
+			| BM_ANADIG_USB1_CHRG_DETECT_CHK_CHRG_B,
+			~0);
+	anatop_write_reg(NULL, HW_ANADIG_USB1_PLL_480_CTRL,
+			BM_ANADIG_USB1_PLL_480_CTRL_EN_USB_CLKS,
+			BM_ANADIG_USB1_PLL_480_CTRL_EN_USB_CLKS);
+}
+
 static void __init imx6q_init_machine(void)
 {
 	/*
@@ -95,6 +153,8 @@ static void __init imx6q_init_machine(void)
 
 	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
 
+	imx6q_post_populate();
+
 	imx6q_pm_init();
 }
 
Signature · 1 line
1.7.5.4
#26 lore raw

PATCH 7/7 ARM: dts: imx6q-sabrelite: add usb devices

In reply to Richard Zhao
---
 arch/arm/boot/dts/imx6q-sabrelite.dts |   19 +++++++++++++-
 arch/arm/boot/dts/imx6q.dtsi          |   44 +++++++++++++++++++++++++++++++-
 2 files changed, 60 insertions(+), 3 deletions(-)
arch/arm/boot/dts/imx6q-sabrelite.dts +18 -1
index 2f631f2..ccc794e 100644
@@ -41,10 +41,27 @@
 					status = "okay";
 				};
 			};
-
+			iomuxc at 020e0000 {
+				gpios {
+					pinctrl_gpio_hog: gpiohog {
+						fsl,pins = <1044 0x80000000
+							   144  0x80000000>;	/* MX6Q_PAD_GPIO_17__GPIO_7_12 */
+					};
+				};
+			};
 		};
 
 		aips-bus at 02100000 { /* AIPS2 */
+			usboh3 at 02184000 { /* USB OTG */
+				fsl,vbus-power = <&gpio3 22 0>;
+				status = "okay";
+			};
+
+			usboh3 at 02184200 { /* USB1 */
+				fsl,hub-reset = <&gpio7 12 0>;
+				status = "okay";
+			};
+
 			ethernet at 02188000 {
 				phy-mode = "rgmii";
 				phy-reset-gpios = <&gpio3 23 0>;
arch/arm/boot/dts/imx6q.dtsi +42 -2

67 lines not on this page · raw message

Signature · 1 line
1.7.5.4
#27 lore raw

cover preliminary work for adding imx6q_sabrelite usb support

In reply to Richard Zhao

Dear Richard Zhao,

Quoted 7 lines

The work is based on usb-next which includes ci13xxx rework by Alexander Shishkin.

Status:
- this version only tested host
- usbotg controler works at host role

Quoted 6 lines

Todo:
- usbh1 has strange system error when enumerate devices for on-board hub.
- high speed usb connecton detect failed for the second time. It needs
hub code hack.

Thanks Peter Chen for greate help.

Hm, I think we should somehow coordinate, because I think we're working on the same thing and we're duplicating work.

Quoted 10 lines

Richard Zhao (7):
usb: chipidea: permit user select USB_EHCI_ROOT_HUB_TT
usb: chipidea: remove zero check of hw_ep_max
usb: chipidea: add imx on-soc utmi phy driver
usb: chipidea: add imx driver binding
ARM: imx6q: correct device name of usbphy and usboh3 clock export
ARM: imx6q: add anatop initialization for usb controllers
ARM: dts: imx6q-sabrelite: add usb devices

Quoted 13 lines
 arch/arm/boot/dts/imx6q-sabrelite.dts |   19 ++++-
 arch/arm/boot/dts/imx6q.dtsi          |   44 ++++++++-
 arch/arm/mach-imx/clk-imx6q.c         |    7 +-
 arch/arm/mach-imx/mach-imx6q.c        |   60 +++++++++++
 drivers/usb/chipidea/Makefile         |    4 +
 drivers/usb/chipidea/ci13xxx_imx.c    |  177
+++++++++++++++++++++++++++++++++ drivers/usb/chipidea/core.c           | 
  2 +-
 drivers/usb/chipidea/phy-imx-utmi.c   |  140 ++++++++++++++++++++++++++
 drivers/usb/host/Kconfig              |    2 +-
 9 files changed, 449 insertions(+), 6 deletions(-)
 create mode 100644 drivers/usb/chipidea/ci13xxx_imx.c
 create mode 100644 drivers/usb/chipidea/phy-imx-utmi.c

Best regards,
Marek Vasut

maintainer
#28 lore raw

cover preliminary work for adding imx6q_sabrelite usb support

Quoted 14 lines · Marek Vasut

Dear Richard Zhao,

Quoted 12 lines

The work is based on usb-next which includes ci13xxx rework by Alexander Shishkin.

Status:
- this version only tested host
- usbotg controler works at host role

Todo:
- usbh1 has strange system error when enumerate devices for on-board hub.
- high speed usb connecton detect failed for the second time. It needs
hub code hack.

Quoted 4 lines
Quoted 1 line

Thanks Peter Chen for greate help.

Hm, I think we should somehow coordinate, because I think we're working on the same thing and we're duplicating work.

Ok, can the two of you work together on this so I don't get conflicting patches, and so you both can verify it works properly for your systems?

thanks,

greg k-h

#29 lore raw

cover preliminary work for adding imx6q_sabrelite usb support

Dear Greg KH,

Quoted 17 lines · Marek Vasut
Quoted 16 lines

Dear Richard Zhao,

Quoted 14 lines

The work is based on usb-next which includes ci13xxx rework by Alexander Shishkin.

Status:
- this version only tested host
- usbotg controler works at host role

Todo:
- usbh1 has strange system error when enumerate devices for on-board hub. - high speed usb connecton detect failed for the second time. It needs

hub code hack.

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

Thanks Peter Chen for greate help.

Hm, I think we should somehow coordinate, because I think we're working on the same thing and we're duplicating work.

Ok, can the two of you work together on this so I don't get conflicting patches, and so you both can verify it works properly for your systems?

Yes, I'd prefer to.

Richard, are these 7 patches the only stuff that I have to apply on top of linux-usb/usb-next to get this working?

Quoted 3 lines

thanks,

greg k-h

Best regards,
Marek Vasut

#30 lore raw

cover preliminary work for adding imx6q_sabrelite usb support

Quoted 21 lines · Marek Vasut

Dear Greg KH,

Quoted 19 lines · Marek Vasut
Quoted 18 lines

Dear Richard Zhao,

Quoted 15 lines

The work is based on usb-next which includes ci13xxx rework by Alexander Shishkin.

Status:
- this version only tested host
- usbotg controler works at host role

Todo:
- usbh1 has strange system error when enumerate devices for on-board hub. - high speed usb connecton detect failed for the second time. It needs

hub code hack.

Thanks Peter Chen for greate help.

Quoted 7 lines
Quoted 5 lines
Quoted 2 lines

Hm, I think we should somehow coordinate, because I think we're working on the same thing and we're duplicating work.

Ok, can the two of you work together on this so I don't get conflicting patches, and so you both can verify it works properly for your systems?

Yes, I'd prefer to.

Me too. I think we can start with review each other's patch.

Quoted 3 lines

Richard, are these 7 patches the only stuff that I have to apply on top of linux-usb/usb-next to get this working?

Yes.

Thanks
Richard

Quoted 8 lines
Quoted 4 lines

thanks,

greg k-h

Best regards,
Marek Vasut

#31 lore raw

cover preliminary work for adding imx6q_sabrelite usb support

Quoted 6 lines
Quoted 1 line

Yes, I'd prefer to.

Me too. I think we can start with review each other's patch.

Quoted 3 lines

Richard, are these 7 patches the only stuff that I have to apply on top of linux-usb/usb-next to get this working?

Yes.

Hi Marek and Richard,

The i.mx28 and i.mx6q use the same USB controller and USB PHY, Your patchset can be totally the same, only different is platform part. Below are my commnets for your patchset:
- Use drivers/usb/otg/mxs-phy.c as PHY's driver.
- Use drivers/usb/chipidea/ci13xxx_imx.c as controller's driver, all freescale usb uses the same controller.
- Use DT for platform support, phandle can work around current multi-phy support problem.

My proposal how you work together (already discussed with richard)
- Richard picks marek's controller's patch, and add DT support for i.mx28, send marek, me, shawn privately.
- Marek tunes this DT patched patchset at i.mx28 platform, and add Signed-off-by: Richard Zhao <[email protected]> for patchset, and send to mail list for review
- Richard tests it and give some comments if there is any problems at i.mx6q.
- Marek re-sends this patchset to mail list for review

Marek, if you agree with it, please take the responsibilities of the owner with this patchset, and be active during this patchset review procedure.

Signature · 2 lines
Best Regards,
Peter Chen
#32 lore raw

cover preliminary work for adding imx6q_sabrelite usb support

Dear Peter Chen,

Quoted 20 lines
Quoted 8 lines
Quoted 1 line

Yes, I'd prefer to.

Me too. I think we can start with review each other's patch.

Quoted 2 lines

Richard, are these 7 patches the only stuff that I have to apply on top of linux-usb/usb-next to get this working?

Yes.

Hi Marek and Richard,

The i.mx28 and i.mx6q use the same USB controller and USB PHY, Your patchset can be totally the same, only different is platform part. Below are my commnets for your patchset:
- Use drivers/usb/otg/mxs-phy.c as PHY's driver.
- Use drivers/usb/chipidea/ci13xxx_imx.c as controller's driver, all freescale usb uses the same controller.
- Use DT for platform support, phandle can work around current multi-phy support problem.

Quoted 5 lines

My proposal how you work together (already discussed with richard)
- Richard picks marek's controller's patch, and add DT support for i.mx28, send marek, me, shawn privately.
- Marek tunes this DT patched patchset at i.mx28 platform, and add Signed-off-by: Richard Zhao <[email protected]>

You mean SoB by me, dont you ? ;-)

Quoted 8 lines

for patchset, and send to mail list for review
- Richard tests it and give some comments if there is any problems at i.mx6q.
- Marek re-sends this patchset to mail list for review

Marek, if you agree with it, please take the responsibilities of the owner with this patchset, and be active during this patchset review procedure.

Fine with me, where can I get DT-enabled MXS kernel tree please?

Best regards,
Marek Vasut

#33 lore raw

cover preliminary work for adding imx6q_sabrelite usb support

Quoted 2 lines · Marek Vasut

Fine with me, where can I get DT-enabled MXS kernel tree please?

linux-next

Signature · 2 lines
Regards,
Shawn