diff mbox series

[v2,2/2] usb: dwc3: gadget: fix high speed multiplier setting

Message ID 20220704141812.1532306-3-m.grzeschik@pengutronix.de (mailing list archive)
State Accepted
Commit 8affe37c525d800a2628c4ecfaed13b77dc5634a
Headers show
Series fix trb handling for high speed isoc transfers | expand

Commit Message

Michael Grzeschik July 4, 2022, 2:18 p.m. UTC
For High-Speed Transfers the prepare_one_trb function is calculating the
multiplier setting for the trb based on the length parameter of the trb
currently prepared. This assumption is wrong. For trbs with a sg list,
the length of the actual request has to be taken instead.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
v1 -> v2: - added refactor patch before this patch
          - using req->request.length as condition value

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

Comments

Greg KH July 8, 2022, 12:45 p.m. UTC | #1
On Mon, Jul 04, 2022 at 04:18:12PM +0200, Michael Grzeschik wrote:
> For High-Speed Transfers the prepare_one_trb function is calculating the
> multiplier setting for the trb based on the length parameter of the trb
> currently prepared. This assumption is wrong. For trbs with a sg list,
> the length of the actual request has to be taken instead.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> ---
> v1 -> v2: - added refactor patch before this patch
>           - using req->request.length as condition value

Does this need to be backported to older kernels or is it ok for
5.20-rc1?

thanks,

greg k-h
Michael Grzeschik July 8, 2022, 12:51 p.m. UTC | #2
Cc: Thinh.Nguyen@synopsys.com

On Fri, Jul 08, 2022 at 02:45:30PM +0200, Greg KH wrote:
>On Mon, Jul 04, 2022 at 04:18:12PM +0200, Michael Grzeschik wrote:
>> For High-Speed Transfers the prepare_one_trb function is calculating the
>> multiplier setting for the trb based on the length parameter of the trb
>> currently prepared. This assumption is wrong. For trbs with a sg list,
>> the length of the actual request has to be taken instead.
>>
>> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
>> ---
>> v1 -> v2: - added refactor patch before this patch
>>           - using req->request.length as condition value
>
>Does this need to be backported to older kernels or is it ok for
>5.20-rc1?

We can add this line to the patch.

Fixes: 40d829fb2ec6 ("usb: dwc3: gadget: Correct ISOC DATA PIDs for short packets")

I would say this could be backported.

Thinh?

>thanks,
>
>greg k-h
>
Thinh Nguyen July 8, 2022, 11 p.m. UTC | #3
On 7/8/2022, Michael Grzeschik wrote:
> Cc: Thinh.Nguyen@synopsys.com
>
> On Fri, Jul 08, 2022 at 02:45:30PM +0200, Greg KH wrote:
>> On Mon, Jul 04, 2022 at 04:18:12PM +0200, Michael Grzeschik wrote:
>>> For High-Speed Transfers the prepare_one_trb function is calculating 
>>> the
>>> multiplier setting for the trb based on the length parameter of the trb
>>> currently prepared. This assumption is wrong. For trbs with a sg list,
>>> the length of the actual request has to be taken instead.
>>>
>>> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
>>> ---
>>> v1 -> v2: - added refactor patch before this patch
>>>           - using req->request.length as condition value
>>
>> Does this need to be backported to older kernels or is it ok for
>> 5.20-rc1?
>
> We can add this line to the patch.
>
> Fixes: 40d829fb2ec6 ("usb: dwc3: gadget: Correct ISOC DATA PIDs for 
> short packets")
>
> I would say this could be backported.
>
> Thinh?

Hi Michael,

Since you have the refactoring patch prior to this, it's a bit difficult 
to backport cleanly without rewriting the patch.

It would be nice to be able to backport fixes. But for this particular 
fix, I don't think will affect too many users. Not many application 
would use SG for simply 3KB or less requests. (Note that periodic ep can 
only transfer up to 3KB/interval in HS). Typically a page size is 4KB, 
and I don't think SG is needed.

Anyway, I think Greg already picked up your patches. Thanks for the fix!

BR,
Thinh
diff mbox series

Patch

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index dcd8fc209ccd6a..4366c45c28cf6d 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1265,10 +1265,10 @@  static void dwc3_prepare_one_trb(struct dwc3_ep *dep,
 				unsigned int mult = 2;
 				unsigned int maxp = usb_endpoint_maxp(ep->desc);
 
-				if (trb_length <= (2 * maxp))
+				if (req->request.length <= (2 * maxp))
 					mult--;
 
-				if (trb_length <= maxp)
+				if (req->request.length <= maxp)
 					mult--;
 
 				trb->size |= DWC3_TRB_SIZE_PCM1(mult);