Hi,
On Fri, Jan 18, 2019 at 10:58 AM Tejas Joglekar
<Tejas.Joglekar@xxxxxxxxxxxx> wrote:
>
> On 12/10/2018 1:06 PM, Felipe Balbi wrote:
> > Hi,
> >
> > Tejas Joglekar <tejas.joglekar@xxxxxxxxxxxx> writes:
> >> For OUT endpoints, zero-length transfers require MaxPacketSize buffer as
> >> per the DWC_usb3 programming guide 3.30a section 4.2.3.3.
> >>
> >> This patch fixes this by explicitly checking zero length
> >> transfer to correctly pad up to MaxPacketSize.
> >>
> >> Signed-off-by: Tejas Joglekar <joglekar@xxxxxxxxxxxx>
> > do you have tracepoints of the problem?
> Yes I have
thanks
> >> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> >> index e2caf9e..f1089ec 100644
> >> --- a/drivers/usb/dwc3/gadget.c
> >> +++ b/drivers/usb/dwc3/gadget.c
> >> @@ -1069,7 +1069,8 @@ static void dwc3_prepare_one_trb_sg(struct dwc3_ep *dep,
> >> if (sg_is_last(s))
> >> chain = false;
> >> - if (rem && usb_endpoint_dir_out(dep->endpoint.desc) && !chain) {
> >> + if ((!length || rem) && usb_endpoint_dir_out(dep->endpoint.desc)
> >> + && !chain) {
> > are you 100% certain we have scatter/gather for a zlp? That doesn't make
> > sense to me.
> No.... my bad not required for scatter/gather implementation
thanks for confirming
> >
> >> @@ -1114,7 +1115,7 @@ static void dwc3_prepare_one_trb_linear(struct dwc3_ep *dep,
> >> unsigned int maxp = usb_endpoint_maxp(dep->endpoint.desc);
> >> unsigned int rem = length % maxp;
> >> - if (rem && usb_endpoint_dir_out(dep->endpoint.desc)) {
> >> + if ((!length || rem) && usb_endpoint_dir_out(dep->endpoint.desc)) {
> > this is already taken care of:
> That path is not taken with the test which I am running
now, with your tracepoints, I can see the problem: gadget driver
queued a request with
length set to 0.
> >> static void dwc3_prepare_one_trb_linear(struct dwc3_ep *dep,
> >> struct dwc3_request *req)
> >> {
> >> unsigned int length = req->request.length;
> >> unsigned int maxp = usb_endpoint_maxp(dep->endpoint.desc);
> >> unsigned int rem = length % maxp;
> >>
> >> if (rem && usb_endpoint_dir_out(dep->endpoint.desc)) {
> > [...]
> >
> >> } else if (req->request.zero && req->request.length &&
> >> (IS_ALIGNED(req->request.length, maxp))) {
> > why isn't this branch triggering for you? Please capture and share
> > tracepoints exposing the problem
>
> Attached trace points here .. with and without my patch ...
>
> You can see the TRB is not padded upto MSP, without patch.
Cool, thanks. Please resend the patch without the hunk of
prepare_one_trb_sg() so I can
apply. Also, make sure to set correct Fixes tag and Cc: stable
--
balbi
