Re: [PATCH 1/2] can: flexcan: Remodel FlexCAN register r/w APIs for big endian FlexCAN controllers. - Marc Kleine-Budde

From: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
To: Pankaj Bansal <pankaj.bansal-3arQi8VN3Tc@public.gmane.org>,
	"wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org"
	<wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>,
	"linux-can-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-can-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Cc: Varun Sethi <V.Sethi-3arQi8VN3Tc@public.gmane.org>,
	Poonam Aggrwal <poonam.aggrwal-3arQi8VN3Tc@public.gmane.org>,
	"devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Subject: Re: [PATCH 1/2] can: flexcan: Remodel FlexCAN register r/w APIs for big endian FlexCAN controllers.
Date: Fri, 10 Nov 2017 13:49:03 +0100	[thread overview]
Message-ID: <b27d6e14-c6d7-5c4b-0588-093684cbe57b@pengutronix.de> (raw)
In-Reply-To: <AM0PR0402MB394051B0FAADBC45AF71439CF1540-mYCQpYF9suc3mfjNbz3WnI3W/0Ik+aLCnBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org>

[-- Attachment #1.1: Type: text/plain, Size: 2326 bytes --]

On 11/10/2017 01:35 PM, Pankaj Bansal wrote:
>>> 3. Regarding backward compatibility with PowerPC, I see that there is only one platform in PowerPC architecture that is using flexcan.
>>>     There is only one platform in "arch/powerpc/boot/dts/" having flexcan node. p1010si-post.dtsi
>>>     I have added the big-endian property in that platform and have tested it on P1010 platform after backporting the patch to Freescale SDK 1.4 linux.
>>>     I have sent the patch for this. See "[PATCH 2/3] powerpc: dts: P1010: Add endianness property to flexcan node"
>>>     I believe these changes can be accepted for both powerpc and arm and other architectures that use flexcan.
>>
>> No, this is not acceptable. You break the device tree. Please add the le, be information to the devtype data.
> 
> I don't understand how this breaks device tree? can you please elaborate? This method is already being used in other specifications.

Boot a new kernel with an old tree on a PPC board -> flexcan will not work.

See this talk for more information for stable device tree ABI:

https://elinux.org/images/0/0e/OSELAS.Presentation-ELCE2017-DT.pdf
https://www.youtube.com/watch?v=6iguKSJJfxo

> You can refer "Documentation/devicetree/bindings/usb/usb-ehci.txt" or
> "Documentation/devicetree/bindings/i2c/i2c-mux-reg.txt" Or
> "Documentation/devicetree/bindings/regmap/regmap.txt".

> In my opinion, keeping this info in devtype data is not good idea.
> E.g. say two platforms which have same FlexCAN hardware revision,
> will have same quirks. BUT these two platforms implement FlexCAN in
> le and be fashion respectively.

Then it's two different platforms. Simply add another compatible to the
driver.

> With current FlexCAN driver, these two platforms can have same
> devtype data. And we need not to change flexcan.c if we want to add
> support for a second platform. We can just add device tree for that
> platform (which would be needed anyway), and it can work. We can
> mention endianness in device tree.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]