[PATCH 0/5] usb: dwc3: handle non maxpacket aligned transfers > 512

[PATCH 0/5] usb: dwc3: handle non maxpacket aligned transfers > 512

[Kishon Vijay Abraham I]

Patch series adds support to handle non maxpacket aligned transfers
greater than bounce buffer size (512). It first adds chained TRB
support and then uses it to handle non maxpacket aligned transfers
greater than bounce buffer size.

Also included a cleanup patch to use 'roundup' macro.

This series is created after applying [1]

Non maxpacket aligned transfers can be initiated by
"./testusb -t 14 -c 1 -s 520 -v 1"

Before this series:
unknown speed	/dev/bus/usb/001/018	0
/dev/bus/usb/001/018 test 14 --> 110 (Connection timed out)

After this series:
unknown speed	/dev/bus/usb/001/023	0
/dev/bus/usb/001/023 test 14,    0.000486 secs

Tested this patch using USB3 Gen X CV (ch9 tests: usb2 and usb3,
link layer testing and MSC tests) and using USB2 X CV (ch9 tests,
MSC tests) and verified this doesn't cause additional failures.

Lecroy compliance tests fail even without this patch series so
deferred testing it.

[1] -> http://permalink.gmane.org/gmane.linux.kernel/1972684

Kishon Vijay Abraham I (5):
  usb: dwc3: ep0: use _roundup_ to calculate the transfer size
  usb: dwc3: ep0: preparation for handling non maxpacket aligned
    transfers > 512
  usb: dwc3; ep0: Modify _dwc3_ep0_start_trans_ API to take 'chain'
    parameter
  usb: dwc3: ep0: Add chained TRB support
  usb: dwc3: ep0: handle non maxpacket aligned transfers > 512

 drivers/usb/dwc3/ep0.c    |   94 ++++++++++++++++++++++++++++++---------------
 drivers/usb/dwc3/gadget.c |    2 +-
 2 files changed, 64 insertions(+), 32 deletions(-)

-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/5] usb: dwc3: ep0: use _roundup_ to calculate the transfer size

[Kishon Vijay Abraham I]

No functional change. Used _roundup_ macro to calculate the transfer
size aligned to maxpacket in  dwc3_ep0_complete_data. It also makes it
similar to how transfer size is calculated in __dwc3_ep0_do_control_data.

Signed-off-by: Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>
---
 drivers/usb/dwc3/ep0.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index 8858c60..713e46a 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -812,10 +812,8 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc,
 	length = trb->size & DWC3_TRB_SIZE_MASK;
 
 	if (dwc->ep0_bounced) {
-		unsigned transfer_size = ur->length;
 		unsigned maxp = ep0->endpoint.maxpacket;

Re: [PATCH 1/5] usb: dwc3: ep0: use _roundup_ to calculate the transfer size

[Felipe Balbi]

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

On Wed, Jun 10, 2015 at 02:48:48PM +0530, Kishon Vijay Abraham I wrote:
> No functional change. Used _roundup_ macro to calculate the transfer
> size aligned to maxpacket in  dwc3_ep0_complete_data. It also makes it
> similar to how transfer size is calculated in __dwc3_ep0_do_control_data.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>

doesn't apply, can you rebase on testing/next ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 1/5] usb: dwc3: ep0: use _roundup_ to calculate the transfer size

[Kishon Vijay Abraham I]

Hi Felipe,

On Tuesday 07 July 2015 12:30 AM, Felipe Balbi wrote:
> On Wed, Jun 10, 2015 at 02:48:48PM +0530, Kishon Vijay Abraham I wrote:
>> No functional change. Used _roundup_ macro to calculate the transfer
>> size aligned to maxpacket in  dwc3_ep0_complete_data. It also makes it
>> similar to how transfer size is calculated in __dwc3_ep0_do_control_data.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> 
> doesn't apply, can you rebase on testing/next ?

sure, will send it by early next week.

Thanks
Kishon

[PATCH 2/5] usb: dwc3: ep0: preparation for handling non maxpacket aligned transfers > 512

[Kishon Vijay Abraham I]

No functional change. This is in preparation for handling non maxpacket
aligned transfers greater than bounce buffer size. This is basically to
avoid code duplication when using chained TRB transfers to handle
non maxpacket aligned transfers greater than bounce buffer size.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/usb/dwc3/ep0.c |   25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index 713e46a..4998074 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -779,7 +779,11 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc,
 	struct usb_request	*ur;
 	struct dwc3_trb		*trb;
 	struct dwc3_ep		*ep0;
-	u32			transferred;
+	unsigned		transfer_size = 0;
+	unsigned		maxp;
+	unsigned		remaining_ur_length;
+	void			*buf;
+	u32			transferred = 0;
 	u32			status;
 	u32			length;
 	u8			epnum;
@@ -808,20 +812,24 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc,
 	}
 
 	ur = &r->request;
+	buf = ur->buf;
+	remaining_ur_length = ur->length;
 
 	length = trb->size & DWC3_TRB_SIZE_MASK;
 
+	maxp = ep0->endpoint.maxpacket;
+
 	if (dwc->ep0_bounced) {
-		unsigned maxp = ep0->endpoint.maxpacket;
-		unsigned transfer_size = roundup(ur->length, maxp);
+		transfer_size = roundup((ur->length - transfer_size),
+					maxp);
 
 		/* Maximum of DWC3_EP0_BOUNCE_SIZE can only be received */
 		if (transfer_size > DWC3_EP0_BOUNCE_SIZE)
 			transfer_size = DWC3_EP0_BOUNCE_SIZE;
 
-		transferred = min_t(u32, ur->length,
-				transfer_size - length);
-		memcpy(ur->buf, dwc->ep0_bounce, transferred);
+		transferred = min_t(u32, remaining_ur_length,
+				    transfer_size - length);
+		memcpy(buf, dwc->ep0_bounce, transferred);
 	} else {
 		transferred = ur->length - length;
 	}
@@ -930,7 +938,7 @@ static void __dwc3_ep0_do_control_data(struct dwc3 *dwc,
 				DWC3_TRBCTL_CONTROL_DATA);
 	} else if (!IS_ALIGNED(req->request.length, dep->endpoint.maxpacket)
 			&& (dep->number == 0)) {
-		u32	transfer_size;
+		u32	transfer_size = 0;
 		u32	maxpacket;
 
 		ret = usb_gadget_map_request(&dwc->gadget, &req->request,
@@ -941,7 +949,8 @@ static void __dwc3_ep0_do_control_data(struct dwc3 *dwc,
 		}
 
 		maxpacket = dep->endpoint.maxpacket;
-		transfer_size = roundup(req->request.length, maxpacket);
+		transfer_size = roundup((req->request.length - transfer_size),
+					maxpacket);
 
 		if (transfer_size > DWC3_EP0_BOUNCE_SIZE) {
 			dev_WARN(dwc->dev, "bounce buf can't handle req len\n");
-- 
1.7.9.5

Re: [PATCH 2/5] usb: dwc3: ep0: preparation for handling non maxpacket aligned transfers > 512

[Sergei Shtylyov]

On 06/10/2015 12:18 PM, Kishon Vijay Abraham I wrote:

> No functional change. This is in preparation for handling non maxpacket
> aligned transfers greater than bounce buffer size. This is basically to
> avoid code duplication when using chained TRB transfers to handle
> non maxpacket aligned transfers greater than bounce buffer size.

> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>   drivers/usb/dwc3/ep0.c |   25 +++++++++++++++++--------
>   1 file changed, 17 insertions(+), 8 deletions(-)

> diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
> index 713e46a..4998074 100644
> --- a/drivers/usb/dwc3/ep0.c
> +++ b/drivers/usb/dwc3/ep0.c
[...]
> @@ -808,20 +812,24 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc,
>   	}
>
>   	ur = &r->request;
> +	buf = ur->buf;
> +	remaining_ur_length = ur->length;
>
>   	length = trb->size & DWC3_TRB_SIZE_MASK;
>
> +	maxp = ep0->endpoint.maxpacket;
> +
>   	if (dwc->ep0_bounced) {
> -		unsigned maxp = ep0->endpoint.maxpacket;
> -		unsigned transfer_size = roundup(ur->length, maxp);
> +		transfer_size = roundup((ur->length - transfer_size),
> +					maxp);

    The innermost parens shouldn't be needed (if thay are, fix the macro instead).

[...]
> @@ -941,7 +949,8 @@ static void __dwc3_ep0_do_control_data(struct dwc3 *dwc,
>   		}
>
>   		maxpacket = dep->endpoint.maxpacket;
> -		transfer_size = roundup(req->request.length, maxpacket);
> +		transfer_size = roundup((req->request.length - transfer_size),
> +					maxpacket);

    Likewise.

[...]

WBR, Sergei

[PATCH 3/5] usb: dwc3; ep0: Modify _dwc3_ep0_start_trans_ API to take 'chain' parameter

[Kishon Vijay Abraham I]

No functional change. Added a new parameter in _dwc3_ep0_start_trans_ to
indicate whether the TRB is a chained TRB or last TRB. This is in
preparation for adding chained TRB support for ep0.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/usb/dwc3/ep0.c |   15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index 4998074..d1a2be1 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -56,7 +56,7 @@ static const char *dwc3_ep0_state_string(enum dwc3_ep0_state state)
 }
 
 static int dwc3_ep0_start_trans(struct dwc3 *dwc, u8 epnum, dma_addr_t buf_dma,
-		u32 len, u32 type)
+		u32 len, u32 type, unsigned chain)
 {
 	struct dwc3_gadget_ep_cmd_params params;
 	struct dwc3_trb			*trb;
@@ -302,7 +302,7 @@ void dwc3_ep0_out_start(struct dwc3 *dwc)
 	int				ret;
 
 	ret = dwc3_ep0_start_trans(dwc, 0, dwc->ctrl_req_addr, 8,
-			DWC3_TRBCTL_CONTROL_SETUP);
+			DWC3_TRBCTL_CONTROL_SETUP, false);
 	WARN_ON(ret < 0);
 }
 
@@ -851,7 +851,7 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc,
 
 			ret = dwc3_ep0_start_trans(dwc, epnum,
 					dwc->ctrl_req_addr, 0,
-					DWC3_TRBCTL_CONTROL_DATA);
+					DWC3_TRBCTL_CONTROL_DATA, false);
 			WARN_ON(ret < 0);
 		}
 	}
@@ -935,7 +935,7 @@ static void __dwc3_ep0_do_control_data(struct dwc3 *dwc,
 	if (req->request.length == 0) {
 		ret = dwc3_ep0_start_trans(dwc, dep->number,
 				dwc->ctrl_req_addr, 0,
-				DWC3_TRBCTL_CONTROL_DATA);
+				DWC3_TRBCTL_CONTROL_DATA, false);
 	} else if (!IS_ALIGNED(req->request.length, dep->endpoint.maxpacket)
 			&& (dep->number == 0)) {
 		u32	transfer_size = 0;
@@ -966,7 +966,7 @@ static void __dwc3_ep0_do_control_data(struct dwc3 *dwc,
 		 */
 		ret = dwc3_ep0_start_trans(dwc, dep->number,
 				dwc->ep0_bounce_addr, transfer_size,
-				DWC3_TRBCTL_CONTROL_DATA);
+				DWC3_TRBCTL_CONTROL_DATA, false);
 	} else {
 		ret = usb_gadget_map_request(&dwc->gadget, &req->request,
 				dep->number);
@@ -976,7 +976,8 @@ static void __dwc3_ep0_do_control_data(struct dwc3 *dwc,
 		}
 
 		ret = dwc3_ep0_start_trans(dwc, dep->number, req->request.dma,
-				req->request.length, DWC3_TRBCTL_CONTROL_DATA);
+				req->request.length, DWC3_TRBCTL_CONTROL_DATA,
+				false);
 	}
 
 	WARN_ON(ret < 0);
@@ -991,7 +992,7 @@ static int dwc3_ep0_start_control_status(struct dwc3_ep *dep)
 		: DWC3_TRBCTL_CONTROL_STATUS2;
 
 	return dwc3_ep0_start_trans(dwc, dep->number,
-			dwc->ctrl_req_addr, 0, type);
+			dwc->ctrl_req_addr, 0, type, false);
 }
 
 static void __dwc3_ep0_do_control_status(struct dwc3 *dwc, struct dwc3_ep *dep)
-- 
1.7.9.5

Re: [PATCH 3/5] usb: dwc3; ep0: Modify _dwc3_ep0_start_trans_ API to take 'chain' parameter

[Sergei Shtylyov]

Hello.

On 06/10/2015 12:18 PM, Kishon Vijay Abraham I wrote:

> No functional change. Added a new parameter in _dwc3_ep0_start_trans_ to
> indicate whether the TRB is a chained TRB or last TRB. This is in
> preparation for adding chained TRB support for ep0.

> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>   drivers/usb/dwc3/ep0.c |   15 ++++++++-------
>   1 file changed, 8 insertions(+), 7 deletions(-)

> diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
> index 4998074..d1a2be1 100644
> --- a/drivers/usb/dwc3/ep0.c
> +++ b/drivers/usb/dwc3/ep0.c
> @@ -56,7 +56,7 @@ static const char *dwc3_ep0_state_string(enum dwc3_ep0_state state)
>   }
>
>   static int dwc3_ep0_start_trans(struct dwc3 *dwc, u8 epnum, dma_addr_t buf_dma,
> -		u32 len, u32 type)
> +		u32 len, u32 type, unsigned chain)

    Why not *bool*? You're passing boolean values anyway...

[...]

WBR, Sergei

[PATCH 4/5] usb: dwc3: ep0: Add chained TRB support

[Kishon Vijay Abraham I]

Add chained TRB support to ep0. Now TRB's can be chained just by
invoking _dwc3_ep0_start_trans_ with 'chain' parameter set to true.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/usb/dwc3/ep0.c    |   16 +++++++++++++---
 drivers/usb/dwc3/gadget.c |    2 +-
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index d1a2be1..6847afe 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -70,7 +70,10 @@ static int dwc3_ep0_start_trans(struct dwc3 *dwc, u8 epnum, dma_addr_t buf_dma,
 		return 0;
 	}
 
-	trb = dwc->ep0_trb;
+	trb = &dwc->ep0_trb[dep->free_slot];
+
+	if (chain)
+		dep->free_slot++;
 
 	trb->bpl = lower_32_bits(buf_dma);
 	trb->bph = upper_32_bits(buf_dma);
@@ -78,10 +81,17 @@ static int dwc3_ep0_start_trans(struct dwc3 *dwc, u8 epnum, dma_addr_t buf_dma,
 	trb->ctrl = type;
 
 	trb->ctrl |= (DWC3_TRB_CTRL_HWO
-			| DWC3_TRB_CTRL_LST
-			| DWC3_TRB_CTRL_IOC
 			| DWC3_TRB_CTRL_ISP_IMI);
 
+	if (chain)
+		trb->ctrl |= DWC3_TRB_CTRL_CHN;
+	else
+		trb->ctrl |= (DWC3_TRB_CTRL_IOC
+				| DWC3_TRB_CTRL_LST);
+
+	if (chain)
+		return 0;
+
 	memset(&params, 0, sizeof(params));
 	params.param0 = upper_32_bits(dwc->ep0_trb_addr);
 	params.param1 = lower_32_bits(dwc->ep0_trb_addr);
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 8946c32..b8d0a84 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2677,7 +2677,7 @@ int dwc3_gadget_init(struct dwc3 *dwc)
 		goto err0;
 	}
 
-	dwc->ep0_trb = dma_alloc_coherent(dwc->dev, sizeof(*dwc->ep0_trb),
+	dwc->ep0_trb = dma_alloc_coherent(dwc->dev, sizeof(*dwc->ep0_trb) * 2,
 			&dwc->ep0_trb_addr, GFP_KERNEL);
 	if (!dwc->ep0_trb) {
 		dev_err(dwc->dev, "failed to allocate ep0 trb\n");
-- 
1.7.9.5

Re: [PATCH 4/5] usb: dwc3: ep0: Add chained TRB support

[Sergei Shtylyov]

Hello.

On 06/10/2015 12:18 PM, Kishon Vijay Abraham I wrote:

> Add chained TRB support to ep0. Now TRB's can be chained just by
> invoking _dwc3_ep0_start_trans_ with 'chain' parameter set to true.

> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>   drivers/usb/dwc3/ep0.c    |   16 +++++++++++++---
>   drivers/usb/dwc3/gadget.c |    2 +-
>   2 files changed, 14 insertions(+), 4 deletions(-)

> diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
> index d1a2be1..6847afe 100644
> --- a/drivers/usb/dwc3/ep0.c
> +++ b/drivers/usb/dwc3/ep0.c
[...]
> @@ -78,10 +81,17 @@ static int dwc3_ep0_start_trans(struct dwc3 *dwc, u8 epnum, dma_addr_t buf_dma,
>   	trb->ctrl = type;
>
>   	trb->ctrl |= (DWC3_TRB_CTRL_HWO
> -			| DWC3_TRB_CTRL_LST
> -			| DWC3_TRB_CTRL_IOC
>   			| DWC3_TRB_CTRL_ISP_IMI);
>
> +	if (chain)
> +		trb->ctrl |= DWC3_TRB_CTRL_CHN;
> +	else
> +		trb->ctrl |= (DWC3_TRB_CTRL_IOC
> +				| DWC3_TRB_CTRL_LST);

    Parens not needed here (and above too).

[...]

WBR, Sergei

[PATCH 5/5] usb: dwc3: ep0: handle non maxpacket aligned transfers > 512

[Kishon Vijay Abraham I]

Use chained TRB mechanism to handle non maxpacket aligned transfers
greater than bounce buffer size. With this the first TRB will be programmed
to receive 'ALIGN(ur->length - maxp, maxp)' data and the second TRB
will be programmed to receive the remaining data using bounce buffer.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/usb/dwc3/ep0.c |   42 ++++++++++++++++++++++++++++--------------
 1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index 6847afe..4c777fe 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -830,13 +830,26 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc,
 	maxp = ep0->endpoint.maxpacket;
 
 	if (dwc->ep0_bounced) {
+		/*
+		 * Handle the first TRB before handling the bounce buffer if
+		 * the request length is greater than the bounce buffer size
+		 */
+		if (ur->length > DWC3_EP0_BOUNCE_SIZE) {
+			transfer_size = ALIGN(ur->length - maxp, maxp);
+			transferred = transfer_size - length;
+			buf = (u8 *)buf + transferred;
+			ur->actual += transferred;
+			remaining_ur_length -= transferred;
+
+			trb++;
+			length = trb->size & DWC3_TRB_SIZE_MASK;
+
+			ep0->free_slot = 0;
+		}
+
 		transfer_size = roundup((ur->length - transfer_size),
 					maxp);
 
-		/* Maximum of DWC3_EP0_BOUNCE_SIZE can only be received */
-		if (transfer_size > DWC3_EP0_BOUNCE_SIZE)
-			transfer_size = DWC3_EP0_BOUNCE_SIZE;
-
 		transferred = min_t(u32, remaining_ur_length,
 				    transfer_size - length);
 		memcpy(buf, dwc->ep0_bounce, transferred);
@@ -959,21 +972,22 @@ static void __dwc3_ep0_do_control_data(struct dwc3 *dwc,
 		}
 
 		maxpacket = dep->endpoint.maxpacket;
-		transfer_size = roundup((req->request.length - transfer_size),
-					maxpacket);
 
-		if (transfer_size > DWC3_EP0_BOUNCE_SIZE) {
-			dev_WARN(dwc->dev, "bounce buf can't handle req len\n");
-			transfer_size = DWC3_EP0_BOUNCE_SIZE;
+		if (req->request.length > DWC3_EP0_BOUNCE_SIZE) {
+			transfer_size = ALIGN(req->request.length - maxpacket,
+					      maxpacket);
+			ret = dwc3_ep0_start_trans(dwc, dep->number,
+						   req->request.dma,
+						   transfer_size,
+						   DWC3_TRBCTL_CONTROL_DATA,
+						   true);
 		}
 
+		transfer_size = roundup((req->request.length - transfer_size),
+					maxpacket);
+
 		dwc->ep0_bounced = true;
 
-		/*
-		 * REVISIT in case request length is bigger than
-		 * DWC3_EP0_BOUNCE_SIZE we will need two chained
-		 * TRBs to handle the transfer.
-		 */
 		ret = dwc3_ep0_start_trans(dwc, dep->number,
 				dwc->ep0_bounce_addr, transfer_size,
 				DWC3_TRBCTL_CONTROL_DATA, false);
-- 
1.7.9.5