> -----Original Message----- > From: Shawn Guo [mailto:shawnguo@xxxxxxxxxx] > 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: > > > > > > + fman@1a00000 { > > > > > > + enet0: ethernet@e0000 { > > > > > > + }; > > > > > > + > > > > > > + enet1: ethernet@e2000 { > > > > > > + }; > > > > > > + > > > > > > + enet2: ethernet@e4000 { > > > > > > + }; > > > > > > + > > > > > > + enet3: ethernet@e6000 { > > > > > > + }; > > > > > > + > > > > > > + enet4: ethernet@e8000 { > > > > > > + }; > > > > > > + > > > > > > + enet5: ethernet@ea000 { > > > > > > + }; > > > > > > + > > > > > > + enet6: ethernet@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 > > > I should wait for you new patches with better commit log and comments > to > > > 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 > > 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. > > > > > > > > > > > > +}; > > > > > > diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a-qds.dts > > > > > b/arch/arm64/boot/dts/freescale/fsl-ls1043a-qds.dts > > > > > > 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. > > > > > > > > > > > > > diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts > > > > > b/arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts > > > > > > 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 > > > > > > > > > > > +&soc { > > > > > > + fman@1a00000 { > > > > > > + ethernet@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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
