From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754895AbcESNaM (ORCPT ); Thu, 19 May 2016 09:30:12 -0400 Received: from mail-db3on0124.outbound.protection.outlook.com ([157.55.234.124]:43189 "EHLO emea01-db3-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1754785AbcESNaJ (ORCPT ); Thu, 19 May 2016 09:30:09 -0400 From: Marcel Ziswiler To: "robh@kernel.org" CC: "linux-kernel@vger.kernel.org" , "devicetree@vger.kernel.org" , "linux@armlinux.org.uk" , "gnurou@gmail.com" , "thierry.reding@gmail.com" , "mark.rutland@arm.com" , "galak@codeaurora.org" , "ijc+devicetree@hellion.org.uk" , "linux-arm-kernel@lists.infradead.org" , "linux-tegra@vger.kernel.org" , "swarren@wwwdotorg.org" Subject: Re: [PATCH] arm: tegra: initial support for apalis tk1 Thread-Topic: [PATCH] arm: tegra: initial support for apalis tk1 Thread-Index: AQHRrEm1zp0kLGTFQkWmFpXb9MmIY5+7vjgAgASKMoA= Date: Thu, 19 May 2016 13:15:24 +0000 Message-ID: <1463663721.4958.18.camel@toradex.com> References: <1463056032-25888-1-git-send-email-marcel.ziswiler@toradex.com> <20160516155542.GA25949@rob-hp-laptop> In-Reply-To: <20160516155542.GA25949@rob-hp-laptop> Accept-Language: en-GB, 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=toradex.com; x-originating-ip: [46.140.72.82] x-ms-office365-filtering-correlation-id: e65fa577-8f0a-4314-7747-08d37fe7a363 x-microsoft-exchange-diagnostics: 1;HE1PR05MB1881;5:9t20eLVKk5iuYOJBsc70O8TRUtt5RJh1b0wo4Og2SqLctcTJmtjIKY4gnCtoSP70WHAL6gw9VRhHY6L4lI3552zJNrmJJUxGdjOpwOFfrDJGKM5o3BXCPYNnYhSqVQ3Cv528RXGuPuP2xBbvCu0NoA==;24:7C6N1Rpy0dsH0MV1ZmiJYQhveG2cLOqgrJzP7s6OVTb7r+eO5AjtiiCA5yC5Er+jjnPdJqpIwDQHtb1I1L6eKRRbIstY5UH1UD21rWEHRCc=;7:VYfXTiO2K9BEpXfzPfLNbyY/xItsNStoowO1p/HhYPXZO3HeDmYeklqwL6kNiJLg3VbdKDSgdMfA2yp5VoGYubjNVilqvkxOu6BqtnFDLVunR1lUhOkQ1gei2tHKNzUy7d0Kj+w8Mqrdf7WZN8MLeR2gTZB+jTCIZZrLsEMjzByAXsTTmWIuCtbQIsO5BY5O x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:HE1PR05MB1881; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:; x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(601004)(2401047)(5005006)(8121501046)(10201501046)(3002001);SRVR:HE1PR05MB1881;BCL:0;PCL:0;RULEID:;SRVR:HE1PR05MB1881; x-forefront-prvs: 094700CA91 x-forefront-antispam-report: SFV:NSPM;SFS:(10019020)(6009001)(52314003)(24454002)(377424004)(189998001)(66066001)(3846002)(1730700003)(81166006)(8936002)(87936001)(8676002)(122556002)(19580405001)(10400500002)(86362001)(3660700001)(2900100001)(586003)(77096005)(4326007)(2950100001)(3280700002)(19580395003)(5640700001)(103116003)(2906002)(102836003)(110136002)(5002640100001)(2501003)(33646002)(106116001)(2351001)(6116002)(92566002)(76176999)(50986999)(11100500001)(54356999)(5008740100001)(36756003)(5004730100002)(1220700001)(2004002);DIR:OUT;SFP:1102;SCL:1;SRVR:HE1PR05MB1881;H:HE1PR05MB1882.eurprd05.prod.outlook.com;FPR:;SPF:None;MLV:sfv;LANG:en; spamdiagnosticoutput: 1:23 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="utf-8" Content-ID: MIME-Version: 1.0 X-OriginatorOrg: toradex.com X-MS-Exchange-CrossTenant-originalarrivaltime: 19 May 2016 13:15:24.0999 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: d9995866-0d9b-4251-8315-093f062abab4 X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1PR05MB1881 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 base64 to 8bit by mail.home.local id u4JDUJu1002646 On Mon, 2016-05-16 at 10:55 -0500, Rob Herring wrote: > On Thu, May 12, 2016 at 02:27:12PM +0200, Marcel Ziswiler wrote: > > > > This patch adds the device tree to support Toradex Apalis TK1 a > > computer on module which can be used on different carrier boards. > > > > The module consists of a Tegra TK1 SoC, a PMIC solution, 2 GB of > > DDR3L > > RAM, a bunch of level shifters, an eMMC, a TMP451 temperature > > sensor > > chip and an I210 gigabit Ethernet controller. Furthermore, there is > > an > > SGTL5000 audio codec plus a Kinetis MK20DN512 companion micro > > controller for analogue, CAN and resistive touch functionality > > which > > are not yet supported. Anything that is not self contained on the > > module is disabled by default. > > > > The device tree for the Evaluation Board includes the module's > > device > > tree and enables the supported peripherals of the carrier board > > (the > > Evaluation Board supports almost all of them). > > > > While at it also add the device tree binding documentation for > > Apalis > > TK1 as well as for Colibri T30 which was missing so far. > "While at it" and "also" are keywords for put in a separate patch. While I do agree in general in this case it is about missing device tree documentation which has absolutely zero potential to break bisectability or anything the like. Anyway, I will split it for a v2. > > Signed-off-by: Marcel Ziswiler > > > > --- > > > >  Documentation/devicetree/bindings/arm/tegra.txt |    4 + > >  arch/arm/boot/dts/Makefile                      |    1 + > >  arch/arm/boot/dts/tegra124-apalis-emc.dtsi      | 2462 > > +++++++++++++++++++++++ > >  arch/arm/boot/dts/tegra124-apalis-eval.dts      |  283 +++ > >  arch/arm/boot/dts/tegra124-apalis.dtsi          | 2058 > > +++++++++++++++++++ > >  5 files changed, 4808 insertions(+) > >  create mode 100644 arch/arm/boot/dts/tegra124-apalis-emc.dtsi > >  create mode 100644 arch/arm/boot/dts/tegra124-apalis-eval.dts > >  create mode 100644 arch/arm/boot/dts/tegra124-apalis.dtsi > > > > diff --git a/Documentation/devicetree/bindings/arm/tegra.txt > > b/Documentation/devicetree/bindings/arm/tegra.txt > > index 73278c6..7709f3d 100644 > > --- a/Documentation/devicetree/bindings/arm/tegra.txt > > +++ b/Documentation/devicetree/bindings/arm/tegra.txt > > @@ -31,8 +31,12 @@ board-specific compatible values: > >    nvidia,ventana > >    nvidia,whistler > >    toradex,apalis_t30 > > +  toradex,apalis_tk1 > >    toradex,apalis_t30-eval > > +  toradex,apalis_tk1-eval > >    toradex,colibri_t20-512 > > +  toradex,colibri_t30 > > +  toradex,colibri_t30-eval-v3 > >    toradex,iris > Please use '-' rather than '_' for new strings even if that's what  > previous strings have. For ones that already in use in upstream dts  > files, then keep the underscore. OK, semantically so far we used '_' aka underscores as having a weaker separation meaning (e.g. like a space) than '-' aka dashes which separated the module from the carrier board part. We even once discussed that the dash between eval and v3 for colibri_t30 should actually rather be an underscore. I guess something like this is now no more feasible or do you have any suggestions for us? > >  Trusted Foundations > > diff --git a/arch/arm/boot/dts/Makefile > > b/arch/arm/boot/dts/Makefile > > index 06b6c2d..3a13d4f 100644 > > --- a/arch/arm/boot/dts/Makefile > > +++ b/arch/arm/boot/dts/Makefile > > @@ -793,6 +793,7 @@ dtb-$(CONFIG_ARCH_TEGRA_114_SOC) += \ > >   tegra114-roth.dtb \ > >   tegra114-tn7.dtb > >  dtb-$(CONFIG_ARCH_TEGRA_124_SOC) += \ > > + tegra124-apalis-eval.dtb \ > >   tegra124-jetson-tk1.dtb \ > >   tegra124-nyan-big.dtb \ > >   tegra124-nyan-blaze.dtb \ > > diff --git a/arch/arm/boot/dts/tegra124-apalis-emc.dtsi > > b/arch/arm/boot/dts/tegra124-apalis-emc.dtsi > > new file mode 100644 > > index 0000000..31b31ea > > --- /dev/null > > +++ b/arch/arm/boot/dts/tegra124-apalis-emc.dtsi > > @@ -0,0 +1,2462 @@ > > +/* > > + * Copyright 2016 Toradex AG > > + * > > + * This file is dual-licensed: you can use it either under the > > terms > > + * of the GPL or the X11 license, at your option. Note that this > > dual > > + * licensing only applies to this file, and not this project as a > > + * whole. > > + * > > + *  a) This file is free software; you can redistribute it and/or > > + *     modify it under the terms of the GNU General Public License > > + *     version 2 as published by the Free Software Foundation. > > + * > > + *     This file is distributed in the hope that it will be useful > > + *     but WITHOUT ANY WARRANTY; without even the implied warranty > > of > > + *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See > > the > > + *     GNU General Public License for more details. > > + * > > + * Or, alternatively > > + * > > + *  b) Permission is hereby granted, free of charge, to any person > > + *     obtaining a copy of this software and associated > > documentation > > + *     files (the "Software"), to deal in the Software without > > + *     restriction, including without limitation the rights to use > > + *     copy, modify, merge, publish, distribute, sublicense, > > and/or > > + *     sell copies of the Software, and to permit persons to whom > > the > > + *     Software is furnished to do so, subject to the following > > + *     conditions: > > + * > > + *     The above copyright notice and this permission notice shall > > be > > + *     included in all copies or substantial portions of the > > Software. > > + * > > + *     THE SOFTWARE IS PROVIDED , WITHOUT WARRANTY OF ANY KIND > > + *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE > > WARRANTIES > > + *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND > > + *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT > > + *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY > > + *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, > > ARISING > > + *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE > > OR > > + *     OTHER DEALINGS IN THE SOFTWARE. > > + */ > > + > > +/ { > > + clock@0,60006000 { > All these commas should be dropped. These have been fixed in -next > and  > will go into 4.7. I guess you mean the zero followed by the comma, right? That only works if tegra124.dtsi and with it all the other boards get migrated to this as well. I can certainly send a patch for that as well if this is the direction we want to go. > > + emc-timings-1 { > > + nvidia,ram-code = <1>; > > + > > + timing-12750000 { > > + clock-frequency = <12750000>; > > + nvidia,parent-clock-frequency = > > <408000000>; > > + clocks = <&tegra_car > > TEGRA124_CLK_PLL_P>; > > + clock-names = "emc-parent"; > > + }; > > + timing-20400000 { > > + clock-frequency = <20400000>; > > + nvidia,parent-clock-frequency = > > <408000000>; > > + clocks = <&tegra_car > > TEGRA124_CLK_PLL_P>; > > + clock-names = "emc-parent"; > > + }; > > + timing-40800000 { > > + clock-frequency = <40800000>; > > + nvidia,parent-clock-frequency = > > <408000000>; > > + clocks = <&tegra_car > > TEGRA124_CLK_PLL_P>; > > + clock-names = "emc-parent"; > > + }; > > + timing-68000000 { > > + clock-frequency = <68000000>; > > + nvidia,parent-clock-frequency = > > <408000000>; > > + clocks = <&tegra_car > > TEGRA124_CLK_PLL_P>; > > + clock-names = "emc-parent"; > > + }; > > + timing-102000000 { > > + clock-frequency = <102000000>; > > + nvidia,parent-clock-frequency = > > <408000000>; > > + clocks = <&tegra_car > > TEGRA124_CLK_PLL_P>; > > + clock-names = "emc-parent"; > > + }; > > + timing-204000000 { > > + clock-frequency = <204000000>; > > + nvidia,parent-clock-frequency = > > <408000000>; > > + clocks = <&tegra_car > > TEGRA124_CLK_PLL_P>; > > + clock-names = "emc-parent"; > > + }; > > + timing-300000000 { > > + clock-frequency = <300000000>; > > + nvidia,parent-clock-frequency = > > <600000000>; > > + clocks = <&tegra_car > > TEGRA124_CLK_PLL_C>; > > + clock-names = "emc-parent"; > > + }; > > + timing-396000000 { > > + clock-frequency = <396000000>; > > + nvidia,parent-clock-frequency = > > <792000000>; > > + clocks = <&tegra_car > > TEGRA124_CLK_PLL_M>; > > + clock-names = "emc-parent"; > > + }; > > + timing-528000000 { > > + clock-frequency = <528000000>; > > + nvidia,parent-clock-frequency = > > <528000000>; > > + clocks = <&tegra_car > > TEGRA124_CLK_PLL_M_UD>; > > + clock-names = "emc-parent"; > > + }; > > + timing-600000000 { > > + clock-frequency = <600000000>; > > + nvidia,parent-clock-frequency = > > <600000000>; > > + clocks = <&tegra_car > > TEGRA124_CLK_PLL_C_UD>; > > + clock-names = "emc-parent"; > > + }; > > + timing-792000000 { > > + clock-frequency = <792000000>; > > + nvidia,parent-clock-frequency = > > <792000000>; > > + clocks = <&tegra_car > > TEGRA124_CLK_PLL_M_UD>; > > + clock-names = "emc-parent"; > > + }; > > + timing-924000000 { > > + clock-frequency = <924000000>; > > + nvidia,parent-clock-frequency = > > <924000000>; > > + clocks = <&tegra_car > > TEGRA124_CLK_PLL_M_UD>; > > + clock-names = "emc-parent"; > > + }; > > + }; > > + }; > > + > > + emc@0,7001b000 { > > + emc-timings-1 { > > + nvidia,ram-code = <1>; > > + > > + timing-12750000 { > > + clock-frequency = <12750000>; > > + > > + nvidia,emc-auto-cal-config = > > <0xa1430000>; > > + nvidia,emc-auto-cal-config2 = > > <0x00000000>; > > + nvidia,emc-auto-cal-config3 = > > <0x00000000>; > > + nvidia,emc-auto-cal-interval = > > <0x001fffff>; > > + nvidia,emc-bgbias-ctl0 = > > <0x00000008>; > > + nvidia,emc-cfg = <0x73240000>; > > + nvidia,emc-cfg-2 = <0x000008c5>; > > + nvidia,emc-ctt-term-ctrl = > > <0x00000802>; > > + nvidia,emc-mode-1 = <0x80100003>; > > + nvidia,emc-mode-2 = <0x80200008>; > > + nvidia,emc-mode-4 = <0x00000000>; > > + nvidia,emc-mode-reset = > > <0x80001221>; > > + nvidia,emc-mrs-wait-cnt = > > <0x000e000e>; > > + nvidia,emc-sel-dpd-ctrl = > > <0x00040128>; > > + nvidia,emc-xm2dqspadctrl2 = > > <0x0130b118>; > > + nvidia,emc-zcal-cnt-long = > > <0x00000042>; > > + nvidia,emc-zcal-interval = > > <0x00000000>; > > + > > + nvidia,emc-configuration = < > > + 0x00000000 > > + 0x00000003 > This is a bit long. Do multiple values per line. OK. If keeping the 80 character limit one could get 3 values per line which otherwise seems rather inconvenient. Should I just do two then or allow for longer lines and putting 4 or even 8? > > + 0x00000000 > > + 0x00000000 > > + 0x00000000 > > + 0x00000004 > > + 0x0000000a > > + 0x00000005 Thanks, Rob.