Subject: Re: [patch 5/7] CAN: Add virtual CAN netdevice driver

From: Oliver Hartkopp <[email-masked]>
Date: Wed, 30 May 2007 20:57:33 +0200

Stephen Hemminger wrote:
> In addition to Patrick's comments.
>
>   
>> +
>> +static void vcan_init(struct net_device *dev)
>> +{
>> +	DBG("dev %s\n", dev->name);
>> +
>> +	ether_setup(dev);
>>     
>
> Do really want to do this?
>   - you set different flags/type after this.
>   - do you really want change_mtu to call eth_change_mtu
> Better off to just copy what you want out of ether_setup if
> your device is not really an ether device.
>   

Yep! That's reasonable. Thanks.

>> +
>> +	dev->type              = ARPHRD_CAN;
>> +	dev->mtu               = sizeof(struct can_frame);
>> +	dev->flags             = IFF_NOARP;
>> +
>> +	/* set flags according to driver capabilities */
>> +	if (loopback)
>> +		dev->flags |= IFF_LOOPBACK;
>> +
>> +	dev->open              = vcan_open;
>> +	dev->stop              = vcan_stop;
>> +	dev->set_config        = NULL;
>>     
> unneeded
>
>   
>> +	dev->hard_start_xmit   = vcan_tx;
>> +	dev->do_ioctl          = vcan_ioctl;
>> +	dev->get_stats         = vcan_get_stats;
>> +	dev->hard_header       = vcan_header;
>> +	dev->rebuild_header    = vcan_rebuild_header;
>> +	dev->hard_header_cache = NULL;
>>     
> unneeded
>
>   

Does 'unneeded' belong to always the last line of the quoted code snippet?

Regards,
Oliver