> -----Original Message----- > From: Shawn Guo [mailto:shawnguo@xxxxxxxxxx] > Sent: Monday, March 27, 2017 9:27 AM > Subject: Re: [PATCH v2 1/2] dts: arm64: add LS1043A DPAA support > > On Wed, Mar 22, 2017 at 10:58:12AM +0000, Madalin-Cristian Bucur wrote: > > > > +&soc { > > > > + > > > > +/include/ "qoriq-fman3-0.dtsi" > > > > +/include/ "qoriq-fman3-0-1g-0.dtsi" > > > > +/include/ "qoriq-fman3-0-1g-1.dtsi" > > > > +/include/ "qoriq-fman3-0-1g-2.dtsi" > > > > +/include/ "qoriq-fman3-0-1g-3.dtsi" > > > > +/include/ "qoriq-fman3-0-1g-4.dtsi" > > > > +/include/ "qoriq-fman3-0-1g-5.dtsi" > > > > +/include/ "qoriq-fman3-0-10g-0.dtsi" > > > > > > We usually put the includes at the beginning of the file, and #include > > > is more recommended than /include/. > > > > I'm not making use of the header file inclusion feature #include > provides > > (nor plan to) in these files thus I've selected /include/ here. > > Let's be simple and consistent. Use #include please. I can do that, running the preprocessor on these files without being required does not add that much time in the end. I'm not a fan of this feature, I never liked the fact one loses the liberty of easily using dtc directly to debug using dtc -O dts when adding #includes. > > > > + 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. > > > > > > +}; > > > > 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. > > > > > > 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. 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
