[PATCH RESEND 0/2] usb: dwc3: gadget: Bug fixes

[PATCH RESEND 0/2] usb: dwc3: gadget: Bug fixes

[Amit Virdi]

This is a re-submission of patches [1/4] and [2/4] from:
	http://www.spinics.net/lists/linux-usb/msg118841.html

Commit log of both these patches has been modified for aided clarity. These
patches have been rebased on Balbi's testing/next.

Patches [3/4] and [4/4] were accepted as they were. 

Amit Virdi (2):
  usb: dwc3: gadget: Fix TRB preparation during SG
  usb: dwc3: gadget: Stop TRB preparation after limit is reached

 drivers/usb/dwc3/gadget.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

-- 
1.8.0

--
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 RESEND 1/2] usb: dwc3: gadget: Fix TRB preparation during SG

[Amit Virdi]

When scatter gather (SG) is used, multiple TRBs are prepared from one DWC3
request (dwc3_request). So while preparing TRBs, the 'last' flag should be set
only when it is the last TRB being prepared from the last dwc3_request entry.

The current implementation uses list_is_last to check if the dwc3_request is the
last entry from the request_list. However, list_is_last returns false for the
last entry too. This is because, while preparing the first TRB from a request,
the function dwc3_prepare_one_trb modifies the request's next and prev pointers
while moving the URB to req_queued. Hence, list_is_last always returns false no
matter what.

The correct way is not to access the modified pointers of dwc3_request but to
use list_empty macro instead.

Fixes: e5ba5ec833aa4a76980b512d6a6779643516b850 ("usb: dwc3: gadget: fix scatter
gather implementation"

Signed-off-by: Amit Virdi <amit.virdi-qxv4g6HH51o@public.gmane.org>
Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> # v3.9+
---
 drivers/usb/dwc3/gadget.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 4e2593993fae..cb8939134c32 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -879,8 +879,7 @@ static void dwc3_prepare_trbs(struct dwc3_ep *dep, bool starting)
 
 				if (i == (request->num_mapped_sgs - 1) ||
 						sg_is_last(s)) {
-					if (list_is_last(&req->list,
-							&dep->request_list))
+					if (list_empty(&dep->request_list))
 						last_one = true;
 					chain = false;
 				}
-- 
1.8.0

--
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 RESEND 2/2] usb: dwc3: gadget: Stop TRB preparation after limit is reached

[Amit Virdi]

DWC3 gadget sets up a pool of 32 TRBs for each EP during initialization. This
means, the max TRBs that can be submitted for an EP is fixed to 32. Since the
request queue for an EP is a linked list, any number of requests can be queued
to it by the gadget layer.  However, the dwc3 driver must not submit TRBs more
than the pool it has created for. This limit wasn't respected when SG was used
resulting in submitting more than the max TRBs, eventually leading to
non-transfer of the TRBs submitted over the max limit.

Root cause:
When SG is used, there are two loops iterating to prepare TRBs:
 - Outer loop over the request_list
 - Inner loop over the SG list
The code was missing break to get out of the outer loop.

Signed-off-by: Amit Virdi <amit.virdi@st.com>
Cc: <stable@vger.kernel.org> # v3.9+
---
 drivers/usb/dwc3/gadget.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index cb8939134c32..6c5e344822b9 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -897,6 +897,9 @@ static void dwc3_prepare_trbs(struct dwc3_ep *dep, bool starting)
 				if (last_one)
 					break;
 			}
+
+			if (last_one)
+				break;
 		} else {
 			dma = req->request.dma;
 			length = req->request.length;
-- 
1.8.0

Re: [PATCH RESEND 0/2] usb: dwc3: gadget: Bug fixes

[Amit Virdi]

On 1/13/2015 12:10 PM, Amit VIRDI wrote:
> This is a re-submission of patches [1/4] and [2/4] from:
> 	http://www.spinics.net/lists/linux-usb/msg118841.html
>
> Commit log of both these patches has been modified for aided clarity. These
> patches have been rebased on Balbi's testing/next.
>
> Patches [3/4] and [4/4] were accepted as they were.
>
> Amit Virdi (2):
>    usb: dwc3: gadget: Fix TRB preparation during SG
>    usb: dwc3: gadget: Stop TRB preparation after limit is reached
>
>   drivers/usb/dwc3/gadget.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>

I was using older git version, so git failed to send patches to 
stable@vger.kernel.org

Please discard this patchset. I'm resending with updated git

[PATCH RESEND(2) 0/2] usb: dwc3: gadget: Bug fixes

[Amit Virdi]

This is a re-submission of patches [1/4] and [2/4] from:
	http://www.spinics.net/lists/linux-usb/msg118841.html

Commit log of both these patches has been modified for aided clarity. These
patches have been rebased on Balbi's testing/next.

Patches [3/4] and [4/4] were accepted as they were. 

Amit Virdi (2):
  usb: dwc3: gadget: Fix TRB preparation during SG
  usb: dwc3: gadget: Stop TRB preparation after limit is reached

 drivers/usb/dwc3/gadget.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

-- 
1.8.0

--
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 RESEND(2) 1/2] usb: dwc3: gadget: Fix TRB preparation during SG

[Amit Virdi]

When scatter gather (SG) is used, multiple TRBs are prepared from one DWC3
request (dwc3_request). So while preparing TRBs, the 'last' flag should be set
only when it is the last TRB being prepared from the last dwc3_request entry.

The current implementation uses list_is_last to check if the dwc3_request is the
last entry from the request_list. However, list_is_last returns false for the
last entry too. This is because, while preparing the first TRB from a request,
the function dwc3_prepare_one_trb modifies the request's next and prev pointers
while moving the URB to req_queued. Hence, list_is_last always returns false no
matter what.

The correct way is not to access the modified pointers of dwc3_request but to
use list_empty macro instead.

Fixes: e5ba5ec833aa4a76980b512d6a6779643516b850 ("usb: dwc3: gadget: fix scatter
gather implementation"

Signed-off-by: Amit Virdi <amit.virdi-qxv4g6HH51o@public.gmane.org>
Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> # v3.9+
---
 drivers/usb/dwc3/gadget.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 4e2593993fae..cb8939134c32 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -879,8 +879,7 @@ static void dwc3_prepare_trbs(struct dwc3_ep *dep, bool starting)
 
 				if (i == (request->num_mapped_sgs - 1) ||
 						sg_is_last(s)) {
-					if (list_is_last(&req->list,
-							&dep->request_list))
+					if (list_empty(&dep->request_list))
 						last_one = true;
 					chain = false;
 				}
-- 
1.8.0

--
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 RESEND(2) 2/2] usb: dwc3: gadget: Stop TRB preparation after limit is reached

[Amit Virdi]

DWC3 gadget sets up a pool of 32 TRBs for each EP during initialization. This
means, the max TRBs that can be submitted for an EP is fixed to 32. Since the
request queue for an EP is a linked list, any number of requests can be queued
to it by the gadget layer.  However, the dwc3 driver must not submit TRBs more
than the pool it has created for. This limit wasn't respected when SG was used
resulting in submitting more than the max TRBs, eventually leading to
non-transfer of the TRBs submitted over the max limit.

Root cause:
When SG is used, there are two loops iterating to prepare TRBs:
 - Outer loop over the request_list
 - Inner loop over the SG list
The code was missing break to get out of the outer loop.

Signed-off-by: Amit Virdi <amit.virdi-qxv4g6HH51o@public.gmane.org>
Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> # v3.9+
---
 drivers/usb/dwc3/gadget.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index cb8939134c32..6c5e344822b9 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -897,6 +897,9 @@ static void dwc3_prepare_trbs(struct dwc3_ep *dep, bool starting)
 				if (last_one)
 					break;
 			}
+
+			if (last_one)
+				break;
 		} else {
 			dma = req->request.dma;
 			length = req->request.length;
-- 
1.8.0

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