| Message ID | 1536326029-16327-2-git-send-email-anurag.kumar.vulisha@xilinx.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | usb: dwc3: Fix broken BULK stream support to dwc3 gadget driver | expand |
Hi Anurag, On 9/7/2018 6:14 AM, Anurag Kumar Vulisha wrote: > Availability of TRB's are calculated using dwc3_calc_trbs_left(), which > determines available TRB's based on the HWO bit set in a TRB. > > __dwc3_prepare_one_trb() is called with a TRB which needs to be prepared > for transfer. This __dwc3_prepare_one_trb() calls dwc3_calc_trbs_left() > to determine total available TRBs and set IOC bit if the total available > TRBs are zero. Since the present working TRB(which is passed as an > argument to __dwc3_prepare_one_trb() ) doesn't have HWO bit already set, > there are chances where dwc3_calc_trbs_left() wrongly calculates this > present working TRB as free(since the HWO bit is not yet set). This could > be a problem. This patch correct this issue by setting HWO bit before > calling dwc3_calc_trbs_left() Please revise your commit message to reflect your new change. > > Signed-off-by: Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com> > Reviewed-by: Thinh Nguyen <thinhn@synopsys.com> > --- > Changes in v3: > 1. Corrected the logic for setting HWO bit as suggested by "Thinh Nguyen" > > Changes in v2: > 1. Changed the commit message > --- > drivers/usb/dwc3/gadget.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > index 032ea7d..8a1622b 100644 > --- a/drivers/usb/dwc3/gadget.c > +++ b/drivers/usb/dwc3/gadget.c > @@ -911,8 +911,6 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep *dep, struct dwc3_trb *trb, > struct usb_gadget *gadget = &dwc->gadget; > enum usb_device_speed speed = gadget->speed; > > - dwc3_ep_inc_enq(dep); > - > trb->size = DWC3_TRB_SIZE_LENGTH(length); > trb->bpl = lower_32_bits(dma); > trb->bph = upper_32_bits(dma); > @@ -991,7 +989,7 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep *dep, struct dwc3_trb *trb, > } > > if ((!no_interrupt && !chain) || > - (dwc3_calc_trbs_left(dep) == 0)) > + (dwc3_calc_trbs_left(dep) == 1)) > trb->ctrl |= DWC3_TRB_CTRL_IOC; > > if (chain) > @@ -1002,6 +1000,8 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep *dep, struct dwc3_trb *trb, > > trb->ctrl |= DWC3_TRB_CTRL_HWO; > > + dwc3_ep_inc_enq(dep); > + > trace_dwc3_prepare_trb(dep, trb); > } > BR, Thinh
Hi Thinh, >On 9/7/2018 6:14 AM, Anurag Kumar Vulisha wrote: >> Availability of TRB's are calculated using dwc3_calc_trbs_left(), >> which determines available TRB's based on the HWO bit set in a TRB. >> >> __dwc3_prepare_one_trb() is called with a TRB which needs to be >> prepared for transfer. This __dwc3_prepare_one_trb() calls >> dwc3_calc_trbs_left() to determine total available TRBs and set IOC >> bit if the total available TRBs are zero. Since the present working >> TRB(which is passed as an argument to __dwc3_prepare_one_trb() ) >> doesn't have HWO bit already set, there are chances where >> dwc3_calc_trbs_left() wrongly calculates this present working TRB as >> free(since the HWO bit is not yet set). This could be a problem. This >> patch correct this issue by setting HWO bit before calling >> dwc3_calc_trbs_left() > >Please revise your commit message to reflect your new change. > Sorry , I missed updating the commit message. Thanks a lot for pointing, will fix it in next version Thanks, Anurag Kumar Vulisha >> >> Signed-off-by: Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com> >> Reviewed-by: Thinh Nguyen <thinhn@synopsys.com> >> --- >> Changes in v3: >> 1. Corrected the logic for setting HWO bit as suggested by "Thinh Nguyen" >> >> Changes in v2: >> 1. Changed the commit message >> --- >> drivers/usb/dwc3/gadget.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c >> index 032ea7d..8a1622b 100644 >> --- a/drivers/usb/dwc3/gadget.c >> +++ b/drivers/usb/dwc3/gadget.c >> @@ -911,8 +911,6 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep *dep, >struct dwc3_trb *trb, >> struct usb_gadget *gadget = &dwc->gadget; >> enum usb_device_speed speed = gadget->speed; >> >> - dwc3_ep_inc_enq(dep); >> - >> trb->size = DWC3_TRB_SIZE_LENGTH(length); >> trb->bpl = lower_32_bits(dma); >> trb->bph = upper_32_bits(dma); >> @@ -991,7 +989,7 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep *dep, >struct dwc3_trb *trb, >> } >> >> if ((!no_interrupt && !chain) || >> - (dwc3_calc_trbs_left(dep) == 0)) >> + (dwc3_calc_trbs_left(dep) == 1)) >> trb->ctrl |= DWC3_TRB_CTRL_IOC; >> >> if (chain) >> @@ -1002,6 +1000,8 @@ static void __dwc3_prepare_one_trb(struct >> dwc3_ep *dep, struct dwc3_trb *trb, >> >> trb->ctrl |= DWC3_TRB_CTRL_HWO; >> >> + dwc3_ep_inc_enq(dep); >> + >> trace_dwc3_prepare_trb(dep, trb); >> } >> > >BR, >Thinh
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 032ea7d..8a1622b 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -911,8 +911,6 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep *dep, struct dwc3_trb *trb, struct usb_gadget *gadget = &dwc->gadget; enum usb_device_speed speed = gadget->speed; - dwc3_ep_inc_enq(dep); - trb->size = DWC3_TRB_SIZE_LENGTH(length); trb->bpl = lower_32_bits(dma); trb->bph = upper_32_bits(dma); @@ -991,7 +989,7 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep *dep, struct dwc3_trb *trb, } if ((!no_interrupt && !chain) || - (dwc3_calc_trbs_left(dep) == 0)) + (dwc3_calc_trbs_left(dep) == 1)) trb->ctrl |= DWC3_TRB_CTRL_IOC; if (chain) @@ -1002,6 +1000,8 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep *dep, struct dwc3_trb *trb, trb->ctrl |= DWC3_TRB_CTRL_HWO; + dwc3_ep_inc_enq(dep); + trace_dwc3_prepare_trb(dep, trb); }