| Message ID | 1666159637-161135-1-git-send-email-pawell@cadence.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: cdnsp: fix issue with ZLP - added TD_SIZE = 1 | expand |
On 22-10-19 02:07:17, Pawel Laszczak wrote: > Patch modifies the TD_SIZE in TRB before ZLP TRB. > The TD_SIZE in TRB before ZLP TRB must be set to 1 to force > processing ZLP TRB by controller. > > Cc: <stable@vger.kernel.org> > Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence USBSSP DRD Driver") > Signed-off-by: Pawel Laszczak <pawell@cadence.com> > --- > drivers/usb/cdns3/cdnsp-ring.c | 15 ++++++++------- > 1 file changed, 8 insertions(+), 7 deletions(-) > > diff --git a/drivers/usb/cdns3/cdnsp-ring.c b/drivers/usb/cdns3/cdnsp-ring.c > index 794e413800ae..4809d0e894bb 100644 > --- a/drivers/usb/cdns3/cdnsp-ring.c > +++ b/drivers/usb/cdns3/cdnsp-ring.c > @@ -1765,18 +1765,19 @@ static u32 cdnsp_td_remainder(struct cdnsp_device *pdev, > struct cdnsp_request *preq, > bool more_trbs_coming) > { > - u32 maxp, total_packet_count; > - > - /* One TRB with a zero-length data packet. */ > - if (!more_trbs_coming || (transferred == 0 && trb_buff_len == 0) || > - trb_buff_len == td_total_len) > - return 0; > + u32 maxp, total_packet_count, remainder; > > maxp = usb_endpoint_maxp(preq->pep->endpoint.desc); > total_packet_count = DIV_ROUND_UP(td_total_len, maxp); > > /* Queuing functions don't count the current TRB into transferred. */ > - return (total_packet_count - ((transferred + trb_buff_len) / maxp)); > + remainder = (total_packet_count - ((transferred + trb_buff_len) / maxp)); > + > + /* Before ZLP driver needs set TD_SIZE=1. */ > + if (!remainder && more_trbs_coming) > + remainder = 1; Without ZLP, TD_SIZE = 0 for the last TRB. With ZLP, TD_SIZE = 1 for current TRB, and TD_SIZE = 0 for the next TRB (the last zero-length packet) right? Peter > + > + return remainder; > } > > static int cdnsp_align_td(struct cdnsp_device *pdev, > -- > 2.25.1 >
> > >On 22-10-19 02:07:17, Pawel Laszczak wrote: >> Patch modifies the TD_SIZE in TRB before ZLP TRB. >> The TD_SIZE in TRB before ZLP TRB must be set to 1 to force processing >> ZLP TRB by controller. >> >> Cc: <stable@vger.kernel.org> >> Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence >> USBSSP DRD Driver") >> Signed-off-by: Pawel Laszczak <pawell@cadence.com> >> --- >> drivers/usb/cdns3/cdnsp-ring.c | 15 ++++++++------- >> 1 file changed, 8 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/usb/cdns3/cdnsp-ring.c >> b/drivers/usb/cdns3/cdnsp-ring.c index 794e413800ae..4809d0e894bb >> 100644 >> --- a/drivers/usb/cdns3/cdnsp-ring.c >> +++ b/drivers/usb/cdns3/cdnsp-ring.c >> @@ -1765,18 +1765,19 @@ static u32 cdnsp_td_remainder(struct >cdnsp_device *pdev, >> struct cdnsp_request *preq, >> bool more_trbs_coming) >> { >> - u32 maxp, total_packet_count; >> - >> - /* One TRB with a zero-length data packet. */ >> - if (!more_trbs_coming || (transferred == 0 && trb_buff_len == 0) || >> - trb_buff_len == td_total_len) >> - return 0; >> + u32 maxp, total_packet_count, remainder; >> >> maxp = usb_endpoint_maxp(preq->pep->endpoint.desc); >> total_packet_count = DIV_ROUND_UP(td_total_len, maxp); >> >> /* Queuing functions don't count the current TRB into transferred. */ >> - return (total_packet_count - ((transferred + trb_buff_len) / maxp)); >> + remainder = (total_packet_count - ((transferred + trb_buff_len) / >> +maxp)); >> + >> + /* Before ZLP driver needs set TD_SIZE=1. */ >> + if (!remainder && more_trbs_coming) >> + remainder = 1; > >Without ZLP, TD_SIZE = 0 for the last TRB. >With ZLP, TD_SIZE = 1 for current TRB, and TD_SIZE = 0 for the next TRB (the >last zero-length packet) right? Yes, you have right. Pawel >Peter > >> + >> + return remainder; >> } >> >> static int cdnsp_align_td(struct cdnsp_device *pdev, >> -- >> 2.25.1 >> > >-- > >Thanks, >Peter Chen Regards, Pawel Laszczak
On 22-10-21 05:03:25, Pawel Laszczak wrote: > > > > > >On 22-10-19 02:07:17, Pawel Laszczak wrote: > >> Patch modifies the TD_SIZE in TRB before ZLP TRB. > >> The TD_SIZE in TRB before ZLP TRB must be set to 1 to force processing > >> ZLP TRB by controller. > >> > >> Cc: <stable@vger.kernel.org> > >> Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence > >> USBSSP DRD Driver") > >> Signed-off-by: Pawel Laszczak <pawell@cadence.com> > >> --- > >> drivers/usb/cdns3/cdnsp-ring.c | 15 ++++++++------- > >> 1 file changed, 8 insertions(+), 7 deletions(-) > >> > >> diff --git a/drivers/usb/cdns3/cdnsp-ring.c > >> b/drivers/usb/cdns3/cdnsp-ring.c index 794e413800ae..4809d0e894bb > >> 100644 > >> --- a/drivers/usb/cdns3/cdnsp-ring.c > >> +++ b/drivers/usb/cdns3/cdnsp-ring.c > >> @@ -1765,18 +1765,19 @@ static u32 cdnsp_td_remainder(struct > >cdnsp_device *pdev, > >> struct cdnsp_request *preq, > >> bool more_trbs_coming) > >> { > >> - u32 maxp, total_packet_count; > >> - > >> - /* One TRB with a zero-length data packet. */ > >> - if (!more_trbs_coming || (transferred == 0 && trb_buff_len == 0) || > >> - trb_buff_len == td_total_len) > >> - return 0; > >> + u32 maxp, total_packet_count, remainder; > >> > >> maxp = usb_endpoint_maxp(preq->pep->endpoint.desc); > >> total_packet_count = DIV_ROUND_UP(td_total_len, maxp); > >> > >> /* Queuing functions don't count the current TRB into transferred. */ > >> - return (total_packet_count - ((transferred + trb_buff_len) / maxp)); > >> + remainder = (total_packet_count - ((transferred + trb_buff_len) / > >> +maxp)); > >> + > >> + /* Before ZLP driver needs set TD_SIZE=1. */ > >> + if (!remainder && more_trbs_coming) > >> + remainder = 1; > > > >Without ZLP, TD_SIZE = 0 for the last TRB. > >With ZLP, TD_SIZE = 1 for current TRB, and TD_SIZE = 0 for the next TRB (the > >last zero-length packet) right? > > Yes, you have right. > Pawel, With your changes, the return value is 1 for function cdnsp_queue_ctrl_tx. Without your changes, it is 0, something wrong?
>> > >> >On 22-10-19 02:07:17, Pawel Laszczak wrote: >> >> Patch modifies the TD_SIZE in TRB before ZLP TRB. >> >> The TD_SIZE in TRB before ZLP TRB must be set to 1 to force >> >> processing ZLP TRB by controller. >> >> >> >> Cc: <stable@vger.kernel.org> >> >> Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence >> >> USBSSP DRD Driver") >> >> Signed-off-by: Pawel Laszczak <pawell@cadence.com> >> >> --- >> >> drivers/usb/cdns3/cdnsp-ring.c | 15 ++++++++------- >> >> 1 file changed, 8 insertions(+), 7 deletions(-) >> >> >> >> diff --git a/drivers/usb/cdns3/cdnsp-ring.c >> >> b/drivers/usb/cdns3/cdnsp-ring.c index 794e413800ae..4809d0e894bb >> >> 100644 >> >> --- a/drivers/usb/cdns3/cdnsp-ring.c >> >> +++ b/drivers/usb/cdns3/cdnsp-ring.c >> >> @@ -1765,18 +1765,19 @@ static u32 cdnsp_td_remainder(struct >> >cdnsp_device *pdev, >> >> struct cdnsp_request *preq, >> >> bool more_trbs_coming) >> >> { >> >> - u32 maxp, total_packet_count; >> >> - >> >> - /* One TRB with a zero-length data packet. */ >> >> - if (!more_trbs_coming || (transferred == 0 && trb_buff_len == 0) || >> >> - trb_buff_len == td_total_len) >> >> - return 0; >> >> + u32 maxp, total_packet_count, remainder; >> >> >> >> maxp = usb_endpoint_maxp(preq->pep->endpoint.desc); >> >> total_packet_count = DIV_ROUND_UP(td_total_len, maxp); >> >> >> >> /* Queuing functions don't count the current TRB into transferred. */ >> >> - return (total_packet_count - ((transferred + trb_buff_len) / maxp)); >> >> + remainder = (total_packet_count - ((transferred + trb_buff_len) / >> >> +maxp)); >> >> + >> >> + /* Before ZLP driver needs set TD_SIZE=1. */ >> >> + if (!remainder && more_trbs_coming) >> >> + remainder = 1; >> > >> >Without ZLP, TD_SIZE = 0 for the last TRB. >> >With ZLP, TD_SIZE = 1 for current TRB, and TD_SIZE = 0 for the next >> >TRB (the last zero-length packet) right? >> >> Yes, you have right. >> > >Pawel, With your changes, the return value is 1 for function >cdnsp_queue_ctrl_tx. Without your changes, it is 0, something wrong? > Yes you have right. It does not cause a problem, but it's not according with controller specification. I posted the fixed version of this patch. Thanks, Pawel
diff --git a/drivers/usb/cdns3/cdnsp-ring.c b/drivers/usb/cdns3/cdnsp-ring.c index 794e413800ae..4809d0e894bb 100644 --- a/drivers/usb/cdns3/cdnsp-ring.c +++ b/drivers/usb/cdns3/cdnsp-ring.c @@ -1765,18 +1765,19 @@ static u32 cdnsp_td_remainder(struct cdnsp_device *pdev, struct cdnsp_request *preq, bool more_trbs_coming) { - u32 maxp, total_packet_count; - - /* One TRB with a zero-length data packet. */ - if (!more_trbs_coming || (transferred == 0 && trb_buff_len == 0) || - trb_buff_len == td_total_len) - return 0; + u32 maxp, total_packet_count, remainder; maxp = usb_endpoint_maxp(preq->pep->endpoint.desc); total_packet_count = DIV_ROUND_UP(td_total_len, maxp); /* Queuing functions don't count the current TRB into transferred. */ - return (total_packet_count - ((transferred + trb_buff_len) / maxp)); + remainder = (total_packet_count - ((transferred + trb_buff_len) / maxp)); + + /* Before ZLP driver needs set TD_SIZE=1. */ + if (!remainder && more_trbs_coming) + remainder = 1; + + return remainder; } static int cdnsp_align_td(struct cdnsp_device *pdev,
Patch modifies the TD_SIZE in TRB before ZLP TRB. The TD_SIZE in TRB before ZLP TRB must be set to 1 to force processing ZLP TRB by controller. Cc: <stable@vger.kernel.org> Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence USBSSP DRD Driver") Signed-off-by: Pawel Laszczak <pawell@cadence.com> --- drivers/usb/cdns3/cdnsp-ring.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)