usb: dwc2: gadget: Fix TX FIFO handling

PATCH In review John Youn 3 participants 10 messages Aug 29, 2016 – Sep 2, 2016 1 message not in the archive

Subsystems (3)

Device tree bindings USB DesignWare USB2 DRD IP driver

lore

Share & tools

lore

John Youn

author

2016-08-29 20:38 UTC #0 lore raw

cover usb: dwc2: gadget: Fix TX FIFO handling

Hi,

This is a resend of a patch series originally submitted by Robert Baldyga back in February.

Somehow I missed it back then. Sorry about that.

It should fix some FIFO programming related issues on RPi and maybe other platforms.

Also, this series makes the g-tx-fifo-size DT property obsolete. I've added a patch at the end of the series to document that.

Regards,
John

John Youn (1):
Documentation: devicetree: dwc2: Deprecate g-tx-fifo-size

Robert Baldyga (5):
usb: dwc2: gadget: use ep->fifo_index in context of FIFO registers
usb: dwc2: gadget: fix TX FIFO size and address initialization
usb: dwc2: gadget: change variable name to more meaningful
usb: dwc2: gadget: remove dead code from dwc2_hsotg_ep_enable()
usb: dwc2: gadget: free TX FIFO after killing requests

 Documentation/devicetree/bindings/usb/dwc2.txt |  5 +-
 drivers/usb/dwc2/core.h                        |  7 ---
 drivers/usb/dwc2/gadget.c                      | 79 +++++++-------------------
 3 files changed, 23 insertions(+), 68 deletions(-)

Signature · 6 lines

2.9.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

John Youn

author

2016-08-29 20:38 UTC #1 lore raw

PATCH 1/6 usb: dwc2: gadget: use ep->fifo_index in context of FIFO registers

From: Robert Baldyga <[email protected]>

In context of FIFO registers we use ep->fifo_index instead of ep->index.

Signed-off-by Robert Baldyga <[email protected]>

Signed-off-by John Youn <[email protected]>

---
 drivers/usb/dwc2/gadget.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

drivers/usb/dwc2/gadget.c +3 -2

index af46adf..f6086d6 100644
@@ -388,7 +388,8 @@ static int dwc2_hsotg_write_fifo(struct dwc2_hsotg *hsotg,
 			return -ENOSPC;
 		}
 	} else if (hsotg->dedicated_fifos && hs_ep->index != 0) {
-		can_write = dwc2_readl(hsotg->regs + DTXFSTS(hs_ep->index));
+		can_write = dwc2_readl(hsotg->regs +
+				DTXFSTS(hs_ep->fifo_index));
 
 		can_write &= 0xffff;
 		can_write *= 4;

@@ -2432,7 +2433,7 @@ static void kill_all_requests(struct dwc2_hsotg *hsotg,
 
 	if (!hsotg->dedicated_fifos)
 		return;
-	size = (dwc2_readl(hsotg->regs + DTXFSTS(ep->index)) & 0xffff) * 4;
+	size = (dwc2_readl(hsotg->regs + DTXFSTS(ep->fifo_index)) & 0xffff) * 4;
 	if (size < ep->fifo_size)
 		dwc2_hsotg_txfifo_flush(hsotg, ep->fifo_index);
 }

Signature · 1 line

2.9.0

John Youn

author

2016-08-29 20:38 UTC #2 lore raw

PATCH 2/6 usb: dwc2: gadget: fix TX FIFO size and address initialization

In reply to John Youn

From: Robert Baldyga <[email protected]>

According to DWC2 documentation, DPTxFSize field of DPTXFSIZn register is read only, which means that software cannot change FIFO size.

Register description says:
"The value of this register is the Largest Device Mode Periodic Tx Data FIFO Depth (parameter OTG_TX_DPERIO_DFIFO_DEPTH_n), as specified during coreConsultant configuration."

That means, that we have to setup only FIFO start addresses (DPTxFStAddr), taking into account reset values of DPTxFSize.

Initialize FIFO start addresses properly and remove unneeded core related to incorrect FIFO size initialization.

Signed-off-by Robert Baldyga <[email protected]>

Signed-off-by John Youn <[email protected]>

---
 drivers/usb/dwc2/core.h   |  7 -------
 drivers/usb/dwc2/gadget.c | 47 ++++++++---------------------------------------
 2 files changed, 8 insertions(+), 46 deletions(-)

drivers/usb/dwc2/core.h +0 -7

index 9fae029..78526ee 100644
@@ -259,13 +259,6 @@ enum dwc2_lx_state {
 	DWC2_L3,	/* Off state */
 };
 
-/*
- * Gadget periodic tx fifo sizes as used by legacy driver
- * EP0 is not included
- */
-#define DWC2_G_P_LEGACY_TX_FIFO_SIZE {256, 256, 256, 256, 768, 768, 768, \
-					   768, 0, 0, 0, 0, 0, 0, 0}
-
 /* Gadget ep0 states */
 enum dwc2_ep0_state {
 	DWC2_EP0_SETUP,

drivers/usb/dwc2/gadget.c +8 ~~-39~~

85 lines not on this page · raw message

Signature · 1 line

2.9.0

John Youn

author

2016-08-29 20:39 UTC #3 lore raw

PATCH 3/6 usb: dwc2: gadget: change variable name to more meaningful

In reply to John Youn

From: Robert Baldyga <[email protected]>

Since we handle FIFOs and endpoint separately, using variable named 'ep' in context of FIFO is misleading, hence we rename it to 'fifo'.

Signed-off-by Robert Baldyga <[email protected]>

Signed-off-by John Youn <[email protected]>

---
 drivers/usb/dwc2/gadget.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

drivers/usb/dwc2/gadget.c +4 -4

index 5c24c85..ac267fd 100644
@@ -186,7 +186,7 @@ static void dwc2_hsotg_ctrl_epint(struct dwc2_hsotg *hsotg,
  */
 static void dwc2_hsotg_init_fifo(struct dwc2_hsotg *hsotg)
 {
-	unsigned int ep;
+	unsigned int fifo;
 	unsigned int addr;
 	int timeout;
 	u32 dptxfsizn;

@@ -217,8 +217,8 @@ static void dwc2_hsotg_init_fifo(struct dwc2_hsotg *hsotg)
 	 * them to endpoints dynamically according to maxpacket size value of
 	 * given endpoint.
 	 */
-	for (ep = 1; ep < MAX_EPS_CHANNELS; ep++) {
-		dptxfsizn = dwc2_readl(hsotg->regs + DPTXFSIZN(ep));
+	for (fifo = 1; fifo < MAX_EPS_CHANNELS; fifo++) {
+		dptxfsizn = dwc2_readl(hsotg->regs + DPTXFSIZN(fifo));
 
 		val = (dptxfsizn & FIFOSIZE_DEPTH_MASK) | addr;
 		addr += dptxfsizn >> FIFOSIZE_DEPTH_SHIFT;

@@ -226,7 +226,7 @@ static void dwc2_hsotg_init_fifo(struct dwc2_hsotg *hsotg)
 		if (addr > hsotg->fifo_mem)
 			break;
 
-		dwc2_writel(val, hsotg->regs + DPTXFSIZN(ep));
+		dwc2_writel(val, hsotg->regs + DPTXFSIZN(fifo));
 	}
 
 	/*

Signature · 1 line

2.9.0

John Youn

author

2016-08-29 20:39 UTC #4 lore raw

PATCH 4/6 usb: dwc2: gadget: remove dead code from dwc2_hsotg_ep_enable()

In reply to John Youn

From: Robert Baldyga <[email protected]>

Since FIFO is always freed in dwc2_hsotg_ep_disable(), ep->fifo_index is always 0 in dwc2_hsotg_ep_enable(), hence code inside if() block is never executed.

Signed-off-by Robert Baldyga <[email protected]>

Signed-off-by John Youn <[email protected]>

---
 drivers/usb/dwc2/gadget.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

drivers/usb/dwc2/gadget.c +1 ~~-12~~

index ac267fd..c8a182c 100644
@@ -3043,22 +3043,11 @@ static int dwc2_hsotg_ep_enable(struct usb_ep *ep,
 		break;
 	}
 
-	/* If fifo is already allocated for this ep */
-	if (hs_ep->fifo_index) {
-		size =  hs_ep->ep.maxpacket * hs_ep->mc;
-		/* If bigger fifo is required deallocate current one */
-		if (size > hs_ep->fifo_size) {
-			hsotg->fifo_map &= ~(1 << hs_ep->fifo_index);
-			hs_ep->fifo_index = 0;
-			hs_ep->fifo_size = 0;
-		}
-	}
-
 	/*
 	 * if the hardware has dedicated fifos, we must give each IN EP
 	 * a unique tx-fifo even if it is non-periodic.
 	 */
-	if (dir_in && hsotg->dedicated_fifos && !hs_ep->fifo_index) {
+	if (dir_in && hsotg->dedicated_fifos) {
 		u32 fifo_index = 0;
 		u32 fifo_size = UINT_MAX;
 		size = hs_ep->ep.maxpacket*hs_ep->mc;

Signature · 1 line

2.9.0

A message that is not in the archive (sent off-list or not yet received): cover.1472502978.git.johnyoun-HKixBCOQz3hWk0Htik3J/[email protected]

John Youn

2016-08-29 20:39 UTC #6 lore raw

PATCH 6/6 Documentation: devicetree: dwc2: Deprecate g-tx-fifo-size

This property is not needed because the periodic fifos are not configurable. So it was incorrect to add this property in the first place.

Signed-off-by John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/[email protected]>

---
 Documentation/devicetree/bindings/usb/dwc2.txt | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Documentation/devicetree/bindings/usb/dwc2.txt +4 -1

index 20a68bf..7d16ebf 100644
@@ -26,7 +26,10 @@ Refer to phy/phy-bindings.txt for generic phy consumer properties
 - g-use-dma: enable dma usage in gadget driver.
 - g-rx-fifo-size: size of rx fifo size in gadget mode.
 - g-np-tx-fifo-size: size of non-periodic tx fifo size in gadget mode.
-- g-tx-fifo-size: size of periodic tx fifo per endpoint (except ep0) in gadget mode.
+
+Deprecated properties:
+- g-tx-fifo-size: size of periodic tx fifo per endpoint (except ep0)
+  in gadget mode.
 
 Example:

Signature · 6 lines

2.9.0

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

John Youn

author

2016-08-29 20:39 UTC #7 lore raw

PATCH 5/6 usb: dwc2: gadget: free TX FIFO after killing requests

In reply to John Youn

From: Robert Baldyga <[email protected]>

As kill_all_requests() potentially flushes TX FIFO, we should should free FIFO after calling it. Otherwise FIFO could stay unflushed properly.

Signed-off-by Robert Baldyga <[email protected]>

Signed-off-by John Youn <[email protected]>

---
 drivers/usb/dwc2/gadget.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

drivers/usb/dwc2/gadget.c +4 -4

index c8a182c..4d84c32 100644
@@ -3120,10 +3120,6 @@ static int dwc2_hsotg_ep_disable(struct usb_ep *ep)
 
 	spin_lock_irqsave(&hsotg->lock, flags);
 
-	hsotg->fifo_map &= ~(1<<hs_ep->fifo_index);
-	hs_ep->fifo_index = 0;
-	hs_ep->fifo_size = 0;
-
 	ctrl = dwc2_readl(hsotg->regs + epctrl_reg);
 	ctrl &= ~DXEPCTL_EPENA;
 	ctrl &= ~DXEPCTL_USBACTEP;

@@ -3138,6 +3134,10 @@ static int dwc2_hsotg_ep_disable(struct usb_ep *ep)
 	/* terminate all requests with shutdown */
 	kill_all_requests(hsotg, hs_ep, -ESHUTDOWN);
 
+	hsotg->fifo_map &= ~(1 << hs_ep->fifo_index);
+	hs_ep->fifo_index = 0;
+	hs_ep->fifo_size = 0;
+
 	spin_unlock_irqrestore(&hsotg->lock, flags);
 	return 0;
 }

Signature · 1 line

2.9.0

John Youn

author

2016-08-29 20:39 UTC #8 lore raw

PATCH 6/6 Documentation: devicetree: dwc2: Deprecate g-tx-fifo-size

In reply to John Youn

This property is not needed because the periodic fifos are not configurable. So it was incorrect to add this property in the first place.

Signed-off-by John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/[email protected]>

---
 Documentation/devicetree/bindings/usb/dwc2.txt | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Documentation/devicetree/bindings/usb/dwc2.txt +4 -1

index 20a68bf..7d16ebf 100644
@@ -26,7 +26,10 @@ Refer to phy/phy-bindings.txt for generic phy consumer properties
 - g-use-dma: enable dma usage in gadget driver.
 - g-rx-fifo-size: size of rx fifo size in gadget mode.
 - g-np-tx-fifo-size: size of non-periodic tx fifo size in gadget mode.
-- g-tx-fifo-size: size of periodic tx fifo per endpoint (except ep0) in gadget mode.
+
+Deprecated properties:
+- g-tx-fifo-size: size of periodic tx fifo per endpoint (except ep0)
+  in gadget mode.
 
 Example:

Signature · 6 lines

2.9.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Rob Herring

maintainer Acked-by on v1 6/6

2016-09-02 14:41 UTC #9 lore raw

Re: [PATCH 6/6] Documentation: devicetree: dwc2: Deprecate g-tx-fifo-size

Quoted 8 lines · John Youn

This property is not needed because the periodic fifos are not configurable. So it was incorrect to add this property in the first place.
Signed-off-by John Youn <[email protected]>
---
 Documentation/devicetree/bindings/usb/dwc2.txt | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Acked-by Rob Herring <[email protected]>

John Youn

author

2016-08-29 20:39 UTC #10 lore raw

PATCH 6/6 Documentation: devicetree: dwc2: Deprecate g-tx-fifo-size

In reply to John Youn

This property is not needed because the periodic fifos are not configurable. So it was incorrect to add this property in the first place.

Signed-off-by John Youn <[email protected]>

---
 Documentation/devicetree/bindings/usb/dwc2.txt | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Documentation/devicetree/bindings/usb/dwc2.txt +4 -1

index 20a68bf..7d16ebf 100644
@@ -26,7 +26,10 @@ Refer to phy/phy-bindings.txt for generic phy consumer properties
 - g-use-dma: enable dma usage in gadget driver.
 - g-rx-fifo-size: size of rx fifo size in gadget mode.
 - g-np-tx-fifo-size: size of non-periodic tx fifo size in gadget mode.
-- g-tx-fifo-size: size of periodic tx fifo per endpoint (except ep0) in gadget mode.
+
+Deprecated properties:
+- g-tx-fifo-size: size of periodic tx fifo per endpoint (except ep0)
+  in gadget mode.
 
 Example:

Signature · 1 line

2.9.0