ems_usb: endianess warning

ems_usb: endianess warning

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 821 bytes --]

Hello,

when compiling with sparse (make C=2), the ems driver throws the
following warning, can someone, preferred the driver author :), take
care of fixing them?

> drivers/net/can/usb/ems_usb.c:312:22: warning: cast to restricted __le32
> drivers/net/can/usb/ems_usb.c:787:29: warning: incorrect type in assignment (different base types)
> drivers/net/can/usb/ems_usb.c:787:29:    expected unsigned int [unsigned] [usertype] id
> drivers/net/can/usb/ems_usb.c:787:29:    got restricted __le32 [usertype] <noident>

regards,
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: 819 bytes --]

Re: ems_usb: endianess warning

[Gerhard Uttenthaler]

Hi Marc,

this took some time, sorry.

Regards
Gerhard

The device expects the id in le format.

Signed-off-by: Gerhard Uttenthaler <uttenthaler@ems-wuensche.com>
---
diff --git a/drivers/net/can/usb/ems_usb.c b/drivers/net/can/usb/ems_usb.c
index 9376f5e..481f539 100644
--- a/drivers/net/can/usb/ems_usb.c
+++ b/drivers/net/can/usb/ems_usb.c
@@ -123,7 +123,7 @@ MODULE_LICENSE("GPL v2");
  * CPC_MSG_TYPE_EXT_CAN_FRAME or CPC_MSG_TYPE_EXT_RTR_FRAME.
  */
 struct cpc_can_msg {
-       u32 id;
+       __le32 id;
        u8 length;
        u8 msg[8];
 };
@@ -765,7 +765,7 @@ static netdev_tx_t ems_usb_start_xmit(struct sk_buff *skb, struct net_device *ne

        msg = (struct ems_cpc_msg *)&buf[CPC_HEADER_SIZE];

-       msg->msg.can_msg.id = cf->can_id & CAN_ERR_MASK;
+       msg->msg.can_msg.id = cpu_to_le32(cf->can_id & CAN_ERR_MASK);
        msg->msg.can_msg.length = cf->can_dlc;

        if (cf->can_id & CAN_RTR_FLAG) {
@@ -783,9 +783,6 @@ static netdev_tx_t ems_usb_start_xmit(struct sk_buff *skb, struct net_device *ne
                msg->length = CPC_CAN_MSG_MIN_SIZE + cf->can_dlc;
        }

-       /* Respect byte order */
-       msg->msg.can_msg.id = cpu_to_le32(msg->msg.can_msg.id);
-
        for (i = 0; i < MAX_TX_URBS; i++) {
                if (dev->tx_contexts[i].echo_index == MAX_TX_URBS) {
                        context = &dev->tx_contexts[i];

--

Am 14.03.2015 um 15:30 schrieb Marc Kleine-Budde:
> Hello,
> 
> when compiling with sparse (make C=2), the ems driver throws the
> following warning, can someone, preferred the driver author :), take
> care of fixing them?
> 
>> drivers/net/can/usb/ems_usb.c:312:22: warning: cast to restricted __le32
>> drivers/net/can/usb/ems_usb.c:787:29: warning: incorrect type in assignment (different base types)
>> drivers/net/can/usb/ems_usb.c:787:29:    expected unsigned int [unsigned] [usertype] id
>> drivers/net/can/usb/ems_usb.c:787:29:    got restricted __le32 [usertype] <noident>
> 
> regards,
> Marc
> 

-- 
EMS Dr. Thomas Wuensche e.K.
Sonnenhang 3
85304 Ilmmuenster
HRA Neuburg a.d. Donau, HR-Nr. 70.106

Phone: +49-8441-490260
Fax  : +49-8441-81860
http://www.ems-wuensche.com

Re: ems_usb: endianess warning

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 510 bytes --]

On 03/24/2015 07:02 PM, Gerhard Uttenthaler wrote:
> Hi Marc,
> 
> this took some time, sorry.

Thanks, applied to can.

Can you please use git send-email for future patches, as git fails to
apply this one directly.

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: 819 bytes --]

Re: ems_usb: endianess warning

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1349 bytes --]

On 03/25/2015 09:34 AM, Marc Kleine-Budde wrote:
> On 03/24/2015 07:02 PM, Gerhard Uttenthaler wrote:
>> this took some time, sorry.
> 
> Thanks, applied to can.

I missed the fact, that the ID is correct in the driver without the
patch, sparse just missed that. So this goes into can-next.

In the driver there's timestamp, which endianess is corret here?

> /* Main message type used between library and application */
> struct __packed ems_cpc_msg {
> 	u8 type;	/* type of message */
> 	u8 length;	/* length of data within union 'msg' */
> 	u8 msgid;	/* confirmation handle */
> 	u32 ts_sec;	/* timestamp in seconds */
> 	u32 ts_nsec;	/* timestamp in nano seconds */
        ^^^^^^^^^^^
> 
> 	union {
> 		u8 generic[64];
> 		struct cpc_can_msg can_msg;
> 		struct cpc_can_params can_params;
> 		struct cpc_confirm confirmation;
> 		struct cpc_overrun overrun;
> 		struct cpc_can_error error;
> 		struct cpc_can_err_counter err_counter;
> 		u8 can_state;
> 	} msg;
> };

I know, the timestamp is not used in the driver, for now.

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: 819 bytes --]

Re: ems_usb: endianess warning

[Gerhard Uttenthaler]

Am 25.03.2015 um 09:44 schrieb Marc Kleine-Budde:
> On 03/25/2015 09:34 AM, Marc Kleine-Budde wrote:
>> On 03/24/2015 07:02 PM, Gerhard Uttenthaler wrote:
>>> this took some time, sorry.
>>
>> Thanks, applied to can.
> 
> I missed the fact, that the ID is correct in the driver without the
> patch, sparse just missed that. So this goes into can-next.
> 
> In the driver there's timestamp, which endianess is corret here?

The device is LE and transmits/receives everything as LE, hence also the timestamp.

Gerhard
-- 
EMS Dr. Thomas Wuensche e.K.
Sonnenhang 3
85304 Ilmmuenster
HRA Neuburg a.d. Donau, HR-Nr. 70.106

Phone: +49-8441-490260
Fax  : +49-8441-81860
http://www.ems-wuensche.com

Re: ems_usb: endianess warning

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 559 bytes --]

On 03/25/2015 10:32 AM, Gerhard Uttenthaler wrote:
>> In the driver there's timestamp, which endianess is corret here?
> 
> The device is LE and transmits/receives everything as LE, hence also
> the timestamp.

Thanks, I've created a patch, can I have your Acked-by.

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: 819 bytes --]

Re: ems_usb: endianess warning

[Gerhard Uttenthaler]

Am 25.03.2015 um 10:38 schrieb Marc Kleine-Budde:
> On 03/25/2015 10:32 AM, Gerhard Uttenthaler wrote:
>>> In the driver there's timestamp, which endianess is corret here?
>>
>> The device is LE and transmits/receives everything as LE, hence also
>> the timestamp.
> 
> Thanks, I've created a patch, can I have your Acked-by.
> 
> Marc
> 
Yes, sure!

Is this OK or do you need a patch? Sorry for this, maybe silly, question, but this formalism is new
for me.

Gerhard
-- 
EMS Dr. Thomas Wuensche e.K.
Sonnenhang 3
85304 Ilmmuenster
HRA Neuburg a.d. Donau, HR-Nr. 70.106

Phone: +49-8441-490260
Fax  : +49-8441-81860
http://www.ems-wuensche.com

Re: ems_usb: endianess warning

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 716 bytes --]

On 03/25/2015 11:17 AM, Gerhard Uttenthaler wrote:
>> Thanks, I've created a patch, can I have your Acked-by.
>>
>> Marc
>>
> Yes, sure!
> 
> Is this OK or do you need a patch? Sorry for this, maybe silly,
> question, but this formalism is new for me.

Legitimate qeustion, the "formal" way would be to reply on my patch and
add: Acked-by: Joe Developer <j.developer@example.com>.

But your "Yes, sure!" is enough :)

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: 819 bytes --]