Skip to contentSkip to conversation

ems_usb: endianess warning

DISCUSSION Marc Kleine-Budde 2 participants 8 messages Mar 14, 2015 – Mar 25, 2015 CAN network drivers

lore
Share & tools
lore

Marc Kleine-Budde

author maintainer
#0 lore raw

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?

Quoted 4 lines

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

Signature · 4 lines
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   |
#1 lore raw

Hi Marc,

this took some time, sorry.

Regards
Gerhard

The device expects the id in le format.

Signed-off-by Gerhard Uttenthaler <[email protected]>

---

drivers/net/can/usb/ems_usb.c +2 -5
index 9376f5e..481f539 100644
@@ -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:

Quoted 14 lines

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?

Quoted 4 lines

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

Signature · 8 lines
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
author maintainer Applied
#2 lore raw
Quoted 3 lines · Gerhard Uttenthaler

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

Signature · 4 lines
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   |
author maintainer
#3 lore raw
Quoted 4 lines · Marc Kleine-Budde
Quoted 1 line · Gerhard Uttenthaler

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?

Quoted 7 lines

/* 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 */

        ^^^^^^^^^^^
Quoted 12 lines
	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

Signature · 4 lines
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   |
#4 lore raw

Am 25.03.2015 um 09:44 schrieb Marc Kleine-Budde:

Quoted 10 lines · Marc Kleine-Budde
Quoted 4 lines · Gerhard Uttenthaler
Quoted 1 line

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

Signature · 8 lines
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
author maintainer
#5 lore raw
Quoted 4 lines · Gerhard Uttenthaler
Quoted 1 line

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

Signature · 4 lines
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   |
#6 lore raw

Am 25.03.2015 um 10:38 schrieb Marc Kleine-Budde:

Quoted 10 lines · Gerhard Uttenthaler
Quoted 4 lines
Quoted 1 line

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

Signature · 8 lines
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
author maintainer
#7 lore raw
Quoted 8 lines · Gerhard Uttenthaler
Quoted 4 lines

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 <[email protected]>.

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

Marc

Signature · 4 lines
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   |