All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/5] usb: dwc2: gadget: Fix TX FIFO handling
@ 2016-02-03 11:36 Robert Baldyga
  2016-02-03 11:36 ` [PATCH v2 1/5] usb: dwc2: gadget: use ep->fifo_index in context of FIFO registers Robert Baldyga
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Robert Baldyga @ 2016-02-03 11:36 UTC (permalink / raw)
  To: balbi, johnyoun
  Cc: gregkh, linux-usb, linux-kernel, m.szyprowski, b.zolnierkie,
	Robert Baldyga

Hello,

This patch series fixes TX FIFO handling in UDC part of DWC2 driver.
It gets rid of few bugs, improves compliance with DWC2 documentation
and cleanes up driver code. More detailed description is provided in
commit messages.

Best regards,
Robert Baldyga

Changelog:

v2:
- Addressed comments from Sergei Shtylyov

v1: https://lkml.org/lkml/2015/12/28/208

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

 drivers/usb/dwc2/core.h   |  7 -----
 drivers/usb/dwc2/gadget.c | 79 ++++++++++++-----------------------------------
 2 files changed, 19 insertions(+), 67 deletions(-)

-- 
1.9.1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 1/5] usb: dwc2: gadget: use ep->fifo_index in context of FIFO registers
  2016-02-03 11:36 [PATCH v2 0/5] usb: dwc2: gadget: Fix TX FIFO handling Robert Baldyga
@ 2016-02-03 11:36 ` Robert Baldyga
  2016-02-04 12:25   ` Vahram Aharonyan
  2016-02-03 11:36 ` [PATCH v2 2/5] usb: dwc2: gadget: fix TX FIFO size and address initialization Robert Baldyga
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Robert Baldyga @ 2016-02-03 11:36 UTC (permalink / raw)
  To: balbi, johnyoun
  Cc: gregkh, linux-usb, linux-kernel, m.szyprowski, b.zolnierkie,
	Robert Baldyga

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

Signed-off-by: Robert Baldyga <r.baldyga@samsung.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 422ab7d..0d0f6fe 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -369,7 +369,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;
@@ -2172,7 +2173,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);
 }
-- 
1.9.1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 2/5] usb: dwc2: gadget: fix TX FIFO size and address initialization
  2016-02-03 11:36 [PATCH v2 0/5] usb: dwc2: gadget: Fix TX FIFO handling Robert Baldyga
  2016-02-03 11:36 ` [PATCH v2 1/5] usb: dwc2: gadget: use ep->fifo_index in context of FIFO registers Robert Baldyga
@ 2016-02-03 11:36 ` Robert Baldyga
  2016-02-04 13:07   ` Vahram Aharonyan
  2016-02-03 11:36 ` [PATCH v2 3/5] usb: dwc2: gadget: change variable name to more meaningful Robert Baldyga
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Robert Baldyga @ 2016-02-03 11:36 UTC (permalink / raw)
  To: balbi, johnyoun
  Cc: gregkh, linux-usb, linux-kernel, m.szyprowski, b.zolnierkie,
	Robert Baldyga

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>
---
 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 7fb6434..441da5c 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -208,13 +208,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 0d0f6fe..5de9236 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -170,6 +170,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 */
@@ -198,13 +199,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));
 	}
@@ -3453,36 +3454,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);
 
@@ -3504,13 +3479,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);
 
@@ -3528,9 +3500,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;
-- 
1.9.1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 3/5] usb: dwc2: gadget: change variable name to more meaningful
  2016-02-03 11:36 [PATCH v2 0/5] usb: dwc2: gadget: Fix TX FIFO handling Robert Baldyga
  2016-02-03 11:36 ` [PATCH v2 1/5] usb: dwc2: gadget: use ep->fifo_index in context of FIFO registers Robert Baldyga
  2016-02-03 11:36 ` [PATCH v2 2/5] usb: dwc2: gadget: fix TX FIFO size and address initialization Robert Baldyga
@ 2016-02-03 11:36 ` Robert Baldyga
  2016-02-03 11:36 ` [PATCH v2 4/5] usb: dwc2: gadget: remove dead code from dwc2_hsotg_ep_enable() Robert Baldyga
  2016-02-03 11:36 ` [PATCH v2 5/5] usb: dwc2: gadget: free TX FIFO after killing requests Robert Baldyga
  4 siblings, 0 replies; 9+ messages in thread
From: Robert Baldyga @ 2016-02-03 11:36 UTC (permalink / raw)
  To: balbi, johnyoun
  Cc: gregkh, linux-usb, linux-kernel, m.szyprowski, b.zolnierkie,
	Robert Baldyga

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>
---
 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 5de9236..f01a936 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -167,7 +167,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;
@@ -198,8 +198,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;
@@ -207,7 +207,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));
 	}
 
 	/*
-- 
1.9.1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 4/5] usb: dwc2: gadget: remove dead code from dwc2_hsotg_ep_enable()
  2016-02-03 11:36 [PATCH v2 0/5] usb: dwc2: gadget: Fix TX FIFO handling Robert Baldyga
                   ` (2 preceding siblings ...)
  2016-02-03 11:36 ` [PATCH v2 3/5] usb: dwc2: gadget: change variable name to more meaningful Robert Baldyga
@ 2016-02-03 11:36 ` Robert Baldyga
  2016-02-03 11:36 ` [PATCH v2 5/5] usb: dwc2: gadget: free TX FIFO after killing requests Robert Baldyga
  4 siblings, 0 replies; 9+ messages in thread
From: Robert Baldyga @ 2016-02-03 11:36 UTC (permalink / raw)
  To: balbi, johnyoun
  Cc: gregkh, linux-usb, linux-kernel, m.szyprowski, b.zolnierkie,
	Robert Baldyga

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>
---
 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 f01a936..8f9d45b 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2702,22 +2702,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;
-- 
1.9.1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 5/5] usb: dwc2: gadget: free TX FIFO after killing requests
  2016-02-03 11:36 [PATCH v2 0/5] usb: dwc2: gadget: Fix TX FIFO handling Robert Baldyga
                   ` (3 preceding siblings ...)
  2016-02-03 11:36 ` [PATCH v2 4/5] usb: dwc2: gadget: remove dead code from dwc2_hsotg_ep_enable() Robert Baldyga
@ 2016-02-03 11:36 ` Robert Baldyga
  4 siblings, 0 replies; 9+ messages in thread
From: Robert Baldyga @ 2016-02-03 11:36 UTC (permalink / raw)
  To: balbi, johnyoun
  Cc: gregkh, linux-usb, linux-kernel, m.szyprowski, b.zolnierkie,
	Robert Baldyga

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>
---
 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 8f9d45b..fa5919a 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2779,10 +2779,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;
@@ -2797,6 +2793,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;
 }
-- 
1.9.1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/5] usb: dwc2: gadget: use ep->fifo_index in context of FIFO registers
  2016-02-03 11:36 ` [PATCH v2 1/5] usb: dwc2: gadget: use ep->fifo_index in context of FIFO registers Robert Baldyga
@ 2016-02-04 12:25   ` Vahram Aharonyan
  2017-01-09  8:13     ` Morgan Chang
  0 siblings, 1 reply; 9+ messages in thread
From: Vahram Aharonyan @ 2016-02-04 12:25 UTC (permalink / raw)
  To: Robert Baldyga, balbi, John.Youn
  Cc: gregkh, linux-usb, linux-kernel, m.szyprowski, b.zolnierkie

On 2/3/2016 3:40 PM, Robert Baldyga wrote:
> In context of FIFO registers we use ep->fifo_index instead of ep->index.
> 
> Signed-off-by: Robert Baldyga <r.baldyga@samsung.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 422ab7d..0d0f6fe 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -369,7 +369,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;
> @@ -2172,7 +2173,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);
>  }
> 

Hi Robert,

DTXFSTS register is linked with endpoint, not FIFO - it contains information about how much space is used in the FIFO assigned to the endpoint. Changing ep->index to ep->fifo_index will work, if FIFO number assigned to that endpoint coincides with ep->index. For example, TX FIFO #1 has been assigned to EP 1 In. If TX FIFO #2 was assigned to EP #1, then with this change DTXFSTS[2] will be used instead of DTXFSTS[1] for EP #1.

Thanks,
Vahram.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/5] usb: dwc2: gadget: fix TX FIFO size and address initialization
  2016-02-03 11:36 ` [PATCH v2 2/5] usb: dwc2: gadget: fix TX FIFO size and address initialization Robert Baldyga
@ 2016-02-04 13:07   ` Vahram Aharonyan
  0 siblings, 0 replies; 9+ messages in thread
From: Vahram Aharonyan @ 2016-02-04 13:07 UTC (permalink / raw)
  To: Robert Baldyga, balbi, John.Youn
  Cc: gregkh, linux-usb, linux-kernel, m.szyprowski, b.zolnierkie

On 2/3/2016 3:38 PM, Robert Baldyga wrote:
> 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>
> ---
>  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 7fb6434..441da5c 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -208,13 +208,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 0d0f6fe..5de9236 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -170,6 +170,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 */
> @@ -198,13 +199,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));
>  	}
> @@ -3453,36 +3454,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);
>  
> @@ -3504,13 +3479,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);
>  
> @@ -3528,9 +3500,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;
> 

Hi Robert,

Memory access type of this HW register's field depends on the core configuration. It's read-only in case of shared FIFO mode of the controller. In dedicated FIFO mode with dynamic FIFO sizing enabled this bit-field is writable and SW is allowed to set user-defined values in the HW during core initialization.

In Shared FIFO operation HW register with offset 104h + (FIFO_num -1) * 04h is named DPTXFSIZn with read-only upper bits. For dedicated FIFO mode SW should base on DIEPTXFn register description. 

Thanks,
Vahram.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/5] usb: dwc2: gadget: use ep->fifo_index in context of FIFO registers
  2016-02-04 12:25   ` Vahram Aharonyan
@ 2017-01-09  8:13     ` Morgan Chang
  0 siblings, 0 replies; 9+ messages in thread
From: Morgan Chang @ 2017-01-09  8:13 UTC (permalink / raw)
  To: Vahram Aharonyan, John.Youn
  Cc: Robert Baldyga, balbi, gregkh, linux-usb, linux-kernel,
	m.szyprowski, b.zolnierkie

Hi, John

On Thu, Feb 4, 2016 at 8:25 PM, Vahram Aharonyan
<Vahram.Aharonyan@synopsys.com> wrote:
> On 2/3/2016 3:40 PM, Robert Baldyga wrote:
>
> Hi Robert,
>
> DTXFSTS register is linked with endpoint, not FIFO - it contains information about how much space is used in the FIFO assigned to the endpoint. Changing ep->index to ep->fifo_index will work, if FIFO number assigned to that endpoint coincides with ep->index. For example, TX FIFO #1 has been assigned to EP 1 In. If TX FIFO #2 was assigned to EP #1, then with this change DTXFSTS[2] will be used instead of DTXFSTS[1] for EP #1.
>
> Thanks,
> Vahram.

As mentioned by Vahram, DTXFSTS registers are indexed by endpoint
number, instead of FIFO number.
It should be reverted.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-01-09  8:13 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-03 11:36 [PATCH v2 0/5] usb: dwc2: gadget: Fix TX FIFO handling Robert Baldyga
2016-02-03 11:36 ` [PATCH v2 1/5] usb: dwc2: gadget: use ep->fifo_index in context of FIFO registers Robert Baldyga
2016-02-04 12:25   ` Vahram Aharonyan
2017-01-09  8:13     ` Morgan Chang
2016-02-03 11:36 ` [PATCH v2 2/5] usb: dwc2: gadget: fix TX FIFO size and address initialization Robert Baldyga
2016-02-04 13:07   ` Vahram Aharonyan
2016-02-03 11:36 ` [PATCH v2 3/5] usb: dwc2: gadget: change variable name to more meaningful Robert Baldyga
2016-02-03 11:36 ` [PATCH v2 4/5] usb: dwc2: gadget: remove dead code from dwc2_hsotg_ep_enable() Robert Baldyga
2016-02-03 11:36 ` [PATCH v2 5/5] usb: dwc2: gadget: free TX FIFO after killing requests Robert Baldyga

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.