RE: [PATCH] usb: cdnsp: Fixes incorrect value in ISOC TRB
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
>>
>> 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
>> ---
>> 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
Re: [PATCH] usb: cdnsp: Fixes incorrect value in ISOC TRB
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 > > > > 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
Re: [PATCH] usb: cdnsp: Fixes incorrect value in ISOC TRB
On 21-03-05 06:10:59, Pawel Laszczak wrote:
> From: Pawel Laszczak
>
> 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
> ---
> 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,
Peter Chen
RE: [PATCH] usb: cdnsp: Fixes incorrect value in ISOC TRB
Hi,
Please ignore this patch. I put incorrect address to Peter. I have sent again
this patch with correct email address.
>-Original Message-
>From: Pawel Laszczak
>Sent: Friday, March 5, 2021 6:00 AM
>To: peter.c...@nxp.com
>Cc: gre...@linuxfoundation.org; linux-...@vger.kernel.org;
>linux-kernel@vger.kernel.org; Rahul Kumar ;
>Sanket Parmar ; Pawel Laszczak
>Subject: [PATCH] usb: cdnsp: Fixes incorrect value in ISOC TRB
>
>From: Pawel Laszczak
>
>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.
>
>Fixes: commit 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence USBSSP
>DRD Driver")
>Signed-off-by: Pawel Laszczak
>---
> 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
Regards,
Pawel Laszczak
[PATCH] usb: cdnsp: Fixes incorrect value in ISOC TRB
From: Pawel Laszczak
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.
Fixes: commit 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence USBSSP
DRD Driver")
Signed-off-by: Pawel Laszczak
---
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
[PATCH] usb: cdnsp: Fixes incorrect value in ISOC TRB
From: Pawel Laszczak
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.
Fixes: commit 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence USBSSP
DRD Driver")
Signed-off-by: Pawel Laszczak
---
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
