Re: [RFC] [PATCH v2] can: fix handling of unmodifiable configuration options

[Oliver Hartkopp <socketcan@hartkopp.net>]

On 03/14/2016 11:15 AM, Ramesh Shanmugasundaram wrote:

>> Oliver Hartkopp <socketcan@hartkopp.net>
>> Subject: [RFC] [PATCH v2] can: fix handling of unmodifiable configuration
>> options
>
> (...)
>
>>
>> +static inline void can_set_static_ctrlmode(struct can_priv *priv,
>> +					   const u32 static_mode)
>> +{
>> +	priv->ctrlmode = static_mode;
>> +	priv->ctrlmode_static = static_mode;
>> +}
>
> Is it worth taking the ndev as arg and change the MTU as well in this function? This way mode & MTU would be consistent and would reflect the same in a configuration like this
>
> "ip link set can0 up type can bitrate 1000000 dbitrate 1000000"
>
> Would this be an overkill?

My first thought was that setting the MTU in this function smells fishy.

But as can_set_static_ctrlmode() is intended to be used at CAN device 
setup time only it makes sense to override the MTU which is set in 
can_setup() to CAN_MTU by default.

It doesn't make sense to pass the MTU to alloc_candev() as setting the 
MTU to CAN_MTU is always to correct default (even for configurable CAN 
FD devices).

I'll sed an updated patch which overrides the MTU in 
can_set_static_ctrlmode() when CAN_CTRLMODE_FD is set in ctrlmode_static.

Regards,
Oliver