Re: [PATCH v2] can: rcar_canfd: Add Renesas R-Car CAN FD driver

Oliver Hartkopp
2016-03-08T17:16:48Z

On 03/08/2016 01:48 PM, Ramesh Shanmugasundaram wrote:

>> In fact you provided a CAN driver which is "CAN-FD-only".
>
> Yes. That's the status of current submission.
>
>> We did not had that before but there's a solution for this kind of setup.
>>
>> There is a similar case with CAN_CTRLMODE_FD_NON_ISO we had on the M_CAN
>> driver which only provides non-ISO configuration for the supported IP core
>> and _no_ option to _change_ this value:
>>
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id
>> =6cfda7fbebe8a4fd33ea5722fa0212f98f643c35
>>
>> If you would do it similar in rcar_canfd.c with
>>
>> /* CAN_CTRLMODE_FD is fixed with R-Car CAN FD */
>> priv->can.ctrlmode = CAN_CTRLMODE_FD;
>>
>> and remove CAN_CTRLMODE_FD from the priv->can.ctrlmode_supported
>> assignment then it should do the entire configuration process correctly
>> for you.
>> Including the proper tests for the two bitrates.
>> (see open_candev() in linux/drivers/net/can/dev.c)
>>
>> Right?
>
> I did try this option earlier but there are two problems with this method.
>
> 1) Below configuration is not possible
>
> ip link set can0 up type can bitrate 1000000 dbitrate 1000000 fd on
>
> "fd on" -> This is not allowed because CAN_CTRLMODE_FD bit is not set in ctrlmode_supported.
>
> 2) If I ignore "fd on", my interface MTU stays as CAN_MTU only. If I have to change the MTU alone to CANFD_MTU using another netlink message, it again checks ctrlmode_supported where it would fail. I have the option of providing my own change_mtu function & ignore this check but two configuration messages are required for my driver alone :-(.
>
> Both these anomalies are addressed with the current check I have.

Oh - you are right with complaining about this inconsistency.

Can you check my RFC patch for Linux stable I just sent on the mailing list?
http://marc.info/?l=linux-can&m=145745724917976&w=2

Many thanks,
Oliver