| Message ID | 20210305051059.31623-1-pawell@gli-login.cadence.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: cdnsp: Fixes incorrect value in ISOC TRB | expand |
On 21-03-05 06:10:59, Pawel Laszczak wrote: > From: Pawel Laszczak <pawell@cadence.com> > > The value "start_cycle ? 0 : 1" in assignment caused > implicit truncation whole value to 1 byte. > To fix the issue, an explicit casting has been added. The root cause for this issue should be operator "|" priority higher than "? :", I think just add () for start_cycle ? 0 : 1 could fix it. Please double confirm it, and change the commit log if necessary Peter > > Fixes: commit 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 | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/cdns3/cdnsp-ring.c b/drivers/usb/cdns3/cdnsp-ring.c > index f9170d177a89..d35bc4490216 100644 > --- a/drivers/usb/cdns3/cdnsp-ring.c > +++ b/drivers/usb/cdns3/cdnsp-ring.c > @@ -2197,7 +2197,7 @@ static int cdnsp_queue_isoc_tx(struct cdnsp_device *pdev, > * inverted in the first TDs isoc TRB. > */ > field = TRB_TYPE(TRB_ISOC) | TRB_TLBPC(last_burst_pkt) | > - start_cycle ? 0 : 1 | TRB_SIA | TRB_TBC(burst_count); > + (u32)(start_cycle ? 0 : 1) | TRB_SIA | TRB_TBC(burst_count); > > /* Fill the rest of the TRB fields, and remaining normal TRBs. */ > for (i = 0; i < trbs_per_td; i++) { > -- > 2.25.1 >
On Sat, Mar 06, 2021 at 08:53:42AM +0800, Peter Chen wrote: > On 21-03-05 06:10:59, Pawel Laszczak wrote: > > From: Pawel Laszczak <pawell@cadence.com> > > > > The value "start_cycle ? 0 : 1" in assignment caused > > implicit truncation whole value to 1 byte. > > To fix the issue, an explicit casting has been added. > > The root cause for this issue should be operator "|" priority higher > than "? :", I think just add () for start_cycle ? 0 : 1 could fix it. > Please double confirm it, and change the commit log if necessary Please do not rely on this type of thing to get the code right. Spell it out with real if () statements so that humans can read it and understand it and maintain it for the next 10+ years. thanks, greg k-h
You have right. It's the operator priority issue. I've made this condition as separate "if" statement as suggested by Greg. V2 has been posted. Pawel > > >On 21-03-05 06:10:59, Pawel Laszczak wrote: >> From: Pawel Laszczak <pawell@cadence.com> >> >> The value "start_cycle ? 0 : 1" in assignment caused >> implicit truncation whole value to 1 byte. >> To fix the issue, an explicit casting has been added. > >The root cause for this issue should be operator "|" priority higher >than "? :", I think just add () for start_cycle ? 0 : 1 could fix it. >Please double confirm it, and change the commit log if necessary > >Peter >> >> Fixes: commit 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 | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/usb/cdns3/cdnsp-ring.c b/drivers/usb/cdns3/cdnsp-ring.c >> index f9170d177a89..d35bc4490216 100644 >> --- a/drivers/usb/cdns3/cdnsp-ring.c >> +++ b/drivers/usb/cdns3/cdnsp-ring.c >> @@ -2197,7 +2197,7 @@ static int cdnsp_queue_isoc_tx(struct cdnsp_device *pdev, >> * inverted in the first TDs isoc TRB. >> */ >> field = TRB_TYPE(TRB_ISOC) | TRB_TLBPC(last_burst_pkt) | >> - start_cycle ? 0 : 1 | TRB_SIA | TRB_TBC(burst_count); >> + (u32)(start_cycle ? 0 : 1) | TRB_SIA | TRB_TBC(burst_count); >> >> /* Fill the rest of the TRB fields, and remaining normal TRBs. */ >> for (i = 0; i < trbs_per_td; i++) { >> -- >> 2.25.1 >> -- Thanks, Pawel Laszczak
diff --git a/drivers/usb/cdns3/cdnsp-ring.c b/drivers/usb/cdns3/cdnsp-ring.c index f9170d177a89..d35bc4490216 100644 --- a/drivers/usb/cdns3/cdnsp-ring.c +++ b/drivers/usb/cdns3/cdnsp-ring.c @@ -2197,7 +2197,7 @@ static int cdnsp_queue_isoc_tx(struct cdnsp_device *pdev, * inverted in the first TDs isoc TRB. */ field = TRB_TYPE(TRB_ISOC) | TRB_TLBPC(last_burst_pkt) | - start_cycle ? 0 : 1 | TRB_SIA | TRB_TBC(burst_count); + (u32)(start_cycle ? 0 : 1) | TRB_SIA | TRB_TBC(burst_count); /* Fill the rest of the TRB fields, and remaining normal TRBs. */ for (i = 0; i < trbs_per_td; i++) {