Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
DomainKey-Signature: a=rsa-sha1; c=nofws; d=bidouilliste.com; h=
        mime-version:content-type:content-transfer-encoding:date:from:to
        :cc:subject:in-reply-to:references:message-id; q=dns; s=mail; b=
        hkb42g0oIOjTGoIxJKE7rBUnLZs/ND3Heq90wHXz6jygPolklHuestv5fNZrMgNI
        VS6PuccNHcoPZd8b7VJbyTT4Gmf6iTKclu9pG+CbzxfHO57TJt6KbbEqYegwTlFl
        M0Qgg50i1v4dSR13is+ABJNAppWIZrkhsa5XevV9Lg4=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8;
 format=flowed
Content-Transfer-Encoding: 8bit
Date:   Fri, 09 Feb 2018 22:20:57 +0100
From:   Emmanuel Vadot <manu@bidouilliste.com>
To:     Icenowy Zheng <icenowy@aosc.io>
Cc:     devicetree@vger.kernel.org,
        Maxime Ripard <maxime.ripard@bootlin.com>,
        linux-sunxi@googlegroups.com, linux-kernel@vger.kernel.org,
        Chen-Yu Tsai <wens@csie.org>,
        linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] Revert "ARM: dts: sunxi: Add regulators for Sinovoip
 BPI-M2"
Organization: Bidouilliste
In-Reply-To: <207CD143-6535-42C9-BEF3-9D2614336482@aosc.io>
References: <20180203112353.13497-1-icenowy@aosc.io>
 <20180205095558.2e713c24cdb7c3232943db52@bidouilliste.com>
 <207CD143-6535-42C9-BEF3-9D2614336482@aosc.io>
Message-ID: <c59ee4bd3246172ec62705967bb8f751@megadrive.org>
User-Agent: Roundcube Webmail/1.1.1
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 2018-02-05 10:05, Icenowy Zheng wrote:
> 于 2018年2月5日 GMT+08:00 下午4:55:58, Emmanuel Vadot <manu@bidouilliste.com> 
> 写到:
>> 
>> Hello,
>> 
>> On Sat,  3 Feb 2018 19:23:53 +0800
>> Icenowy Zheng <icenowy@aosc.io> 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.

> 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.

>> 
>> 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 <icenowy@aosc.io>
>>> ---
>>>  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 = <&reg_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"
>>> -
>>> -&reg_dc5ldo {
>>> -	regulator-min-microvolt = <700000>;
>>> -	regulator-max-microvolt = <1320000>;
>>> -	regulator-name = "vdd-cpus";
>>> -};
>>> -
>>> -&reg_dcdc1 {
>>> -	regulator-always-on;
>>> -	regulator-min-microvolt = <3000000>;
>>> -	regulator-max-microvolt = <3000000>;
>>> -	regulator-name = "vdd-3v0";
>>> -};
>>> -
>>> -&reg_dcdc2 {
>>> -	regulator-min-microvolt = <700000>;
>>> -	regulator-max-microvolt = <1320000>;
>>> -	regulator-name = "vdd-gpu";
>>> -};
>>> -
>>> -&reg_dcdc3 {
>>> -	regulator-always-on;
>>> -	regulator-min-microvolt = <700000>;
>>> -	regulator-max-microvolt = <1320000>;
>>> -	regulator-name = "vdd-cpu";
>>> -};
>>> -
>>> -&reg_dcdc4 {
>>> -	regulator-always-on;
>>> -	regulator-min-microvolt = <700000>;
>>> -	regulator-max-microvolt = <1320000>;
>>> -	regulator-name = "vdd-sys-dll";
>>> -};
>>> -
>>> -&reg_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 <manu@bidouilliste.com> <manu@freebsd.org>