From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1162783AbeBNTRB (ORCPT ); Wed, 14 Feb 2018 14:17:01 -0500 Received: from mail.blih.net ([212.83.177.182]:13994 "EHLO mail.blih.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1162753AbeBNTQ5 (ORCPT ); Wed, 14 Feb 2018 14:16:57 -0500 Date: Wed, 14 Feb 2018 20:16:54 +0100 From: Emmanuel Vadot To: Chen-Yu Tsai Cc: devicetree , Maxime Ripard , linux-kernel , linux-sunxi , linux-arm-kernel , Icenowy Zheng Subject: Re: [PATCH] Revert "ARM: dts: sunxi: Add regulators for Sinovoip BPI-M2" Message-Id: <20180214201654.cb8cb64bf50b958a53ebebbd@bidouilliste.com> In-Reply-To: References: <20180203112353.13497-1-icenowy@aosc.io> <20180205095558.2e713c24cdb7c3232943db52@bidouilliste.com> <207CD143-6535-42C9-BEF3-9D2614336482@aosc.io> X-Mailer: Sylpheed 3.6.0 (GTK+ 2.24.31; amd64-portbld-freebsd12.0) Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 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 w1EJH76e009150 On Tue, 13 Feb 2018 18:36:24 +0800 Chen-Yu Tsai wrote: > On Sat, Feb 10, 2018 at 5:20 AM, Emmanuel Vadot wrote: > > On 2018-02-05 10:05, Icenowy Zheng wrote: > >> > >> ? 2018?2?5? GMT+08:00 ??4:55:58, Emmanuel Vadot > >> ??: > >>> > >>> > >>> Hello, > >>> > >>> On Sat, 3 Feb 2018 19:23:53 +0800 > >>> Icenowy Zheng wrote: > >>> > >>>> This reverts commit 7daa213700758b5b08fc0daab09bb139dd334165. > >>>> > >>>> The original commit has several problems: > >>>> > >>>> - vdd-cpus and aldo3 (AVCC of the SoC) are not set to always-on, > >>> > >>> which > >>>> > >>>> leads to system hang when disabling unused regulators. > >>> > >>> > >>> Indeed I should have make those always-on. > >>> > >>>> - GMAC (which uses dldo1 and aldo2) and Wi-Fi (which uses aldo1) are > >>> > >>> not > >>>> > >>>> considered, and will fail to work after adding this commit. > >>> > >>> > >>> While I understand the problem with vdd-cpus and aldo3 I don't see why > >>> when you don't declare regulator the code should do something with it. > >>> DT is supposed to describe the hardware and the code should not use > >>> hardware not described right ? > >>> The gmac node doesn't declare any regulators and the mmc2 uses > >>> reg_vcc3v0 (haven't checked on the schematics yet if it is correct). > >> > >> > >> It's because the regulator support isn't present before > >> this commit. However these parts really need special > >> regulators. I don't have M2 schematics at hand, so you'd > >> check it by yourself. > > > > > > Yes but why does the PMIC should disable regulators not defined in the DTS > > ? That the part I don't understand and want to know where it is > > described/documented. > > They are defined. See axp22x.dtsi, which you included in your patch. > > Now the system is free to do whatever it wants under the constraints > of the device tree. Since you do not reference the regulator, the > kernel is free to turn it off to save power. Yeah I realized that now and feel stupid ... Thanks for the clarification. > > > >> P.S. a proper device tree with AXP shouldn't use > >> reg_vcc3v0/3v3/1v8/etc. They're dummy > >> regulator nodes for > >> not implemented or not controllable regulators. > >> > >>> > >>>> This indicates that this patch should be not tested at all. > >>> > >>> > >>> This have indeed not been tested with linux. > >>> I think that this commit should not be reverted, I'll send a proper > >>> patch tonight or tomorow night max. > >> > >> > >> Please test patches sent to Linux on Linux :-) > > > > > > If my patches adhere to the bindings I don't see why. > > It adheres to the bindings, but does not accurately describe the > hardware constraints. Is there a place where there is build of the linux kernel for sunxi/multi7 and initrd available for download so I can add that to my test bench ? Cheers, > ChenYu > > > > > > >>> > >>> P.S.: Also as I'm the original sender I think I should have been in CC > >>> no ? > >> > >> > >> get_maintainer.pl didn't mention you and I forgot... sorry. > >> > >>> > >>> Cheers, > >>> > >>>> Signed-off-by: Icenowy Zheng > >>>> --- > >>>> arch/arm/boot/dts/sun6i-a31s-sinovoip-bpi-m2.dts | 57 > >>> > >>> ------------------------ > >>>> > >>>> 1 file changed, 57 deletions(-) > >>>> > >>>> diff --git a/arch/arm/boot/dts/sun6i-a31s-sinovoip-bpi-m2.dts > >>> > >>> b/arch/arm/boot/dts/sun6i-a31s-sinovoip-bpi-m2.dts > >>>> > >>>> index 51e6f1d21c32..a565316eb340 100644 > >>>> --- a/arch/arm/boot/dts/sun6i-a31s-sinovoip-bpi-m2.dts > >>>> +++ b/arch/arm/boot/dts/sun6i-a31s-sinovoip-bpi-m2.dts > >>>> @@ -86,10 +86,6 @@ > >>>> }; > >>>> }; > >>>> > >>>> -&cpu0 { > >>>> - cpu-supply = <®_dcdc3>; > >>>> -}; > >>>> - > >>>> &ehci0 { > >>>> status = "okay"; > >>>> }; > >>>> @@ -155,17 +151,6 @@ > >>>> status = "okay"; > >>>> }; > >>>> > >>>> -&p2wi { > >>>> - status = "okay"; > >>>> - > >>>> - axp22x: pmic@68 { > >>>> - compatible = "x-powers,axp221"; > >>>> - reg = <0x68>; > >>>> - interrupt-parent = <&nmi_intc>; > >>>> - interrupts = <0 IRQ_TYPE_LEVEL_LOW>; > >>>> - }; > >>>> -}; > >>>> - > >>>> &pio { > >>>> gmac_phy_reset_pin_bpi_m2: gmac_phy_reset_pin@0 { > >>>> pins = "PA21"; > >>>> @@ -191,48 +176,6 @@ > >>>> }; > >>>> }; > >>>> > >>>> -#include "axp22x.dtsi" > >>>> - > >>>> -®_dc5ldo { > >>>> - regulator-min-microvolt = <700000>; > >>>> - regulator-max-microvolt = <1320000>; > >>>> - regulator-name = "vdd-cpus"; > >>>> -}; > >>>> - > >>>> -®_dcdc1 { > >>>> - regulator-always-on; > >>>> - regulator-min-microvolt = <3000000>; > >>>> - regulator-max-microvolt = <3000000>; > >>>> - regulator-name = "vdd-3v0"; > >>>> -}; > >>>> - > >>>> -®_dcdc2 { > >>>> - regulator-min-microvolt = <700000>; > >>>> - regulator-max-microvolt = <1320000>; > >>>> - regulator-name = "vdd-gpu"; > >>>> -}; > >>>> - > >>>> -®_dcdc3 { > >>>> - regulator-always-on; > >>>> - regulator-min-microvolt = <700000>; > >>>> - regulator-max-microvolt = <1320000>; > >>>> - regulator-name = "vdd-cpu"; > >>>> -}; > >>>> - > >>>> -®_dcdc4 { > >>>> - regulator-always-on; > >>>> - regulator-min-microvolt = <700000>; > >>>> - regulator-max-microvolt = <1320000>; > >>>> - regulator-name = "vdd-sys-dll"; > >>>> -}; > >>>> - > >>>> -®_dcdc5 { > >>>> - regulator-always-on; > >>>> - regulator-min-microvolt = <1500000>; > >>>> - regulator-max-microvolt = <1500000>; > >>>> - regulator-name = "vcc-dram"; > >>>> -}; > >>>> - > >>>> &uart0 { > >>>> pinctrl-names = "default"; > >>>> pinctrl-0 = <&uart0_pins_a>; > >>>> -- > >>>> 2.15.1 > >>>> > >>>> > >>>> _______________________________________________ > >>>> linux-arm-kernel mailing list > >>>> linux-arm-kernel@lists.infradead.org > >>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > >> > >> > >> _______________________________________________ > >> linux-arm-kernel mailing list > >> linux-arm-kernel@lists.infradead.org > >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > > > > > -- > > Emmanuel Vadot > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- Emmanuel Vadot