From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752083AbdESIzi (ORCPT ); Fri, 19 May 2017 04:55:38 -0400 Received: from foss.arm.com ([217.140.101.70]:41054 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751274AbdESIzg (ORCPT ); Fri, 19 May 2017 04:55:36 -0400 Subject: Re: [linux-sunxi] [PATCH v6 8/9] arm64: allwinner: a64: enable AXP803 regulators for Pine64 To: Icenowy Zheng , Thomas Gleixner , Jason Cooper , Marc Zyngier , Rob Herring , Maxime Ripard , Chen-Yu Tsai , Lee Jones , Liam Girdwood , Mark Brown Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-sunxi@googlegroups.com References: <20170518071653.36561-1-icenowy@aosc.io> <20170518071653.36561-9-icenowy@aosc.io> <98ae1ea8-ef08-8859-63d8-57c56a6348a6@arm.com> <4B129FD0-4AEB-4DC3-AF33-C279468BD197@aosc.io> From: Andre Przywara Message-ID: <77045320-c19a-6c18-ed68-108fa5416c2a@arm.com> Date: Fri, 19 May 2017 09:55:32 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 MIME-Version: 1.0 In-Reply-To: <4B129FD0-4AEB-4DC3-AF33-C279468BD197@aosc.io> Content-Type: text/plain; charset=utf-8 Content-Language: en-GB Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On 19/05/17 09:29, Icenowy Zheng wrote: > > > 于 2017年5月19日 GMT+08:00 下午4:27:21, Andre Przywara 写到: >> Hi, >> >> On 18/05/17 08:16, Icenowy Zheng wrote: >>> Add support of AXP803 regulators in the Pine64 device tree, in order >> to >>> enable many future functionalities, e.g. Wi-Fi. >>> >>> Signed-off-by: Icenowy Zheng >>> --- >>> Changes in v6: >>> - Rebased on next-20170517. >>> >>> .../arm64/boot/dts/allwinner/sun50i-a64-pine64.dts | 109 >> +++++++++++++++++++++ >>> 1 file changed, 109 insertions(+) >>> >>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts >> b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts >>> index 36001884ed33..40921bacb39c 100644 >>> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts >>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts >>> @@ -118,6 +118,115 @@ >>> }; >>> }; >>> >>> +#include "axp803.dtsi" >>> + >>> +®_aldo1 { >>> + regulator-min-microvolt = <2800000>; >>> + regulator-max-microvolt = <2800000>; >>> + regulator-name = "vcc-csi"; >>> +}; >>> + >>> +®_aldo2 { >>> + regulator-always-on; >>> + regulator-min-microvolt = <1800000>; >>> + regulator-max-microvolt = <3300000>; >>> + regulator-name = "vcc-pl"; >>> +}; >>> + >>> +®_aldo3 { >>> + regulator-always-on; >>> + regulator-min-microvolt = <2700000>; >>> + regulator-max-microvolt = <3300000>; >>> + regulator-name = "vcc-pll-avcc"; >>> +}; The schematic puts this at a fixed 3.0V, so why the range here? Are we expected to tune the voltage of the PLLs? >>> + >>> +®_dc1sw { >>> + regulator-name = "vcc-phy"; >>> +}; >>> + >>> +®_dcdc1 { >>> + regulator-always-on; >>> + regulator-min-microvolt = <3300000>; >>> + regulator-max-microvolt = <3300000>; >>> + regulator-name = "vcc-3v3"; >>> +}; >>> + >>> +®_dcdc2 { >>> + regulator-always-on; >>> + regulator-min-microvolt = <1000000>; >>> + regulator-max-microvolt = <1300000>; >>> + regulator-name = "vdd-cpux"; >>> +}; >>> + >>> +/* DCDC3 is polyphased with DCDC2 */ >>> + >>> +®_dcdc5 { >>> + regulator-always-on; >>> + regulator-min-microvolt = <1500000>; >>> + regulator-max-microvolt = <1500000>; >>> + regulator-name = "vcc-dram"; >>> +}; >> >> I think I mentioned this before, but the Pine64 has DDR3L DRAM, which >> is >> specified to run at 1.35V (1.36V with the 20mV granularity of the AXP). >> The reset value is even (wrongly?) configured to 1.24V. >> >> So is there any reason you set the voltage to 1.5V? Is that what the >> BSP >> does? Or did you see any problems with 1.36V? > > I just set it based on the schematics. I wouldn't trust the schematics too much. They are rather generic, see the Ethernet page, for instance, showing *different* PHYs, not just the ones used. For the DRAM the Pine64 schematic does not even tell the DRAM chip used, the name under the chip is just describing the package. > And 1.35v cannot be accurately achieved by dcdc5 and it's a problem whether to use 1.34v or 1.36v ;-) Well, as I wrote above, 1.36V is the voltage to go with. I think 10mV more is well within the tolerance ;-) >>> +®_dcdc6 { >>> + regulator-always-on; >>> + regulator-min-microvolt = <1100000>; >>> + regulator-max-microvolt = <1100000>; >>> + regulator-name = "vdd-sys"; >>> +}; >>> + >>> +®_dldo1 { >>> + regulator-min-microvolt = <3300000>; >>> + regulator-max-microvolt = <3300000>; >>> + regulator-name = "vcc-hdmi"; >>> +}; >>> + >>> +®_dldo2 { >>> + regulator-min-microvolt = <3300000>; >>> + regulator-max-microvolt = <3300000>; >>> + regulator-name = "vcc-mipi"; >>> +}; >>> + >>> +®_dldo3 { >>> + regulator-min-microvolt = <3300000>; >>> + regulator-max-microvolt = <3300000>; >>> + regulator-name = "avdd-csi"; >>> +}; If you stick to the schematic, this should be 2.8V. >>> + >>> +®_dldo4 { >>> + regulator-min-microvolt = <3300000>; >>> + regulator-max-microvolt = <3300000>; >>> + regulator-name = "vcc-wifi"; >>> +}; >>> + >>> +®_eldo1 { >>> + regulator-min-microvolt = <1800000>; >>> + regulator-max-microvolt = <1800000>; >>> + regulator-name = "cpvdd"; >>> +}; >>> + >>> +®_eldo3 { >>> + regulator-min-microvolt = <1800000>; >>> + regulator-max-microvolt = <1800000>; >>> + regulator-name = "vdd-1v8-csi"; >>> +}; The schematic lists 1.2V/1.5V/1.8V here, so should we have a range? >>> + >>> +®_fldo1 { >>> + regulator-min-microvolt = <1200000>; >>> + regulator-max-microvolt = <1200000>; >>> + regulator-name = "vcc-1v2-hsic"; >>> +}; >>> + >>> +®_fldo2 { >>> + regulator-always-on; Why do we need to turn this on? Upstream firmware does not use the arisc, so it can stay off. Also in general I think Linux should not tinker with the management processor at all. Cheers, Andre. >>> + regulator-min-microvolt = <1100000>; >>> + regulator-max-microvolt = <1100000>; >>> + regulator-name = "vdd-cpus"; >>> +}; >>> + >>> +®_rtc_ldo { >>> + regulator-name = "vcc-rtc"; >>> +}; >>> + >>> /* On Exp and Euler connectors */ >>> &uart0 { >>> pinctrl-names = "default"; >>> From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andre Przywara Subject: Re: [linux-sunxi] [PATCH v6 8/9] arm64: allwinner: a64: enable AXP803 regulators for Pine64 Date: Fri, 19 May 2017 09:55:32 +0100 Message-ID: <77045320-c19a-6c18-ed68-108fa5416c2a@arm.com> References: <20170518071653.36561-1-icenowy@aosc.io> <20170518071653.36561-9-icenowy@aosc.io> <98ae1ea8-ef08-8859-63d8-57c56a6348a6@arm.com> <4B129FD0-4AEB-4DC3-AF33-C279468BD197@aosc.io> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Return-path: In-Reply-To: <4B129FD0-4AEB-4DC3-AF33-C279468BD197-h8G6r0blFSE@public.gmane.org> Content-Language: en-GB Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Icenowy Zheng , Thomas Gleixner , Jason Cooper , Marc Zyngier , Rob Herring , Maxime Ripard , Chen-Yu Tsai , Lee Jones , Liam Girdwood , Mark Brown Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org List-Id: devicetree@vger.kernel.org Hi, On 19/05/17 09:29, Icenowy Zheng wrote: > > > 于 2017年5月19日 GMT+08:00 下午4:27:21, Andre Przywara 写到: >> Hi, >> >> On 18/05/17 08:16, Icenowy Zheng wrote: >>> Add support of AXP803 regulators in the Pine64 device tree, in order >> to >>> enable many future functionalities, e.g. Wi-Fi. >>> >>> Signed-off-by: Icenowy Zheng >>> --- >>> Changes in v6: >>> - Rebased on next-20170517. >>> >>> .../arm64/boot/dts/allwinner/sun50i-a64-pine64.dts | 109 >> +++++++++++++++++++++ >>> 1 file changed, 109 insertions(+) >>> >>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts >> b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts >>> index 36001884ed33..40921bacb39c 100644 >>> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts >>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts >>> @@ -118,6 +118,115 @@ >>> }; >>> }; >>> >>> +#include "axp803.dtsi" >>> + >>> +®_aldo1 { >>> + regulator-min-microvolt = <2800000>; >>> + regulator-max-microvolt = <2800000>; >>> + regulator-name = "vcc-csi"; >>> +}; >>> + >>> +®_aldo2 { >>> + regulator-always-on; >>> + regulator-min-microvolt = <1800000>; >>> + regulator-max-microvolt = <3300000>; >>> + regulator-name = "vcc-pl"; >>> +}; >>> + >>> +®_aldo3 { >>> + regulator-always-on; >>> + regulator-min-microvolt = <2700000>; >>> + regulator-max-microvolt = <3300000>; >>> + regulator-name = "vcc-pll-avcc"; >>> +}; The schematic puts this at a fixed 3.0V, so why the range here? Are we expected to tune the voltage of the PLLs? >>> + >>> +®_dc1sw { >>> + regulator-name = "vcc-phy"; >>> +}; >>> + >>> +®_dcdc1 { >>> + regulator-always-on; >>> + regulator-min-microvolt = <3300000>; >>> + regulator-max-microvolt = <3300000>; >>> + regulator-name = "vcc-3v3"; >>> +}; >>> + >>> +®_dcdc2 { >>> + regulator-always-on; >>> + regulator-min-microvolt = <1000000>; >>> + regulator-max-microvolt = <1300000>; >>> + regulator-name = "vdd-cpux"; >>> +}; >>> + >>> +/* DCDC3 is polyphased with DCDC2 */ >>> + >>> +®_dcdc5 { >>> + regulator-always-on; >>> + regulator-min-microvolt = <1500000>; >>> + regulator-max-microvolt = <1500000>; >>> + regulator-name = "vcc-dram"; >>> +}; >> >> I think I mentioned this before, but the Pine64 has DDR3L DRAM, which >> is >> specified to run at 1.35V (1.36V with the 20mV granularity of the AXP). >> The reset value is even (wrongly?) configured to 1.24V. >> >> So is there any reason you set the voltage to 1.5V? Is that what the >> BSP >> does? Or did you see any problems with 1.36V? > > I just set it based on the schematics. I wouldn't trust the schematics too much. They are rather generic, see the Ethernet page, for instance, showing *different* PHYs, not just the ones used. For the DRAM the Pine64 schematic does not even tell the DRAM chip used, the name under the chip is just describing the package. > And 1.35v cannot be accurately achieved by dcdc5 and it's a problem whether to use 1.34v or 1.36v ;-) Well, as I wrote above, 1.36V is the voltage to go with. I think 10mV more is well within the tolerance ;-) >>> +®_dcdc6 { >>> + regulator-always-on; >>> + regulator-min-microvolt = <1100000>; >>> + regulator-max-microvolt = <1100000>; >>> + regulator-name = "vdd-sys"; >>> +}; >>> + >>> +®_dldo1 { >>> + regulator-min-microvolt = <3300000>; >>> + regulator-max-microvolt = <3300000>; >>> + regulator-name = "vcc-hdmi"; >>> +}; >>> + >>> +®_dldo2 { >>> + regulator-min-microvolt = <3300000>; >>> + regulator-max-microvolt = <3300000>; >>> + regulator-name = "vcc-mipi"; >>> +}; >>> + >>> +®_dldo3 { >>> + regulator-min-microvolt = <3300000>; >>> + regulator-max-microvolt = <3300000>; >>> + regulator-name = "avdd-csi"; >>> +}; If you stick to the schematic, this should be 2.8V. >>> + >>> +®_dldo4 { >>> + regulator-min-microvolt = <3300000>; >>> + regulator-max-microvolt = <3300000>; >>> + regulator-name = "vcc-wifi"; >>> +}; >>> + >>> +®_eldo1 { >>> + regulator-min-microvolt = <1800000>; >>> + regulator-max-microvolt = <1800000>; >>> + regulator-name = "cpvdd"; >>> +}; >>> + >>> +®_eldo3 { >>> + regulator-min-microvolt = <1800000>; >>> + regulator-max-microvolt = <1800000>; >>> + regulator-name = "vdd-1v8-csi"; >>> +}; The schematic lists 1.2V/1.5V/1.8V here, so should we have a range? >>> + >>> +®_fldo1 { >>> + regulator-min-microvolt = <1200000>; >>> + regulator-max-microvolt = <1200000>; >>> + regulator-name = "vcc-1v2-hsic"; >>> +}; >>> + >>> +®_fldo2 { >>> + regulator-always-on; Why do we need to turn this on? Upstream firmware does not use the arisc, so it can stay off. Also in general I think Linux should not tinker with the management processor at all. Cheers, Andre. >>> + regulator-min-microvolt = <1100000>; >>> + regulator-max-microvolt = <1100000>; >>> + regulator-name = "vdd-cpus"; >>> +}; >>> + >>> +®_rtc_ldo { >>> + regulator-name = "vcc-rtc"; >>> +}; >>> + >>> /* On Exp and Euler connectors */ >>> &uart0 { >>> pinctrl-names = "default"; >>> -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 From: andre.przywara@arm.com (Andre Przywara) Date: Fri, 19 May 2017 09:55:32 +0100 Subject: [linux-sunxi] [PATCH v6 8/9] arm64: allwinner: a64: enable AXP803 regulators for Pine64 In-Reply-To: <4B129FD0-4AEB-4DC3-AF33-C279468BD197@aosc.io> References: <20170518071653.36561-1-icenowy@aosc.io> <20170518071653.36561-9-icenowy@aosc.io> <98ae1ea8-ef08-8859-63d8-57c56a6348a6@arm.com> <4B129FD0-4AEB-4DC3-AF33-C279468BD197@aosc.io> Message-ID: <77045320-c19a-6c18-ed68-108fa5416c2a@arm.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi, On 19/05/17 09:29, Icenowy Zheng wrote: > > > ? 2017?5?19? GMT+08:00 ??4:27:21, Andre Przywara ??: >> Hi, >> >> On 18/05/17 08:16, Icenowy Zheng wrote: >>> Add support of AXP803 regulators in the Pine64 device tree, in order >> to >>> enable many future functionalities, e.g. Wi-Fi. >>> >>> Signed-off-by: Icenowy Zheng >>> --- >>> Changes in v6: >>> - Rebased on next-20170517. >>> >>> .../arm64/boot/dts/allwinner/sun50i-a64-pine64.dts | 109 >> +++++++++++++++++++++ >>> 1 file changed, 109 insertions(+) >>> >>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts >> b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts >>> index 36001884ed33..40921bacb39c 100644 >>> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts >>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts >>> @@ -118,6 +118,115 @@ >>> }; >>> }; >>> >>> +#include "axp803.dtsi" >>> + >>> +®_aldo1 { >>> + regulator-min-microvolt = <2800000>; >>> + regulator-max-microvolt = <2800000>; >>> + regulator-name = "vcc-csi"; >>> +}; >>> + >>> +®_aldo2 { >>> + regulator-always-on; >>> + regulator-min-microvolt = <1800000>; >>> + regulator-max-microvolt = <3300000>; >>> + regulator-name = "vcc-pl"; >>> +}; >>> + >>> +®_aldo3 { >>> + regulator-always-on; >>> + regulator-min-microvolt = <2700000>; >>> + regulator-max-microvolt = <3300000>; >>> + regulator-name = "vcc-pll-avcc"; >>> +}; The schematic puts this at a fixed 3.0V, so why the range here? Are we expected to tune the voltage of the PLLs? >>> + >>> +®_dc1sw { >>> + regulator-name = "vcc-phy"; >>> +}; >>> + >>> +®_dcdc1 { >>> + regulator-always-on; >>> + regulator-min-microvolt = <3300000>; >>> + regulator-max-microvolt = <3300000>; >>> + regulator-name = "vcc-3v3"; >>> +}; >>> + >>> +®_dcdc2 { >>> + regulator-always-on; >>> + regulator-min-microvolt = <1000000>; >>> + regulator-max-microvolt = <1300000>; >>> + regulator-name = "vdd-cpux"; >>> +}; >>> + >>> +/* DCDC3 is polyphased with DCDC2 */ >>> + >>> +®_dcdc5 { >>> + regulator-always-on; >>> + regulator-min-microvolt = <1500000>; >>> + regulator-max-microvolt = <1500000>; >>> + regulator-name = "vcc-dram"; >>> +}; >> >> I think I mentioned this before, but the Pine64 has DDR3L DRAM, which >> is >> specified to run at 1.35V (1.36V with the 20mV granularity of the AXP). >> The reset value is even (wrongly?) configured to 1.24V. >> >> So is there any reason you set the voltage to 1.5V? Is that what the >> BSP >> does? Or did you see any problems with 1.36V? > > I just set it based on the schematics. I wouldn't trust the schematics too much. They are rather generic, see the Ethernet page, for instance, showing *different* PHYs, not just the ones used. For the DRAM the Pine64 schematic does not even tell the DRAM chip used, the name under the chip is just describing the package. > And 1.35v cannot be accurately achieved by dcdc5 and it's a problem whether to use 1.34v or 1.36v ;-) Well, as I wrote above, 1.36V is the voltage to go with. I think 10mV more is well within the tolerance ;-) >>> +®_dcdc6 { >>> + regulator-always-on; >>> + regulator-min-microvolt = <1100000>; >>> + regulator-max-microvolt = <1100000>; >>> + regulator-name = "vdd-sys"; >>> +}; >>> + >>> +®_dldo1 { >>> + regulator-min-microvolt = <3300000>; >>> + regulator-max-microvolt = <3300000>; >>> + regulator-name = "vcc-hdmi"; >>> +}; >>> + >>> +®_dldo2 { >>> + regulator-min-microvolt = <3300000>; >>> + regulator-max-microvolt = <3300000>; >>> + regulator-name = "vcc-mipi"; >>> +}; >>> + >>> +®_dldo3 { >>> + regulator-min-microvolt = <3300000>; >>> + regulator-max-microvolt = <3300000>; >>> + regulator-name = "avdd-csi"; >>> +}; If you stick to the schematic, this should be 2.8V. >>> + >>> +®_dldo4 { >>> + regulator-min-microvolt = <3300000>; >>> + regulator-max-microvolt = <3300000>; >>> + regulator-name = "vcc-wifi"; >>> +}; >>> + >>> +®_eldo1 { >>> + regulator-min-microvolt = <1800000>; >>> + regulator-max-microvolt = <1800000>; >>> + regulator-name = "cpvdd"; >>> +}; >>> + >>> +®_eldo3 { >>> + regulator-min-microvolt = <1800000>; >>> + regulator-max-microvolt = <1800000>; >>> + regulator-name = "vdd-1v8-csi"; >>> +}; The schematic lists 1.2V/1.5V/1.8V here, so should we have a range? >>> + >>> +®_fldo1 { >>> + regulator-min-microvolt = <1200000>; >>> + regulator-max-microvolt = <1200000>; >>> + regulator-name = "vcc-1v2-hsic"; >>> +}; >>> + >>> +®_fldo2 { >>> + regulator-always-on; Why do we need to turn this on? Upstream firmware does not use the arisc, so it can stay off. Also in general I think Linux should not tinker with the management processor at all. Cheers, Andre. >>> + regulator-min-microvolt = <1100000>; >>> + regulator-max-microvolt = <1100000>; >>> + regulator-name = "vdd-cpus"; >>> +}; >>> + >>> +®_rtc_ldo { >>> + regulator-name = "vcc-rtc"; >>> +}; >>> + >>> /* On Exp and Euler connectors */ >>> &uart0 { >>> pinctrl-names = "default"; >>>