[PATCH 2/2] can: usb: esd_usb2: Fix can_dlc value for received RTR, frames

[PATCH 2/2] can: usb: esd_usb2: Fix can_dlc value for received RTR, frames

[Stefan Mätje]

The dlc member of the struct rx_msg contains also the ESD_RTR flag to mark
received RTR frames. Without the fix the can_dlc value for received RTR
frames would always be set to 8 by get_can_dlc() instead of the received
value.

Signed-off-by: Stefan Mätje <stefan.maetje@esd.eu>
---
 drivers/net/can/usb/esd_usb2.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/usb/esd_usb2.c b/drivers/net/can/usb/esd_usb2.c
index 6183b7707ad7..20f2aa21677f 100644
--- a/drivers/net/can/usb/esd_usb2.c
+++ b/drivers/net/can/usb/esd_usb2.c
@@ -49,7 +49,7 @@ MODULE_LICENSE("GPL v2");
 #define CMD_IDADD              6 /* also used for IDADD_REPLY */

 /* esd CAN message flags - dlc field */
-#define ESD_RTR                        0x10
+#define ESD_RTR                        0x10u

 /* esd CAN message flags - id field */
 #define ESD_EXTID              0x20000000
@@ -334,7 +334,7 @@ static void esd_usb2_rx_can_msg(struct
esd_usb2_net_priv *priv,
                }

                cf->can_id = id & ESD_IDMASK;
-               cf->can_dlc = get_can_dlc(msg->msg.rx.dlc);
+               cf->can_dlc = get_can_dlc(msg->msg.rx.dlc & ~ESD_RTR);

                if (id & ESD_EXTID)
                        cf->can_id |= CAN_EFF_FLAG;
-- 
2.11.0

Re: [PATCH 2/2] can: usb: esd_usb2: Fix can_dlc value for received RTR, frames

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 1736 bytes --]

On 10/18/2017 01:25 PM, Stefan Mätje wrote:
> The dlc member of the struct rx_msg contains also the ESD_RTR flag to mark
> received RTR frames. Without the fix the can_dlc value for received RTR
> frames would always be set to 8 by get_can_dlc() instead of the received
> value.
> 
> Signed-off-by: Stefan Mätje <stefan.maetje@esd.eu>
> ---
>  drivers/net/can/usb/esd_usb2.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/can/usb/esd_usb2.c b/drivers/net/can/usb/esd_usb2.c
> index 6183b7707ad7..20f2aa21677f 100644
> --- a/drivers/net/can/usb/esd_usb2.c
> +++ b/drivers/net/can/usb/esd_usb2.c
> @@ -49,7 +49,7 @@ MODULE_LICENSE("GPL v2");
>  #define CMD_IDADD              6 /* also used for IDADD_REPLY */
> 
>  /* esd CAN message flags - dlc field */
> -#define ESD_RTR                        0x10
> +#define ESD_RTR                        0x10u

I think this is not needed, is it?

> 
>  /* esd CAN message flags - id field */
>  #define ESD_EXTID              0x20000000
> @@ -334,7 +334,7 @@ static void esd_usb2_rx_can_msg(struct
> esd_usb2_net_priv *priv,
>                 }
> 
>                 cf->can_id = id & ESD_IDMASK;
> -               cf->can_dlc = get_can_dlc(msg->msg.rx.dlc);
> +               cf->can_dlc = get_can_dlc(msg->msg.rx.dlc & ~ESD_RTR);
> 
>                 if (id & ESD_EXTID)
>                         cf->can_id |= CAN_EFF_FLAG;
> 

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/2] can: usb: esd_usb2: Fix can_dlc value for received RTR, frames

[Stefan Mätje]

Am 18.10.2017 um 13:49 schrieb Marc Kleine-Budde:
> On 10/18/2017 01:25 PM, Stefan Mätje wrote:
>> The dlc member of the struct rx_msg contains also the ESD_RTR flag to mark
>> received RTR frames. Without the fix the can_dlc value for received RTR
>> frames would always be set to 8 by get_can_dlc() instead of the received
>> value.
>>
>> Signed-off-by: Stefan Mätje <stefan.maetje@esd.eu>
>> ---
>>  drivers/net/can/usb/esd_usb2.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/can/usb/esd_usb2.c b/drivers/net/can/usb/esd_usb2.c
>> index 6183b7707ad7..20f2aa21677f 100644
>> --- a/drivers/net/can/usb/esd_usb2.c
>> +++ b/drivers/net/can/usb/esd_usb2.c
>> @@ -49,7 +49,7 @@ MODULE_LICENSE("GPL v2");
>>  #define CMD_IDADD              6 /* also used for IDADD_REPLY */
>>
>>  /* esd CAN message flags - dlc field */
>> -#define ESD_RTR                        0x10
>> +#define ESD_RTR                        0x10u
> 
> I think this is not needed, is it?

Yes, that's not really needed. It's a glitch of my personal style to use
unsigned declarations for values used as masks. I'll change the patch
and send it again.

>>
>>  /* esd CAN message flags - id field */
>>  #define ESD_EXTID              0x20000000
>> @@ -334,7 +334,7 @@ static void esd_usb2_rx_can_msg(struct
>> esd_usb2_net_priv *priv,
>>                 }
>>
>>                 cf->can_id = id & ESD_IDMASK;
>> -               cf->can_dlc = get_can_dlc(msg->msg.rx.dlc);
>> +               cf->can_dlc = get_can_dlc(msg->msg.rx.dlc & ~ESD_RTR);
>>
>>                 if (id & ESD_EXTID)
>>                         cf->can_id |= CAN_EFF_FLAG;
>>
> 
> Marc

Stefan

Re: [PATCH 2/2] can: usb: esd_usb2: Fix can_dlc value for received RTR, frames

[Stefan Mätje]

Hi Marc,

I've seen you already added the changed patch to your last pull request
for the main line kernel.

Thank you for doing so without waiting for me sending a changed patch.

Stefan

Am 18.10.2017 um 16:45 schrieb Stefan Mätje:
> Am 18.10.2017 um 13:49 schrieb Marc Kleine-Budde:
>> On 10/18/2017 01:25 PM, Stefan Mätje wrote:
>>> The dlc member of the struct rx_msg contains also the ESD_RTR flag to mark
>>> received RTR frames. Without the fix the can_dlc value for received RTR
>>> frames would always be set to 8 by get_can_dlc() instead of the received
>>> value.
>>>
>>> Signed-off-by: Stefan Mätje <stefan.maetje@esd.eu>
>>> ---
>>>  drivers/net/can/usb/esd_usb2.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/can/usb/esd_usb2.c b/drivers/net/can/usb/esd_usb2.c
>>> index 6183b7707ad7..20f2aa21677f 100644
>>> --- a/drivers/net/can/usb/esd_usb2.c
>>> +++ b/drivers/net/can/usb/esd_usb2.c
>>> @@ -49,7 +49,7 @@ MODULE_LICENSE("GPL v2");
>>>  #define CMD_IDADD              6 /* also used for IDADD_REPLY */
>>>
>>>  /* esd CAN message flags - dlc field */
>>> -#define ESD_RTR                        0x10
>>> +#define ESD_RTR                        0x10u
>>
>> I think this is not needed, is it?
> 
> Yes, that's not really needed. It's a glitch of my personal style to use
> unsigned declarations for values used as masks. I'll change the patch
> and send it again.
> 
>>>
>>>  /* esd CAN message flags - id field */
>>>  #define ESD_EXTID              0x20000000
>>> @@ -334,7 +334,7 @@ static void esd_usb2_rx_can_msg(struct
>>> esd_usb2_net_priv *priv,
>>>                 }
>>>
>>>                 cf->can_id = id & ESD_IDMASK;
>>> -               cf->can_dlc = get_can_dlc(msg->msg.rx.dlc);
>>> +               cf->can_dlc = get_can_dlc(msg->msg.rx.dlc & ~ESD_RTR);
>>>
>>>                 if (id & ESD_EXTID)
>>>                         cf->can_id |= CAN_EFF_FLAG;
>>>
>>
>> Marc

Re: [PATCH 2/2] can: usb: esd_usb2: Fix can_dlc value for received RTR, frames

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 536 bytes --]

On 10/19/2017 01:40 PM, Stefan Mätje wrote:
> Hi Marc,
> 
> I've seen you already added the changed patch to your last pull request
> for the main line kernel.
> 
> Thank you for doing so without waiting for me sending a changed patch.

np.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 484 bytes --]