On 09/09/2009 03:39 PM, Oliver Hartkopp wrote:
> Wolfgang Grandegger wrote:
>> Hi Sebastian,
>>
>> you just fix the lines I pointed to. There are more similar issues.
>> For consitancy reasons, could you please do a global:
>>
>> s/canstate/can_state/g
>> s/canmsg/canmsg/g
>
> What's the difference here? :-)
Don't know ;-).
>>> +
>>> +/* Representation of the CAN parameters for the M16C controller
>>> + * in basic CAN mode (means no full CAN)
>>> + */
>>> +struct cpc_m16c_basic_params {
>>> + u8 con0;
>
> (..)
>
>>> +/* Table of devices that work with this driver
>>> + * NOTE: This driver supports only CPC-USB/ARM7 (LPC2119) yet.
>>> + */
>>> +static struct usb_device_id ems_usb_table[] = {
>>> + /*{USB_DEVICE(USB_CPCUSB_VENDOR_ID, USB_CPCUSB_M16C_PRODUCT_ID)},*/
>>
>> Please remove.
>>
>
> I assume the M16C support will be added before upstream.
>
> So to me this is a step in between and we do not need to remove all the stuff
> that will get in before mainlining anyway.
Agreed, I don't care much for the SVN commit. Sebastian, feel free to
keep the preliminary M16C stuff.
>>> +static void ems_usb_read_bulk_callback(struct urb *urb)
>>> +{
>>> + struct ems_usb *dev = urb->context;
>>> + struct net_device *netdev;
>>> +
>>> + int retval;
>>
>> Remove empty line above, please.
>> s/retval/err ?
>>
>
> s/retval/ret/ ?
>
> I think this is ok.
It's not that I just dislike the name. My point here is: If ret != 0
means an error condition, then err should be used instead for the sake
of readability:
if (err)
return err;
If ret != 0 might also indicate success, like the return value of
read(), then err would not be appropriate. That's why I put a "?".
Please check.
> Btw. This function is a *void* function, right?
> This needs to be checked :-)
I think the compiler would complain.
Wolfgang.
_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core
