-----Original Message-----
From: Shawn Guo [mailto:shawnguo at kernel.org]
Sent: Monday, March 27, 2017 10:56 AM
Subject: Re: [PATCH v2 1/2] dts: arm64: add LS1043A DPAA support

On Mon, Mar 27, 2017 at 07:03:40AM +0000, Madalin-Cristian Bucur wrote:

quoted

quoted

quoted

quoted

quoted

+	fman at 1a00000 {
+		enet0: ethernet at e0000 {
+		};
+
+		enet1: ethernet at e2000 {
+		};
+
+		enet2: ethernet at e4000 {
+		};
+
+		enet3: ethernet at e6000 {
+		};
+
+		enet4: ethernet at e8000 {
+		};
+
+		enet5: ethernet at ea000 {
+		};
+
+		enet6: ethernet at f0000 {
+		};
+	};

I do not quite understand why these nodes are empty.

These nodes provide the aliases (and custom SoC mapping) for the
FMan ports that are used on this particular SoC. The particular
node details are found in the port dtsi file thus no information
is required here. Given the fact that the numbering and actual
ports that are in use can vary between SoCs, the aliases cannot
be included in the port dtsi nor in the FMan dtsi.

Do not completely follow.  What do you mean by 'port dtsi file'?

Maybe

quoted

quoted

I should wait for you new patches with better commit log and comments

to

quoted

quoted

understand these odd empty nodes.

The DPAA IP can have a certain number of ports. Out of those, a certain
SoC can use all or only a subset, with diverse decisions on actual

numbering

quoted

of the used ports. Next, when using the SoC on a particular board, some
ports will be used, some will not. The file hierarchy relates to this
hierarchy - you have individual port files that are included by the
SoC dtsi which in turn is included by the board dts. These nodes do not
need any new content as all the node details are provided by the port
dtsi files. The information they provide is the alias used for each

port.

My impression is that such hierarchy mapping is not really necessary and
only makes the device tree source messy and hard to follow.  I do not
like it.

Hi Shawn, I respect your opinion on this, I'm sure it is the result of an
extensive experience dealing with less complicated devices. Before breaking
a construct that to date has served the DPAA users well I'd like to hear
more thoughts on this topic.

quoted

quoted

quoted

quoted

quoted

+};

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a-qds.dts

b/arch/arm64/boot/dts/freescale/fsl-ls1043a-qds.dts

quoted

index 0989d63..ee66bb2 100644

--- a/arch/arm64/boot/dts/freescale/fsl-ls1043a-qds.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a-qds.dts

@@ -181,3 +181,5 @@
 		reg = <0>;
 	};
 };
+
+/include/ "fsl-ls1043-post.dtsi"

Move it to header of the file.

This is to be included at the end, to make sure the references are
met and to allow overrides if needed.

What is broken if you move the include to header?

Not much besides the structure we've always used for our SoCs device
trees. The file is called "-post.dtsi" because here is the place any
required overrides can be made, if needed. Moving to the top renders
having this separate file useless.

That's great, and let's kill it then.

quoted

quoted

quoted

quoted

quoted

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts

b/arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts

quoted

index c37110b..d94f003 100644

--- a/arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts

@@ -139,3 +139,78 @@
 &duart1 {
 	status = "okay";
 };
+
+/include/ "fsl-ls1043-post.dtsi"
+

Ditto

quoted

+&soc {
+	fman at 1a00000 {
+		ethernet at e0000 {

You defined enet0 label.  Why don't you use it?

The enet0 label is used by u-boot for fix-ups, providing the
actual offset here makes it easier to follow.

You will not need to construct the node hierarchy with label.  And
alias/label name is more easier to follow than offset.

Shawn

When I said easier to follow I was referring to someone creating a
new device tree for his custom board, not someone reading the device
tree. If you have the board and SoC reference manuals in your hands
and you are writing a new board device tree, having the offset here
makes things easier. The benefit of having one less indentation level
is lesser than that.

The while complex and messy file hierarchy makes users' life harder,
both the ones reading the device tree and the ones creating board device
tree.  I would suggest you go the opposite, making the device tree
simple and easy for users by allowing data duplication.  In arm/arm64
device tree world, we do not consider DT data reusing/sharing among
different SoCs that much.

Shawn

Complex it is, mirroring the IP, but messy it is a word I would not use.

Regards,
Madalin