From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752103AbdC0INT (ORCPT ); Mon, 27 Mar 2017 04:13:19 -0400 Received: from mail-db5eur01on0045.outbound.protection.outlook.com ([104.47.2.45]:10883 "EHLO EUR01-DB5-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751640AbdC0INL (ORCPT ); Mon, 27 Mar 2017 04:13:11 -0400 From: Madalin-Cristian Bucur To: Shawn Guo , "devicetree@vger.kernel.org" , "robh+dt@kernel.org" , "mark.rutland@arm.com" , Roy Pledge , "will.deacon@arm.com" , "catalin.marinas@arm.com" CC: "linux-kernel@vger.kernel.org" , "Russell King - ARM Linux" , Kumar Gala , "linux-arm-kernel@lists.infradead.org" Subject: RE: [PATCH v2 1/2] dts: arm64: add LS1043A DPAA support Thread-Topic: [PATCH v2 1/2] dts: arm64: add LS1043A DPAA support Thread-Index: AQHSkqFuhh5lPIEB7UueKCvnkQraTKGT/gsAgAzAoNCAB6JzgIAAA4yQgAAVKgCAAAItMA== Date: Mon, 27 Mar 2017 08:11:33 +0000 Message-ID: References: <1488382505-17554-1-git-send-email-madalin.bucur@nxp.com> <1488382505-17554-2-git-send-email-madalin.bucur@nxp.com> <20170314070732.GH3618@dragon> <20170327062719.GQ30608@dragon> <20170327075546.GR30608@dragon> In-Reply-To: <20170327075546.GR30608@dragon> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: kernel.org; dkim=none (message not signed) header.d=none;kernel.org; dmarc=none action=none header.from=nxp.com; x-originating-ip: [192.88.146.1] x-microsoft-exchange-diagnostics: 1;HE1PR04MB3209;7:Pca6BwW7qhkOTZOJcm/E5YeDsKDTClDHTReYQsZsutVykzw8PPalsSI7IKARJSbk6tN54DJj/XmPOvPVQ5tGpGk+gbsgwkFqTPhgCiem8rA61u6Bufe7W87iRuld8Y4Pt/pUik0IarM6Nl7QplDDI3E6h19fsNybkzeVJhQA+3j9erXxIq8o5GaP6i+CIDK2ya3GKYCeOO4PCbeiQDrwyrbsYor3B5dvYPb+deuIz9RAp5tS71vIbB7U0J5FMiLwL80shR1NqdO8JTz48LLGIXJquHlmpvuveDQWXayp8sekVZhcdcyoA60f3Wtp8UzFqe4G6wyDEq8+zokHaYit4Q== x-forefront-antispam-report: SFV:SKI;SCL:-1SFV:NSPM;SFS:(10009020)(6009001)(39410400002)(39840400002)(39450400003)(39860400002)(13464003)(24454002)(377454003)(99286003)(2950100002)(9686003)(55016002)(86362001)(7696004)(54906002)(2201001)(8936002)(7736002)(74316002)(305945005)(6116002)(102836003)(3846002)(5660300001)(189998001)(7416002)(2906002)(6506006)(6436002)(81166006)(3280700002)(93886004)(8676002)(77096006)(3660700001)(2501003)(53936002)(53546009)(25786009)(33656002)(4326008)(6246003)(38730400002)(50986999)(76176999)(54356999)(2900100001)(122556002)(66066001);DIR:OUT;SFP:1101;SCL:1;SRVR:HE1PR04MB3209;H:HE1PR04MB1129.eurprd04.prod.outlook.com;FPR:;SPF:None;MLV:sfv;LANG:en; x-ms-office365-filtering-correlation-id: 8fc6fd7c-3f27-4c77-a545-08d474e8e1b9 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001)(2017030254075)(48565401081);SRVR:HE1PR04MB3209; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:; x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(6040375)(601004)(2401047)(8121501046)(5005006)(10201501046)(3002001)(6055026)(6041248)(20161123558025)(20161123555025)(20161123560025)(20161123564025)(20161123562025)(6072148);SRVR:HE1PR04MB3209;BCL:0;PCL:0;RULEID:;SRVR:HE1PR04MB3209; x-forefront-prvs: 02596AB7DA spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 X-OriginatorOrg: nxp.com X-MS-Exchange-CrossTenant-originalarrivaltime: 27 Mar 2017 08:11:33.1955 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635 X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1PR04MB3209 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id v2R8DSU9023960 > -----Original Message----- > From: Shawn Guo [mailto:shawnguo@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: > > > > > > + 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