Re: [PATCH v2] usb_8dev: Add support for USB2CAN interface from 8 devices

[Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 1998 bytes --]

On 12/03/2012 09:42 PM, krumboeck-Hi41barv6paIERSsAYjmKA@public.gmane.org wrote:
> Hi Marc!
> 
> 
>>> +/* Send open command to device */
>>> +static int usb_8dev_cmd_open(struct usb_8dev *dev)
>>> +{
>>> +    struct can_bittiming *bt = &dev->can.bittiming;
>>> +    struct usb_8dev_cmd_msg outmsg;
>>> +    struct usb_8dev_cmd_msg inmsg;
>>> +    u32 flags = 0;
>>> +    u32 beflags;
>>> +    u16 bebrp;
>>> +    u32 ctrlmode = dev->can.ctrlmode;
>>> +
>>> +    if (ctrlmode & CAN_CTRLMODE_LOOPBACK)
>>> +        flags |= USB_8DEV_LOOPBACK;
>>> +    if (ctrlmode & CAN_CTRLMODE_LISTENONLY)
>>> +        flags |= USB_8DEV_SILENT;
>>> +    if (ctrlmode & CAN_CTRLMODE_ONE_SHOT)
>>> +        flags |= USB_8DEV_DISABLE_AUTO_RESTRANS;
>>> +
>>> +    flags |= USB_8DEV_STATUS_FRAME;
>>> +
>>> +    memset(&outmsg, 0, sizeof(struct usb_8dev_cmd_msg));
>>> +    outmsg.command = USB_8DEV_OPEN;
>>> +    outmsg.opt1 = USB_8DEV_BAUD_MANUAL;
>>> +    outmsg.data[0] = (bt->prop_seg + bt->phase_seg1);
>>> +    outmsg.data[1] = bt->phase_seg2;
>>> +    outmsg.data[2] = bt->sjw;
>>
>> But you should not use usb_bulk_msg() to send data which is on the
>> stack, please use you already allocated memory in priv or kmalloc
>> something here.
> 
> The function usb_8dev_send_cmd copies the data to dev->cmd_msg_buffer
> (allocated with kzalloc, GFP_KERNEL).

Okay, then you might want to set outmsg via C99 initializers.

>>> +
>>> +    /* BRP */
>>> +    bebrp = cpu_to_be16((u16) bt->brp);
>>> +    memcpy(&outmsg.data[3], &bebrp, sizeof(bebrp));
>>
>> Are you sure about the endianess? Some data types are BE some are LE?
> 
> Where is LE used?

Opps, yeah right. only BE.


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: 261 bytes --]