* re: can: usb: PEAK-System Technik PCAN-USB specific part
@ 2012-03-06 11:21 Dan Carpenter
2012-03-07 8:56 ` Stephane Grosjean
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Dan Carpenter @ 2012-03-06 11:21 UTC (permalink / raw)
To: s.grosjean; +Cc: linux-can
Hello Stephane Grosjean,
The patch 46be265d3388: "can: usb: PEAK-System Technik PCAN-USB
specific part" from Mar 2, 2012, leads to the following warning:
drivers/net/can/usb/peak_usb/pcan_usb.c:751 pcan_usb_encode_msg()
error: wrong number of bits for 'cpu_to_le32' (16 vs 32)
drivers/net/can/usb/peak_usb/pcan_usb.c
742 /* can id */
743 if (cf->can_id & CAN_EFF_FLAG) {
744 __le32 tmp32 = cpu_to_le32(cf->can_id & CAN_ERR_MASK);
745
746 tmp32 <<= 3;
747 *pc |= PCAN_USB_STATUSLEN_EXT_ID;
748 memcpy(++pc, &tmp32, 4);
749 pc += 4;
750 } else {
751 __le16 tmp16 = cpu_to_le32(cf->can_id & CAN_ERR_MASK);
^^^^^^^^^^^^
A little endian 32 bit can't fit here.
752
753 tmp16 <<= 5;
754 memcpy(++pc, &tmp16, 2);
755 pc += 2;
regards,
dan carpenter
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: can: usb: PEAK-System Technik PCAN-USB specific part 2012-03-06 11:21 can: usb: PEAK-System Technik PCAN-USB specific part Dan Carpenter @ 2012-03-07 8:56 ` Stephane Grosjean 2012-03-07 9:24 ` Marc Kleine-Budde 2012-03-07 9:05 ` Stephane Grosjean 2012-03-07 12:21 ` Dan Carpenter 2 siblings, 1 reply; 14+ messages in thread From: Stephane Grosjean @ 2012-03-07 8:56 UTC (permalink / raw) To: linux-can@vger.kernel.org Hi everybody, Happy to have seen some of you last Monday, at iCC, it was great. It's always easier when we're able to attach a face to a name. Back to work now: Le 06/03/2012 12:21, Dan Carpenter a écrit : > Hello Stephane Grosjean, > > The patch 46be265d3388: "can: usb: PEAK-System Technik PCAN-USB > specific part" from Mar 2, 2012, leads to the following warning: > > drivers/net/can/usb/peak_usb/pcan_usb.c:751 pcan_usb_encode_msg() > error: wrong number of bits for 'cpu_to_le32' (16 vs 32) > > drivers/net/can/usb/peak_usb/pcan_usb.c > 742 /* can id */ > 743 if (cf->can_id& CAN_EFF_FLAG) { > 744 __le32 tmp32 = cpu_to_le32(cf->can_id& CAN_ERR_MASK); > 745 > 746 tmp32<<= 3; > 747 *pc |= PCAN_USB_STATUSLEN_EXT_ID; > 748 memcpy(++pc,&tmp32, 4); > 749 pc += 4; > 750 } else { > 751 __le16 tmp16 = cpu_to_le32(cf->can_id& CAN_ERR_MASK); > ^^^^^^^^^^^^ > A little endian 32 bit can't fit here. How may this error happen while it didn't before? Is this is due to some (new) options of gcc, or because of a new version used? In the future, how could I avoid that? I mean, how to synchronize with the final GCC options/version? FYI, the original code was: > > + else { > } else { > > + u16 tmp16 = (u16 )*cpu_to_le32*(cf->can_id& CAN_ERR_MASK); > ^^^^^^^^^^^^^^^^^ cf->can_id is a 32-bits field which I want to keep only the lo-word part... How to do that in the correct way, please? Anyway, what next? I suppose I have to submit a new patch, but comparing to what? Thanks for your help and regards, Stéphane -- PEAK-System Technik GmbH, Otto-Roehm-Strasse 69, D-64293 Darmstadt Geschaeftsleitung: A.Gach/U.Wilhelm,St.Nr.:007/241/13586 FA Darmstadt HRB-9183 Darmstadt, Ust.IdNr.:DE 202220078, WEE-Reg.-Nr.: DE39305391 Tel.+49 (0)6151-817320 / Fax:+49 (0)6151-817329, info@peak-system.com ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: can: usb: PEAK-System Technik PCAN-USB specific part 2012-03-07 8:56 ` Stephane Grosjean @ 2012-03-07 9:24 ` Marc Kleine-Budde 2012-03-07 11:16 ` Stephane Grosjean 0 siblings, 1 reply; 14+ messages in thread From: Marc Kleine-Budde @ 2012-03-07 9:24 UTC (permalink / raw) To: Stephane Grosjean; +Cc: linux-can@vger.kernel.org [-- Attachment #1: Type: text/plain, Size: 2391 bytes --] On 03/07/2012 09:56 AM, Stephane Grosjean wrote: > Happy to have seen some of you last Monday, at iCC, it was great. It's > always easier when we're able to attach a face to a name. Yes, I always like that occasions. > Back to work now: > > Le 06/03/2012 12:21, Dan Carpenter a écrit : >> Hello Stephane Grosjean, >> >> The patch 46be265d3388: "can: usb: PEAK-System Technik PCAN-USB >> specific part" from Mar 2, 2012, leads to the following warning: >> >> drivers/net/can/usb/peak_usb/pcan_usb.c:751 pcan_usb_encode_msg() >> error: wrong number of bits for 'cpu_to_le32' (16 vs 32) >> >> drivers/net/can/usb/peak_usb/pcan_usb.c >> 742 /* can id */ >> 743 if (cf->can_id& CAN_EFF_FLAG) { >> 744 __le32 tmp32 = cpu_to_le32(cf->can_id& >> CAN_ERR_MASK); >> 745 >> 746 tmp32<<= 3; >> 747 *pc |= PCAN_USB_STATUSLEN_EXT_ID; >> 748 memcpy(++pc,&tmp32, 4); >> 749 pc += 4; >> 750 } else { >> 751 __le16 tmp16 = cpu_to_le32(cf->can_id& >> CAN_ERR_MASK); >> ^^^^^^^^^^^^ >> A little endian 32 bit can't fit here. > > > How may this error happen while it didn't before? Is this is due to some > (new) options of gcc, or because of a new version used? In the future, > how could I avoid that? I mean, how to synchronize with the final GCC > options/version? There's no final gcc version. Gcc and the kernel will always evolve. > > FYI, the original code was: > >> > + else { >> } else { >> > + u16 tmp16 = (u16 )*cpu_to_le32*(cf->can_id& CAN_ERR_MASK); >> ^^^^^^^^^^^^^^^^^ > > cf->can_id is a 32-bits field which I want to keep only the lo-word part... > > How to do that in the correct way, please? > > Anyway, what next? I suppose I have to submit a new patch, but comparing > to what? Please post your patch against the can-next git repo. https://gitorious.org/linux-can/linux-can-next 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: 262 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: can: usb: PEAK-System Technik PCAN-USB specific part 2012-03-07 9:24 ` Marc Kleine-Budde @ 2012-03-07 11:16 ` Stephane Grosjean 0 siblings, 0 replies; 14+ messages in thread From: Stephane Grosjean @ 2012-03-07 11:16 UTC (permalink / raw) To: Marc Kleine-Budde; +Cc: linux-can@vger.kernel.org Le 07/03/2012 10:24, Marc Kleine-Budde a écrit : > On 03/07/2012 09:56 AM, Stephane Grosjean wrote: > Le 06/03/2012 12:21, Dan Carpenter a écrit : >>> Hello Stephane Grosjean, >>> >>> The patch 46be265d3388: "can: usb: PEAK-System Technik PCAN-USB >>> specific part" from Mar 2, 2012, leads to the following warning: >>> >>> drivers/net/can/usb/peak_usb/pcan_usb.c:751 pcan_usb_encode_msg() >>> error: wrong number of bits for 'cpu_to_le32' (16 vs 32) >>> >>> drivers/net/can/usb/peak_usb/pcan_usb.c >>> 742 /* can id */ >>> 743 if (cf->can_id& CAN_EFF_FLAG) { >>> 744 __le32 tmp32 = cpu_to_le32(cf->can_id& >>> CAN_ERR_MASK); >>> 745 >>> 746 tmp32<<= 3; >>> 747 *pc |= PCAN_USB_STATUSLEN_EXT_ID; >>> 748 memcpy(++pc,&tmp32, 4); >>> 749 pc += 4; >>> 750 } else { >>> 751 __le16 tmp16 = cpu_to_le32(cf->can_id& >>> CAN_ERR_MASK); >>> ^^^^^^^^^^^^ >>> A little endian 32 bit can't fit here. >> >> How may this error happen while it didn't before? Is this is due to some >> (new) options of gcc, or because of a new version used? In the future, >> how could I avoid that? I mean, how to synchronize with the final GCC >> options/version? > There's no final gcc version. Gcc and the kernel will always evolve. Yes I know, this is Ok... But the question is: how did that guy manage to get that error? I "git pull"ed my own linux-can-next then rebuilt my drivers and don't even see that error! I suppose that he set some flags somewhere in the Kernel Makefile(s), but where? How can I know whether my next patch will fail or not in his configuration? Thanks for your reply, Stéphane -- PEAK-System Technik GmbH, Otto-Roehm-Strasse 69, D-64293 Darmstadt Geschaeftsleitung: A.Gach/U.Wilhelm,St.Nr.:007/241/13586 FA Darmstadt HRB-9183 Darmstadt, Ust.IdNr.:DE 202220078, WEE-Reg.-Nr.: DE39305391 Tel.+49 (0)6151-817320 / Fax:+49 (0)6151-817329, info@peak-system.com ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: can: usb: PEAK-System Technik PCAN-USB specific part 2012-03-06 11:21 can: usb: PEAK-System Technik PCAN-USB specific part Dan Carpenter 2012-03-07 8:56 ` Stephane Grosjean @ 2012-03-07 9:05 ` Stephane Grosjean 2012-03-07 12:21 ` Dan Carpenter 2 siblings, 0 replies; 14+ messages in thread From: Stephane Grosjean @ 2012-03-07 9:05 UTC (permalink / raw) To: linux-can@vger.kernel.org Same but without the stars around "cpu_to_le32()"... (Sorry for that wrong copy/paste) > FYI, the original code was: > >> > + else { >> } else { >> > + u16 tmp16 = (u16 )cpu_to_le32(cf->can_id & CAN_ERR_MASK); >> ^^^^^^^^^^^^^^^^^ Regards, Stéphane -- PEAK-System Technik GmbH, Otto-Roehm-Strasse 69, D-64293 Darmstadt Geschaeftsleitung: A.Gach/U.Wilhelm,St.Nr.:007/241/13586 FA Darmstadt HRB-9183 Darmstadt, Ust.IdNr.:DE 202220078, WEE-Reg.-Nr.: DE39305391 Tel.+49 (0)6151-817320 / Fax:+49 (0)6151-817329, info@peak-system.com ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: can: usb: PEAK-System Technik PCAN-USB specific part 2012-03-06 11:21 can: usb: PEAK-System Technik PCAN-USB specific part Dan Carpenter 2012-03-07 8:56 ` Stephane Grosjean 2012-03-07 9:05 ` Stephane Grosjean @ 2012-03-07 12:21 ` Dan Carpenter 2012-03-07 12:29 ` Marc Kleine-Budde 2 siblings, 1 reply; 14+ messages in thread From: Dan Carpenter @ 2012-03-07 12:21 UTC (permalink / raw) To: s.grosjean; +Cc: linux-can [-- Attachment #1: Type: text/plain, Size: 872 bytes --] On Tue, Mar 06, 2012 at 02:21:08PM +0300, Dan Carpenter wrote: > Hello Stephane Grosjean, > > The patch 46be265d3388: "can: usb: PEAK-System Technik PCAN-USB > specific part" from Mar 2, 2012, leads to the following warning: > > drivers/net/can/usb/peak_usb/pcan_usb.c:751 pcan_usb_encode_msg() > error: wrong number of bits for 'cpu_to_le32' (16 vs 32) > > drivers/net/can/usb/peak_usb/pcan_usb.c > 742 /* can id */ > 743 if (cf->can_id & CAN_EFF_FLAG) { > 744 __le32 tmp32 = cpu_to_le32(cf->can_id & CAN_ERR_MASK); > 745 > 746 tmp32 <<= 3; Also we can't be doing this shift on little endian data. > 747 *pc |= PCAN_USB_STATUSLEN_EXT_ID; > 748 memcpy(++pc, &tmp32, 4); > 749 pc += 4; regards, dan carpenter [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: can: usb: PEAK-System Technik PCAN-USB specific part 2012-03-07 12:21 ` Dan Carpenter @ 2012-03-07 12:29 ` Marc Kleine-Budde 2012-03-07 13:17 ` Dan Carpenter 0 siblings, 1 reply; 14+ messages in thread From: Marc Kleine-Budde @ 2012-03-07 12:29 UTC (permalink / raw) To: Dan Carpenter; +Cc: s.grosjean, linux-can [-- Attachment #1: Type: text/plain, Size: 1264 bytes --] On 03/07/2012 01:21 PM, Dan Carpenter wrote: > On Tue, Mar 06, 2012 at 02:21:08PM +0300, Dan Carpenter wrote: >> Hello Stephane Grosjean, >> >> The patch 46be265d3388: "can: usb: PEAK-System Technik PCAN-USB >> specific part" from Mar 2, 2012, leads to the following warning: >> >> drivers/net/can/usb/peak_usb/pcan_usb.c:751 pcan_usb_encode_msg() >> error: wrong number of bits for 'cpu_to_le32' (16 vs 32) >> >> drivers/net/can/usb/peak_usb/pcan_usb.c >> 742 /* can id */ >> 743 if (cf->can_id & CAN_EFF_FLAG) { >> 744 __le32 tmp32 = cpu_to_le32(cf->can_id & CAN_ERR_MASK); >> 745 >> 746 tmp32 <<= 3; > > Also we can't be doing this shift on little endian data. This should be first shift, then convert to little endian, isn't it? >> 747 *pc |= PCAN_USB_STATUSLEN_EXT_ID; >> 748 memcpy(++pc, &tmp32, 4); >> 749 pc += 4; 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: 262 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: can: usb: PEAK-System Technik PCAN-USB specific part 2012-03-07 12:29 ` Marc Kleine-Budde @ 2012-03-07 13:17 ` Dan Carpenter 2012-03-07 13:26 ` Marc Kleine-Budde 0 siblings, 1 reply; 14+ messages in thread From: Dan Carpenter @ 2012-03-07 13:17 UTC (permalink / raw) To: Marc Kleine-Budde; +Cc: s.grosjean, linux-can [-- Attachment #1: Type: text/plain, Size: 1366 bytes --] On Wed, Mar 07, 2012 at 01:29:19PM +0100, Marc Kleine-Budde wrote: > On 03/07/2012 01:21 PM, Dan Carpenter wrote: > > On Tue, Mar 06, 2012 at 02:21:08PM +0300, Dan Carpenter wrote: > >> Hello Stephane Grosjean, > >> > >> The patch 46be265d3388: "can: usb: PEAK-System Technik PCAN-USB > >> specific part" from Mar 2, 2012, leads to the following warning: > >> > >> drivers/net/can/usb/peak_usb/pcan_usb.c:751 pcan_usb_encode_msg() > >> error: wrong number of bits for 'cpu_to_le32' (16 vs 32) > >> > >> drivers/net/can/usb/peak_usb/pcan_usb.c > >> 742 /* can id */ > >> 743 if (cf->can_id & CAN_EFF_FLAG) { > >> 744 __le32 tmp32 = cpu_to_le32(cf->can_id & CAN_ERR_MASK); > >> 745 > >> 746 tmp32 <<= 3; > > > > Also we can't be doing this shift on little endian data. > > This should be first shift, then convert to little endian, isn't it? > That's my guess too. __le32 tmp32 = cpu_to_le32((cf->can_id & CAN_ERR_MASK) << 3); Btw, it's slightly odd to me that CAN_EFF_MASK and CAN_ERR_MASK are the same. I wouldn't comment on it except that I was wondering if it might be a copy and paste bug. #define CAN_EFF_MASK 0x1FFFFFFFU /* extended frame format (EFF) */ #define CAN_ERR_MASK 0x1FFFFFFFU /* omit EFF, RTR, ERR flags */ regards, dan carpenter [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: can: usb: PEAK-System Technik PCAN-USB specific part 2012-03-07 13:17 ` Dan Carpenter @ 2012-03-07 13:26 ` Marc Kleine-Budde 2012-03-07 13:36 ` Dan Carpenter 2012-03-07 14:05 ` Stephane Grosjean 0 siblings, 2 replies; 14+ messages in thread From: Marc Kleine-Budde @ 2012-03-07 13:26 UTC (permalink / raw) To: Dan Carpenter; +Cc: s.grosjean, linux-can [-- Attachment #1: Type: text/plain, Size: 1920 bytes --] On 03/07/2012 02:17 PM, Dan Carpenter wrote: > On Wed, Mar 07, 2012 at 01:29:19PM +0100, Marc Kleine-Budde wrote: >> On 03/07/2012 01:21 PM, Dan Carpenter wrote: >>> On Tue, Mar 06, 2012 at 02:21:08PM +0300, Dan Carpenter wrote: >>>> Hello Stephane Grosjean, >>>> >>>> The patch 46be265d3388: "can: usb: PEAK-System Technik PCAN-USB >>>> specific part" from Mar 2, 2012, leads to the following warning: >>>> >>>> drivers/net/can/usb/peak_usb/pcan_usb.c:751 pcan_usb_encode_msg() >>>> error: wrong number of bits for 'cpu_to_le32' (16 vs 32) >>>> >>>> drivers/net/can/usb/peak_usb/pcan_usb.c >>>> 742 /* can id */ >>>> 743 if (cf->can_id & CAN_EFF_FLAG) { >>>> 744 __le32 tmp32 = cpu_to_le32(cf->can_id & CAN_ERR_MASK); >>>> 745 >>>> 746 tmp32 <<= 3; >>> >>> Also we can't be doing this shift on little endian data. >> >> This should be first shift, then convert to little endian, isn't it? >> > > That's my guess too. > __le32 tmp32 = cpu_to_le32((cf->can_id & CAN_ERR_MASK) << 3); Stéphane, do you want to make that patch? Add my Acked-by and a "Reported-by: Dan Carpenter <dan.carpenter@oracle.com>" > Btw, it's slightly odd to me that CAN_EFF_MASK and CAN_ERR_MASK are > the same. I wouldn't comment on it except that I was wondering if > it might be a copy and paste bug. You're right, it odd, but it's intended to be the same. Do you want to make a patch to add an annotation? > #define CAN_EFF_MASK 0x1FFFFFFFU /* extended frame format (EFF) */ > #define CAN_ERR_MASK 0x1FFFFFFFU /* omit EFF, RTR, ERR flags */ 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: 262 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: can: usb: PEAK-System Technik PCAN-USB specific part 2012-03-07 13:26 ` Marc Kleine-Budde @ 2012-03-07 13:36 ` Dan Carpenter 2012-03-07 13:43 ` Marc Kleine-Budde 2012-03-07 14:05 ` Stephane Grosjean 1 sibling, 1 reply; 14+ messages in thread From: Dan Carpenter @ 2012-03-07 13:36 UTC (permalink / raw) To: Marc Kleine-Budde; +Cc: s.grosjean, linux-can [-- Attachment #1: Type: text/plain, Size: 350 bytes --] On Wed, Mar 07, 2012 at 02:26:10PM +0100, Marc Kleine-Budde wrote: > You're right, it odd, but it's intended to be the same. Do you want to > make a patch to add an annotation? > No. I was just curious. I should have noticed how old the definitions were and realized that they were well tested at this point. regards, dan carpenter [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: can: usb: PEAK-System Technik PCAN-USB specific part 2012-03-07 13:36 ` Dan Carpenter @ 2012-03-07 13:43 ` Marc Kleine-Budde 0 siblings, 0 replies; 14+ messages in thread From: Marc Kleine-Budde @ 2012-03-07 13:43 UTC (permalink / raw) To: Dan Carpenter; +Cc: s.grosjean, linux-can [-- Attachment #1: Type: text/plain, Size: 743 bytes --] On 03/07/2012 02:36 PM, Dan Carpenter wrote: > On Wed, Mar 07, 2012 at 02:26:10PM +0100, Marc Kleine-Budde wrote: >> You're right, it odd, but it's intended to be the same. Do you want to >> make a patch to add an annotation? >> > > No. I was just curious. I should have noticed how old the > definitions were and realized that they were well tested at this > point. It's always good to ask, old code doesn't automatically mean it's tested :) 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: 262 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: can: usb: PEAK-System Technik PCAN-USB specific part 2012-03-07 13:26 ` Marc Kleine-Budde 2012-03-07 13:36 ` Dan Carpenter @ 2012-03-07 14:05 ` Stephane Grosjean 2012-03-07 14:07 ` Marc Kleine-Budde 1 sibling, 1 reply; 14+ messages in thread From: Stephane Grosjean @ 2012-03-07 14:05 UTC (permalink / raw) To: Marc Kleine-Budde; +Cc: Dan Carpenter, linux-can Le 07/03/2012 14:26, Marc Kleine-Budde a écrit : > On 03/07/2012 02:17 PM, Dan Carpenter wrote: >> On Wed, Mar 07, 2012 at 01:29:19PM +0100, Marc Kleine-Budde wrote: >>> On 03/07/2012 01:21 PM, Dan Carpenter wrote: >>>> On Tue, Mar 06, 2012 at 02:21:08PM +0300, Dan Carpenter wrote: >>>>> Hello Stephane Grosjean, >>>>> >>>>> The patch 46be265d3388: "can: usb: PEAK-System Technik PCAN-USB >>>>> specific part" from Mar 2, 2012, leads to the following warning: >>>>> >>>>> drivers/net/can/usb/peak_usb/pcan_usb.c:751 pcan_usb_encode_msg() >>>>> error: wrong number of bits for 'cpu_to_le32' (16 vs 32) >>>>> >>>>> drivers/net/can/usb/peak_usb/pcan_usb.c >>>>> 742 /* can id */ >>>>> 743 if (cf->can_id& CAN_EFF_FLAG) { >>>>> 744 __le32 tmp32 = cpu_to_le32(cf->can_id& CAN_ERR_MASK); >>>>> 745 >>>>> 746 tmp32<<= 3; >>>> Also we can't be doing this shift on little endian data. >>> This should be first shift, then convert to little endian, isn't it? >>> >> That's my guess too. >> __le32 tmp32 = cpu_to_le32((cf->can_id& CAN_ERR_MASK)<< 3); > Stéphane, do you want to make that patch? > Add my Acked-by and a > "Reported-by: Dan Carpenter<dan.carpenter@oracle.com>" Ok I'll do it as soon as I did small tests to check. FYI, the patch will also contain the same fix but for the tmp16 local, declared into the "else" block: __le16 tmp16 = cpu_to_le16((cf->can_id & CAN_ERR_MASK) << 5); Regards, Stéphane -- PEAK-System Technik GmbH, Otto-Roehm-Strasse 69, D-64293 Darmstadt Geschaeftsleitung: A.Gach/U.Wilhelm,St.Nr.:007/241/13586 FA Darmstadt HRB-9183 Darmstadt, Ust.IdNr.:DE 202220078, WEE-Reg.-Nr.: DE39305391 Tel.+49 (0)6151-817320 / Fax:+49 (0)6151-817329, info@peak-system.com ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: can: usb: PEAK-System Technik PCAN-USB specific part 2012-03-07 14:05 ` Stephane Grosjean @ 2012-03-07 14:07 ` Marc Kleine-Budde 2012-03-07 14:13 ` Stephane Grosjean 0 siblings, 1 reply; 14+ messages in thread From: Marc Kleine-Budde @ 2012-03-07 14:07 UTC (permalink / raw) To: Stephane Grosjean; +Cc: Dan Carpenter, linux-can [-- Attachment #1: Type: text/plain, Size: 918 bytes --] On 03/07/2012 03:05 PM, Stephane Grosjean wrote: >>> That's my guess too. >>> __le32 tmp32 = cpu_to_le32((cf->can_id& CAN_ERR_MASK)<< 3); >> Stéphane, do you want to make that patch? >> Add my Acked-by and a >> "Reported-by: Dan Carpenter<dan.carpenter@oracle.com>" > > Ok I'll do it as soon as I did small tests to check. Have you got a big endian machine (powerpc or an ARM IXP in Big Endian Mode) somewhere? > FYI, the patch will also contain the same fix but for the tmp16 local, > declared into the "else" block: > > __le16 tmp16 = cpu_to_le16((cf->can_id & CAN_ERR_MASK) << 5); good point, 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: 262 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: can: usb: PEAK-System Technik PCAN-USB specific part 2012-03-07 14:07 ` Marc Kleine-Budde @ 2012-03-07 14:13 ` Stephane Grosjean 0 siblings, 0 replies; 14+ messages in thread From: Stephane Grosjean @ 2012-03-07 14:13 UTC (permalink / raw) To: Marc Kleine-Budde; +Cc: Dan Carpenter, linux-can Le 07/03/2012 15:07, Marc Kleine-Budde a écrit : > On 03/07/2012 03:05 PM, Stephane Grosjean wrote: >>>> That's my guess too. >>>> __le32 tmp32 = cpu_to_le32((cf->can_id& CAN_ERR_MASK)<< 3); >>> Stéphane, do you want to make that patch? >>> Add my Acked-by and a >>> "Reported-by: Dan Carpenter<dan.carpenter@oracle.com>" >> Ok I'll do it as soon as I did small tests to check. > Have you got a big endian machine (powerpc or an ARM IXP in Big Endian > Mode) somewhere? No sorry, I only have little-endian machines here (x86 and ARMle). I just talked about small non-regression tests. Regards, Stéphane -- PEAK-System Technik GmbH, Otto-Roehm-Strasse 69, D-64293 Darmstadt Geschaeftsleitung: A.Gach/U.Wilhelm,St.Nr.:007/241/13586 FA Darmstadt HRB-9183 Darmstadt, Ust.IdNr.:DE 202220078, WEE-Reg.-Nr.: DE39305391 Tel.+49 (0)6151-817320 / Fax:+49 (0)6151-817329, info@peak-system.com ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2012-03-07 14:13 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-03-06 11:21 can: usb: PEAK-System Technik PCAN-USB specific part Dan Carpenter 2012-03-07 8:56 ` Stephane Grosjean 2012-03-07 9:24 ` Marc Kleine-Budde 2012-03-07 11:16 ` Stephane Grosjean 2012-03-07 9:05 ` Stephane Grosjean 2012-03-07 12:21 ` Dan Carpenter 2012-03-07 12:29 ` Marc Kleine-Budde 2012-03-07 13:17 ` Dan Carpenter 2012-03-07 13:26 ` Marc Kleine-Budde 2012-03-07 13:36 ` Dan Carpenter 2012-03-07 13:43 ` Marc Kleine-Budde 2012-03-07 14:05 ` Stephane Grosjean 2012-03-07 14:07 ` Marc Kleine-Budde 2012-03-07 14:13 ` Stephane Grosjean
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox