[PATCH 0/6] usb: dwc2: gadget: Fix TX FIFO handling

[PATCH 0/6] usb: dwc2: gadget: Fix TX FIFO handling

[John Youn]

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

-- 
2.9.0

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

[John Youn]

From: Robert Baldyga <r.baldyga@samsung.com>

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

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
Signed-off-by: John Youn <johnyoun@synopsys.com>
---
 drivers/usb/dwc2/gadget.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index af46adf..f6086d6 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -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);
 }
-- 
2.9.0

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

[John Youn]

From: Robert Baldyga <r.baldyga@samsung.com>

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 <r.baldyga@samsung.com>
Signed-off-by: John Youn <johnyoun@synopsys.com>
---
 drivers/usb/dwc2/core.h   |  7 -------
 drivers/usb/dwc2/gadget.c | 47 ++++++++---------------------------------------
 2 files changed, 8 insertions(+), 46 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 9fae029..78526ee 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -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,
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index f6086d6..5c24c85 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -189,6 +189,7 @@ static void dwc2_hsotg_init_fifo(struct dwc2_hsotg *hsotg)
 	unsigned int ep;
 	unsigned int addr;
 	int timeout;
+	u32 dptxfsizn;
 	u32 val;
 
 	/* Reset fifo map if not correctly cleared during previous session */
@@ -217,13 +218,13 @@ static void dwc2_hsotg_init_fifo(struct dwc2_hsotg *hsotg)
 	 * given endpoint.
 	 */
 	for (ep = 1; ep < MAX_EPS_CHANNELS; ep++) {
-		if (!hsotg->g_tx_fifo_sz[ep])
-			continue;
-		val = addr;
-		val |= hsotg->g_tx_fifo_sz[ep] << FIFOSIZE_DEPTH_SHIFT;
-		WARN_ONCE(addr + hsotg->g_tx_fifo_sz[ep] > hsotg->fifo_mem,
-			  "insufficient fifo memory");
-		addr += hsotg->g_tx_fifo_sz[ep];
+		dptxfsizn = dwc2_readl(hsotg->regs + DPTXFSIZN(ep));
+
+		val = (dptxfsizn & FIFOSIZE_DEPTH_MASK) | addr;
+		addr += dptxfsizn >> FIFOSIZE_DEPTH_SHIFT;
+
+		if (addr > hsotg->fifo_mem)
+			break;
 
 		dwc2_writel(val, hsotg->regs + DPTXFSIZN(ep));
 	}
@@ -3814,36 +3815,10 @@ static void dwc2_hsotg_dump(struct dwc2_hsotg *hsotg)
 static void dwc2_hsotg_of_probe(struct dwc2_hsotg *hsotg)
 {
 	struct device_node *np = hsotg->dev->of_node;
-	u32 len = 0;
-	u32 i = 0;
 
 	/* Enable dma if requested in device tree */
 	hsotg->g_using_dma = of_property_read_bool(np, "g-use-dma");
 
-	/*
-	* Register TX periodic fifo size per endpoint.
-	* EP0 is excluded since it has no fifo configuration.
-	*/
-	if (!of_find_property(np, "g-tx-fifo-size", &len))
-		goto rx_fifo;
-
-	len /= sizeof(u32);
-
-	/* Read tx fifo sizes other than ep0 */
-	if (of_property_read_u32_array(np, "g-tx-fifo-size",
-						&hsotg->g_tx_fifo_sz[1], len))
-		goto rx_fifo;
-
-	/* Add ep0 */
-	len++;
-
-	/* Make remaining TX fifos unavailable */
-	if (len < MAX_EPS_CHANNELS) {
-		for (i = len; i < MAX_EPS_CHANNELS; i++)
-			hsotg->g_tx_fifo_sz[i] = 0;
-	}
-
-rx_fifo:
 	/* Register RX fifo size */
 	of_property_read_u32(np, "g-rx-fifo-size", &hsotg->g_rx_fifo_sz);
 
@@ -3865,13 +3840,10 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
 	struct device *dev = hsotg->dev;
 	int epnum;
 	int ret;
-	int i;
-	u32 p_tx_fifo[] = DWC2_G_P_LEGACY_TX_FIFO_SIZE;
 
 	/* Initialize to legacy fifo configuration values */
 	hsotg->g_rx_fifo_sz = 2048;
 	hsotg->g_np_g_tx_fifo_sz = 1024;
-	memcpy(&hsotg->g_tx_fifo_sz[1], p_tx_fifo, sizeof(p_tx_fifo));
 	/* Device tree specific probe */
 	dwc2_hsotg_of_probe(hsotg);
 
@@ -3889,9 +3861,6 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
 	dev_dbg(dev, "NonPeriodic TXFIFO size: %d\n",
 						hsotg->g_np_g_tx_fifo_sz);
 	dev_dbg(dev, "RXFIFO size: %d\n", hsotg->g_rx_fifo_sz);
-	for (i = 0; i < MAX_EPS_CHANNELS; i++)
-		dev_dbg(dev, "Periodic TXFIFO%2d size: %d\n", i,
-						hsotg->g_tx_fifo_sz[i]);
 
 	hsotg->gadget.max_speed = USB_SPEED_HIGH;
 	hsotg->gadget.ops = &dwc2_hsotg_gadget_ops;
-- 
2.9.0

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

[John Youn]

From: Robert Baldyga <r.baldyga@samsung.com>

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 <r.baldyga@samsung.com>
Signed-off-by: John Youn <johnyoun@synopsys.com>
---
 drivers/usb/dwc2/gadget.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 5c24c85..ac267fd 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -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));
 	}
 
 	/*
-- 
2.9.0

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

[John Youn]

From: Robert Baldyga <r.baldyga@samsung.com>

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 <r.baldyga@samsung.com>
Signed-off-by: John Youn <johnyoun@synopsys.com>
---
 drivers/usb/dwc2/gadget.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index ac267fd..c8a182c 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -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;
-- 
2.9.0

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

[John Youn]

From: Robert Baldyga <r.baldyga@samsung.com>

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 <r.baldyga@samsung.com>
Signed-off-by: John Youn <johnyoun@synopsys.com>
---
 drivers/usb/dwc2/gadget.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index c8a182c..4d84c32 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -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;
 }
-- 
2.9.0

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

[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@synopsys.com>
---
 Documentation/devicetree/bindings/usb/dwc2.txt | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
index 20a68bf..7d16ebf 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.txt
+++ b/Documentation/devicetree/bindings/usb/dwc2.txt
@@ -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:
 
-- 
2.9.0

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

[Rob Herring]

On Mon, Aug 29, 2016 at 01:39:03PM -0700, John Youn wrote:
> 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@synopsys.com>
> ---
>  Documentation/devicetree/bindings/usb/dwc2.txt | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)

Acked-by: Rob Herring <robh@kernel.org>