Re: [PATCH RFC] can fd: Add separate bittiming infrastructure

[Oliver Hartkopp <socketcan@hartkopp.net>]

On 21.01.2014 09:42, Stephane Grosjean wrote:
> Le 18/01/2014 18:34, Oliver Hartkopp a écrit :

>> 1. Is it a good idea to provide CANFD8 frames to legacy applications?
> 
> Well, at first, I didn't very agree with this idea ... But I'm not able to
> find an example against, so ...

Ok. Me too :-)

I'll better create a new thread for this compatibility question ...

>> My idea behind CAN_CTRLMODE_FD is to enable/disable the CAN FD mode in the
>> CAN FD capable controller chip.
>>
>> Depending on this CAN_CTRLMODE_FD the CAN FD functionality is enabled when
>> this feature is supported(!) by the driver, see priv->ctrlmode_supported:
> 
> Ok I agree with the usage of this new flag in "ctrmode_supported".
> 
>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/net/can/dev.c#n669
>>
>>
>> Depending on CAN_CTRLMODE_FD the MTU is then set to CAN_MTU or CANFD_MTU
> 
> So? Do you mean that the *CAN core* will set the MTU to CAN_MTU or CANFD_MTU
> according to "priv->ctrlmode_supported & CAN_CTRLMODE_FD" ?
> (Atm, the MTU is changed from user space only).

Not really. Not the CAN core but the driver does so.

What you see is, that the MTU for the VCAN is set at configuration(!) time.
But this is an exception to me, as the VCAN does not use the stuff from dev.c
as it has no bitrate setting, etc.

Therefore the MTU for the VCAN is set by root - and not by any application.

Usually AFAIK the MTU is not set by root but from the driver side.

Here: Setting CAN_CTRLMODE_FD at configuration time leads to CAN_MTU or
CANFD_MTU set by the driver when the interface is set to 'up'.

The mode of the driver can be checked by *reading* dev->mtu then.

> 
>> (note that the interface has to be 'down' when configuring this feature -
>> that's just the same as with configuring the bitrate).
>>
>> When CAN_CTRLMODE_FD was set but the second bitrate was *not* set before the
>> interface is requested to be set to 'up' this leads to the same error as if
>> the (single and only) bitrate is missing today.
> 
> So, this means that the arbitration bitrate could not be the default data
> bitrate, doesn't it?
> 

You think about copying the arbitration bitrate into the data bitrate when the
data bitrate was not set (with CAN_CTRLMODE_FD), right?

Yes, this could be an idea too.

The question is if this convenience functionality is confusing people or not.
IMO your idea is nice.

When you set a different bitrate this will be used. If not, the arbitration
bitrate is used for data bitrate too. No objections.

Regards,
Oliver