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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <56DEC49D.4@hartkopp.net>
Date:	Tue, 8 Mar 2016 13:25:01 +0100
From:	Oliver Hartkopp <socketcan@...tkopp.net>
To:	Ramesh Shanmugasundaram <ramesh.shanmugasundaram@...renesas.com>,
	Marc Kleine-Budde <mkl@...gutronix.de>,
	"wg@...ndegger.com" <wg@...ndegger.com>,
	"robh+dt@...nel.org" <robh+dt@...nel.org>,
	"pawel.moll@....com" <pawel.moll@....com>,
	"mark.rutland@....com" <mark.rutland@....com>,
	"ijc+devicetree@...lion.org.uk" <ijc+devicetree@...lion.org.uk>,
	"galak@...eaurora.org" <galak@...eaurora.org>,
	"corbet@....net" <corbet@....net>
Cc:	"linux-renesas-soc@...r.kernel.org" 
	<linux-renesas-soc@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-can@...r.kernel.org" <linux-can@...r.kernel.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
	"geert+renesas@...der.be" <geert+renesas@...der.be>,
	Chris Paterson <Chris.Paterson2@...esas.com>
Subject: Re: [PATCH v2] can: rcar_canfd: Add Renesas R-Car CAN FD driver

Hi Ramesh,

On 08.03.2016 09:57, Ramesh Shanmugasundaram wrote:

>
> 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.
>
> 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.
>

ok - got it now.

In fact you provided a CAN driver which is "CAN-FD-only".
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?

Best regards,
Oliver