Re: [PATCH] Add PEAK System USB adapters core driver

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

Hi Sebastian,

Le 22/12/2011 22:41, Sebastian Haas a écrit :
> Hi again,
>
> some nitpicking this time. ;-)
>
... I certainly agree with you! But it's good news anyway.

Generally speaking:

- ok for the two missing ending \n
- ok for strncpy instead of strcpy()
- but what about the followings?

> +    struct can_bittiming *bt =&dev->can.bittiming;
                                  ^^^ remove whitespace
...

> +    if (sizeof_candev<  sizeof(struct peak_usb_device))
                         ^^^^^^ cleanup

...

> +    netdev->netdev_ops =&peak_usb_netdev_ops;
                            ^^^^ whitespace
...

>> +
>> +    SET_NETDEV_DEV(netdev,&intf->dev);
>                              ^^^
>> +
>> +        dev->state&= ~PCAN_USB_STATE_CONNECTED;
>                          ^^^^
>> +            if (*pc<  32 || *pc>  127)
>                               ^^^^         ^^
It looks like missing whitespaces sometimes, and sometimes there were 
too in your text... which is different from the patch I sent yesterday. 
Or do I misunderstand your comments?

>> +
>> +    /* Last chance do send some synchronous commands here */
>> +    err = driver_for_each_device(&peak_usb_driver.drvwrap.driver, NULL,
>> +        NULL, peak_usb_do_device_exit);
>> +    if (err)
>> +        err = 0;
> \err\ is neither checked nor printed, why not removing it?
>

Because of that (see <linux/device.h>):

extern int __must_check driver_for_each_device(struct device_driver *drv,
            ~~~~~~~~~~~~

... Is there any other way to get around that?

> You are mixing pr_*, dev_* and netdev_* please check if it is possible 
> to harmonize the usage.
>
Ok I'll try do that, whenever it is possible.

> Cheers,
>  Sebastian +

Thanks and regards,

Stéphane

> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-can" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html