Re: can: usb: PEAK-System Technik PCAN-USB specific part

[Stephane Grosjean <s.grosjean@peak-system.com>]

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