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

Hi Oliver,

Thanks for the comments.

On 08.03.2016 09:57, Ramesh Shanmugasundaram wrote:

quoted

As you mentioned further down, when a user does this

"ip link set can0 up type can bitrate 1000000"

the intention is to put the controller in CAN 2.0 mode. Even if we use a

status flag or copy the data bitrate equal to the nominal bitrate, what
would it achieve? It still cannot be a CAN 2.0 node - it is a CAN FD node
configured with same nominal & data bitrate.

quoted

This is why I have this check in ndo_open, so that the user is aware it

is a CAN FD node always and avoid misconfiguration like above with
EOPNOTSUPP.

quoted

ok - got it now.

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. 

Thanks,
Ramesh