From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751998AbdC0G2S (ORCPT ); Mon, 27 Mar 2017 02:28:18 -0400 Received: from mail.kernel.org ([198.145.29.136]:40490 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751583AbdC0G2I (ORCPT ); Mon, 27 Mar 2017 02:28:08 -0400 Date: Mon, 27 Mar 2017 14:27:21 +0800 From: Shawn Guo To: Madalin-Cristian Bucur Cc: Roy Pledge , "mark.rutland@arm.com" , "devicetree@vger.kernel.org" , "catalin.marinas@arm.com" , "will.deacon@arm.com" , "linux-kernel@vger.kernel.org" , Russell King - ARM Linux , "robh+dt@kernel.org" , Kumar Gala , "linux-arm-kernel@lists.infradead.org" Subject: Re: [PATCH v2 1/2] dts: arm64: add LS1043A DPAA support Message-ID: <20170327062719.GQ30608@dragon> References: <1488382505-17554-1-git-send-email-madalin.bucur@nxp.com> <1488382505-17554-2-git-send-email-madalin.bucur@nxp.com> <20170314070732.GH3618@dragon> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. > > > + 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. > > > > +}; > > > 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? > > > > 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