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

[Dan Carpenter <dan.carpenter@oracle.com>]

[-- 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 --]