Skip to contentSkip to conversation

usb: dwc3: gadget: Revise maxpacket_limit calculation

PATCH In review Thinh Nguyen 2 participants 5 messages Dec 11, 2019 – Mar 30, 2020

Subsystems (2)
lore
Share & tools
lore

Thinh Nguyen

author maintainer
#0 lore raw

v2 cover usb: dwc3: gadget: Revise maxpacket_limit calculation

This series revises how dwc3 calculate maxpacket_limit for IN and OUT endpoints to follow the databook.

Changes in v2:
- Add Fixes tag for macro name change

Thinh Nguyen (2):
usb: dwc3: Fix GTXFIFOSIZ.TXFDEP macro name
usb: dwc3: gadget: Properly set maxpacket limit

 drivers/usb/dwc3/core.h   |  8 +++++--
 drivers/usb/dwc3/gadget.c | 56 ++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 49 insertions(+), 15 deletions(-)
Signature · 1 line
2.11.0
author maintainer
#1 lore raw

PATCH v2 1/2 usb: dwc3: Fix GTXFIFOSIZ.TXFDEP macro name

Change the macro name DWC3_GTXFIFOSIZ_TXFDEF to DWC3_GTXFIFOSIZ_TXFDEP to match with the register name GTXFIFOSIZ.TXFDEP.

Fixes 457e84b6624b ("usb: dwc3: gadget: dynamically re-size TxFifos")
Fixes 0cab8d26d6e5 ("usb: dwc3: Update DWC_usb31 GTXFIFOSIZ reg fields")

---
Changes in v2 -
Added Fixes tags

 drivers/usb/dwc3/core.h   | 4 ++--
 drivers/usb/dwc3/gadget.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)
drivers/usb/dwc3/core.h +2 -2
index 1c8b349379af..6a6baadcb697 100644
@@ -305,8 +305,8 @@
 
 /* Global TX Fifo Size Register */
 #define DWC31_GTXFIFOSIZ_TXFRAMNUM	BIT(15)		/* DWC_usb31 only */
-#define DWC31_GTXFIFOSIZ_TXFDEF(n)	((n) & 0x7fff)	/* DWC_usb31 only */
-#define DWC3_GTXFIFOSIZ_TXFDEF(n)	((n) & 0xffff)
+#define DWC31_GTXFIFOSIZ_TXFDEP(n)	((n) & 0x7fff)	/* DWC_usb31 only */
+#define DWC3_GTXFIFOSIZ_TXFDEP(n)	((n) & 0xffff)
 #define DWC3_GTXFIFOSIZ_TXFSTADDR(n)	((n) & 0xffff0000)
 
 /* Global Event Size Registers */
drivers/usb/dwc3/gadget.c +2 -2
index 3dcdde9080f5..5da266173146 100644
@@ -2216,9 +2216,9 @@ static int dwc3_gadget_init_in_endpoint(struct dwc3_ep *dep)
 
 	size = dwc3_readl(dwc->regs, DWC3_GTXFIFOSIZ(dep->number >> 1));
 	if (dwc3_is_usb31(dwc))
-		size = DWC31_GTXFIFOSIZ_TXFDEF(size);
+		size = DWC31_GTXFIFOSIZ_TXFDEP(size);
 	else
-		size = DWC3_GTXFIFOSIZ_TXFDEF(size);
+		size = DWC3_GTXFIFOSIZ_TXFDEP(size);
 
 	/* FIFO Depth is in MDWDITH bytes. Multiply */
 	size *= mdwidth;
Signature · 1 line
2.11.0
author maintainer
#2 lore raw

PATCH v2 2/2 usb: dwc3: gadget: Properly set maxpacket limit

In reply to Thinh Nguyen

Currently the calculation of max packet size limit for IN endpoints is too restrictive. This prevents a matching of a capable hardware endpoint during configuration. Below is the minimum recommended HW configuration to support a particular endpoint setup from the databook:

For OUT endpoints, the databook recommended the minimum RxFIFO size to be at least 3x MaxPacketSize + 3x setup packets size (8 bytes each) + clock crossing margin (16 bytes).

For IN endpoints, the databook recommended the minimum TxFIFO size to be at least 3x MaxPacketSize for endpoints that support burst. If the endpoint doesn't support burst or when the device is operating in USB 2.0 mode, a minimum TxFIFO size of 2x MaxPacketSize is recommended.

Base on these recommendations, we can calculate the MaxPacketSize limit of each endpoint. This patch revises the IN endpoint MaxPacketSize limit and also sets the MaxPacketSize limit for OUT endpoints.

Reference: Databook 3.30a section 3.2.2 and 3.2.3

---
Changes in v2 -
None

 drivers/usb/dwc3/core.h   |  4 ++++
 drivers/usb/dwc3/gadget.c | 52 +++++++++++++++++++++++++++++++++++++----------
 2 files changed, 45 insertions(+), 11 deletions(-)
drivers/usb/dwc3/core.h +4 -0
index 6a6baadcb697..0f019db5e125 100644
@@ -309,6 +309,10 @@
 #define DWC3_GTXFIFOSIZ_TXFDEP(n)	((n) & 0xffff)
 #define DWC3_GTXFIFOSIZ_TXFSTADDR(n)	((n) & 0xffff0000)
 
+/* Global RX Fifo Size Register */
+#define DWC31_GRXFIFOSIZ_RXFDEP(n)	((n) & 0x7fff)	/* DWC_usb31 only */
+#define DWC3_GRXFIFOSIZ_RXFDEP(n)	((n) & 0xffff)
+
 /* Global Event Size Registers */
 #define DWC3_GEVNTSIZ_INTMASK		BIT(31)
 #define DWC3_GEVNTSIZ_SIZE(n)		((n) & 0xffff)
drivers/usb/dwc3/gadget.c +41 -11

73 lines not on this page · raw message

Signature · 1 line
2.11.0
author maintainer
#3 lore raw

Re: [RESEND PATCH v2 2/2] usb: dwc3: gadget: Properly set maxpacket limit

Hi,

Quoted 114 lines · Thinh Nguyen

Currently the calculation of max packet size limit for IN endpoints is too restrictive. This prevents a matching of a capable hardware endpoint during configuration. Below is the minimum recommended HW configuration to support a particular endpoint setup from the databook:

For OUT endpoints, the databook recommended the minimum RxFIFO size to be at least 3x MaxPacketSize + 3x setup packets size (8 bytes each) + clock crossing margin (16 bytes).

For IN endpoints, the databook recommended the minimum TxFIFO size to be at least 3x MaxPacketSize for endpoints that support burst. If the endpoint doesn't support burst or when the device is operating in USB 2.0 mode, a minimum TxFIFO size of 2x MaxPacketSize is recommended.

Base on these recommendations, we can calculate the MaxPacketSize limit of each endpoint. This patch revises the IN endpoint MaxPacketSize limit and also sets the MaxPacketSize limit for OUT endpoints.

Reference: Databook 3.30a section 3.2.2 and 3.2.3

---
Changes in v2 -
None

  drivers/usb/dwc3/core.h   |  4 ++++
  drivers/usb/dwc3/gadget.c | 52 +++++++++++++++++++++++++++++++++++++----------
  2 files changed, 45 insertions(+), 11 deletions(-)
drivers/usb/dwc3/core.h +4 -0
index 6a6baadcb697..0f019db5e125 100644
@@ -309,6 +309,10 @@
  #define DWC3_GTXFIFOSIZ_TXFDEP(n)	((n) & 0xffff)
  #define DWC3_GTXFIFOSIZ_TXFSTADDR(n)	((n) & 0xffff0000)
 
+/* Global RX Fifo Size Register */
+#define DWC31_GRXFIFOSIZ_RXFDEP(n)	((n) & 0x7fff)	/* DWC_usb31 only */
+#define DWC3_GRXFIFOSIZ_RXFDEP(n)	((n) & 0xffff)
+
  /* Global Event Size Registers */
  #define DWC3_GEVNTSIZ_INTMASK		BIT(31)
  #define DWC3_GEVNTSIZ_SIZE(n)		((n) & 0xffff)
drivers/usb/dwc3/gadget.c +34 -11
index 5da266173146..3b3e6a655c6a 100644
@@ -2207,7 +2207,6 @@ static int dwc3_gadget_init_in_endpoint(struct dwc3_ep *dep)
  {
  	struct dwc3 *dwc = dep->dwc;
  	int mdwidth;
-	int kbytes;
  	int size;
 
  	mdwidth = DWC3_MDWIDTH(dwc->hwparams.hwparams0);
@@ -2223,17 +2222,17 @@ static int dwc3_gadget_init_in_endpoint(struct dwc3_ep *dep)
  	/* FIFO Depth is in MDWDITH bytes. Multiply */
  	size *= mdwidth;
 
-	kbytes = size / 1024;
-	if (kbytes == 0)
-		kbytes = 1;
-
  	/*
-	 * FIFO sizes account an extra MDWIDTH * (kbytes + 1) bytes for
-	 * internal overhead. We don't really know how these are used,
-	 * but documentation say it exists.
+	 * To meet performance requirement, a minimum TxFIFO size of 3x
+	 * MaxPacketSize is recommended for endpoints that support burst and a
+	 * minimum TxFIFO size of 2x MaxPacketSize for endpoints that don't
+	 * support burst. Use those numbers and we can calculate the max packet
+	 * limit as below.
  	 */
-	size -= mdwidth * (kbytes + 1);
-	size /= kbytes;
+	if (dwc->maximum_speed >= USB_SPEED_SUPER)
+		size /= 3;
+	else
+		size /= 2;
 
  	usb_ep_set_maxpacket_limit(&dep->endpoint, size);
 
@@ -2251,8 +2250,39 @@ static int dwc3_gadget_init_in_endpoint(struct dwc3_ep *dep)
  static int dwc3_gadget_init_out_endpoint(struct dwc3_ep *dep)
  {
  	struct dwc3 *dwc = dep->dwc;
+	int mdwidth;
+	int size;
+
+	mdwidth = DWC3_MDWIDTH(dwc->hwparams.hwparams0);
+
+	/* MDWIDTH is represented in bits, convert to bytes */
+	mdwidth /= 8;
 
-	usb_ep_set_maxpacket_limit(&dep->endpoint, 1024);
+	/* All OUT endpoints share a single RxFIFO space */
+	size = dwc3_readl(dwc->regs, DWC3_GRXFIFOSIZ(0));
+	if (dwc3_is_usb31(dwc))
+		size = DWC31_GRXFIFOSIZ_RXFDEP(size);
+	else
+		size = DWC3_GRXFIFOSIZ_RXFDEP(size);
+
+	/* FIFO depth is in MDWDITH bytes */
+	size *= mdwidth;
+
+	/*
+	 * To meet performance requirement, a minimum recommended RxFIFO size
+	 * is defined as follow:
+	 * RxFIFO size >= (3 x MaxPacketSize) +
+	 * (3 x 8 bytes setup packets size) + (16 bytes clock crossing margin)
+	 *
+	 * Then calculate the max packet limit as below.
+	 */

drivers/usb/dwc3/gadget.c static int dwc3_gadget_init_in_endpoint(struct dwc3_ep *dep) · line 2288

Quoted 10 lines
drivers/usb/dwc3/gadget.c +7 -0
+	size -= (3 * 8) + 16;
+	if (size < 0)
+		size = 0;
+	else
+		size /= 3;
+
+	usb_ep_set_maxpacket_limit(&dep->endpoint, size);
  	dep->endpoint.max_streams = 15;
  	dep->endpoint.ops = &dwc3_gadget_ep_ops;
  	list_add_tail(&dep->endpoint.ep_list,

Please let me know if these patches are not clear or that they are lost somewhere. I'll try my best to clarify if anything's not clear.

Thanks,
Thinh

#4 lore raw

Re: [RESEND PATCH v2 2/2] usb: dwc3: gadget: Properly set maxpacket limit

Quoted 122 lines · Thinh Nguyen

Hi,

Quoted 119 lines · Thinh Nguyen

Currently the calculation of max packet size limit for IN endpoints is too restrictive. This prevents a matching of a capable hardware endpoint during configuration. Below is the minimum recommended HW configuration to support a particular endpoint setup from the databook:

For OUT endpoints, the databook recommended the minimum RxFIFO size to be at least 3x MaxPacketSize + 3x setup packets size (8 bytes each) + clock crossing margin (16 bytes).

For IN endpoints, the databook recommended the minimum TxFIFO size to be at least 3x MaxPacketSize for endpoints that support burst. If the endpoint doesn't support burst or when the device is operating in USB 2.0 mode, a minimum TxFIFO size of 2x MaxPacketSize is recommended.

Base on these recommendations, we can calculate the MaxPacketSize limit of each endpoint. This patch revises the IN endpoint MaxPacketSize limit and also sets the MaxPacketSize limit for OUT endpoints.

Reference: Databook 3.30a section 3.2.2 and 3.2.3

---
Changes in v2 -
None

  drivers/usb/dwc3/core.h   |  4 ++++
  drivers/usb/dwc3/gadget.c | 52 +++++++++++++++++++++++++++++++++++++----------
  2 files changed, 45 insertions(+), 11 deletions(-)
drivers/usb/dwc3/core.h +4 -0
index 6a6baadcb697..0f019db5e125 100644
@@ -309,6 +309,10 @@
  #define DWC3_GTXFIFOSIZ_TXFDEP(n)	((n) & 0xffff)
  #define DWC3_GTXFIFOSIZ_TXFSTADDR(n)	((n) & 0xffff0000)
 
+/* Global RX Fifo Size Register */
+#define DWC31_GRXFIFOSIZ_RXFDEP(n)	((n) & 0x7fff)	/* DWC_usb31 only */
+#define DWC3_GRXFIFOSIZ_RXFDEP(n)	((n) & 0xffff)
+
  /* Global Event Size Registers */
  #define DWC3_GEVNTSIZ_INTMASK		BIT(31)
  #define DWC3_GEVNTSIZ_SIZE(n)		((n) & 0xffff)
drivers/usb/dwc3/gadget.c +39 -11
index 5da266173146..3b3e6a655c6a 100644
@@ -2207,7 +2207,6 @@ static int dwc3_gadget_init_in_endpoint(struct dwc3_ep *dep)
  {
  	struct dwc3 *dwc = dep->dwc;
  	int mdwidth;
-	int kbytes;
  	int size;
 
  	mdwidth = DWC3_MDWIDTH(dwc->hwparams.hwparams0);
@@ -2223,17 +2222,17 @@ static int dwc3_gadget_init_in_endpoint(struct dwc3_ep *dep)
  	/* FIFO Depth is in MDWDITH bytes. Multiply */
  	size *= mdwidth;
 
-	kbytes = size / 1024;
-	if (kbytes == 0)
-		kbytes = 1;
-
  	/*
-	 * FIFO sizes account an extra MDWIDTH * (kbytes + 1) bytes for
-	 * internal overhead. We don't really know how these are used,
-	 * but documentation say it exists.
+	 * To meet performance requirement, a minimum TxFIFO size of 3x
+	 * MaxPacketSize is recommended for endpoints that support burst and a
+	 * minimum TxFIFO size of 2x MaxPacketSize for endpoints that don't
+	 * support burst. Use those numbers and we can calculate the max packet
+	 * limit as below.
  	 */
-	size -= mdwidth * (kbytes + 1);
-	size /= kbytes;
+	if (dwc->maximum_speed >= USB_SPEED_SUPER)
+		size /= 3;
+	else
+		size /= 2;
 
  	usb_ep_set_maxpacket_limit(&dep->endpoint, size);
 
@@ -2251,8 +2250,39 @@ static int dwc3_gadget_init_in_endpoint(struct dwc3_ep *dep)
  static int dwc3_gadget_init_out_endpoint(struct dwc3_ep *dep)
  {
  	struct dwc3 *dwc = dep->dwc;
+	int mdwidth;
+	int size;
+
+	mdwidth = DWC3_MDWIDTH(dwc->hwparams.hwparams0);
+
+	/* MDWIDTH is represented in bits, convert to bytes */
+	mdwidth /= 8;
 
-	usb_ep_set_maxpacket_limit(&dep->endpoint, 1024);
+	/* All OUT endpoints share a single RxFIFO space */
+	size = dwc3_readl(dwc->regs, DWC3_GRXFIFOSIZ(0));
+	if (dwc3_is_usb31(dwc))
+		size = DWC31_GRXFIFOSIZ_RXFDEP(size);
+	else
+		size = DWC3_GRXFIFOSIZ_RXFDEP(size);
+
+	/* FIFO depth is in MDWDITH bytes */
+	size *= mdwidth;
+
+	/*
+	 * To meet performance requirement, a minimum recommended RxFIFO size
+	 * is defined as follow:
+	 * RxFIFO size >= (3 x MaxPacketSize) +
+	 * (3 x 8 bytes setup packets size) + (16 bytes clock crossing margin)
+	 *
+	 * Then calculate the max packet limit as below.
+	 */
+	size -= (3 * 8) + 16;
+	if (size < 0)
+		size = 0;
+	else
+		size /= 3;
Quoted 8 lines

drivers/usb/dwc3/gadget.c static int dwc3_gadget_init_in_endpoint(struct dwc3_ep *dep) · line 2288

Quoted 5 lines
drivers/usb/dwc3/gadget.c +2 -0
+
+	usb_ep_set_maxpacket_limit(&dep->endpoint, size);
  	dep->endpoint.max_streams = 15;
  	dep->endpoint.ops = &dwc3_gadget_ep_ops;
  	list_add_tail(&dep->endpoint.ep_list,

Please let me know if these patches are not clear or that they are lost somewhere. I'll try my best to clarify if anything's not clear.

They're now in my testing/fixes. Thanks

Signature · 1 line
balbi