linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] usb: dwc3: Fix GTXFIFOSIZ.TXFDEP macro name
@ 2019-09-10  1:58 Thinh Nguyen
  2019-09-10  1:58 ` [PATCH 2/2] usb: dwc3: gadget: Properly set maxpacket limit Thinh Nguyen
  0 siblings, 1 reply; 4+ messages in thread
From: Thinh Nguyen @ 2019-09-10  1:58 UTC (permalink / raw)
  To: Felipe Balbi, linux-usb; +Cc: John Youn, Thinh Nguyen

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

Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
---
 drivers/usb/dwc3/core.h   | 4 ++--
 drivers/usb/dwc3/gadget.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 1c8b349379af..6a6baadcb697 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -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 */
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 8adb59f8e4f1..292e5e672868 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2214,9 +2214,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;
-- 
2.11.0


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

* [PATCH 2/2] usb: dwc3: gadget: Properly set maxpacket limit
  2019-09-10  1:58 [PATCH 1/2] usb: dwc3: Fix GTXFIFOSIZ.TXFDEP macro name Thinh Nguyen
@ 2019-09-10  1:58 ` Thinh Nguyen
  2019-12-04  1:55   ` Thinh Nguyen
  0 siblings, 1 reply; 4+ messages in thread
From: Thinh Nguyen @ 2019-09-10  1:58 UTC (permalink / raw)
  To: Felipe Balbi, linux-usb; +Cc: John Youn, 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

Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
---
 drivers/usb/dwc3/core.h   |  4 ++++
 drivers/usb/dwc3/gadget.c | 52 +++++++++++++++++++++++++++++++++++++----------
 2 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 6a6baadcb697..0f019db5e125 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -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)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 292e5e672868..cbda3bb4c1c0 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2205,7 +2205,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);
@@ -2221,17 +2220,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);
 
@@ -2249,8 +2248,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;
+
+	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,
-- 
2.11.0


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

* Re: [PATCH 2/2] usb: dwc3: gadget: Properly set maxpacket limit
  2019-09-10  1:58 ` [PATCH 2/2] usb: dwc3: gadget: Properly set maxpacket limit Thinh Nguyen
@ 2019-12-04  1:55   ` Thinh Nguyen
  2019-12-10 12:57     ` Felipe Balbi
  0 siblings, 1 reply; 4+ messages in thread
From: Thinh Nguyen @ 2019-12-04  1:55 UTC (permalink / raw)
  To: Thinh Nguyen, Felipe Balbi, linux-usb; +Cc: John Youn

Hi Felipe,

Thinh Nguyen wrote:
> 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
>
> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
> ---
>   drivers/usb/dwc3/core.h   |  4 ++++
>   drivers/usb/dwc3/gadget.c | 52 +++++++++++++++++++++++++++++++++++++----------
>   2 files changed, 45 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 6a6baadcb697..0f019db5e125 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -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)
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 292e5e672868..cbda3bb4c1c0 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -2205,7 +2205,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);
> @@ -2221,17 +2220,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);
>   
> @@ -2249,8 +2248,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;
> +
> +	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,

I sent these couple of patches some time ago. When you have time, can 
you take a look and see if they can go on the next tree.

Thanks,
Thinh

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

* Re: [PATCH 2/2] usb: dwc3: gadget: Properly set maxpacket limit
  2019-12-04  1:55   ` Thinh Nguyen
@ 2019-12-10 12:57     ` Felipe Balbi
  0 siblings, 0 replies; 4+ messages in thread
From: Felipe Balbi @ 2019-12-10 12:57 UTC (permalink / raw)
  To: Thinh Nguyen, Thinh Nguyen, linux-usb; +Cc: John Youn

[-- Attachment #1: Type: text/plain, Size: 1394 bytes --]


Hi,

Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
> Thinh Nguyen wrote:
>> 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

<snip>

> I sent these couple of patches some time ago. When you have time, can 
> you take a look and see if they can go on the next tree.

I don't seem to have these patches in my inbox anymore. Could you resend
on top of v5.5-rc1?

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

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

end of thread, other threads:[~2019-12-10 12:56 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-10  1:58 [PATCH 1/2] usb: dwc3: Fix GTXFIFOSIZ.TXFDEP macro name Thinh Nguyen
2019-09-10  1:58 ` [PATCH 2/2] usb: dwc3: gadget: Properly set maxpacket limit Thinh Nguyen
2019-12-04  1:55   ` Thinh Nguyen
2019-12-10 12:57     ` Felipe Balbi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).