Skip to contentSkip to conversation

Make mv_udc work for i.mx6

PATCH Superseded Troy Kisky 3 participants 44 messages Sep 26, 2013 – Sep 28, 2013

Subsystems (5)
Arm (32-bit) USB USB ehci driver MUSB multipoint high speed dual-role controller +1 more subsystems

v5 of 6 Superseded Latest is v6 →

lore
Share & tools
lore

Troy Kisky

author
#0 lore raw

v5 cover Make mv_udc work for i.mx6

Hi Marek, Stefano

This series is based on u-boot-usb/master branch. After this series, nitrogen6x works with tftpboot to transfer files over usb, and will recognize a usb stick in the OTG port.

This V5 has been rebased to the latest u-boot-usb/master branch and changed to address Marek comments.

The most noticeable change is the removal of ehci_hcd_init_after_reset using CONFIG_EHCI_HCD_INIT_AFTER_RESET instead.

Stefano, if you would you like to take the last 3 patches, you should apply "i.MX6Q/DLS: Add MX6_PAD_GPIO_1__USB_OTG_ID" from Eric Nelson first.

The patches are mostly independent, so if a patch has
an issue, please feel free to skip it.
An exception is 1/17 should be before 2/17 if Stefano's branch merges upstream first, otherwise nitrogen6x will get a build error.

Thanks

Troy Kisky (18):
usb: gadget: mv_udc: don't check CONFIG_USB_MAX_CONTROLLER_COUNT
usb: ehci-mx6: add support for host mode otg port
usb: echi-hcd: add usb_lowlevel_init_device
usb: gadget: ether set wMaxPacketSize
usb: gadget: ether: return error from rx_submit if no request
usb: gadget: mv_udc: split mv_udc.h file
usb: udc: add udc.h include file
usb: gadget: mv_udc: fix typo in error message
usb: gadget: mv_udc: set is_dualspeed = 1
usb: gadget: mv_udc: fix full speed connections
usb: gadget: mv_udc: optimize bounce
usb: gadget: mv_udc: flush item before head
usb: gadget: mv_udc: optimize ep_enable
usb: gadget: mv_udc: zero transfer descriptor memory on probe
usb: gadget: mv_udc: clear desc upon ep_disable
mx6: iomux: add GPR1 defines for use with nitrogen6x
nitrogen6x: add otg usb host/device mode support
nitrogen6x: add CONFIG_MV_UDC

 arch/arm/include/asm/arch-mx6/iomux.h  |   6 ++
 board/boundary/nitrogen6x/nitrogen6x.c |  33 +++++++++
 drivers/serial/usbtty.h                |   3 +-
 drivers/usb/gadget/designware_udc.c    |   1 +
 drivers/usb/gadget/ether.c             |   4 ++
 drivers/usb/gadget/mpc8xx_udc.c        |   1 +
 drivers/usb/gadget/mv_udc.c            | 115 +++++++++++++++++-------------
 drivers/usb/gadget/mv_udc.h            | 115 ++++++++++++++++++++++++++++++
 drivers/usb/gadget/omap1510_udc.c      |   1 +
 drivers/usb/gadget/pxa27x_udc.c        |   1 +
 drivers/usb/host/ehci-hcd.c            |  13 ++++
 drivers/usb/host/ehci-mx6.c            | 123 ++++++++++++++++++++++-----------
 drivers/usb/musb/musb_udc.c            |   3 +-
 include/configs/mx6qsabreauto.h        |   3 +-
 include/configs/nitrogen6x.h           |   9 ++-
 include/usb.h                          |   1 +
 include/usb/designware_udc.h           |  31 ---------
 include/usb/mpc8xx_udc.h               |  19 +----
 include/usb/musb_udc.h                 |  40 -----------
 include/usb/mv_udc.h                   | 118 -------------------------------
 include/usb/omap1510_udc.h             |  31 ++-------
 include/usb/pxa27x_udc.h               |  26 +------
 include/usb/udc.h                      |  53 ++++++++++++++
 23 files changed, 401 insertions(+), 349 deletions(-)
 create mode 100644 drivers/usb/gadget/mv_udc.h
 delete mode 100644 include/usb/musb_udc.h
 create mode 100644 include/usb/udc.h
Signature · 1 line
1.8.1.2
author
#1 lore raw

PATCH v5 1/18 usb: gadget: mv_udc: don't check CONFIG_USB_MAX_CONTROLLER_COUNT

i.mx6 has 1 otg controller, and 3 host ports. So, CONFIG_USB_MAX_CONTROLLER_COUNT can be greater than 1 even though only 1 device mode controller is supported.

Signed-off-by Troy Kisky <[email protected]>

---
v4: new patch
v5: no change

---
 drivers/usb/gadget/mv_udc.c | 4 ----
 1 file changed, 4 deletions(-)
drivers/usb/gadget/mv_udc.c +0 -4
index 7574e31..b87119c 100644
@@ -17,10 +17,6 @@
 #include <linux/types.h>
 #include <usb/mv_udc.h>
 
-#if CONFIG_USB_MAX_CONTROLLER_COUNT > 1
-#error This driver only supports one single controller.
-#endif
-
 /*
  * Check if the system has too long cachelines. If the cachelines are
  * longer then 128b, the driver will not be able flush/invalidate data
Signature · 1 line
1.8.1.2
author
#2 lore raw

PATCH v5 2/18 usb: ehci-mx6: add support for host mode otg port

In reply to Troy Kisky

Previously, only host1 was supported using an index of 0. Now, otg has index 0, host1 is 1, host2 is 2, host3 is 3. Since OTG requires usbmode to be set after reset, I added CONFIG_EHCI_HCD_INIT_AFTER_RESET to nitrogen6x.h and mx6qsabreauto.h.

I also added a weak function board_ehci_power to handle turning power on/off for otg.

Signed-off-by Troy Kisky <[email protected]>

---
V4: new patch, replaces "usb: gadget: mv_udc: fix hardware udc address for i.MX6" and has the bonus of giving OTG host mode support

V5: use CONFIG_EHCI_HCD_INIT_AFTER_RESET instead of a weak function. Return error if otg_id is high.

---
 drivers/usb/host/ehci-mx6.c     | 123 +++++++++++++++++++++++++++-------------
 include/configs/mx6qsabreauto.h |   3 +-
 include/configs/nitrogen6x.h    |   3 +-
 3 files changed, 87 insertions(+), 42 deletions(-)
drivers/usb/host/ehci-mx6.c +83 -40
index eb24af5..ff4424d 100644
@@ -35,6 +35,7 @@
 #define USBPHY_CTRL_CLKGATE			0x40000000
 #define USBPHY_CTRL_ENUTMILEVEL3		0x00008000
 #define USBPHY_CTRL_ENUTMILEVEL2		0x00004000
+#define USBPHY_CTRL_OTD_ID_BIT			27
 
 #define ANADIG_USB2_CHRG_DETECT_EN_B		0x00100000
 #define ANADIG_USB2_CHRG_DETECT_CHK_CHRG_B	0x00080000

@@ -49,52 +50,84 @@
 #define UCTRL_OVER_CUR_DIS	(1 << 7) /* Disable OTG Overcurrent Detection */
 
 /* USBCMD */
-#define UH1_USBCMD_OFFSET	0x140
 #define UCMD_RUN_STOP           (1 << 0) /* controller run/stop */
 #define UCMD_RESET		(1 << 1) /* controller reset */
 
-static void usbh1_internal_phy_clock_gate(int on)
+static const unsigned phy_bases[] = {
+	USB_PHY0_BASE_ADDR,
+	USB_PHY1_BASE_ADDR,
+};
+
+static void usb_internal_phy_clock_gate(int index, int on)
 {
-	void __iomem *phy_reg = (void __iomem *)USB_PHY1_BASE_ADDR;
+	void __iomem *phy_reg;
+
+	if (index >= ARRAY_SIZE(phy_bases))
+		return;
 
+	phy_reg = (void __iomem *)phy_bases[index];
 	phy_reg += on ? USBPHY_CTRL_CLR : USBPHY_CTRL_SET;
 	__raw_writel(USBPHY_CTRL_CLKGATE, phy_reg);
 }
 
-static void usbh1_power_config(void)
+static void usb_power_config(int index)
 {
 	struct anatop_regs __iomem *anatop =
 		(struct anatop_regs __iomem *)ANATOP_BASE_ADDR;
+	void __iomem *chrg_detect;
+	void __iomem *pll_480_ctrl_clr;
+	void __iomem *pll_480_ctrl_set;
+
+	switch (index) {
+	case 0:
+		chrg_detect = &anatop->usb1_chrg_detect;
+		pll_480_ctrl_clr = &anatop->usb1_pll_480_ctrl_clr;
+		pll_480_ctrl_set = &anatop->usb1_pll_480_ctrl_set;
+		break;
+	case 1:
+		chrg_detect = &anatop->usb2_chrg_detect;
+		pll_480_ctrl_clr = &anatop->usb2_pll_480_ctrl_clr;
+		pll_480_ctrl_set = &anatop->usb2_pll_480_ctrl_set;
+		break;
+	default:
+		return;
+	}
 	/*
-	 * Some phy and power's special controls for host1
+	 * Some phy and power's special controls
 	 * 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
+	 * 2. The PLL's power and output to usb
 	 * is totally controlled by IC, so the Software only needs
 	 * to enable them at initializtion.
 	 */
 	__raw_writel(ANADIG_USB2_CHRG_DETECT_EN_B |
 		     ANADIG_USB2_CHRG_DETECT_CHK_CHRG_B,
-		     &anatop->usb2_chrg_detect);
+		     chrg_detect);
 
 	__raw_writel(ANADIG_USB2_PLL_480_CTRL_BYPASS,
-		     &anatop->usb2_pll_480_ctrl_clr);
+		     pll_480_ctrl_clr);
 
 	__raw_writel(ANADIG_USB2_PLL_480_CTRL_ENABLE |
 		     ANADIG_USB2_PLL_480_CTRL_POWER |
 		     ANADIG_USB2_PLL_480_CTRL_EN_USB_CLKS,
-		     &anatop->usb2_pll_480_ctrl_set);
+		     pll_480_ctrl_set);
 }
 
-static int usbh1_phy_enable(void)
+/* Return 0 : host node, 1 : device mode */
+static int usb_phy_enable(int index, struct usb_ehci *ehci)
 {
-	void __iomem *phy_reg = (void __iomem *)USB_PHY1_BASE_ADDR;
-	void __iomem *phy_ctrl = (void __iomem *)(phy_reg + USBPHY_CTRL);
-	void __iomem *usb_cmd =	(void __iomem *)(USBOH3_USB_BASE_ADDR +
-						 USB_H1REGS_OFFSET +
-						 UH1_USBCMD_OFFSET);
+	void __iomem *phy_reg;
+	void __iomem *phy_ctrl;
+	void __iomem *usb_cmd;
 	u32 val;
 
+	if (index >= ARRAY_SIZE(phy_bases))
+		return 0;
+
+	phy_reg = (void __iomem *)phy_bases[index];
+	phy_ctrl = (void __iomem *)(phy_reg + USBPHY_CTRL);
+	usb_cmd = (void __iomem *)&ehci->usbcmd;
+
 	/* Stop then Reset */
 	val = __raw_readl(usb_cmd);
 	val &= ~UCMD_RUN_STOP;

@@ -123,31 +156,36 @@ static int usbh1_phy_enable(void)
 	/* Power up the PHY */
 	__raw_writel(0, phy_reg + USBPHY_PWD);
 	/* enable FS/LS device */
-	val = __raw_readl(phy_reg + USBPHY_CTRL);
+	val = __raw_readl(phy_ctrl);
 	val |= (USBPHY_CTRL_ENUTMILEVEL2 | USBPHY_CTRL_ENUTMILEVEL3);
-	__raw_writel(val, phy_reg + USBPHY_CTRL);
+	__raw_writel(val, phy_ctrl);
 
-	return 0;
+	return (val >> USBPHY_CTRL_OTD_ID_BIT) & 1;
 }
 
-static void usbh1_oc_config(void)
+struct usbnc_regs {
+	u32	ctrl[4];	/* otg/host1-3 */
+};
+
+static void usb_oc_config(int index)
 {
-	void __iomem *usb_base = (void __iomem *)USBOH3_USB_BASE_ADDR;
-	void __iomem *usbother_base = usb_base + USB_OTHERREGS_OFFSET;
+	struct usbnc_regs *usbnc = (struct usbnc_regs *)(USBOH3_USB_BASE_ADDR +
+			USB_OTHERREGS_OFFSET);
+	void __iomem *ctrl = (void __iomem *)(&usbnc->ctrl[index]);
 	u32 val;
 
-	val = __raw_readl(usbother_base + USB_H1_CTRL_OFFSET);
+	val = __raw_readl(ctrl);
 #if CONFIG_MACH_TYPE == MACH_TYPE_MX6Q_ARM2
 	/* mx6qarm2 seems to required a different setting*/
 	val &= ~UCTRL_OVER_CUR_POL;
 #else
 	val |= UCTRL_OVER_CUR_POL;
 #endif
-	__raw_writel(val, usbother_base + USB_H1_CTRL_OFFSET);
+	__raw_writel(val, ctrl);
 
-	val = __raw_readl(usbother_base + USB_H1_CTRL_OFFSET);
+	val = __raw_readl(ctrl);
 	val |= UCTRL_OVER_CUR_DIS;
-	__raw_writel(val, usbother_base + USB_H1_CTRL_OFFSET);
+	__raw_writel(val, ctrl);
 }
 
 int __weak board_ehci_hcd_init(int port)

@@ -155,33 +193,38 @@ int __weak board_ehci_hcd_init(int port)
 	return 0;
 }
 
+int __weak board_ehci_power(int port, int on)
+{
+	return 0;
+}
+
 int ehci_hcd_init(int index, struct ehci_hccr **hccr, struct ehci_hcor **hcor)
 {
-	struct usb_ehci *ehci;
+	int otg_id;
+	struct usb_ehci *ehci = (struct usb_ehci *)(USBOH3_USB_BASE_ADDR +
+		(0x200 * index));
 
+	if (index > 3)
+		return -EINVAL;
 	enable_usboh3_clk(1);
 	mdelay(1);
 
 	/* Do board specific initialization */
-	board_ehci_hcd_init(CONFIG_MXC_USB_PORT);
-
-#if CONFIG_MXC_USB_PORT == 1
-	/* USB Host 1 */
-	usbh1_power_config();
-	usbh1_oc_config();
-	usbh1_internal_phy_clock_gate(1);
-	usbh1_phy_enable();
-#else
-#error "MXC USB port not yet supported"
-#endif
+	board_ehci_hcd_init(index);
+
+	usb_power_config(index);
+	usb_oc_config(index);
+	usb_internal_phy_clock_gate(index, 1);
+	otg_id = usb_phy_enable(index, ehci);
 
-	ehci = (struct usb_ehci *)(USBOH3_USB_BASE_ADDR +
-		(0x200 * CONFIG_MXC_USB_PORT));
 	*hccr = (struct ehci_hccr *)((uint32_t)&ehci->caplength);
 	*hcor = (struct ehci_hcor *)((uint32_t)*hccr +
 			HC_LENGTH(ehci_readl(&(*hccr)->cr_capbase)));
-	setbits_le32(&ehci->usbmode, CM_HOST);
 
+	board_ehci_power(index, otg_id ? 0 : 1);
+	if (otg_id)
+		return -ENODEV;	/* Device mode */
+	setbits_le32(&ehci->usbmode, CM_HOST);
 	__raw_writel(CONFIG_MXC_USB_PORTSC, &ehci->portsc);
 	setbits_le32(&ehci->portsc, USB_EN);
include/configs/mx6qsabreauto.h +2 -1
index 5530fc6..9e48a49 100644
@@ -23,7 +23,8 @@
 #define CONFIG_USB_STORAGE
 #define CONFIG_USB_HOST_ETHER
 #define CONFIG_USB_ETHER_ASIX
-#define CONFIG_MXC_USB_PORT	1
+#define CONFIG_USB_MAX_CONTROLLER_COUNT 2
+#define CONFIG_EHCI_HCD_INIT_AFTER_RESET	/* For OTG port */
 #define CONFIG_MXC_USB_PORTSC	(PORT_PTS_UTMI | PORT_PTS_PTW)
 #define CONFIG_MXC_USB_FLAGS	0
include/configs/nitrogen6x.h +2 -1
index 3454b86..2e5ae13 100644
@@ -119,7 +119,8 @@
 #define CONFIG_USB_HOST_ETHER
 #define CONFIG_USB_ETHER_ASIX
 #define CONFIG_USB_ETHER_SMSC95XX
-#define CONFIG_MXC_USB_PORT	1
+#define CONFIG_USB_MAX_CONTROLLER_COUNT 2
+#define CONFIG_EHCI_HCD_INIT_AFTER_RESET	/* For OTG port */
 #define CONFIG_MXC_USB_PORTSC	(PORT_PTS_UTMI | PORT_PTS_PTW)
 #define CONFIG_MXC_USB_FLAGS	0
Signature · 1 line
1.8.1.2
#3 lore raw

PATCH v5 2/18 usb: ehci-mx6: add support for host mode otg port

Dear Troy Kisky,

Quoted 29 lines

Previously, only host1 was supported using an index of 0. Now, otg has index 0, host1 is 1, host2 is 2, host3 is 3. Since OTG requires usbmode to be set after reset, I added CONFIG_EHCI_HCD_INIT_AFTER_RESET to nitrogen6x.h and mx6qsabreauto.h.

I also added a weak function board_ehci_power to handle turning power on/off for otg.

Signed-off-by Troy Kisky <[email protected]>

---
V4: new patch, replaces "usb: gadget: mv_udc: fix hardware udc address for i.MX6" and has the bonus of giving OTG host mode support

V5: use CONFIG_EHCI_HCD_INIT_AFTER_RESET instead of a weak function. Return error if otg_id is high.

---
 drivers/usb/host/ehci-mx6.c     | 123
+++++++++++++++++++++++++++------------- include/configs/mx6qsabreauto.h |
  3 +-
 include/configs/nitrogen6x.h    |   3 +-
 3 files changed, 87 insertions(+), 42 deletions(-)
drivers/usb/host/ehci-mx6.c +0 -0
index eb24af5..ff4424d 100644

drivers/usb/host/ehci-mx6.c line 38

Quoted 4 lines
drivers/usb/host/ehci-mx6.c +1 -0
@@ -35,6 +35,7 @@
 #define USBPHY_CTRL_CLKGATE			0x40000000
 #define USBPHY_CTRL_ENUTMILEVEL3		0x00008000
 #define USBPHY_CTRL_ENUTMILEVEL2		0x00004000
+#define USBPHY_CTRL_OTD_ID_BIT			27

Why is this anarchism used here? Why is it not defined as the rest of the values? Btw. its OTG_ID_VALUE , not OTD.

[...]

drivers/usb/host/ehci-mx6.c static int usbh1_phy_enable(void) · line 163

Quoted 12 lines
drivers/usb/host/ehci-mx6.c +3 -3
@@ -123,31 +156,36 @@ static int usbh1_phy_enable(void)
 	/* Power up the PHY */
 	__raw_writel(0, phy_reg + USBPHY_PWD);
 	/* enable FS/LS device */
-	val = __raw_readl(phy_reg + USBPHY_CTRL);
+	val = __raw_readl(phy_ctrl);
 	val |= (USBPHY_CTRL_ENUTMILEVEL2 | USBPHY_CTRL_ENUTMILEVEL3);
-	__raw_writel(val, phy_reg + USBPHY_CTRL);
+	__raw_writel(val, phy_ctrl);
 
-	return 0;
+	return (val >> USBPHY_CTRL_OTD_ID_BIT) & 1;

See above, you can use return (val & USBPHY_CTRL_OTG_ID_VALUE); here instead.

drivers/usb/host/ehci-mx6.c static int usbh1_phy_enable(void) · line 167

Quoted 5 lines
drivers/usb/host/ehci-mx6.c +2 -1
 }
 
-static void usbh1_oc_config(void)
+struct usbnc_regs {
+	u32	ctrl[4];	/* otg/host1-3 */

This won't scale. Align this with the # of controllers you claim you have here, why dont you?

[...]

Quoted 19 lines
(quoted diff) +0 -0

b/include/configs/mx6qsabreauto.h index 5530fc6..9e48a49 100644

include/configs/mx6qsabreauto.h +2 -1
@@ -23,7 +23,8 @@
 #define CONFIG_USB_STORAGE
 #define CONFIG_USB_HOST_ETHER
 #define CONFIG_USB_ETHER_ASIX
-#define CONFIG_MXC_USB_PORT	1
+#define CONFIG_USB_MAX_CONTROLLER_COUNT 2
+#define CONFIG_EHCI_HCD_INIT_AFTER_RESET	/* For OTG port */
 #define CONFIG_MXC_USB_PORTSC	(PORT_PTS_UTMI | PORT_PTS_PTW)
 #define CONFIG_MXC_USB_FLAGS	0
include/configs/nitrogen6x.h +0 -0
index 3454b86..2e5ae13 100644

include/configs/nitrogen6x.h line 125

Quoted 8 lines
include/configs/nitrogen6x.h +2 -1
@@ -119,7 +119,8 @@
 #define CONFIG_USB_HOST_ETHER
 #define CONFIG_USB_ETHER_ASIX
 #define CONFIG_USB_ETHER_SMSC95XX
-#define CONFIG_MXC_USB_PORT	1
+#define CONFIG_USB_MAX_CONTROLLER_COUNT 2
+#define CONFIG_EHCI_HCD_INIT_AFTER_RESET	/* For OTG port */
 #define CONFIG_MXC_USB_PORTSC	(PORT_PTS_UTMI | PORT_PTS_PTW)
 #define CONFIG_MXC_USB_FLAGS	0

Does this patch do two things here ? I think it does, if I'm right, split this board adjustment away please.

Best regards,
Marek Vasut

author
#4 lore raw

PATCH v5 2/18 usb: ehci-mx6: add support for host mode otg port

Quoted 26 lines · Marek Vasut

Dear Troy Kisky,

Quoted 24 lines

Previously, only host1 was supported using an index of 0. Now, otg has index 0, host1 is 1, host2 is 2, host3 is 3. Since OTG requires usbmode to be set after reset, I added CONFIG_EHCI_HCD_INIT_AFTER_RESET to nitrogen6x.h and mx6qsabreauto.h.

I also added a weak function board_ehci_power to handle turning power on/off for otg.

Signed-off-by Troy Kisky <[email protected]>

---
V4: new patch, replaces "usb: gadget: mv_udc: fix hardware udc address for i.MX6" and has the bonus of giving OTG host mode support

V5: use CONFIG_EHCI_HCD_INIT_AFTER_RESET instead of a weak function. Return error if otg_id is high.

---
  drivers/usb/host/ehci-mx6.c     | 123
+++++++++++++++++++++++++++------------- include/configs/mx6qsabreauto.h |
   3 +-
  include/configs/nitrogen6x.h    |   3 +-
  3 files changed, 87 insertions(+), 42 deletions(-)
Quoted 11 lines

drivers/usb/host/ehci-mx6.c line 38

Quoted 9 lines
drivers/usb/host/ehci-mx6.c +1 -0
index eb24af5..ff4424d 100644
@@ -35,6 +35,7 @@
  #define USBPHY_CTRL_CLKGATE			0x40000000
  #define USBPHY_CTRL_ENUTMILEVEL3		0x00008000
  #define USBPHY_CTRL_ENUTMILEVEL2		0x00004000
+#define USBPHY_CTRL_OTD_ID_BIT			27

Why is this anarchism used here? Why is it not defined as the rest of the values? Btw. its OTG_ID_VALUE , not OTD.

Good catch on the typo!

Quoted 12 lines

[...]

Quoted 10 lines
drivers/usb/host/ehci-mx6.c +2 -2
@@ -123,31 +156,36 @@ static int usbh1_phy_enable(void)
  	/* Power up the PHY */
  	__raw_writel(0, phy_reg + USBPHY_PWD);
  	/* enable FS/LS device */
-	val = __raw_readl(phy_reg + USBPHY_CTRL);
+	val = __raw_readl(phy_ctrl);
  	val |= (USBPHY_CTRL_ENUTMILEVEL2 | USBPHY_CTRL_ENUTMILEVEL3);
-	__raw_writel(val, phy_reg + USBPHY_CTRL);
+	__raw_writel(val, phy_ctrl);
Quoted 3 lines

drivers/usb/host/ehci-mx6.c static int usbh1_phy_enable(void) · line 163

Quoted 2 lines
drivers/usb/host/ehci-mx6.c +1 -1
-	return 0;
+	return (val >> USBPHY_CTRL_OTD_ID_BIT) & 1;

See above, you can use return (val & USBPHY_CTRL_OTG_ID_VALUE); here instead.

Yes, I can do that, and also change my documentation along with it.

Quoted 8 lines

drivers/usb/host/ehci-mx6.c static int usbh1_phy_enable(void) · line 167

Quoted 6 lines
drivers/usb/host/ehci-mx6.c +2 -1
  }
 
-static void usbh1_oc_config(void)
+struct usbnc_regs {
+	u32	ctrl[4];	/* otg/host1-3 */

This won't scale. Align this with the # of controllers you claim you have here, why dont you?

You've lost me here. Can you post the struct you'd like to see?

Quoted 17 lines

[...]

Quoted 14 lines
(quoted diff) +0 -0

b/include/configs/mx6qsabreauto.h index 5530fc6..9e48a49 100644

include/configs/mx6qsabreauto.h +2 -1
@@ -23,7 +23,8 @@
  #define CONFIG_USB_STORAGE
  #define CONFIG_USB_HOST_ETHER
  #define CONFIG_USB_ETHER_ASIX
-#define CONFIG_MXC_USB_PORT	1
+#define CONFIG_USB_MAX_CONTROLLER_COUNT 2
+#define CONFIG_EHCI_HCD_INIT_AFTER_RESET	/* For OTG port */
  #define CONFIG_MXC_USB_PORTSC	(PORT_PTS_UTMI | PORT_PTS_PTW)
  #define CONFIG_MXC_USB_FLAGS	0
Quoted 17 lines

include/configs/nitrogen6x.h line 125

Quoted 13 lines
include/configs/nitrogen6x.h +2 -1
index 3454b86..2e5ae13 100644
@@ -119,7 +119,8 @@
  #define CONFIG_USB_HOST_ETHER
  #define CONFIG_USB_ETHER_ASIX
  #define CONFIG_USB_ETHER_SMSC95XX
-#define CONFIG_MXC_USB_PORT	1
+#define CONFIG_USB_MAX_CONTROLLER_COUNT 2
+#define CONFIG_EHCI_HCD_INIT_AFTER_RESET	/* For OTG port */
  #define CONFIG_MXC_USB_PORTSC	(PORT_PTS_UTMI | PORT_PTS_PTW)
  #define CONFIG_MXC_USB_FLAGS	0

Does this patch do two things here ? I think it does, if I'm right, split this board adjustment away please.

If I split this, USB will stop working between the 2 patches. The OTG port will be initialized
instead of the main USB port, and it won't work because it requires CONFIG_EHCI_HCD_INIT_AFTER_RESET.

BR

Troy

#5 lore raw

PATCH v5 2/18 usb: ehci-mx6: add support for host mode otg port

Dear Troy Kisky,

Quoted 75 lines · Marek Vasut
Quoted 37 lines

Dear Troy Kisky,

Quoted 35 lines

Previously, only host1 was supported using an index of 0. Now, otg has index 0, host1 is 1, host2 is 2, host3 is 3. Since OTG requires usbmode to be set after reset, I added CONFIG_EHCI_HCD_INIT_AFTER_RESET to nitrogen6x.h and mx6qsabreauto.h.

I also added a weak function board_ehci_power to handle turning power on/off for otg.

Signed-off-by Troy Kisky <[email protected]>

---
V4: new patch, replaces "usb: gadget: mv_udc: fix hardware udc address for i.MX6" and has the bonus of giving OTG host mode support

V5: use CONFIG_EHCI_HCD_INIT_AFTER_RESET instead of a weak function. Return error if otg_id is high.
---

  drivers/usb/host/ehci-mx6.c     | 123

+++++++++++++++++++++++++++------------- include/configs/mx6qsabreauto.h |

   3 +-
  include/configs/nitrogen6x.h    |   3 +-
  3 files changed, 87 insertions(+), 42 deletions(-)
drivers/usb/host/ehci-mx6.c +0 -0
index eb24af5..ff4424d 100644
@@ -35,6 +35,7 @@
Quoted 8 lines

drivers/usb/host/ehci-mx6.c

Quoted 5 lines
drivers/usb/host/ehci-mx6.c +1 -0
  #define USBPHY_CTRL_CLKGATE			0x40000000
  #define USBPHY_CTRL_ENUTMILEVEL3		0x00008000
  #define USBPHY_CTRL_ENUTMILEVEL2		0x00004000
 
+#define USBPHY_CTRL_OTD_ID_BIT			27

Why is this anarchism used here? Why is it not defined as the rest of the values? Btw. its OTG_ID_VALUE , not OTD.

Good catch on the typo!

Quoted 13 lines

[...]

Quoted 11 lines
drivers/usb/host/ehci-mx6.c +1 -1
@@ -123,31 +156,36 @@ static int usbh1_phy_enable(void)
 
  	/* Power up the PHY */
  	__raw_writel(0, phy_reg + USBPHY_PWD);
  	/* enable FS/LS device */
 
-	val = __raw_readl(phy_reg + USBPHY_CTRL);
+	val = __raw_readl(phy_ctrl);
 
  	val |= (USBPHY_CTRL_ENUTMILEVEL2 | USBPHY_CTRL_ENUTMILEVEL3);
Quoted 8 lines

drivers/usb/host/ehci-mx6.c static int usbh1_phy_enable(void) · line 163

Quoted 5 lines
drivers/usb/host/ehci-mx6.c +2 -2
-	__raw_writel(val, phy_reg + USBPHY_CTRL);
+	__raw_writel(val, phy_ctrl);
 
-	return 0;
+	return (val >> USBPHY_CTRL_OTD_ID_BIT) & 1;

See above, you can use return (val & USBPHY_CTRL_OTG_ID_VALUE); here instead.

Yes, I can do that, and also change my documentation along with it.

Quoted 2 lines
Quoted 2 lines
drivers/usb/host/ehci-mx6.c +0 -0
  }
Quoted 8 lines
Quoted 6 lines

drivers/usb/host/ehci-mx6.c static int usbh1_phy_enable(void) · line 167

Quoted 3 lines
drivers/usb/host/ehci-mx6.c +2 -1
-static void usbh1_oc_config(void)
+struct usbnc_regs {
+	u32	ctrl[4];	/* otg/host1-3 */

This won't scale. Align this with the # of controllers you claim you have here, why dont you?

You've lost me here. Can you post the struct you'd like to see?

Something like u32 ctrl[CONFIG_HOW_MANY_PORTS_YOU_HAVE] or so would suffice. I think you have such a MAX_PORTS #define already.

Quoted 40 lines
Quoted 34 lines

[...]

Quoted 31 lines
(quoted diff) +0 -0

b/include/configs/mx6qsabreauto.h index 5530fc6..9e48a49 100644

include/configs/mx6qsabreauto.h +2 -1
@@ -23,7 +23,8 @@
 
  #define CONFIG_USB_STORAGE
  #define CONFIG_USB_HOST_ETHER
  #define CONFIG_USB_ETHER_ASIX
 
-#define CONFIG_MXC_USB_PORT	1
+#define CONFIG_USB_MAX_CONTROLLER_COUNT 2
+#define CONFIG_EHCI_HCD_INIT_AFTER_RESET	/* For OTG port */
 

  #define CONFIG_MXC_USB_PORTSC	(PORT_PTS_UTMI | PORT_PTS_PTW)
  #define CONFIG_MXC_USB_FLAGS	0
include/configs/nitrogen6x.h +2 -1
index 3454b86..2e5ae13 100644
@@ -119,7 +119,8 @@
 
  #define CONFIG_USB_HOST_ETHER
  #define CONFIG_USB_ETHER_ASIX
  #define CONFIG_USB_ETHER_SMSC95XX
 
-#define CONFIG_MXC_USB_PORT	1
+#define CONFIG_USB_MAX_CONTROLLER_COUNT 2
+#define CONFIG_EHCI_HCD_INIT_AFTER_RESET	/* For OTG port */
Quoted 5 lines
Quoted 2 lines
  #define CONFIG_MXC_USB_PORTSC	(PORT_PTS_UTMI | PORT_PTS_PTW)
  #define CONFIG_MXC_USB_FLAGS	0

Does this patch do two things here ? I think it does, if I'm right, split this board adjustment away please.

Quoted 4 lines

If I split this, USB will stop working between the 2 patches. The OTG port will be initialized
instead of the main USB port, and it won't work because it requires CONFIG_EHCI_HCD_INIT_AFTER_RESET.

OK, keep it.

Best regards,
Marek Vasut

author
#6 lore raw

PATCH v5 2/18 usb: ehci-mx6: add support for host mode otg port

Quoted 11 lines · Marek Vasut
   }
drivers/usb/host/ehci-mx6.c +2 -1
-static void usbh1_oc_config(void)
+struct usbnc_regs {
+	u32	ctrl[4];	/* otg/host1-3 */
Quoted 3 lines
Quoted 2 lines

This won't scale. Align this with the # of controllers you claim you have here, why dont you?

You've lost me here. Can you post the struct you'd like to see?

Something like u32 ctrl[CONFIG_HOW_MANY_PORTS_YOU_HAVE] or so would suffice. I think you have such a MAX_PORTS #define already.

This structure is a description of the i.mx6 hardware. I could add the unreferenced 4 registers that are next
to make this clearer?

#7 lore raw

PATCH v5 2/18 usb: ehci-mx6: add support for host mode otg port

Dear Troy Kisky,

Quoted 10 lines · Marek Vasut
Quoted 9 lines
   }
drivers/usb/host/ehci-mx6.c +2 -1
-static void usbh1_oc_config(void)
+struct usbnc_regs {
+	u32	ctrl[4];	/* otg/host1-3 */
Quoted 4 lines
Quoted 3 lines

This won't scale. Align this with the # of controllers you claim you have here, why dont you?

Quoted 8 lines
Quoted 4 lines
Quoted 1 line

You've lost me here. Can you post the struct you'd like to see?

Something like u32 ctrl[CONFIG_HOW_MANY_PORTS_YOU_HAVE] or so would suffice. I think you have such a MAX_PORTS #define already.

This structure is a description of the i.mx6 hardware. I could add the unreferenced 4 registers that are next
to make this clearer?

Ah, now I get it. Might be a good idea to add them and also add a comment above.

btw. (strictly for subsequent patchset) I keep wondering why we have EHCI MX6 and EHCI MXS files, even if both use the exact same USB IP block and exact same USB PHY. Sigh ...

Best regards,
Marek Vasut

author
#8 lore raw

PATCH v5 3/18 usb: echi-hcd: add usb_lowlevel_init_device

In reply to Troy Kisky

Use this function so that we can verify the OTG_ID pin is high and device mode should be activated.

Signed-off-by Troy Kisky <[email protected]>

---
v5: new patch

---
 drivers/usb/gadget/mv_udc.c |  2 +-
 drivers/usb/host/ehci-hcd.c | 13 +++++++++++++
 include/usb.h               |  1 +
 3 files changed, 15 insertions(+), 1 deletion(-)
drivers/usb/gadget/mv_udc.c +1 -1
index b87119c..ecd11d7 100644
@@ -684,7 +684,7 @@ int usb_gadget_register_driver(struct usb_gadget_driver *driver)
 	if (driver->speed != USB_SPEED_FULL && driver->speed != USB_SPEED_HIGH)
 		return -EINVAL;
 
-	ret = usb_lowlevel_init(0, (void **)&controller.ctrl);
+	ret = usb_lowlevel_init_device(0, (void **)&controller.ctrl);
 	if (ret)
 		return ret;
drivers/usb/host/ehci-hcd.c +13 -0
index fdad739..dbea756 100644
@@ -1004,6 +1004,19 @@ int usb_lowlevel_init(int index, void **controller)
 	return 0;
 }
 
+int usb_lowlevel_init_device(int index, void **controller)
+{
+	int rc = ehci_hcd_init(index, &ehcic[index].hccr, &ehcic[index].hcor);
+
+	/* rc == 0 means host mode, failure for us */
+	if (!rc)
+		return -EINVAL;
+	if (rc != -ENODEV)
+		return rc;
+	*controller = &ehcic[index];
+	return 0;
+}
+
 int
 submit_bulk_msg(struct usb_device *dev, unsigned long pipe, void *buffer,
 		int length)
include/usb.h +1 -0
index 60db897..4bc50cc 100644
@@ -141,6 +141,7 @@ struct usb_device {
 	defined(CONFIG_USB_MUSB_OMAP2PLUS)
 
 int usb_lowlevel_init(int index, void **controller);
+int usb_lowlevel_init_device(int index, void **controller);
 int usb_lowlevel_stop(int index);
 
 int submit_bulk_msg(struct usb_device *dev, unsigned long pipe,
Signature · 1 line
1.8.1.2
#9 lore raw

PATCH v5 3/18 usb: echi-hcd: add usb_lowlevel_init_device

Dear Troy Kisky,

Quoted 18 lines

Use this function so that we can verify the OTG_ID pin is high and device mode should be activated.

Signed-off-by Troy Kisky <[email protected]>

---
v5: new patch

---
 drivers/usb/gadget/mv_udc.c |  2 +-
 drivers/usb/host/ehci-hcd.c | 13 +++++++++++++
 include/usb.h               |  1 +
 3 files changed, 15 insertions(+), 1 deletion(-)
drivers/usb/gadget/mv_udc.c +0 -0
index b87119c..ecd11d7 100644

drivers/usb/gadget/mv_udc.c int usb_gadget_register_driver(struct usb_gadget_driver · line 687

Quoted 5 lines
drivers/usb/gadget/mv_udc.c +1 -1
@@ -684,7 +684,7 @@ int usb_gadget_register_driver(struct usb_gadget_driver
 *driver) if (driver->speed != USB_SPEED_FULL && driver->speed !=
 USB_SPEED_HIGH) return -EINVAL;
 
-	ret = usb_lowlevel_init(0, (void **)&controller.ctrl);
+	ret = usb_lowlevel_init_device(0, (void **)&controller.ctrl);

You're crafting a new undocumented API here :-(

Quoted 8 lines
drivers/usb/gadget/mv_udc.c +0 -0
 	if (ret)
 		return ret;
drivers/usb/host/ehci-hcd.c +0 -0
index fdad739..dbea756 100644

drivers/usb/host/ehci-hcd.c int usb_lowlevel_init(int index, void **controller) · line 1011

Quoted 8 lines
drivers/usb/host/ehci-hcd.c +5 -0
@@ -1004,6 +1004,19 @@ int usb_lowlevel_init(int index, void **controller)
 	return 0;
 }
 
+int usb_lowlevel_init_device(int index, void **controller)
+{
+	int rc = ehci_hcd_init(index, &ehcic[index].hccr, &ehcic[index].hcor);
+
+	/* rc == 0 means host mode, failure for us */

This looks pretty error-prone. A much better idea would be to use usb_lowlevel_init() and add a flag to init the controller in either device or host mode. That way you'd not even have to craft this strange new API.

Quoted 16 lines
drivers/usb/host/ehci-hcd.c +8 -0
+	if (!rc)
+		return -EINVAL;
+	if (rc != -ENODEV)
+		return rc;
+	*controller = &ehcic[index];
+	return 0;
+}
+
 int
 submit_bulk_msg(struct usb_device *dev, unsigned long pipe, void *buffer,
 		int length)
include/usb.h +0 -0
index 60db897..4bc50cc 100644

include/usb.h struct usb_device { · line 147

Quoted 7 lines
include/usb.h +1 -0
@@ -141,6 +141,7 @@ struct usb_device {
 	defined(CONFIG_USB_MUSB_OMAP2PLUS)
 
 int usb_lowlevel_init(int index, void **controller);
+int usb_lowlevel_init_device(int index, void **controller);
 int usb_lowlevel_stop(int index);
 
 int submit_bulk_msg(struct usb_device *dev, unsigned long pipe,

Best regards,
Marek Vasut

author
#10 lore raw

PATCH v5 3/18 usb: echi-hcd: add usb_lowlevel_init_device

Quoted 38 lines · Marek Vasut

Dear Troy Kisky,

Quoted 18 lines

Use this function so that we can verify the OTG_ID pin is high and device mode should be activated.

Signed-off-by Troy Kisky <[email protected]>

---
v5: new patch

---
  drivers/usb/gadget/mv_udc.c |  2 +-
  drivers/usb/host/ehci-hcd.c | 13 +++++++++++++
  include/usb.h               |  1 +
  3 files changed, 15 insertions(+), 1 deletion(-)
drivers/usb/gadget/mv_udc.c +0 -0
index b87119c..ecd11d7 100644

drivers/usb/gadget/mv_udc.c int usb_gadget_register_driver(struct usb_gadget_driver · line 687

Quoted 5 lines
drivers/usb/gadget/mv_udc.c +1 -1
@@ -684,7 +684,7 @@ int usb_gadget_register_driver(struct usb_gadget_driver
 *driver) if (driver->speed != USB_SPEED_FULL && driver->speed !=
 USB_SPEED_HIGH) return -EINVAL;
 
-	ret = usb_lowlevel_init(0, (void **)&controller.ctrl);
+	ret = usb_lowlevel_init_device(0, (void **)&controller.ctrl);

You're crafting a new undocumented API here :-(

Quoted 11 lines
drivers/usb/gadget/mv_udc.c +0 -0
  	if (ret)
  		return ret;
drivers/usb/host/ehci-hcd.c +0 -0
index fdad739..dbea756 100644
@@ -1004,6 +1004,19 @@ int usb_lowlevel_init(int index, void **controller)
  	return 0;
  }
Quoted 8 lines

drivers/usb/host/ehci-hcd.c int usb_lowlevel_init(int index, void **controller) · line 1011

Quoted 5 lines
drivers/usb/host/ehci-hcd.c +5 -0
+int usb_lowlevel_init_device(int index, void **controller)
+{
+	int rc = ehci_hcd_init(index, &ehcic[index].hccr, &ehcic[index].hcor);
+
+	/* rc == 0 means host mode, failure for us */

This looks pretty error-prone. A much better idea would be to use usb_lowlevel_init() and add a flag to init the controller in either device or host mode. That way you'd not even have to craft this strange new API.

How would you like the last parameter?

Like "[PATCH v4] usb: new board-specific USB init interface"

enum board_usb_init_type {
USB_INIT_HOST,
USB_INIT_DEVICE
};

Or just a plain int device_mode ?

Should I wait for you to apply the above referenced patch before resending ?

Should I add the same parameter to ehci_hcd_init ?

And just to be perfectly clear, you want me to add the parameter to all 19 current occurrences of usb_lowlevel_init ?

Quoted 27 lines
Quoted 17 lines
drivers/usb/host/ehci-hcd.c +8 -0
+	if (!rc)
+		return -EINVAL;
+	if (rc != -ENODEV)
+		return rc;
+	*controller = &ehcic[index];
+	return 0;
+}
+
  int
  submit_bulk_msg(struct usb_device *dev, unsigned long pipe, void *buffer,
  		int length)
include/usb.h +0 -0
index 60db897..4bc50cc 100644

include/usb.h struct usb_device { · line 147

Quoted 7 lines
include/usb.h +1 -0
@@ -141,6 +141,7 @@ struct usb_device {
  	defined(CONFIG_USB_MUSB_OMAP2PLUS)
 
  int usb_lowlevel_init(int index, void **controller);
+int usb_lowlevel_init_device(int index, void **controller);
  int usb_lowlevel_stop(int index);
 
  int submit_bulk_msg(struct usb_device *dev, unsigned long pipe,

Best regards,
Marek Vasut

#11 lore raw

PATCH v5 3/18 usb: echi-hcd: add usb_lowlevel_init_device

Dear Troy Kisky,

Quoted 65 lines · Marek Vasut
Quoted 44 lines

Dear Troy Kisky,

Quoted 22 lines

Use this function so that we can verify the OTG_ID pin is high and device mode should be activated.

Signed-off-by Troy Kisky <[email protected]>

---
v5: new patch
---

  drivers/usb/gadget/mv_udc.c |  2 +-
  drivers/usb/host/ehci-hcd.c | 13 +++++++++++++
  include/usb.h               |  1 +
  3 files changed, 15 insertions(+), 1 deletion(-)
drivers/usb/gadget/mv_udc.c +0 -0
index b87119c..ecd11d7 100644
@@ -684,7 +684,7 @@ int usb_gadget_register_driver(struct

usb_gadget_driver *driver) if (driver->speed != USB_SPEED_FULL && driver->speed != USB_SPEED_HIGH) return -EINVAL;

drivers/usb/gadget/mv_udc.c int usb_gadget_register_driver(struct usb_gadget_driver *driver) · line 687

Quoted 2 lines
drivers/usb/gadget/mv_udc.c +1 -1
-	ret = usb_lowlevel_init(0, (void **)&controller.ctrl);
+	ret = usb_lowlevel_init_device(0, (void **)&controller.ctrl);

You're crafting a new undocumented API here :-(

Quoted 15 lines

if (ret)

return ret;

drivers/usb/host/ehci-hcd.c +0 -0
index fdad739..dbea756 100644
@@ -1004,6 +1004,19 @@ int usb_lowlevel_init(int index, void
 **controller)
 
  	return 0;
 
  }
Quoted 10 lines

drivers/usb/host/ehci-hcd.c int usb_lowlevel_init(int index, void **controller) · line 1011

Quoted 5 lines
drivers/usb/host/ehci-hcd.c +5 -0
+int usb_lowlevel_init_device(int index, void **controller)
+{
+	int rc = ehci_hcd_init(index, &ehcic[index].hccr, &ehcic[index].hcor);
+
+	/* rc == 0 means host mode, failure for us */

This looks pretty error-prone. A much better idea would be to use usb_lowlevel_init() and add a flag to init the controller in either device or host mode. That way you'd not even have to craft this strange new API.

How would you like the last parameter?

Like "[PATCH v4] usb: new board-specific USB init interface"

enum board_usb_init_type {
USB_INIT_HOST,
USB_INIT_DEVICE
};

Quoted 4 lines

Or just a plain int device_mode ?

Should I wait for you to apply the above referenced patch before resending ?

Which one? I applied and pushed all I could.

Quoted 1 line

Should I add the same parameter to ehci_hcd_init ?

Makes sense, no ?

Quoted 2 lines

And just to be perfectly clear, you want me to add the parameter to all 19 current occurrences of usb_lowlevel_init ?

Yes, it does make sense, does it not? But look at the last take of this patch here:

http://patchwork.ozlabs.org/patch/273911/

I'm sure some cooperation on this API would be in place now [...]

Best regards,
Marek Vasut

author
#12 lore raw

PATCH v5 4/18 usb: gadget: ether set wMaxPacketSize

In reply to Troy Kisky

set wMaxPacketSize for full speed descriptors
fs_source_desc, fs_sink_desc to 64.

Full-speed bulk endpoint can have a maximum packet size of 8, 16, 32, or 64 bytes, so choice 64.

The hs_source_desc, hs_sink_desc, already have their wMaxPacketSize set to 512. That is the only legal value for high speed bulk endpoints.

Strictly speaking, this patch is not needed because usb_ep_autoconfig will call ep_matches which will override wMaxPacketSize for BULK endpoints only with the size associated with the endpoint setup by the udc driver. But if you want to rely on this, you may as well combine the full speed descriptor with the high speed descriptor to minimize confusion.

Signed-off-by Troy Kisky <[email protected]>

---
v4: expanded commit message
v5: expanded commit message

---
 drivers/usb/gadget/ether.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/usb/gadget/ether.c +2 -0
index 700d5fb..988cffb 100644
@@ -635,6 +635,7 @@ fs_source_desc = {
 
 	.bEndpointAddress =	USB_DIR_IN,
 	.bmAttributes =		USB_ENDPOINT_XFER_BULK,
+	.wMaxPacketSize =	__constant_cpu_to_le16(64),
 };
 
 static struct usb_endpoint_descriptor

@@ -644,6 +645,7 @@ fs_sink_desc = {
 
 	.bEndpointAddress =	USB_DIR_OUT,
 	.bmAttributes =		USB_ENDPOINT_XFER_BULK,
+	.wMaxPacketSize =	__constant_cpu_to_le16(64),
 };
 
 static const struct usb_descriptor_header *fs_eth_function[11] = {
Signature · 1 line
1.8.1.2
#13 lore raw

PATCH v5 4/18 usb: gadget: ether set wMaxPacketSize

Dear Troy Kisky,

Quoted 17 lines

set wMaxPacketSize for full speed descriptors
fs_source_desc, fs_sink_desc to 64.

Full-speed bulk endpoint can have a maximum packet size of 8, 16, 32, or 64 bytes, so choice 64.

The hs_source_desc, hs_sink_desc, already have their wMaxPacketSize set to 512. That is the only legal value for high speed bulk endpoints.

Strictly speaking, this patch is not needed because usb_ep_autoconfig will call ep_matches which will override wMaxPacketSize for BULK endpoints only with the size associated with the endpoint setup by the udc driver. But if you want to rely on this, you may as well combine the full speed descriptor with the high speed descriptor to minimize confusion.

Quoted 1 line
Signed-off-by Troy Kisky <[email protected]>

Picking for -next, thanks

Best regards,
Marek Vasut

author
#14 lore raw

PATCH v5 5/18 usb: gadget: ether: return error from rx_submit if no request

In reply to Troy Kisky

This prevents a crash if tftpboot is given a bad filename.

rx_req will be released by eth_reset_config
which is called by eth_disconnect,
which is called using the .disconnect member of usb_gadget_driver by mv_pullup in mv_udc which is called using the .pullup member of usb_gadget_ops by usb_gadget_disconnect which is called by usb_eth_halt
which is called using the .halt member of eth_device by eth_halt
which is called by TftpHandler when TFTP_ERR_FILE_NOT_FOUND or TFTP_ERR_ACCESS_DENIED occurs

I trigger this with the following commands
setenv ipaddr 10.0.0.2 && setenv netmask 255.255.255.0 && setenv serverip 10.0.0.1 setenv usbnet_devaddr 00:11:22:33:44:55 && setenv usbnet_hostaddr 00:aa:bb:cc:dd:ee setenv ethprime usb_ether && setenv ethact usb_ether && setenv ncip 10.0.0.1 tftpboot 10800000 10.0.0.1:missing_file

Signed-off-by Troy Kisky <[email protected]>

---
v4: expanded commit message
v5: added trigger to commit message

---
 drivers/usb/gadget/ether.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/usb/gadget/ether.c +2 -0
index 988cffb..cc6cc1f 100644
@@ -1536,6 +1536,8 @@ static int rx_submit(struct eth_dev *dev, struct usb_request *req,
 	 */
 
 	debug("%s\n", __func__);
+	if (!req)
+		return -EINVAL;
 
 	size = (ETHER_HDR_SIZE + dev->mtu + RX_EXTRA);
 	size += dev->out_ep->maxpacket - 1;
Signature · 1 line
1.8.1.2
#15 lore raw

PATCH v5 5/18 usb: gadget: ether: return error from rx_submit if no request

Dear Troy Kisky,

Quoted 11 lines

This prevents a crash if tftpboot is given a bad filename.

rx_req will be released by eth_reset_config
which is called by eth_disconnect,
which is called using the .disconnect member of usb_gadget_driver by mv_pullup in mv_udc which is called using the .pullup member of usb_gadget_ops by usb_gadget_disconnect which is called by usb_eth_halt which is called using the .halt member of eth_device by eth_halt which is called by TftpHandler when TFTP_ERR_FILE_NOT_FOUND or TFTP_ERR_ACCESS_DENIED occurs

Quoted 8 lines

I trigger this with the following commands
setenv ipaddr 10.0.0.2 && setenv netmask 255.255.255.0 && setenv serverip 10.0.0.1 setenv usbnet_devaddr 00:11:22:33:44:55 && setenv usbnet_hostaddr 00:aa:bb:cc:dd:ee setenv ethprime usb_ether && setenv ethact usb_ether && setenv ncip 10.0.0.1 tftpboot 10800000 10.0.0.1:missing_file

Signed-off-by Troy Kisky <[email protected]>

Picking for -next, thanks

Best regards,
Marek Vasut

author
#16 lore raw

PATCH v5 6/18 usb: gadget: mv_udc: split mv_udc.h file

In reply to Troy Kisky

Move defines only needed by mv_udc.c to a file
in the same directory.

This allows usbtty to compile for mv_udc,
but it still doesn't link.

Signed-off-by Troy Kisky <[email protected]>

---
v4: unchanged
v5: unchanged

---
 drivers/usb/gadget/mv_udc.c |   6 +++
 drivers/usb/gadget/mv_udc.h | 115 ++++++++++++++++++++++++++++++++++++++++++
 include/usb/mv_udc.h        | 118 --------------------------------------------
 3 files changed, 121 insertions(+), 118 deletions(-)
 create mode 100644 drivers/usb/gadget/mv_udc.h
drivers/usb/gadget/mv_udc.c +6 -0
index ecd11d7..38d1eb5 100644
@@ -13,9 +13,15 @@
 #include <config.h>
 #include <net.h>
 #include <malloc.h>
+#include <asm/byteorder.h>
 #include <asm/io.h>
+#include <asm/errno.h>
 #include <linux/types.h>
+#include <linux/usb/ch9.h>
+#include <linux/usb/gadget.h>
 #include <usb/mv_udc.h>
+#include "../host/ehci.h"
+#include "mv_udc.h"
 
 /*
  * Check if the system has too long cachelines. If the cachelines are
drivers/usb/gadget/mv_udc.h new file +115 -0

115 lines not on this page · raw message

include/usb/mv_udc.h +0 -118

124 lines not on this page · raw message

Signature · 1 line
1.8.1.2
author
#17 lore raw

PATCH v5 7/18 usb: udc: add udc.h include file

In reply to Troy Kisky

Move common definitions to udc.h
This allows musb_udc.h to be removed as well.

Signed-off-by Troy Kisky <[email protected]>

---
v4: updated commit message
removed ifdef UDC_BULK_HS_PACKET_SIZE since 512 is the only legal value, it shouldn't be overridden.

v5: use #if !defined(CONFIG_PPC) && !defined(CONFIG_OMAP1510) for UDC_xxx_PACKET_SIZE setting

---
 drivers/serial/usbtty.h             |  3 +--
 drivers/usb/gadget/designware_udc.c |  1 +
 drivers/usb/gadget/mpc8xx_udc.c     |  1 +
 drivers/usb/gadget/omap1510_udc.c   |  1 +
 drivers/usb/gadget/pxa27x_udc.c     |  1 +
 drivers/usb/musb/musb_udc.c         |  3 ++-
 include/usb/designware_udc.h        | 31 ----------------------
 include/usb/mpc8xx_udc.h            | 19 +------------
 include/usb/musb_udc.h              | 40 ----------------------------
 include/usb/omap1510_udc.h          | 31 +++++-----------------
 include/usb/pxa27x_udc.h            | 26 +-----------------
 include/usb/udc.h                   | 53 +++++++++++++++++++++++++++++++++++++
 12 files changed, 68 insertions(+), 142 deletions(-)
 delete mode 100644 include/usb/musb_udc.h
 create mode 100644 include/usb/udc.h
drivers/serial/usbtty.h +1 -2
index e243a8e..819dec6 100644
@@ -16,8 +16,6 @@
 #include <usb/mpc8xx_udc.h>
 #elif defined(CONFIG_OMAP1510)
 #include <usb/omap1510_udc.h>
-#elif defined(CONFIG_MUSB_UDC)
-#include <usb/musb_udc.h>
 #elif defined(CONFIG_CPU_PXA27X)
 #include <usb/pxa27x_udc.h>
 #elif defined(CONFIG_DW_UDC)

@@ -26,6 +24,7 @@
 #include <usb/mv_udc.h>
 #endif
 
+#include <usb/udc.h>
 #include <version.h>
 
 /* If no VendorID/ProductID is defined in config.h, pretend to be Linux
drivers/usb/gadget/designware_udc.c +1 -0
index 1aab31b..b7c1038 100644
@@ -14,6 +14,7 @@
 #include <usbdevice.h>
 #include "ep0.h"
 #include <usb/designware_udc.h>
+#include <usb/udc.h>
 #include <asm/arch/hardware.h>
 
 #define UDC_INIT_MDELAY		80	/* Device settle delay */
drivers/usb/gadget/mpc8xx_udc.c +1 -0
index 0207d39..7f72972 100644
@@ -47,6 +47,7 @@
 #include <commproc.h>
 #include <usbdevice.h>
 #include <usb/mpc8xx_udc.h>
+#include <usb/udc.h>
 
 #include "ep0.h"
drivers/usb/gadget/omap1510_udc.c +1 -0
index 8553fe5..bdc1b88 100644
@@ -20,6 +20,7 @@
 #endif
 #include <usbdevice.h>
 #include <usb/omap1510_udc.h>
+#include <usb/udc.h>
 
 #include "ep0.h"
drivers/usb/gadget/pxa27x_udc.c +1 -0
index 05d1b56..733558d 100644
@@ -16,6 +16,7 @@
 #include <asm/arch/hardware.h>
 #include <asm/io.h>
 #include <usb/pxa27x_udc.h>
+#include <usb/udc.h>
 
 #include "ep0.h"
drivers/usb/musb/musb_udc.c +2 -1
index 3e3e05e..87640f4 100644
@@ -39,7 +39,8 @@
  */
 
 #include <common.h>
-#include <usb/musb_udc.h>
+#include <usbdevice.h>
+#include <usb/udc.h>
 #include "../gadget/ep0.h"
 #include "musb_core.h"
 #if defined(CONFIG_USB_OMAP3)
include/usb/designware_udc.h +0 -31
index 2e29a7e..2e1cdf1 100644
@@ -174,19 +174,6 @@ struct udcfifo_regs {
 };
 
 /*
- * USBTTY definitions
- */
-#define  EP0_MAX_PACKET_SIZE		64
-#define  UDC_INT_ENDPOINT		1
-#define  UDC_INT_PACKET_SIZE		64
-#define  UDC_OUT_ENDPOINT		2
-#define  UDC_BULK_PACKET_SIZE		64
-#define  UDC_BULK_HS_PACKET_SIZE	512
-#define  UDC_IN_ENDPOINT		3
-#define  UDC_OUT_PACKET_SIZE		64
-#define  UDC_IN_PACKET_SIZE		64
-
-/*
  * UDC endpoint definitions
  */
 #define  UDC_EP0			0

@@ -194,22 +181,4 @@ struct udcfifo_regs {
 #define  UDC_EP2			2
 #define  UDC_EP3			3
 
-/*
- * Function declarations
- */
-
-void udc_irq(void);
-
-void udc_set_nak(int epid);
-void udc_unset_nak(int epid);
-int udc_endpoint_write(struct usb_endpoint_instance *endpoint);
-int udc_init(void);
-void udc_enable(struct usb_device_instance *device);
-void udc_disable(void);
-void udc_connect(void);
-void udc_disconnect(void);
-void udc_startup_events(struct usb_device_instance *device);
-void udc_setup_ep(struct usb_device_instance *device, unsigned int ep,
-		  struct usb_endpoint_instance *endpoint);
-
 #endif /* __DW_UDC_H */
include/usb/mpc8xx_udc.h +1 -18
index 475dd41..9906c75 100644
@@ -111,11 +111,9 @@
 
 /* UDC device defines */
 #define EP0_MAX_PACKET_SIZE	EP_MAX_PKT
-#define UDC_OUT_ENDPOINT	0x02
+
 #define UDC_OUT_PACKET_SIZE	EP_MIN_PACKET_SIZE
-#define UDC_IN_ENDPOINT		0x03
 #define UDC_IN_PACKET_SIZE	EP_MIN_PACKET_SIZE
-#define UDC_INT_ENDPOINT	0x01
 #define UDC_INT_PACKET_SIZE	UDC_IN_PACKET_SIZE
 #define UDC_BULK_PACKET_SIZE	EP_MIN_PACKET_SIZE
 

@@ -178,18 +176,3 @@ typedef enum mpc8xx_udc_state{
 	STATE_READY,
 }mpc8xx_udc_state_t;
 
-/* Declarations */
-int udc_init(void);
-void udc_irq(void);
-int udc_endpoint_write(struct usb_endpoint_instance *endpoint);
-void udc_setup_ep(struct usb_device_instance *device, unsigned int ep,
-		  struct usb_endpoint_instance *endpoint);
-void udc_connect(void);
-void udc_disconnect(void);
-void udc_enable(struct usb_device_instance *device);
-void udc_disable(void);
-void udc_startup_events(struct usb_device_instance *device);
-
-/* Flow control */
-void udc_set_nak(int epid);
-void udc_unset_nak (int epid);
include/usb/musb_udc.h deleted +0 -40
deleted file mode 100644
index 3500c7a..0000000
@@ -1,40 +0,0 @@
-/*
- * Copyright (c) 2009 Wind River Systems, Inc.
- * Tom Rix <[email protected]>
- *
- * SPDX-License-Identifier:	GPL-2.0+
- */
-#ifndef __MUSB_UDC_H__
-#define __MUSB_UDC_H__
-
-#include <usbdevice.h>
-
-/* UDC level routines */
-void udc_irq(void);
-void udc_set_nak(int ep_num);
-void udc_unset_nak(int ep_num);
-int udc_endpoint_write(struct usb_endpoint_instance *endpoint);
-void udc_setup_ep(struct usb_device_instance *device, unsigned int id,
-		  struct usb_endpoint_instance *endpoint);
-void udc_connect(void);
-void udc_disconnect(void);
-void udc_enable(struct usb_device_instance *device);
-void udc_disable(void);
-void udc_startup_events(struct usb_device_instance *device);
-int udc_init(void);
-
-/* usbtty */
-#ifdef CONFIG_USB_TTY
-
-#define EP0_MAX_PACKET_SIZE	64 /* MUSB_EP0_FIFOSIZE */
-#define UDC_INT_ENDPOINT	1
-#define UDC_INT_PACKET_SIZE	64
-#define UDC_OUT_ENDPOINT	2
-#define UDC_OUT_PACKET_SIZE	64
-#define UDC_IN_ENDPOINT		3
-#define UDC_IN_PACKET_SIZE	64
-#define UDC_BULK_PACKET_SIZE	64
-
-#endif /* CONFIG_USB_TTY */
-
-#endif /* __MUSB_UDC_H__ */
include/usb/omap1510_udc.h +6 -25
index ece0e95..adfbf54 100644
@@ -162,32 +162,13 @@
 #define UDC_VBUS_MODE	    (1 << 18)
 
 /* OMAP Endpoint parameters */
-#define EP0_MAX_PACKET_SIZE 64
-#define UDC_OUT_ENDPOINT 2
-#define UDC_OUT_PACKET_SIZE 64
-#define UDC_IN_ENDPOINT	1
-#define UDC_IN_PACKET_SIZE 64
-#define UDC_INT_ENDPOINT 5
+#define UDC_OUT_PACKET_SIZE	64
+#define UDC_IN_PACKET_SIZE	64
 #define UDC_INT_PACKET_SIZE	16
-#define UDC_BULK_PACKET_SIZE 16
-
-void udc_irq (void);
-/* Flow control */
-void udc_set_nak(int epid);
-void udc_unset_nak (int epid);
-
-/* Higher level functions for abstracting away from specific device */
-int udc_endpoint_write(struct usb_endpoint_instance *endpoint);
-
-int  udc_init (void);
+#define UDC_BULK_PACKET_SIZE	16
 
-void udc_enable(struct usb_device_instance *device);
-void udc_disable(void);
-
-void udc_connect(void);
-void udc_disconnect(void);
-
-void udc_startup_events(struct usb_device_instance *device);
-void udc_setup_ep(struct usb_device_instance *device, unsigned int ep, struct usb_endpoint_instance *endpoint);
+#define UDC_INT_ENDPOINT 5
+#define UDC_OUT_ENDPOINT 2
+#define UDC_IN_ENDPOINT	1
 
 #endif
include/usb/pxa27x_udc.h +1 -25
index 7fdbe2a..7eaa000 100644
@@ -22,35 +22,11 @@
 
 /* Endpoint parameters */
 #define MAX_ENDPOINTS		4
-#define EP_MAX_PACKET_SIZE	64
 
 #define EP0_MAX_PACKET_SIZE     16
+
 #define UDC_OUT_ENDPOINT        0x02
-#define UDC_OUT_PACKET_SIZE     EP_MAX_PACKET_SIZE
 #define UDC_IN_ENDPOINT         0x01
-#define UDC_IN_PACKET_SIZE      EP_MAX_PACKET_SIZE
 #define UDC_INT_ENDPOINT        0x05
-#define UDC_INT_PACKET_SIZE     EP_MAX_PACKET_SIZE
-#define UDC_BULK_PACKET_SIZE    EP_MAX_PACKET_SIZE
-
-void udc_irq(void);
-/* Flow control */
-void udc_set_nak(int epid);
-void udc_unset_nak(int epid);
-
-/* Higher level functions for abstracting away from specific device */
-int udc_endpoint_write(struct usb_endpoint_instance *endpoint);
-
-int  udc_init(void);
-
-void udc_enable(struct usb_device_instance *device);
-void udc_disable(void);
-
-void udc_connect(void);
-void udc_disconnect(void);
-
-void udc_startup_events(struct usb_device_instance *device);
-void udc_setup_ep(struct usb_device_instance *device,
-	 unsigned int ep, struct usb_endpoint_instance *endpoint);
 
 #endif
include/usb/udc.h new file +53 -0

53 lines not on this page · raw message

Signature · 1 line
1.8.1.2
#18 lore raw

PATCH v5 7/18 usb: udc: add udc.h include file

Dear Troy Kisky,

Quoted 4 lines

Move common definitions to udc.h
This allows musb_udc.h to be removed as well.

Signed-off-by Troy Kisky <[email protected]>

Did you get any feedback from the MPC8xx or OMAP1 people? I would say reordering of this patchset so that the more controversial changes go last would be a good idea. I dont want to stall the easy patches.

Best regards,
Marek Vasut

author
#19 lore raw

PATCH v5 7/18 usb: udc: add udc.h include file

Quoted 5 lines · Marek Vasut

Dear Troy Kisky,

Quoted 3 lines

Move common definitions to udc.h
This allows musb_udc.h to be removed as well.

Quoted 8 lines
Quoted 1 line
Signed-off-by Troy Kisky <[email protected]>

Did you get any feedback from the MPC8xx or OMAP1 people? I would say reordering of this patchset so that the more controversial changes go last would be a good idea. I dont want to stall the easy patches.

Best regards,
Marek Vasut

No, my email bounced. But this patch merely saves lines of code. It can be dropped without an effect.

Troy

author
#20 lore raw

PATCH v5 8/18 usb: gadget: mv_udc: fix typo in error message

In reply to Troy Kisky

Change 'nfo=' to 'info='

Signed-off-by Troy Kisky <[email protected]>

---
v4: no change
v5: no change

---
 drivers/usb/gadget/mv_udc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/usb/gadget/mv_udc.c +2 -2
index 38d1eb5..4910659 100644
@@ -368,8 +368,8 @@ static void handle_ep_complete(struct mv_ep *ep)
 	mv_invalidate_qtd(num);
 	
 	if (item->info & 0xff)
-		printf("EP%d/%s FAIL nfo=%x pg0=%x\n",
-			num, in ? "in" : "out", item->info, item->page0);
+		printf("EP%d/%s FAIL info=%x pg0=%x\n",
+		       num, in ? "in" : "out", item->info, item->page0);
 
 	len = (item->info >> 16) & 0x7fff;
Signature · 1 line
1.8.1.2
author
#22 lore raw

PATCH v5 9/18 usb: gadget: mv_udc: set is_dualspeed = 1

In reply to Troy Kisky

This controller support full and high speed.

Signed-off-by Troy Kisky <[email protected]>

---
v4: no change
v5: no change

---
 drivers/usb/gadget/mv_udc.c | 1 +
 1 file changed, 1 insertion(+)
drivers/usb/gadget/mv_udc.c +1 -0
index 4910659..0591111 100644
@@ -109,6 +109,7 @@ static struct mv_drv controller = {
 	.gadget	= {
 		.name	= "mv_udc",
 		.ops	= &mv_udc_ops,
+		.is_dualspeed = 1,
 	},
 };
Signature · 1 line
1.8.1.2
#23 lore raw

PATCH v5 9/18 usb: gadget: mv_udc: set is_dualspeed = 1

Dear Troy Kisky,

Quoted 4 lines

This controller support full and high speed.

Signed-off-by Troy Kisky <[email protected]>

Picking for -next, thanks

Best regards,
Marek Vasut

author
#24 lore raw

PATCH v5 10/18 usb: gadget: mv_udc: fix full speed connections

In reply to Troy Kisky

Set maximum packet length in queue header to wMaxPacketSize of endpoint.

Signed-off-by Troy Kisky <[email protected]>

---
v4: no change
v5: no change

---
 drivers/usb/gadget/mv_udc.c | 48 ++++++++++++++++++++++++++++++++-------------
 1 file changed, 34 insertions(+), 14 deletions(-)
drivers/usb/gadget/mv_udc.c +34 -14
index 0591111..36cf47f 100644
@@ -16,6 +16,7 @@
 #include <asm/byteorder.h>
 #include <asm/io.h>
 #include <asm/errno.h>
+#include <asm/unaligned.h>
 #include <linux/types.h>
 #include <linux/usb/ch9.h>
 #include <linux/usb/gadget.h>

@@ -213,7 +214,7 @@ static void mv_ep_free_request(struct usb_ep *ep, struct usb_request *_req)
 	return;
 }
 
-static void ep_enable(int num, int in)
+static void ep_enable(int num, int in, int maxpacket)
 {
 	struct ept_queue_head *head;
 	struct mv_udc *udc = (struct mv_udc *)controller.ctrl->hcor;

@@ -227,7 +228,7 @@ static void ep_enable(int num, int in)
 		n |= (CTRL_RXE | CTRL_RXR | CTRL_RXT_BULK);
 
 	if (num != 0) {
-		head->config = CONFIG_MAX_PKT(EP_MAX_PACKET_SIZE) | CONFIG_ZLT;
+		head->config = CONFIG_MAX_PKT(maxpacket) | CONFIG_ZLT;
 		mv_flush_qh(num);
 	}
 	writel(n, &udc->epctrl[num]);

@@ -240,8 +241,21 @@ static int mv_ep_enable(struct usb_ep *ep,
 	int num, in;
 	num = desc->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK;
 	in = (desc->bEndpointAddress & USB_DIR_IN) != 0;
-	ep_enable(num, in);
 	mv_ep->desc = desc;
+
+	if (num) {
+		int max = get_unaligned_le16(&desc->wMaxPacketSize);
+
+		if ((max > 64) && (controller.gadget.speed == USB_SPEED_FULL))
+			max = 64;
+		if (ep->maxpacket != max) {
+			DBG("%s: from %d to %d\n", __func__,
+			    ep->maxpacket, max);
+			ep->maxpacket = max;
+		}
+	}
+	ep_enable(num, in, ep->maxpacket);
+	DBG("%s: num=%d maxpacket=%d\n", __func__, num, ep->maxpacket);
 	return 0;
 }
 

@@ -414,14 +428,16 @@ static void handle_setup(void)
 		if ((r.wValue == 0) && (r.wLength == 0)) {
 			req->length = 0;
 			for (i = 0; i < NUM_ENDPOINTS; i++) {
-				if (!controller.ep[i].desc)
+				struct mv_ep *ep = &controller.ep[i];
+
+				if (!ep->desc)
 					continue;
-				num = controller.ep[i].desc->bEndpointAddress
+				num = ep->desc->bEndpointAddress
 						& USB_ENDPOINT_NUMBER_MASK;
-				in = (controller.ep[i].desc->bEndpointAddress
+				in = (ep->desc->bEndpointAddress
 						& USB_DIR_IN) != 0;
 				if ((num == _num) && (in == _in)) {
-					ep_enable(num, in);
+					ep_enable(num, in, ep->ep.maxpacket);
 					usb_ep_queue(controller.gadget.ep0,
 							req, 0);
 					break;

@@ -505,15 +521,19 @@ void udc_irq(void)
 		DBG("-- suspend --\n");
 
 	if (n & STS_PCI) {
-		DBG("-- portchange --\n");
+		int max = 64;
+		int speed = USB_SPEED_FULL;
+
 		bit = (readl(&udc->portsc) >> 26) & 3;
+		DBG("-- portchange %x %s\n", bit, (bit == 2) ? "High" : "Full");
 		if (bit == 2) {
-			controller.gadget.speed = USB_SPEED_HIGH;
-			for (i = 1; i < NUM_ENDPOINTS && n; i++)
-				if (controller.ep[i].desc)
-					controller.ep[i].ep.maxpacket = 512;
-		} else {
-			controller.gadget.speed = USB_SPEED_FULL;
+			speed = USB_SPEED_HIGH;
+			max = 512;
+		}
+		controller.gadget.speed = speed;
+		for (i = 1; i < NUM_ENDPOINTS; i++) {
+			if (controller.ep[i].ep.maxpacket > max)
+				controller.ep[i].ep.maxpacket = max;
 		}
 	}
Signature · 1 line
1.8.1.2
#25 lore raw

PATCH v5 10/18 usb: gadget: mv_udc: fix full speed connections

Dear Troy Kisky,

Quoted 5 lines

Set maximum packet length in queue header to wMaxPacketSize of endpoint.

Signed-off-by Troy Kisky <[email protected]>
Quoted 7 lines

---
v4: no change
v5: no change

---
 drivers/usb/gadget/mv_udc.c | 48

++++++++++++++++++++++++++++++++------------- 1 file changed, 34 insertions(+), 14 deletions(-)

Can you rebase on top of usb/master please?

Best regards,
Marek Vasut

author
#26 lore raw

PATCH v5 10/18 usb: gadget: mv_udc: fix full speed connections

Quoted 14 lines · Marek Vasut

Dear Troy Kisky,

Quoted 5 lines

Set maximum packet length in queue header to wMaxPacketSize of endpoint.

Signed-off-by Troy Kisky <[email protected]>
Quoted 7 lines

---
v4: no change
v5: no change

---
  drivers/usb/gadget/mv_udc.c | 48

++++++++++++++++++++++++++++++++------------- 1 file changed, 34 insertions(+), 14 deletions(-)

Quoted 5 lines

Can you rebase on top of usb/master please?

Best regards,
Marek Vasut

Do you mean usb/next ??

Thanks
Troy

#27 lore raw

PATCH v5 10/18 usb: gadget: mv_udc: fix full speed connections

Dear Troy Kisky,

Quoted 18 lines · Marek Vasut
Quoted 17 lines

Dear Troy Kisky,

Quoted 14 lines

Set maximum packet length in queue header to wMaxPacketSize of endpoint.

Signed-off-by Troy Kisky <[email protected]>

---
v4: no change
v5: no change
---

  drivers/usb/gadget/mv_udc.c | 48

++++++++++++++++++++++++++++++++------------- 1 file changed, 34 insertions(+), 14 deletions(-)

Quoted 6 lines
Quoted 4 lines

Can you rebase on top of usb/master please?

Best regards,
Marek Vasut

Do you mean usb/next ??

git://git.denx.de/u-boot-usb.git :: next branch

Best regards,
Marek Vasut

author
#28 lore raw

PATCH v5 11/18 usb: gadget: mv_udc: optimize bounce

In reply to Troy Kisky

Only perform one copy, either in the bounce routine for IN transfers, or the debounce rtn for OUT transfer.

On out transfers, only copy the number of bytes received from the bounce buffer

Signed-off-by Troy Kisky <[email protected]>

---
v4: no change
v5: no change

---
 drivers/usb/gadget/mv_udc.c | 37 ++++++++++++++++---------------------
 1 file changed, 16 insertions(+), 21 deletions(-)
drivers/usb/gadget/mv_udc.c +16 -21
index 36cf47f..a2cbe67 100644
@@ -264,7 +264,7 @@ static int mv_ep_disable(struct usb_ep *ep)
 	return 0;
 }
 
-static int mv_bounce(struct mv_ep *ep)
+static int mv_bounce(struct mv_ep *ep, int in)
 {
 	uint32_t addr = (uint32_t)ep->req.buf;
 	uint32_t ba;

@@ -293,8 +293,8 @@ align:
 		if (!ep->b_buf)
 			return -ENOMEM;
 	}
-
-	memcpy(ep->b_buf, ep->req.buf, ep->req.length);
+	if (in)
+		memcpy(ep->b_buf, ep->req.buf, ep->req.length);
 
 flush:
 	ba = (uint32_t)ep->b_buf;

@@ -303,29 +303,25 @@ flush:
 	return 0;
 }
 
-static void mv_debounce(struct mv_ep *ep)
+static void mv_debounce(struct mv_ep *ep, int in)
 {
 	uint32_t addr = (uint32_t)ep->req.buf;
 	uint32_t ba = (uint32_t)ep->b_buf;
 
+	if (in) {
+		if (addr == ba)
+			return;		/* not a bounce */
+		goto free;
+	}
 	invalidate_dcache_range(ba, ba + ep->b_len);
 
-	/* Input buffer address is not aligned. */
-	if (addr & (ARCH_DMA_MINALIGN - 1))
-		goto copy;
-
-	/* Input buffer length is not aligned. */
-	if (ep->req.length & (ARCH_DMA_MINALIGN - 1))
-		goto copy;
-
-	/* The buffer is well aligned, only invalidate cache. */
-	return;
+	if (addr == ba)
+		return;		/* not a bounce */
 
-copy:
 	memcpy(ep->req.buf, ep->b_buf, ep->req.length);
-
+free:
 	/* Large payloads use allocated buffer, free it. */
-	if (ep->req.length > 64)
+	if (ep->b_buf != ep->b_fast)
 		free(ep->b_buf);
 }
 

@@ -343,7 +339,7 @@ static int mv_ep_queue(struct usb_ep *ep,
 	head = mv_get_qh(num, in);
 	len = req->length;
 
-	ret = mv_bounce(mv_ep);
+	ret = mv_bounce(mv_ep, in);
 	if (ret)
 		return ret;
 

@@ -387,10 +383,9 @@ static void handle_ep_complete(struct mv_ep *ep)
 		       num, in ? "in" : "out", item->info, item->page0);
 
 	len = (item->info >> 16) & 0x7fff;
-
-	mv_debounce(ep);
-
 	ep->req.length -= len;
+	mv_debounce(ep, in);
+
 	DBG("ept%d %s complete %x\n",
 			num, in ? "in" : "out", len);
 	ep->req.complete(&ep->ep, &ep->req);
Signature · 1 line
1.8.1.2
author
#29 lore raw

PATCH v5 12/18 usb: gadget: mv_udc: flush item before head

In reply to Troy Kisky

Make sure the transfer descriptor is flushed before the queue is updated so that the controller will not see old information.

Signed-off-by Troy Kisky <[email protected]>

---
v4: no change
v5: no change

---
 drivers/usb/gadget/mv_udc.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
drivers/usb/gadget/mv_udc.c +2 -3
index a2cbe67..3509aa8 100644
@@ -347,21 +347,20 @@ static int mv_ep_queue(struct usb_ep *ep,
 	item->info = INFO_BYTES(len) | INFO_IOC | INFO_ACTIVE;
 	item->page0 = (uint32_t)mv_ep->b_buf;
 	item->page1 = ((uint32_t)mv_ep->b_buf & 0xfffff000) + 0x1000;
+	mv_flush_qtd(num);
 
 	head->next = (unsigned) item;
 	head->info = 0;
 
 	DBG("ept%d %s queue len %x, buffer %p\n",
 	    num, in ? "in" : "out", len, mv_ep->b_buf);
+	mv_flush_qh(num);
 
 	if (in)
 		bit = EPT_TX(num);
 	else
 		bit = EPT_RX(num);
 
-	mv_flush_qh(num);
-	mv_flush_qtd(num);
-
 	writel(bit, &udc->epprime);
 
 	return 0;
Signature · 1 line
1.8.1.2
#30 lore raw

PATCH v5 12/18 usb: gadget: mv_udc: flush item before head

Dear Troy Kisky,

Quoted 6 lines

Make sure the transfer descriptor is flushed before the queue is updated so that the controller will not see old information.

Signed-off-by Troy Kisky <[email protected]>

Picking for -next, thanks
Best regards,
Marek Vasut

author
#31 lore raw

PATCH v5 13/18 usb: gadget: mv_udc: optimize ep_enable

In reply to Troy Kisky

Only get head if not ep0.

Signed-off-by Troy Kisky <[email protected]>

---
v4: no change
v5: no change

---
 drivers/usb/gadget/mv_udc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/usb/gadget/mv_udc.c +2 -2
index 3509aa8..ff3f3b5 100644
@@ -216,10 +216,8 @@ static void mv_ep_free_request(struct usb_ep *ep, struct usb_request *_req)
 
 static void ep_enable(int num, int in, int maxpacket)
 {
-	struct ept_queue_head *head;
 	struct mv_udc *udc = (struct mv_udc *)controller.ctrl->hcor;
 	unsigned n;
-	head = mv_get_qh(num, in);
 
 	n = readl(&udc->epctrl[num]);
 	if (in)

@@ -228,6 +226,8 @@ static void ep_enable(int num, int in, int maxpacket)
 		n |= (CTRL_RXE | CTRL_RXR | CTRL_RXT_BULK);
 
 	if (num != 0) {
+		struct ept_queue_head *head = mv_get_qh(num, in);
+
 		head->config = CONFIG_MAX_PKT(maxpacket) | CONFIG_ZLT;
 		mv_flush_qh(num);
 	}
Signature · 1 line
1.8.1.2
author
#32 lore raw

PATCH v5 14/18 usb: gadget: mv_udc: zero transfer descriptor memory on probe

In reply to Troy Kisky

Since we flush the TD, we may as well set it to a known value.

Signed-off-by Troy Kisky <[email protected]>

---
v4: no change
v5: no change

---
 drivers/usb/gadget/mv_udc.c | 1 +
 1 file changed, 1 insertion(+)
drivers/usb/gadget/mv_udc.c +1 -0
index ff3f3b5..091c566 100644
@@ -643,6 +643,7 @@ static int mvudc_probe(void)
 		free(controller.epts);
 		return -ENOMEM;
 	}
+	memset(controller.items_mem, 0, ilist_sz);
 
 	for (i = 0; i < 2 * NUM_ENDPOINTS; i++) {
 		/*
Signature · 1 line
1.8.1.2
#33 lore raw

PATCH v5 14/18 usb: gadget: mv_udc: zero transfer descriptor memory on probe

Dear Troy Kisky,

Quoted 4 lines

Since we flush the TD, we may as well set it to a known value.

Signed-off-by Troy Kisky <[email protected]>

Picking for -next, thanks

Best regards,
Marek Vasut

author
#34 lore raw

PATCH v5 15/18 usb: gadget: mv_udc: clear desc upon ep_disable

In reply to Troy Kisky

desc is set at ep_enable, so for symmetry,
clear it at ep_disable.

Signed-off-by Troy Kisky <[email protected]>

---
v4: no change
v5: no change

---
 drivers/usb/gadget/mv_udc.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/usb/gadget/mv_udc.c +3 -0
index 091c566..bfe099b 100644
@@ -261,6 +261,9 @@ static int mv_ep_enable(struct usb_ep *ep,
 
 static int mv_ep_disable(struct usb_ep *ep)
 {
+	struct mv_ep *mv_ep = container_of(ep, struct mv_ep, ep);
+
+	mv_ep->desc = NULL;
 	return 0;
 }
Signature · 1 line
1.8.1.2
#35 lore raw

PATCH v5 15/18 usb: gadget: mv_udc: clear desc upon ep_disable

Dear Troy Kisky,

Quoted 5 lines

desc is set at ep_enable, so for symmetry,
clear it at ep_disable.

Signed-off-by Troy Kisky <[email protected]>

Picking for -next, thanks

Best regards,
Marek Vasut

author
#36 lore raw

PATCH v5 16/18 mx6: iomux: add GPR1 defines for use with nitrogen6x

In reply to Troy Kisky

Select GPIO1 as the USB OTG ID pin for Nitrogen6x

Signed-off-by Troy Kisky <[email protected]>

---
v4: move the Nitrogen6x bits into this patch as well
v5: no change

---
 arch/arm/include/asm/arch-mx6/iomux.h  | 6 ++++++
 board/boundary/nitrogen6x/nitrogen6x.c | 7 +++++++
 2 files changed, 13 insertions(+)
arch/arm/include/asm/arch-mx6/iomux.h +6 -0
index f4cfd4f..9e6d40f 100644
@@ -10,6 +10,12 @@
 #define MX6_IOMUXC_GPR7		0x020e001c
 
 /*
+ * IOMUXC_GPR1 bit fields
+ */
+#define IOMUXC_GPR1_OTG_ID_ENET_RX_ERR	(0<<13)
+#define IOMUXC_GPR1_OTG_ID_GPIO1	(1<<13)
+#define IOMUXC_GPR1_OTG_ID_MASK		(1<<13)
+/*
  * IOMUXC_GPR13 bit fields
  */
 #define IOMUXC_GPR13_SDMA_STOP_REQ	(1<<30)
board/boundary/nitrogen6x/nitrogen6x.c +7 -0
index f664f6d..c6efafc 100644
@@ -704,6 +704,13 @@ int overwrite_console(void)
 
 int board_init(void)
 {
+	struct iomuxc_base_regs *const iomuxc_regs
+		= (struct iomuxc_base_regs *)IOMUXC_BASE_ADDR;
+
+	clrsetbits_le32(&iomuxc_regs->gpr[1],
+			IOMUXC_GPR1_OTG_ID_MASK,
+			IOMUXC_GPR1_OTG_ID_GPIO1);
+
 	/* address of boot parameters */
 	gd->bd->bi_boot_params = PHYS_SDRAM + 0x100;
Signature · 1 line
1.8.1.2
author
#37 lore raw

PATCH v5 17/18 nitrogen6x: add otg usb host/device mode support

In reply to Troy Kisky
Signed-off-by Troy Kisky <[email protected]>

---
v4: similar to Nitrogen6x: add otg usb ethernet gadget support But I added pad setup for over current, otg_id, and power control Added board_ehci_power function to enable power for host mode. Removed otg_id selection done now in previous patch.

v5: changed MX6_PAD_GPIO_1__USBOTG_ID to MX6_PAD_GPIO_1__USB_OTG_ID.

---
 board/boundary/nitrogen6x/nitrogen6x.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)
board/boundary/nitrogen6x/nitrogen6x.c +26 -0
index c6efafc..8523133 100644
@@ -30,6 +30,7 @@
 #include <i2c.h>
 
 DECLARE_GLOBAL_DATA_PTR;
+#define GP_USB_OTG_PWR	IMX_GPIO_NR(3, 22)
 
 #define UART_PAD_CTRL  (PAD_CTL_PUS_100K_UP |			\
 	PAD_CTL_SPEED_MED | PAD_CTL_DSE_40ohm |			\

@@ -179,6 +180,14 @@ iomux_v3_cfg_t const enet_pads2[] = {
 	MX6_PAD_RGMII_RX_CTL__RGMII_RX_CTL	| MUX_PAD_CTRL(ENET_PAD_CTRL),
 };
 
+static iomux_v3_cfg_t const misc_pads[] = {
+	MX6_PAD_GPIO_1__USB_OTG_ID		| MUX_PAD_CTRL(WEAK_PULLUP),
+	MX6_PAD_KEY_COL4__USBOH3_USBOTG_OC	| MUX_PAD_CTRL(WEAK_PULLUP),
+	MX6_PAD_EIM_D30__USBOH3_USBH1_OC	| MUX_PAD_CTRL(WEAK_PULLUP),
+	/* OTG Power enable */
+	MX6_PAD_EIM_D22__GPIO_3_22		| MUX_PAD_CTRL(OUTPUT_40OHM),
+};
+
 /* wl1271 pads on nitrogen6x */
 iomux_v3_cfg_t const wl12xx_pads[] = {
 	(MX6_PAD_NANDF_CS1__GPIO_6_14 & ~MUX_PAD_CTRL_MASK)

@@ -250,6 +259,15 @@ int board_ehci_hcd_init(int port)
 
 	return 0;
 }
+
+int board_ehci_power(int port, int on)
+{
+	if (port)
+		return 0;
+	gpio_set_value(GP_USB_OTG_PWR, on);
+	return 0;
+}
+
 #endif
 
 #ifdef CONFIG_FSL_ESDHC

@@ -369,6 +387,11 @@ int board_eth_init(bd_t *bis)
 		free(bus);
 	}
 #endif
+
+#ifdef CONFIG_MV_UDC
+	/* For otg ethernet*/
+	usb_eth_initialize(bis);
+#endif
 	return 0;
 }
 

@@ -683,6 +706,7 @@ int board_early_init_f(void)
 	gpio_direction_input(WL12XX_WL_IRQ_GP);
 	gpio_direction_output(WL12XX_WL_ENABLE_GP, 0);
 	gpio_direction_output(WL12XX_BT_ENABLE_GP, 0);
+	gpio_direction_output(GP_USB_OTG_PWR, 0); /* OTG power off */
 
 	imx_iomux_v3_setup_multiple_pads(wl12xx_pads, ARRAY_SIZE(wl12xx_pads));
 	setup_buttons();

@@ -711,6 +735,8 @@ int board_init(void)
 			IOMUXC_GPR1_OTG_ID_MASK,
 			IOMUXC_GPR1_OTG_ID_GPIO1);
 
+	imx_iomux_v3_setup_multiple_pads(misc_pads, ARRAY_SIZE(misc_pads));
+
 	/* address of boot parameters */
 	gd->bd->bi_boot_params = PHYS_SDRAM + 0x100;
Signature · 1 line
1.8.1.2
author
#38 lore raw

PATCH v5 18/18 nitrogen6x: add CONFIG_MV_UDC

In reply to Troy Kisky

Also, add other USB related config items.

Signed-off-by Troy Kisky <[email protected]>

---
v4: Updated commit message
v5: no change

---
 include/configs/nitrogen6x.h | 6 ++++++
 1 file changed, 6 insertions(+)
include/configs/nitrogen6x.h +6 -0
index 2e5ae13..a08eea6 100644
@@ -30,6 +30,12 @@
 #define CONFIG_BOARD_EARLY_INIT_F
 #define CONFIG_MISC_INIT_R
 #define CONFIG_MXC_GPIO
+#define CONFIG_MV_UDC
+#define CONFIG_USBD_HS
+#define CONFIG_USB_GADGET_DUALSPEED
+#define CONFIG_USB_ETHER
+#define CONFIG_USB_ETH_CDC
+#define CONFIG_NETCONSOLE
 
 #define CONFIG_CMD_FUSE
 #ifdef CONFIG_CMD_FUSE
Signature · 1 line
1.8.1.2
#39 lore raw

v5 cover Make mv_udc work for i.mx6

In reply to Troy Kisky

Hi Troy, Marek,

Quoted 3 lines · Troy Kisky

Stefano, if you would you like to take the last 3 patches, you should apply "i.MX6Q/DLS: Add MX6_PAD_GPIO_1__USB_OTG_ID" from Eric Nelson first.

Eric's patch is already in my queue, I will apply it.

Quoted 5 lines

The patches are mostly independent, so if a patch has
an issue, please feel free to skip it.
An exception is 1/17 should be before 2/17 if Stefano's branch merges upstream first, otherwise nitrogen6x will get a build error.

Maybe it is better if we agree who merge the patchset to avoid breaking the boards (Nitrogen). Mostly the patchset is Marek's stuff, and I have nothing again if Marek will apply also the i.MX relevant parts. It is better to merge this patchset at whole in the same tree as to split it.

Marek, do you agree ?

Regards,
Stefano

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
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
=====================================================================
author
#40 lore raw

v5 cover Make mv_udc work for i.mx6

Quoted 18 lines · Stefano Babic

Hi Troy, Marek,

Quoted 3 lines · Troy Kisky

Stefano, if you would you like to take the last 3 patches, you should apply "i.MX6Q/DLS: Add MX6_PAD_GPIO_1__USB_OTG_ID" from Eric Nelson first.

Eric's patch is already in my queue, I will apply it.

Quoted 4 lines

The patches are mostly independent, so if a patch has
an issue, please feel free to skip it.
An exception is 1/17 should be before 2/17 if Stefano's branch merges upstream first, otherwise nitrogen6x will get a build error.

Maybe it is better if we agree who merge the patchset to avoid breaking the boards (Nitrogen). Mostly the patchset is Marek's stuff, and I have nothing again if Marek will apply also the i.MX relevant parts. It is better to merge this patchset at whole in the same tree as to split it.

Quoted 6 lines

Marek, do you agree ?

Regards,
Stefano

Then Marek would need Eric's patch, unless a temporary build error is acceptable.

Thanks
Troy

#41 lore raw

v5 cover Make mv_udc work for i.mx6

Dear Troy Kisky,

Quoted 20 lines · Stefano Babic
Quoted 19 lines

Hi Troy, Marek,

Quoted 3 lines · Troy Kisky

Stefano, if you would you like to take the last 3 patches, you should apply "i.MX6Q/DLS: Add MX6_PAD_GPIO_1__USB_OTG_ID" from Eric Nelson first.

Eric's patch is already in my queue, I will apply it.

Quoted 4 lines

The patches are mostly independent, so if a patch has
an issue, please feel free to skip it.
An exception is 1/17 should be before 2/17 if Stefano's branch merges upstream first, otherwise nitrogen6x will get a build error.

Maybe it is better if we agree who merge the patchset to avoid breaking the boards (Nitrogen). Mostly the patchset is Marek's stuff, and I have nothing again if Marek will apply also the i.MX relevant parts. It is better to merge this patchset at whole in the same tree as to split it.

Quoted 7 lines
Quoted 4 lines

Marek, do you agree ?

Regards,
Stefano

Then Marek would need Eric's patch, unless a temporary build error is acceptable.

This will go to -next anyway, so if Stefano can pick this OTG_ID mux patch into master and push it out into current MW, there will be no problem at all.

I'll pick the whole thing otherwise but only AFTER I reviewed it properly.

Best regards,
Marek Vasut

#42 lore raw

v5 cover Make mv_udc work for i.mx6

In reply to Troy Kisky

Dear Troy Kisky,

Quoted 5 lines

Hi Marek, Stefano

This series is based on u-boot-usb/master branch. After this series, nitrogen6x works with tftpboot to transfer files over usb, and will recognize a usb stick in the OTG port.

I applied and pushed what was OK to usb/next. Can you please rebase and repost the rest?

Thanks!

Best regards,
Marek Vasut

author
#43 lore raw

v5 cover Make mv_udc work for i.mx6

Quoted 7 lines · Marek Vasut

Dear Troy Kisky,

Quoted 5 lines

Hi Marek, Stefano

This series is based on u-boot-usb/master branch. After this series, nitrogen6x works with tftpboot to transfer files over usb, and will recognize a usb stick in the OTG port.

Quoted 8 lines

I applied and pushed what was OK to usb/next. Can you please rebase and repost the rest?

Thanks!

Best regards,
Marek Vasut

Sure. Thanks for the review.

Troy