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