Re: incompatible device trees between u-boot and linux

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 11208 bytes --]

On Wed, Aug 25, 2021 at 05:18:16PM +0300, Vladimir Oltean wrote:
> On Wed, Aug 25, 2021 at 10:00:45AM -0400, Tom Rini wrote:
> > On Wed, Aug 25, 2021 at 03:58:10PM +0200, Michael Walle wrote:
> >
> > > Hi,
> > >
> > > I noticed that there is a fallback to the u-boot device tree for linux
> > > (esp. EFI boot) if no other device tree was found, see [1]. It seems this
> > > is working fine for imx devices, for example, where you can just boot a
> > > stock installer iso via EFI. It will just work and it is quite a nice
> > > feature as a fallback.
> > >
> > > Now for the layerscape architecture, the ls1028a in my case, things are
> > > more difficult because the bindings differ between u-boot and linux - one
> > > which comes to mind is DSA and ethernet.
> > >
> > > Which begs the general question, is it encouraged to have both bindings
> > > diverge? To me it seems, that most bindings in u-boot are ad-hoc and there
> > > is no real review or alignment but just added as needed, which is ok if
> > > they are local to u-boot. But since they are nowadays passed to linux
> > > (by default!) I'm not so sure anymore.
> > >
> > > OTOH The whole structure around a .dts{,i} and -u-boot.dtsi looks like
> > > they should (could?) be shared between linux and u-boot.
> > >
> > > -michael
> > >
> > > [1]
> > > https://elixir.bootlin.com/u-boot/v2021.10-rc2/source/common/board_r.c#L471
> >
> > The U-Boot device tree is supposed to be able to be passed on to Linux
> > and Just Work.  The bindings are not supposed to be different between
> > the two (except for when we take the binding while it's being hashed out
> > upstream BUT THEN RESYNCED).
> 
> You might need to spell that out a bit clearer.
> 
> You are saying that both U-Boot and Linux are allowed to have their own
> custom properties (like 'u-boot,dm-spl' for U-Boot, and 'managed = "in-band-status"'
> for Linux), as long as the device tree files themselves are in sync, and
> the subset of the device tree blob understood by Linux (i.e. the U-Boot
> blob sans the U-Boot specifics) is compatible with the Linux DT blob?

I don't know what about the Linux example makes it Linux specific.  But
yes, 'u-boot,dm-spl' is clearly in our namespace and should be ignored
by Linux.  The whole reason we have the -u-boot.dtsi automatic drop-in
logic (as much as it can be used is that device trees are device trees
and describe the hardware and developers don't need to write a device
tree for Linux and a device tree for U-Boot and a device tree for
FreeBSD and ...  So yes, you're supposed to use the device tree for a
platform and it works here and there and every where.

> To expand even further on that, it means we should put 'managed = "in-band-status"'
> in U-Boot, which is a Linux phylink device tree property, even if U-Boot
> does not use phylink?

We should be able to drop in the device trees from Linux and use them.
Custodians should be re-syncing them periodically.  Some are, even.

> > Incompatible device trees / bindings are a
> > bug that needs to be fixed.
> 
> Curious that Michael mentions Ethernet and DSA on LS1028A.
> 
> I decompiled the two:
> 
> dtc -I dtb -O dts < arch/arm/dts/fsl-ls1028a-rdb.dtb > u-boot.dts
> dtc -I dtb -O dts < arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dtb > linux.dts
> 
> and analyzed the Ethernet portion.
> 
> U-Boot looks like this:
> 
> /dts-v1/;
> 
> / {
> 	compatible = "fsl,ls1028a-rdb", "fsl,ls1028a";
> 	interrupt-parent = <0x1>;
> 	#address-cells = <0x2>;
> 	#size-cells = <0x2>;
> 	model = "NXP Layerscape 1028a RDB Board";
> 
> 	pcie@1f0000000 {
> 		compatible = "pci-host-ecam-generic";
> 		bus-range = <0x0 0x0>;
> 		reg = <0x1 0xf0000000 0x0 0x100000>;
> 		#address-cells = <0x3>;
> 		#size-cells = <0x2>;
> 		device_type = "pci";
> 		ranges = <0x82000000 0x0 0x0 0x1 0xf8000000 0x0 0x160000>;
> 
> 		pci@0,0 {
> 			reg = <0x0 0x0 0x0 0x0 0x0>;
> 			status = "okay";
> 			phy-mode = "sgmii";
> 			phy-handle = <0x4>;
> 			phandle = <0x11>;
> 		};
> 
> 		pci@0,1 {
> 			reg = <0x100 0x0 0x0 0x0 0x0>;
> 			status = "disabled";
> 			phandle = <0x12>;
> 		};
> 
> 		pci@0,2 {
> 			reg = <0x200 0x0 0x0 0x0 0x0>;
> 			status = "okay";
> 			phy-mode = "internal";
> 			phandle = <0x9>;
> 
> 			fixed-link {
> 				speed = <0x9c4>;
> 				full-duplex;
> 			};
> 		};
> 
> 		pci@0,3 {
> 			#address-cells = <0x0>;
> 			#size-cells = <0x1>;
> 			reg = <0x300 0x0 0x0 0x0 0x0>;
> 			status = "okay";
> 			phandle = <0x13>;
> 
> 			fixed-link {
> 				speed = <0x3e8>;
> 				full-duplex;
> 			};
> 
> 			phy@2 {
> 				reg = <0x2>;
> 				phandle = <0x4>;
> 			};
> 
> 			phy@10 {
> 				reg = <0x10>;
> 				phandle = <0x5>;
> 			};
> 
> 			phy@11 {
> 				reg = <0x11>;
> 				phandle = <0x6>;
> 			};
> 
> 			phy@12 {
> 				reg = <0x12>;
> 				phandle = <0x7>;
> 			};
> 
> 			phy@13 {
> 				reg = <0x13>;
> 				phandle = <0x8>;
> 			};
> 		};
> 
> 		pci@0,5 {
> 			reg = <0x500 0x0 0x0 0x0 0x0>;
> 			status = "okay";
> 			phandle = <0x14>;
> 
> 			ports {
> 				#address-cells = <0x1>;
> 				#size-cells = <0x0>;
> 
> 				port@0 {
> 					reg = <0x0>;
> 					status = "okay";
> 					label = "swp0";
> 					phy-handle = <0x5>;
> 					phy-mode = "qsgmii";
> 					phandle = <0x15>;
> 				};
> 
> 				port@1 {
> 					reg = <0x1>;
> 					status = "okay";
> 					label = "swp1";
> 					phy-handle = <0x6>;
> 					phy-mode = "qsgmii";
> 					phandle = <0x16>;
> 				};
> 
> 				port@2 {
> 					reg = <0x2>;
> 					status = "okay";
> 					label = "swp2";
> 					phy-handle = <0x7>;
> 					phy-mode = "qsgmii";
> 					phandle = <0x17>;
> 				};
> 
> 				port@3 {
> 					reg = <0x3>;
> 					status = "okay";
> 					label = "swp3";
> 					phy-handle = <0x8>;
> 					phy-mode = "qsgmii";
> 					phandle = <0x18>;
> 				};
> 
> 				port@4 {
> 					reg = <0x4>;
> 					phy-mode = "internal";
> 					status = "okay";
> 					ethernet = <0x9>;
> 					phandle = <0x19>;
> 
> 					fixed-link {
> 						speed = <0x9c4>;
> 						full-duplex;
> 					};
> 				};
> 
> 				port@5 {
> 					reg = <0x5>;
> 					phy-mode = "internal";
> 					status = "disabled";
> 					phandle = <0x1a>;
> 
> 					fixed-link {
> 						speed = <0x3e8>;
> 						full-duplex;
> 					};
> 				};
> 			};
> 		};
> 
> 		pci@0,6 {
> 			reg = <0x600 0x0 0x0 0x0 0x0>;
> 			status = "disabled";
> 			phy-mode = "internal";
> 			phandle = <0x1b>;
> 		};
> 	};
> };
> 
> While Linux looks like this:
> 
> /dts-v1/;
> 
> / {
> 	soc {
> 		compatible = "simple-bus";
> 		#address-cells = <0x2>;
> 		#size-cells = <0x2>;
> 		ranges;
> 
> 		pcie@1f0000000 {
> 			compatible = "pci-host-ecam-generic";
> 			reg = <0x1 0xf0000000 0x0 0x100000>;
> 			#address-cells = <0x3>;
> 			#size-cells = <0x2>;
> 			msi-parent = <0x11>;
> 			device_type = "pci";
> 			bus-range = <0x0 0x0>;
> 			dma-coherent;
> 			msi-map = <0x0 0x11 0x17 0xe>;
> 			iommu-map = <0x0 0x12 0x17 0xe>;
> 			ranges = <0x82000000 0x1 0xf8000000 0x1 0xf8000000 0x0 0x160000 0xc2000000 0x1 0xf8160000 0x1 0xf8160000 0x0 0x70000 0x82000000 0x1 0xf81d0000 0x1 0xf81d0000 0x0 0x20000 0xc2000000 0x1 0xf81f0000 0x1 0xf81f0000 0x0 0x20000 0x82000000 0x1 0xf8210000 0x1 0xf8210000 0x0 0x20000 0xc2000000 0x1 0xf8230000 0x1 0xf8230000 0x0 0x20000 0x82000000 0x1 0xfc000000 0x1 0xfc000000 0x0 0x400000>;
> 
> 			ethernet@0,0 {
> 				compatible = "fsl,enetc";
> 				reg = <0x0 0x0 0x0 0x0 0x0>;
> 				status = "okay";
> 				phy-handle = <0x13>;
> 				phy-connection-type = "sgmii";
> 				managed = "in-band-status";
> 
> 				mdio {
> 					#address-cells = <0x1>;
> 					#size-cells = <0x0>;
> 
> 					ethernet-phy@2 {
> 						reg = <0x2>;
> 						phandle = <0x13>;
> 					};
> 				};
> 			};
> 
> 			ethernet@0,1 {
> 				compatible = "fsl,enetc";
> 				reg = <0x100 0x0 0x0 0x0 0x0>;
> 				status = "disabled";
> 			};
> 
> 			ethernet@0,2 {
> 				compatible = "fsl,enetc";
> 				reg = <0x200 0x0 0x0 0x0 0x0>;
> 				phy-mode = "internal";
> 				status = "okay";
> 				phandle = <0x18>;
> 
> 				fixed-link {
> 					speed = <0x9c4>;
> 					full-duplex;
> 				};
> 			};
> 
> 			mdio@0,3 {
> 				compatible = "fsl,enetc-mdio";
> 				reg = <0x300 0x0 0x0 0x0 0x0>;
> 				#address-cells = <0x1>;
> 				#size-cells = <0x0>;
> 
> 				ethernet-phy@10 {
> 					reg = <0x10>;
> 					phandle = <0x14>;
> 				};
> 
> 				ethernet-phy@11 {
> 					reg = <0x11>;
> 					phandle = <0x15>;
> 				};
> 
> 				ethernet-phy@12 {
> 					reg = <0x12>;
> 					phandle = <0x16>;
> 				};
> 
> 				ethernet-phy@13 {
> 					reg = <0x13>;
> 					phandle = <0x17>;
> 				};
> 			};
> 
> 			ethernet@0,4 {
> 				compatible = "fsl,enetc-ptp";
> 				reg = <0x400 0x0 0x0 0x0 0x0>;
> 				clocks = <0x2 0x2 0x3>;
> 				little-endian;
> 				fsl,extts-fifo;
> 			};
> 
> 			ethernet-switch@0,5 {
> 				reg = <0x500 0x0 0x0 0x0 0x0>;
> 				interrupts = <0x0 0x5f 0x4>;
> 				status = "okay";
> 
> 				ports {
> 					#address-cells = <0x1>;
> 					#size-cells = <0x0>;
> 
> 					port@0 {
> 						reg = <0x0>;
> 						status = "okay";
> 						label = "swp0";
> 						managed = "in-band-status";
> 						phy-handle = <0x14>;
> 						phy-mode = "qsgmii";
> 					};
> 
> 					port@1 {
> 						reg = <0x1>;
> 						status = "okay";
> 						label = "swp1";
> 						managed = "in-band-status";
> 						phy-handle = <0x15>;
> 						phy-mode = "qsgmii";
> 					};
> 
> 					port@2 {
> 						reg = <0x2>;
> 						status = "okay";
> 						label = "swp2";
> 						managed = "in-band-status";
> 						phy-handle = <0x16>;
> 						phy-mode = "qsgmii";
> 					};
> 
> 					port@3 {
> 						reg = <0x3>;
> 						status = "okay";
> 						label = "swp3";
> 						managed = "in-band-status";
> 						phy-handle = <0x17>;
> 						phy-mode = "qsgmii";
> 					};
> 
> 					port@4 {
> 						reg = <0x4>;
> 						phy-mode = "internal";
> 						status = "okay";
> 						ethernet = <0x18>;
> 
> 						fixed-link {
> 							speed = <0x9c4>;
> 							full-duplex;
> 						};
> 					};
> 
> 					port@5 {
> 						reg = <0x5>;
> 						phy-mode = "internal";
> 						status = "disabled";
> 
> 						fixed-link {
> 							speed = <0x3e8>;
> 							full-duplex;
> 						};
> 					};
> 				};
> 			};
> 
> 			ethernet@0,6 {
> 				compatible = "fsl,enetc";
> 				reg = <0x600 0x0 0x0 0x0 0x0>;
> 				phy-mode = "internal";
> 				status = "disabled";
> 
> 				fixed-link {
> 					speed = <0x3e8>;
> 					full-duplex;
> 				};
> 			};
> 
> 			rcec@1f,0 {
> 				reg = <0xf800 0x0 0x0 0x0 0x0>;
> 				interrupts = <0x0 0x5e 0x4>;
> 			};
> 		};
> 	};
> };
> 
> I mean, they look pretty similar to me? The biggest difference is that
> the ENETC ECAM is under the /soc node in Linux, but under / in U-Boot,
> as well as some BAR memory areas that are not marked as prefetchable or
> non-prefetchable in the U-Boot device tree. But excuse me, that isn't an
> Ethernet/DSA problem, is it?

I'll leave this part to Michael.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]