From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753265AbcBONie (ORCPT ); Mon, 15 Feb 2016 08:38:34 -0500 Received: from lists.s-osg.org ([54.187.51.154]:48379 "EHLO lists.s-osg.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751298AbcBONic (ORCPT ); Mon, 15 Feb 2016 08:38:32 -0500 Subject: Re: [PATCH] ARM: dts: Move syscon reboot/poweroff to common dtsi for Exynos To: Krzysztof Kozlowski , linux-kernel@vger.kernel.org References: <1455042606-10075-1-git-send-email-javier@osg.samsung.com> <56C02265.40103@samsung.com> Cc: k.kozlowski.k@gmail.com, devicetree@vger.kernel.org, linux-samsung-soc@vger.kernel.org, Andi Shyti , Alim Akhtar , Kukjin Kim , linux-arm-kernel@lists.infradead.org From: Javier Martinez Canillas Message-ID: <56C1D4D1.2050108@osg.samsung.com> Date: Mon, 15 Feb 2016 10:38:25 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0 MIME-Version: 1.0 In-Reply-To: <56C02265.40103@samsung.com> Content-Type: text/plain; charset=iso-8859-2; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello Krzysztof, On 02/14/2016 03:44 AM, Krzysztof Kozlowski wrote: > W dniu 10.02.2016 o 03:30, Javier Martinez Canillas pisze: >> All Exynos SoCs have the same syscon reboot and poweroff device nodes so >> there is no need to duplicate the same on each SoC dtsi and can be moved >> to a common dtsi that can be included by all the SoCs dtsi files. >> >> Signed-off-by: Javier Martinez Canillas >> >> --- >> Hello, >> >> The patch was tested on an Exynos5800 Peach Pi Chromebook and an >> Exynos5422 Odroid XU4 board. Reboot and poweroff worked for both. >> >> But testing on Exynos3 and Exynos4 will be highly appreciated. > > Would you be so kind and add my suggested-by (since it comes from > https://wiki.tizen.org/wiki/Exynos_Mainline_Kernel_TODO :) ) > Right, sorry for missing that. > I'll test it on Exynos4 at work and then apply... but few comments below: > >> >> Best regards, >> Javier >> >> arch/arm/boot/dts/exynos-syscon-restart.dtsi | 23 +++++++++++++++++++++++ >> arch/arm/boot/dts/exynos3250.dtsi | 15 +-------------- >> arch/arm/boot/dts/exynos4.dtsi | 15 +-------------- >> arch/arm/boot/dts/exynos5.dtsi | 15 +-------------- >> arch/arm/boot/dts/exynos5410.dtsi | 15 +-------------- >> 5 files changed, 27 insertions(+), 56 deletions(-) >> create mode 100644 arch/arm/boot/dts/exynos-syscon-restart.dtsi >> >> diff --git a/arch/arm/boot/dts/exynos-syscon-restart.dtsi b/arch/arm/boot/dts/exynos-syscon-restart.dtsi >> new file mode 100644 >> index 000000000000..2b8acf554881 >> --- /dev/null >> +++ b/arch/arm/boot/dts/exynos-syscon-restart.dtsi >> @@ -0,0 +1,23 @@ >> +/* >> + * Samsung's Exynos SoC syscon reboot/poweroff nodes common definition. >> + * >> + * This program 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. >> + */ >> + >> +/ { > > In some of the DTSI this was under "soc" node, not at top-level. > Unfortunately we do not have consistency here - some DTSI have "soc", Yes, I noticed this but as you said not all DTSI have a soc node and that's why I made it top level in the DTSI. > some not. Anyway I think we should move to "soc" version. > Not sure I'm following, did you mean to do it as a follow up or to add a soc node for the missing DTSI as a part of this series? >> + poweroff: syscon-poweroff { >> + compatible = "syscon-poweroff"; >> + regmap = <&pmu_system_controller>; >> + offset = <0x330C>; /* PS_HOLD_CONTROL */ >> + mask = <0x5200>; /* reset value */ >> + }; >> + >> + reboot: syscon-reboot { >> + compatible = "syscon-reboot"; >> + regmap = <&pmu_system_controller>; >> + offset = <0x0400>; /* SWRESET */ >> + mask = <0x1>; >> + }; >> +}; >> diff --git a/arch/arm/boot/dts/exynos3250.dtsi b/arch/arm/boot/dts/exynos3250.dtsi >> index 18e3deffbf48..d9c221517935 100644 >> --- a/arch/arm/boot/dts/exynos3250.dtsi >> +++ b/arch/arm/boot/dts/exynos3250.dtsi >> @@ -19,6 +19,7 @@ >> >> #include "skeleton.dtsi" >> #include "exynos4-cpu-thermal.dtsi" >> +#include "exynos-syscon-restart.dtsi" >> #include >> >> / { >> @@ -152,20 +153,6 @@ >> interrupt-parent = <&gic>; >> }; >> >> - poweroff: syscon-poweroff { >> - compatible = "syscon-poweroff"; >> - regmap = <&pmu_system_controller>; >> - offset = <0x330C>; /* PS_HOLD_CONTROL */ >> - mask = <0x5200>; /* Reset value */ >> - }; >> - >> - reboot: syscon-reboot { >> - compatible = "syscon-reboot"; >> - regmap = <&pmu_system_controller>; >> - offset = <0x0400>; /* SWRESET */ >> - mask = <0x1>; >> - }; >> - >> mipi_phy: video-phy@10020710 { >> compatible = "samsung,s5pv210-mipi-video-phy"; >> #phy-cells = <1>; >> diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi >> index ca621a92319e..e6f6939ccdfa 100644 >> --- a/arch/arm/boot/dts/exynos4.dtsi >> +++ b/arch/arm/boot/dts/exynos4.dtsi >> @@ -21,6 +21,7 @@ >> >> #include >> #include >> +#include "exynos-syscon-restart.dtsi" > > Just to keep it consistent and logic - include after skeleton. > Ok, for include I usually try to keep alphabetically sorted but I'll move it after skeleton if you prefer to keep it consistent. > Best regards, > Krzysztof > Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America