* [PATCH] Revert "ARM: dts: sunxi: Add regulators for Sinovoip BPI-M2" @ 2018-02-03 11:23 Icenowy Zheng [not found] ` <20180203112353.13497-1-icenowy-h8G6r0blFSE@public.gmane.org> 0 siblings, 1 reply; 7+ messages in thread From: Icenowy Zheng @ 2018-02-03 11:23 UTC (permalink / raw) To: Maxime Ripard, Chen-Yu Tsai Cc: devicetree, linux-arm-kernel, linux-kernel, linux-sunxi, Icenowy Zheng 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. - GMAC (which uses dldo1 and aldo2) and Wi-Fi (which uses aldo1) are not considered, and will fail to work after adding this commit. This indicates that this patch should be not tested at all. 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 = <®_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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
[parent not found: <20180203112353.13497-1-icenowy-h8G6r0blFSE@public.gmane.org>]
* Re: [PATCH] Revert "ARM: dts: sunxi: Add regulators for Sinovoip BPI-M2" [not found] ` <20180203112353.13497-1-icenowy-h8G6r0blFSE@public.gmane.org> @ 2018-02-05 8:55 ` Emmanuel Vadot [not found] ` <20180205095558.2e713c24cdb7c3232943db52-xXdDKFdH5B3kFDPD4ZthVA@public.gmane.org> 0 siblings, 1 reply; 7+ messages in thread From: Emmanuel Vadot @ 2018-02-05 8:55 UTC (permalink / raw) To: Icenowy Zheng Cc: Maxime Ripard, Chen-Yu Tsai, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r Hello, On Sat, 3 Feb 2018 19:23:53 +0800 Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> 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). > 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. P.S.: Also as I'm the original sender I think I should have been in CC no ? Cheers, > Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> > --- > 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-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- Emmanuel Vadot <manu-xXdDKFdH5B3kFDPD4ZthVA@public.gmane.org> <manu-h+KGxgPPiopAfugRpC6u6w@public.gmane.org> -- 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <20180205095558.2e713c24cdb7c3232943db52-xXdDKFdH5B3kFDPD4ZthVA@public.gmane.org>]
* Re: [PATCH] Revert "ARM: dts: sunxi: Add regulators for Sinovoip BPI-M2" [not found] ` <20180205095558.2e713c24cdb7c3232943db52-xXdDKFdH5B3kFDPD4ZthVA@public.gmane.org> @ 2018-02-05 9:05 ` Icenowy Zheng 2018-02-09 21:20 ` Emmanuel Vadot 0 siblings, 1 reply; 7+ messages in thread From: Icenowy Zheng @ 2018-02-05 9:05 UTC (permalink / raw) To: Emmanuel Vadot Cc: Maxime Ripard, Chen-Yu Tsai, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r 于 2018年2月5日 GMT+08:00 下午4:55:58, Emmanuel Vadot <manu-xXdDKFdH5B3kFDPD4ZthVA@public.gmane.org> 写到: > > Hello, > >On Sat, 3 Feb 2018 19:23:53 +0800 >Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> 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. 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 :-) > > 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-h8G6r0blFSE@public.gmane.org> >> --- >> 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-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Revert "ARM: dts: sunxi: Add regulators for Sinovoip BPI-M2" 2018-02-05 9:05 ` Icenowy Zheng @ 2018-02-09 21:20 ` Emmanuel Vadot 2018-02-13 10:30 ` Maxime Ripard [not found] ` <c59ee4bd3246172ec62705967bb8f751-/Q/jCV+WVIbNLxjTenLetw@public.gmane.org> 0 siblings, 2 replies; 7+ messages in thread From: Emmanuel Vadot @ 2018-02-09 21:20 UTC (permalink / raw) To: Icenowy Zheng Cc: devicetree, Maxime Ripard, Chen-Yu Tsai, linux-kernel, linux-sunxi, linux-arm-kernel 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 = <®_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 <manu@bidouilliste.com> <manu@freebsd.org> _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Revert "ARM: dts: sunxi: Add regulators for Sinovoip BPI-M2" 2018-02-09 21:20 ` Emmanuel Vadot @ 2018-02-13 10:30 ` Maxime Ripard [not found] ` <c59ee4bd3246172ec62705967bb8f751-/Q/jCV+WVIbNLxjTenLetw@public.gmane.org> 1 sibling, 0 replies; 7+ messages in thread From: Maxime Ripard @ 2018-02-13 10:30 UTC (permalink / raw) To: Emmanuel Vadot Cc: Icenowy Zheng, devicetree, linux-sunxi, linux-kernel, Chen-Yu Tsai, linux-arm-kernel [-- Attachment #1: Type: text/plain, Size: 868 bytes --] On Fri, Feb 09, 2018 at 10:20:57PM +0100, Emmanuel Vadot wrote: > > 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. Adhering to a binding and being functional is a completely different story. And the latter is the most important. Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering http://bootlin.com [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <c59ee4bd3246172ec62705967bb8f751-/Q/jCV+WVIbNLxjTenLetw@public.gmane.org>]
* Re: [PATCH] Revert "ARM: dts: sunxi: Add regulators for Sinovoip BPI-M2" [not found] ` <c59ee4bd3246172ec62705967bb8f751-/Q/jCV+WVIbNLxjTenLetw@public.gmane.org> @ 2018-02-13 10:36 ` Chen-Yu Tsai 2018-02-14 19:16 ` Emmanuel Vadot 0 siblings, 1 reply; 7+ messages in thread From: Chen-Yu Tsai @ 2018-02-13 10:36 UTC (permalink / raw) To: Emmanuel Vadot Cc: Icenowy Zheng, devicetree, Maxime Ripard, linux-sunxi, linux-kernel, linux-arm-kernel On Sat, Feb 10, 2018 at 5:20 AM, Emmanuel Vadot <manu-xXdDKFdH5B3kFDPD4ZthVA@public.gmane.org> wrote: > On 2018-02-05 10:05, Icenowy Zheng wrote: >> >> 于 2018年2月5日 GMT+08:00 下午4:55:58, Emmanuel Vadot <manu-xXdDKFdH5B3kFDPD4ZthVA@public.gmane.org> >> 写到: >>> >>> >>> Hello, >>> >>> On Sat, 3 Feb 2018 19:23:53 +0800 >>> Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> 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. > >> 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. 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 <icenowy-h8G6r0blFSE@public.gmane.org> >>>> --- >>>> 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-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org >>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >> >> >> _______________________________________________ >> linux-arm-kernel mailing list >> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > > -- > Emmanuel Vadot <manu-xXdDKFdH5B3kFDPD4ZthVA@public.gmane.org> <manu-h+KGxgPPiopAfugRpC6u6w@public.gmane.org> -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Revert "ARM: dts: sunxi: Add regulators for Sinovoip BPI-M2" 2018-02-13 10:36 ` Chen-Yu Tsai @ 2018-02-14 19:16 ` Emmanuel Vadot 0 siblings, 0 replies; 7+ messages in thread From: Emmanuel Vadot @ 2018-02-14 19:16 UTC (permalink / raw) To: Chen-Yu Tsai Cc: devicetree, Maxime Ripard, linux-kernel, linux-sunxi, linux-arm-kernel, Icenowy Zheng On Tue, 13 Feb 2018 18:36:24 +0800 Chen-Yu Tsai <wens@csie.org> wrote: > On Sat, Feb 10, 2018 at 5:20 AM, Emmanuel Vadot <manu@bidouilliste.com> wrote: > > 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. > > 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 <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 = <®_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 <manu@bidouilliste.com> <manu@freebsd.org> > > _______________________________________________ > 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> ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-02-14 19:16 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-02-03 11:23 [PATCH] Revert "ARM: dts: sunxi: Add regulators for Sinovoip BPI-M2" Icenowy Zheng [not found] ` <20180203112353.13497-1-icenowy-h8G6r0blFSE@public.gmane.org> 2018-02-05 8:55 ` Emmanuel Vadot [not found] ` <20180205095558.2e713c24cdb7c3232943db52-xXdDKFdH5B3kFDPD4ZthVA@public.gmane.org> 2018-02-05 9:05 ` Icenowy Zheng 2018-02-09 21:20 ` Emmanuel Vadot 2018-02-13 10:30 ` Maxime Ripard [not found] ` <c59ee4bd3246172ec62705967bb8f751-/Q/jCV+WVIbNLxjTenLetw@public.gmane.org> 2018-02-13 10:36 ` Chen-Yu Tsai 2018-02-14 19:16 ` Emmanuel Vadot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).