linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] usb: cdnsp: Fixes incorrect value in ISOC TRB
@ 2021-03-05  5:00 Pawel Laszczak
  2021-03-05  5:16 ` Pawel Laszczak
  0 siblings, 1 reply; 6+ messages in thread
From: Pawel Laszczak @ 2021-03-05  5:00 UTC (permalink / raw)
  To: peter.chen
  Cc: gregkh, linux-usb, linux-kernel, kurahul, sparmar, Pawel Laszczak

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.

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* RE: [PATCH] usb: cdnsp: Fixes incorrect value in ISOC TRB
  2021-03-05  5:00 [PATCH] usb: cdnsp: Fixes incorrect value in ISOC TRB Pawel Laszczak
@ 2021-03-05  5:16 ` Pawel Laszczak
  0 siblings, 0 replies; 6+ messages in thread
From: Pawel Laszczak @ 2021-03-05  5:16 UTC (permalink / raw)
  To: Pawel Laszczak, peter.chen
  Cc: gregkh, linux-usb, linux-kernel, Rahul Kumar, Sanket Parmar

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 <pawell@cadence.com>
>Sent: Friday, March 5, 2021 6:00 AM
>To: peter.chen@nxp.com
>Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org; Rahul Kumar <kurahul@cadence.com>;
>Sanket Parmar <sparmar@cadence.com>; Pawel Laszczak <pawell@cadence.com>
>Subject: [PATCH] usb: cdnsp: Fixes incorrect value in ISOC TRB
>
>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.
>
>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

Regards,
Pawel Laszczak

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] usb: cdnsp: Fixes incorrect value in ISOC TRB
  2021-03-06  0:53 ` Peter Chen
  2021-03-06  8:19   ` Greg KH
@ 2021-03-08  7:37   ` Pawel Laszczak
  1 sibling, 0 replies; 6+ messages in thread
From: Pawel Laszczak @ 2021-03-08  7:37 UTC (permalink / raw)
  To: Peter Chen; +Cc: gregkh, linux-usb, linux-kernel, Rahul Kumar, Sanket Parmar


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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] usb: cdnsp: Fixes incorrect value in ISOC TRB
  2021-03-06  0:53 ` Peter Chen
@ 2021-03-06  8:19   ` Greg KH
  2021-03-08  7:37   ` Pawel Laszczak
  1 sibling, 0 replies; 6+ messages in thread
From: Greg KH @ 2021-03-06  8:19 UTC (permalink / raw)
  To: Peter Chen; +Cc: Pawel Laszczak, linux-usb, linux-kernel, kurahul, sparmar

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] usb: cdnsp: Fixes incorrect value in ISOC TRB
  2021-03-05  5:10 Pawel Laszczak
@ 2021-03-06  0:53 ` Peter Chen
  2021-03-06  8:19   ` Greg KH
  2021-03-08  7:37   ` Pawel Laszczak
  0 siblings, 2 replies; 6+ messages in thread
From: Peter Chen @ 2021-03-06  0:53 UTC (permalink / raw)
  To: Pawel Laszczak; +Cc: gregkh, linux-usb, linux-kernel, kurahul, sparmar

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,
Peter Chen


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] usb: cdnsp: Fixes incorrect value in ISOC TRB
@ 2021-03-05  5:10 Pawel Laszczak
  2021-03-06  0:53 ` Peter Chen
  0 siblings, 1 reply; 6+ messages in thread
From: Pawel Laszczak @ 2021-03-05  5:10 UTC (permalink / raw)
  To: peter.chen
  Cc: gregkh, linux-usb, linux-kernel, kurahul, sparmar, Pawel Laszczak

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.

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-03-08  7:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-05  5:00 [PATCH] usb: cdnsp: Fixes incorrect value in ISOC TRB Pawel Laszczak
2021-03-05  5:16 ` Pawel Laszczak
2021-03-05  5:10 Pawel Laszczak
2021-03-06  0:53 ` Peter Chen
2021-03-06  8:19   ` Greg KH
2021-03-08  7:37   ` Pawel Laszczak

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).