* [PATCH v2 1/3] ARM: dts: exynos: Define vqmmc for SD card and allow disabling regulators on Odroid X/X2/U3
@ 2016-04-27 14:00 Krzysztof Kozlowski
2016-04-27 14:00 ` [PATCH v2 2/3] ARM: dts: exynos: Define vqmmc for eMMC card " Krzysztof Kozlowski
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Krzysztof Kozlowski @ 2016-04-27 14:00 UTC (permalink / raw)
To: Kukjin Kim, Krzysztof Kozlowski, devicetree, linux-arm-kernel,
linux-samsung-soc, linux-kernel, Tobias Jakobi, Marek Szyprowski
Cc: linux-mmc, linux.amoon, Bartlomiej Zolnierkiewicz
The SD-card vmmc-supply contained incorrectly two regulators. The second
one is ignored. Fix this by defining proper vmmc and vqmmc supplies.
Additionally these regulators do not have to be always on, so allow
disabling them to reduce energy consumption.
Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
Changes since v1:
1. None
---
arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
index cab0f07d7d28..3d0d44581fbd 100644
--- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
+++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
@@ -249,7 +249,6 @@
regulator-name = "VDDQ_MMC2_2.8V";
regulator-min-microvolt = <2800000>;
regulator-max-microvolt = <2800000>;
- regulator-always-on;
regulator-boot-on;
};
@@ -345,7 +344,6 @@
regulator-name = "LDO21_3.3V";
regulator-min-microvolt = <3300000>;
regulator-max-microvolt = <3300000>;
- regulator-always-on;
regulator-boot-on;
};
@@ -482,7 +480,8 @@
bus-width = <4>;
pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>;
pinctrl-names = "default";
- vmmc-supply = <&ldo4_reg &ldo21_reg>;
+ vmmc-supply = <&ldo21_reg>;
+ vqmmc-supply = <&ldo4_reg>;
cd-gpios = <&gpk2 2 GPIO_ACTIVE_HIGH>;
cd-inverted;
status = "okay";
--
1.9.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v2 2/3] ARM: dts: exynos: Define vqmmc for eMMC card on Odroid X/X2/U3 2016-04-27 14:00 [PATCH v2 1/3] ARM: dts: exynos: Define vqmmc for SD card and allow disabling regulators on Odroid X/X2/U3 Krzysztof Kozlowski @ 2016-04-27 14:00 ` Krzysztof Kozlowski 2016-04-27 21:29 ` Javier Martinez Canillas 2016-04-27 14:00 ` [PATCH v2 3/3] ARM: dts: exynos: Lower SD card interface voltage to 2.8v " Krzysztof Kozlowski 2016-04-27 21:09 ` [PATCH v2 1/3] ARM: dts: exynos: Define vqmmc for SD card and allow disabling regulators " Javier Martinez Canillas 2 siblings, 1 reply; 8+ messages in thread From: Krzysztof Kozlowski @ 2016-04-27 14:00 UTC (permalink / raw) To: Kukjin Kim, Krzysztof Kozlowski, devicetree, linux-arm-kernel, linux-samsung-soc, linux-kernel, Tobias Jakobi, Marek Szyprowski Cc: linux-mmc, linux.amoon, Bartlomiej Zolnierkiewicz The eMMC card vmmc-supply contained incorrectly two regulators: LDO20 and buck8. The second one is ignored. Additionally the buck8 is a vqmmc supply only on X and X2. On U3 the buck8 is providing power to the LAN (SMSC95xx) so instead the LDO22 should be used. Fix this by defining proper vmmc and vqmmc supplies for respective boards. Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> --- Changes since v1: 1. buck8 is used on X/X2 so differentiate the configuration (hint by Tobias Jakobi). --- arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 13 ++++++++++--- arch/arm/boot/dts/exynos4412-odroidu3.dts | 18 ++++++++++++++++++ arch/arm/boot/dts/exynos4412-odroidx.dts | 11 +++++++++++ arch/arm/boot/dts/exynos4412-odroidx2.dts | 11 +++++++++++ 4 files changed, 50 insertions(+), 3 deletions(-) diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi index 3d0d44581fbd..34a5b3daced0 100644 --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi @@ -347,6 +347,13 @@ regulator-boot-on; }; + ldo22_reg: LDO22 { + regulator-name = "LDO22"; + regulator-min-microvolt = <800000>; + regulator-max-microvolt = <3950000>; + regulator-boot-on; + }; + ldo25_reg: LDO25 { regulator-name = "VDDQ_LCD_1.8V"; regulator-min-microvolt = <1800000>; @@ -411,8 +418,8 @@ buck8_reg: BUCK8 { regulator-name = "BUCK8_2.8V"; - regulator-min-microvolt = <2800000>; - regulator-max-microvolt = <2800000>; + regulator-min-microvolt = <750000>; + regulator-max-microvolt = <3900000>; }; }; }; @@ -456,7 +463,7 @@ &mshc_0 { pinctrl-0 = <&sd4_clk &sd4_cmd &sd4_bus4 &sd4_bus8>; pinctrl-names = "default"; - vmmc-supply = <&ldo20_reg &buck8_reg>; + vmmc-supply = <&ldo20_reg>; mmc-pwrseq = <&emmc_pwrseq>; status = "okay"; diff --git a/arch/arm/boot/dts/exynos4412-odroidu3.dts b/arch/arm/boot/dts/exynos4412-odroidu3.dts index dd89f7b37c9f..d73aa6c58fe3 100644 --- a/arch/arm/boot/dts/exynos4412-odroidu3.dts +++ b/arch/arm/boot/dts/exynos4412-odroidu3.dts @@ -69,6 +69,24 @@ }; }; +/* Supply for LAN9730/SMSC95xx */ +&buck8_reg { + regulator-name = "BUCK8_P3V3"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; +}; + +/* VDDQ for MSHC (eMMC card) */ +&ldo22_reg { + regulator-name = "LDO22_VDDQ_MMC4_2.8V"; + regulator-min-microvolt = <2800000>; + regulator-max-microvolt = <2800000>; +}; + +&mshc_0 { + vqmmc-supply = <&ldo22_reg>; +}; + &pwm { pinctrl-0 = <&pwm0_out>; pinctrl-names = "default"; diff --git a/arch/arm/boot/dts/exynos4412-odroidx.dts b/arch/arm/boot/dts/exynos4412-odroidx.dts index bf7b21b817e4..2af235151301 100644 --- a/arch/arm/boot/dts/exynos4412-odroidx.dts +++ b/arch/arm/boot/dts/exynos4412-odroidx.dts @@ -63,12 +63,23 @@ }; }; +/* VDDQ for MSHC (eMMC card) */ +&buck8_reg { + regulator-name = "BUCK8_VDDQ_MMC4_2.8V"; + regulator-min-microvolt = <2800000>; + regulator-max-microvolt = <2800000>; +}; + &ehci { port@1 { status = "okay"; }; }; +&mshc_0 { + vqmmc-supply = <&buck8_reg>; +}; + &pinctrl_1 { gpio_home_key: home_key { samsung,pins = "gpx2-2"; diff --git a/arch/arm/boot/dts/exynos4412-odroidx2.dts b/arch/arm/boot/dts/exynos4412-odroidx2.dts index 6e33678562ae..3e3584270e00 100644 --- a/arch/arm/boot/dts/exynos4412-odroidx2.dts +++ b/arch/arm/boot/dts/exynos4412-odroidx2.dts @@ -22,6 +22,17 @@ }; }; +/* VDDQ for MSHC (eMMC card) */ +&buck8_reg { + regulator-name = "BUCK8_VDDQ_MMC4_2.8V"; + regulator-min-microvolt = <2800000>; + regulator-max-microvolt = <2800000>; +}; + +&mshc_0 { + vqmmc-supply = <&buck8_reg>; +}; + &sound { simple-audio-card,name = "Odroid-X2"; simple-audio-card,widgets = -- 1.9.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/3] ARM: dts: exynos: Define vqmmc for eMMC card on Odroid X/X2/U3 2016-04-27 14:00 ` [PATCH v2 2/3] ARM: dts: exynos: Define vqmmc for eMMC card " Krzysztof Kozlowski @ 2016-04-27 21:29 ` Javier Martinez Canillas 2016-04-28 5:50 ` Krzysztof Kozlowski 0 siblings, 1 reply; 8+ messages in thread From: Javier Martinez Canillas @ 2016-04-27 21:29 UTC (permalink / raw) To: Krzysztof Kozlowski, Kukjin Kim, devicetree, linux-arm-kernel, linux-samsung-soc, linux-kernel, Tobias Jakobi, Marek Szyprowski Cc: linux-mmc, linux.amoon, Bartlomiej Zolnierkiewicz Hello Krzysztof, On 04/27/2016 10:00 AM, Krzysztof Kozlowski wrote: > The eMMC card vmmc-supply contained incorrectly two regulators: LDO20 > and buck8. The second one is ignored. Additionally the buck8 is a vqmmc > supply only on X and X2. On U3 the buck8 is providing power to the LAN > (SMSC95xx) so instead the LDO22 should be used. > > Fix this by defining proper vmmc and vqmmc supplies for respective > boards. > > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> > > --- > > Changes since v1: > 1. buck8 is used on X/X2 so differentiate the configuration (hint by > Tobias Jakobi). > --- > arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 13 ++++++++++--- > arch/arm/boot/dts/exynos4412-odroidu3.dts | 18 ++++++++++++++++++ > arch/arm/boot/dts/exynos4412-odroidx.dts | 11 +++++++++++ > arch/arm/boot/dts/exynos4412-odroidx2.dts | 11 +++++++++++ > 4 files changed, 50 insertions(+), 3 deletions(-) > > diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > index 3d0d44581fbd..34a5b3daced0 100644 > --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > @@ -347,6 +347,13 @@ > regulator-boot-on; > }; > > + ldo22_reg: LDO22 { > + regulator-name = "LDO22"; > + regulator-min-microvolt = <800000>; > + regulator-max-microvolt = <3950000>; > + regulator-boot-on; > + }; > + I don't have a datasheet for the max77686 but I guess these min and max values are the actual voltage range that the ldo22 regulator can support? If that's the case, then I don't think setting these are correct since the DT binding says that the regulator-{min,max}-microvolt properties describe the voltage range that consumers may set. I've seen Mark mention this many times, the last one I remember is at [0]. Probably would be better to leave the ldo22_reg definition to odroidu3.dts to avoid setting these constraint in a .dtsi file. > ldo25_reg: LDO25 { > regulator-name = "VDDQ_LCD_1.8V"; > regulator-min-microvolt = <1800000>; > @@ -411,8 +418,8 @@ > > buck8_reg: BUCK8 { > regulator-name = "BUCK8_2.8V"; > - regulator-min-microvolt = <2800000>; > - regulator-max-microvolt = <2800000>; > + regulator-min-microvolt = <750000>; > + regulator-max-microvolt = <3900000>; Same here, since not all boards have the same constraint for this regulator, it would be better to remove it from the .dtsi and let each dts to define it. > }; > }; > }; > @@ -456,7 +463,7 @@ > &mshc_0 { > pinctrl-0 = <&sd4_clk &sd4_cmd &sd4_bus4 &sd4_bus8>; > pinctrl-names = "default"; > - vmmc-supply = <&ldo20_reg &buck8_reg>; > + vmmc-supply = <&ldo20_reg>; > mmc-pwrseq = <&emmc_pwrseq>; > status = "okay"; > > diff --git a/arch/arm/boot/dts/exynos4412-odroidu3.dts b/arch/arm/boot/dts/exynos4412-odroidu3.dts > index dd89f7b37c9f..d73aa6c58fe3 100644 > --- a/arch/arm/boot/dts/exynos4412-odroidu3.dts > +++ b/arch/arm/boot/dts/exynos4412-odroidu3.dts > @@ -69,6 +69,24 @@ > }; > }; > > +/* Supply for LAN9730/SMSC95xx */ > +&buck8_reg { > + regulator-name = "BUCK8_P3V3"; I think it would be better to name it "BUCK8_3.3V" for consistency. > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > +}; > + > +/* VDDQ for MSHC (eMMC card) */ > +&ldo22_reg { > + regulator-name = "LDO22_VDDQ_MMC4_2.8V"; > + regulator-min-microvolt = <2800000>; > + regulator-max-microvolt = <2800000>; > +}; > + > +&mshc_0 { > + vqmmc-supply = <&ldo22_reg>; > +}; > + > &pwm { > pinctrl-0 = <&pwm0_out>; > pinctrl-names = "default"; > diff --git a/arch/arm/boot/dts/exynos4412-odroidx.dts b/arch/arm/boot/dts/exynos4412-odroidx.dts > index bf7b21b817e4..2af235151301 100644 > --- a/arch/arm/boot/dts/exynos4412-odroidx.dts > +++ b/arch/arm/boot/dts/exynos4412-odroidx.dts > @@ -63,12 +63,23 @@ > }; > }; > > +/* VDDQ for MSHC (eMMC card) */ > +&buck8_reg { > + regulator-name = "BUCK8_VDDQ_MMC4_2.8V"; > + regulator-min-microvolt = <2800000>; > + regulator-max-microvolt = <2800000>; > +}; > + > &ehci { > port@1 { > status = "okay"; > }; > }; > > +&mshc_0 { > + vqmmc-supply = <&buck8_reg>; > +}; > + > &pinctrl_1 { > gpio_home_key: home_key { > samsung,pins = "gpx2-2"; > diff --git a/arch/arm/boot/dts/exynos4412-odroidx2.dts b/arch/arm/boot/dts/exynos4412-odroidx2.dts > index 6e33678562ae..3e3584270e00 100644 > --- a/arch/arm/boot/dts/exynos4412-odroidx2.dts > +++ b/arch/arm/boot/dts/exynos4412-odroidx2.dts > @@ -22,6 +22,17 @@ > }; > }; > > +/* VDDQ for MSHC (eMMC card) */ > +&buck8_reg { > + regulator-name = "BUCK8_VDDQ_MMC4_2.8V"; > + regulator-min-microvolt = <2800000>; > + regulator-max-microvolt = <2800000>; > +}; > + > +&mshc_0 { > + vqmmc-supply = <&buck8_reg>; > +}; > + > &sound { > simple-audio-card,name = "Odroid-X2"; > simple-audio-card,widgets = > Rest looks good, so if you do those changes feel free to add: Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com> [0]: http://www.spinics.net/lists/linux-omap/msg127035.html Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/3] ARM: dts: exynos: Define vqmmc for eMMC card on Odroid X/X2/U3 2016-04-27 21:29 ` Javier Martinez Canillas @ 2016-04-28 5:50 ` Krzysztof Kozlowski 2016-04-28 6:21 ` Javier Martinez Canillas 0 siblings, 1 reply; 8+ messages in thread From: Krzysztof Kozlowski @ 2016-04-28 5:50 UTC (permalink / raw) To: Javier Martinez Canillas, Kukjin Kim, devicetree, linux-arm-kernel, linux-samsung-soc, linux-kernel, Tobias Jakobi, Marek Szyprowski Cc: linux-mmc, linux.amoon, Bartlomiej Zolnierkiewicz On 04/27/2016 11:29 PM, Javier Martinez Canillas wrote: > Hello Krzysztof, > > On 04/27/2016 10:00 AM, Krzysztof Kozlowski wrote: >> The eMMC card vmmc-supply contained incorrectly two regulators: LDO20 >> and buck8. The second one is ignored. Additionally the buck8 is a vqmmc >> supply only on X and X2. On U3 the buck8 is providing power to the LAN >> (SMSC95xx) so instead the LDO22 should be used. >> >> Fix this by defining proper vmmc and vqmmc supplies for respective >> boards. >> >> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> >> >> --- >> >> Changes since v1: >> 1. buck8 is used on X/X2 so differentiate the configuration (hint by >> Tobias Jakobi). >> --- >> arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 13 ++++++++++--- >> arch/arm/boot/dts/exynos4412-odroidu3.dts | 18 ++++++++++++++++++ >> arch/arm/boot/dts/exynos4412-odroidx.dts | 11 +++++++++++ >> arch/arm/boot/dts/exynos4412-odroidx2.dts | 11 +++++++++++ >> 4 files changed, 50 insertions(+), 3 deletions(-) >> >> diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi >> index 3d0d44581fbd..34a5b3daced0 100644 >> --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi >> +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi >> @@ -347,6 +347,13 @@ >> regulator-boot-on; >> }; >> >> + ldo22_reg: LDO22 { >> + regulator-name = "LDO22"; >> + regulator-min-microvolt = <800000>; >> + regulator-max-microvolt = <3950000>; >> + regulator-boot-on; >> + }; >> + > > I don't have a datasheet for the max77686 but I guess these min and max > values are the actual voltage range that the ldo22 regulator can support? Yes. > If that's the case, then I don't think setting these are correct since the > DT binding says that the regulator-{min,max}-microvolt properties describe > the voltage range that consumers may set. I've seen Mark mention this many > times, the last one I remember is at [0]. For LDO22 there is no consumer so these are the constraints. Leaving them undefined (here and in DTS for X/X2) would result in the same effect. The point is that these values are valid constraints. Please have in mind that the binding describe this as "smallest/largest voltage consumer may set" which is true for the code above. > > Probably would be better to leave the ldo22_reg definition to odroidu3.dts > to avoid setting these constraint in a .dtsi file. What about X/X2? >> ldo25_reg: LDO25 { >> regulator-name = "VDDQ_LCD_1.8V"; >> regulator-min-microvolt = <1800000>; >> @@ -411,8 +418,8 @@ >> >> buck8_reg: BUCK8 { >> regulator-name = "BUCK8_2.8V"; >> - regulator-min-microvolt = <2800000>; >> - regulator-max-microvolt = <2800000>; >> + regulator-min-microvolt = <750000>; >> + regulator-max-microvolt = <3900000>; > > Same here, since not all boards have the same constraint for this regulator, > it would be better to remove it from the .dtsi and let each dts to define it. Since all of the boards define them, it does not bring any difference having it here... so I can remove them. >> }; >> }; >> }; >> @@ -456,7 +463,7 @@ >> &mshc_0 { >> pinctrl-0 = <&sd4_clk &sd4_cmd &sd4_bus4 &sd4_bus8>; >> pinctrl-names = "default"; >> - vmmc-supply = <&ldo20_reg &buck8_reg>; >> + vmmc-supply = <&ldo20_reg>; >> mmc-pwrseq = <&emmc_pwrseq>; >> status = "okay"; >> >> diff --git a/arch/arm/boot/dts/exynos4412-odroidu3.dts b/arch/arm/boot/dts/exynos4412-odroidu3.dts >> index dd89f7b37c9f..d73aa6c58fe3 100644 >> --- a/arch/arm/boot/dts/exynos4412-odroidu3.dts >> +++ b/arch/arm/boot/dts/exynos4412-odroidu3.dts >> @@ -69,6 +69,24 @@ >> }; >> }; >> >> +/* Supply for LAN9730/SMSC95xx */ >> +&buck8_reg { >> + regulator-name = "BUCK8_P3V3"; > > I think it would be better to name it "BUCK8_3.3V" for consistency. Consistency of naming is nice but in the same time we want the regulator names to match the hardware because it is easier to read the code and check the schematics. On the schematics this is unfortunately called P3V3. :) > >> + regulator-min-microvolt = <3300000>; >> + regulator-max-microvolt = <3300000>; >> +}; >> + >> +/* VDDQ for MSHC (eMMC card) */ >> +&ldo22_reg { >> + regulator-name = "LDO22_VDDQ_MMC4_2.8V"; >> + regulator-min-microvolt = <2800000>; >> + regulator-max-microvolt = <2800000>; >> +}; >> + >> +&mshc_0 { >> + vqmmc-supply = <&ldo22_reg>; >> +}; >> + >> &pwm { >> pinctrl-0 = <&pwm0_out>; >> pinctrl-names = "default"; >> diff --git a/arch/arm/boot/dts/exynos4412-odroidx.dts b/arch/arm/boot/dts/exynos4412-odroidx.dts >> index bf7b21b817e4..2af235151301 100644 >> --- a/arch/arm/boot/dts/exynos4412-odroidx.dts >> +++ b/arch/arm/boot/dts/exynos4412-odroidx.dts >> @@ -63,12 +63,23 @@ >> }; >> }; >> >> +/* VDDQ for MSHC (eMMC card) */ >> +&buck8_reg { >> + regulator-name = "BUCK8_VDDQ_MMC4_2.8V"; >> + regulator-min-microvolt = <2800000>; >> + regulator-max-microvolt = <2800000>; >> +}; >> + >> &ehci { >> port@1 { >> status = "okay"; >> }; >> }; >> >> +&mshc_0 { >> + vqmmc-supply = <&buck8_reg>; >> +}; >> + >> &pinctrl_1 { >> gpio_home_key: home_key { >> samsung,pins = "gpx2-2"; >> diff --git a/arch/arm/boot/dts/exynos4412-odroidx2.dts b/arch/arm/boot/dts/exynos4412-odroidx2.dts >> index 6e33678562ae..3e3584270e00 100644 >> --- a/arch/arm/boot/dts/exynos4412-odroidx2.dts >> +++ b/arch/arm/boot/dts/exynos4412-odroidx2.dts >> @@ -22,6 +22,17 @@ >> }; >> }; >> >> +/* VDDQ for MSHC (eMMC card) */ >> +&buck8_reg { >> + regulator-name = "BUCK8_VDDQ_MMC4_2.8V"; >> + regulator-min-microvolt = <2800000>; >> + regulator-max-microvolt = <2800000>; >> +}; >> + >> +&mshc_0 { >> + vqmmc-supply = <&buck8_reg>; >> +}; >> + >> &sound { >> simple-audio-card,name = "Odroid-X2"; >> simple-audio-card,widgets = >> > > Rest looks good, so if you do those changes feel free to add: > > Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com> > > [0]: http://www.spinics.net/lists/linux-omap/msg127035.html Thanks for feedback! Krzysztof ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/3] ARM: dts: exynos: Define vqmmc for eMMC card on Odroid X/X2/U3 2016-04-28 5:50 ` Krzysztof Kozlowski @ 2016-04-28 6:21 ` Javier Martinez Canillas 0 siblings, 0 replies; 8+ messages in thread From: Javier Martinez Canillas @ 2016-04-28 6:21 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Javier Martinez Canillas, Kukjin Kim, devicetree, linux-arm-kernel, linux-samsung-soc, Linux Kernel, Tobias Jakobi, Marek Szyprowski, linux-mmc, Anand Moon, Bartlomiej Zolnierkiewicz Hello Krzysztof, On Thu, Apr 28, 2016 at 1:50 AM, Krzysztof Kozlowski <k.kozlowski@samsung.com> wrote: > On 04/27/2016 11:29 PM, Javier Martinez Canillas wrote: >> Hello Krzysztof, >> >> On 04/27/2016 10:00 AM, Krzysztof Kozlowski wrote: >>> The eMMC card vmmc-supply contained incorrectly two regulators: LDO20 >>> and buck8. The second one is ignored. Additionally the buck8 is a vqmmc >>> supply only on X and X2. On U3 the buck8 is providing power to the LAN >>> (SMSC95xx) so instead the LDO22 should be used. >>> >>> Fix this by defining proper vmmc and vqmmc supplies for respective >>> boards. >>> >>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> >>> >>> --- >>> >>> Changes since v1: >>> 1. buck8 is used on X/X2 so differentiate the configuration (hint by >>> Tobias Jakobi). >>> --- >>> arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 13 ++++++++++--- >>> arch/arm/boot/dts/exynos4412-odroidu3.dts | 18 ++++++++++++++++++ >>> arch/arm/boot/dts/exynos4412-odroidx.dts | 11 +++++++++++ >>> arch/arm/boot/dts/exynos4412-odroidx2.dts | 11 +++++++++++ >>> 4 files changed, 50 insertions(+), 3 deletions(-) >>> >>> diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi >>> index 3d0d44581fbd..34a5b3daced0 100644 >>> --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi >>> +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi >>> @@ -347,6 +347,13 @@ >>> regulator-boot-on; >>> }; >>> >>> + ldo22_reg: LDO22 { >>> + regulator-name = "LDO22"; >>> + regulator-min-microvolt = <800000>; >>> + regulator-max-microvolt = <3950000>; >>> + regulator-boot-on; >>> + }; >>> + >> >> I don't have a datasheet for the max77686 but I guess these min and max >> values are the actual voltage range that the ldo22 regulator can support? > > Yes. > >> If that's the case, then I don't think setting these are correct since the >> DT binding says that the regulator-{min,max}-microvolt properties describe >> the voltage range that consumers may set. I've seen Mark mention this many >> times, the last one I remember is at [0]. > > For LDO22 there is no consumer so these are the constraints. Leaving That was my point, there's no consumers for X/X2 and there is only a consumer in U3 (vqmmc-supply for mshc_0) added by $SUBJECT. That's why I thought that it should be defined in odroidu3.dts and removed from the .dtsi. > them undefined (here and in DTS for X/X2) would result in the same > effect. The point is that these values are valid constraints. Please > have in mind that the binding describe this as "smallest/largest voltage > consumer may set" which is true for the code above. > >> >> Probably would be better to leave the ldo22_reg definition to odroidu3.dts >> to avoid setting these constraint in a .dtsi file. > > What about X/X2? > Mmm, I see what you mean. If the regulators are not defined then these for example won't be turned off by the regulator subsystem if were enabled by the bootloader. Then I guess your change makes sense. >>> ldo25_reg: LDO25 { >>> regulator-name = "VDDQ_LCD_1.8V"; >>> regulator-min-microvolt = <1800000>; >>> @@ -411,8 +418,8 @@ >>> >>> buck8_reg: BUCK8 { >>> regulator-name = "BUCK8_2.8V"; >>> - regulator-min-microvolt = <2800000>; >>> - regulator-max-microvolt = <2800000>; >>> + regulator-min-microvolt = <750000>; >>> + regulator-max-microvolt = <3900000>; >> >> Same here, since not all boards have the same constraint for this regulator, >> it would be better to remove it from the .dtsi and let each dts to define it. > > Since all of the boards define them, it does not bring any difference > having it here... so I can remove them. > > Ok. I don't have a strong opinion on this though now that I realized that not defining some regulators could lead to unused ones not being disabled on boot. >>> }; >>> }; >>> }; >>> @@ -456,7 +463,7 @@ >>> &mshc_0 { >>> pinctrl-0 = <&sd4_clk &sd4_cmd &sd4_bus4 &sd4_bus8>; >>> pinctrl-names = "default"; >>> - vmmc-supply = <&ldo20_reg &buck8_reg>; >>> + vmmc-supply = <&ldo20_reg>; >>> mmc-pwrseq = <&emmc_pwrseq>; >>> status = "okay"; >>> >>> diff --git a/arch/arm/boot/dts/exynos4412-odroidu3.dts b/arch/arm/boot/dts/exynos4412-odroidu3.dts >>> index dd89f7b37c9f..d73aa6c58fe3 100644 >>> --- a/arch/arm/boot/dts/exynos4412-odroidu3.dts >>> +++ b/arch/arm/boot/dts/exynos4412-odroidu3.dts >>> @@ -69,6 +69,24 @@ >>> }; >>> }; >>> >>> +/* Supply for LAN9730/SMSC95xx */ >>> +&buck8_reg { >>> + regulator-name = "BUCK8_P3V3"; >> >> I think it would be better to name it "BUCK8_3.3V" for consistency. > > Consistency of naming is nice but in the same time we want the regulator > names to match the hardware because it is easier to read the code and > check the schematics. On the schematics this is unfortunately called > P3V3. :) > oh, I missed that. It's pity but I agree with you that matching the schematics is more important that naming consistency. Thanks a lot for your explanations, the patch looks good to me as it is now: Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com> Best regards, Javier ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 3/3] ARM: dts: exynos: Lower SD card interface voltage to 2.8v on Odroid X/X2/U3 2016-04-27 14:00 [PATCH v2 1/3] ARM: dts: exynos: Define vqmmc for SD card and allow disabling regulators on Odroid X/X2/U3 Krzysztof Kozlowski 2016-04-27 14:00 ` [PATCH v2 2/3] ARM: dts: exynos: Define vqmmc for eMMC card " Krzysztof Kozlowski @ 2016-04-27 14:00 ` Krzysztof Kozlowski 2016-04-27 21:30 ` Javier Martinez Canillas 2016-04-27 21:09 ` [PATCH v2 1/3] ARM: dts: exynos: Define vqmmc for SD card and allow disabling regulators " Javier Martinez Canillas 2 siblings, 1 reply; 8+ messages in thread From: Krzysztof Kozlowski @ 2016-04-27 14:00 UTC (permalink / raw) To: Kukjin Kim, Krzysztof Kozlowski, devicetree, linux-arm-kernel, linux-samsung-soc, linux-kernel, Tobias Jakobi, Marek Szyprowski Cc: linux-mmc, linux.amoon, Bartlomiej Zolnierkiewicz Odroid X/X2/U3 schematics say that SD card vmmc regulator (LDO21/TFLASH) operates on 2.8 V. Mainline U-Boot uses that value as well. 2.8 V is common on Exynos-based boards. Additionally use some descriptive name for this regulator. Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> --- Changes since v1: 1. None --- arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi index 34a5b3daced0..260601d2f72f 100644 --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi @@ -341,9 +341,9 @@ }; ldo21_reg: LDO21 { - regulator-name = "LDO21_3.3V"; - regulator-min-microvolt = <3300000>; - regulator-max-microvolt = <3300000>; + regulator-name = "TFLASH_2.8V"; + regulator-min-microvolt = <2800000>; + regulator-max-microvolt = <2800000>; regulator-boot-on; }; -- 1.9.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 3/3] ARM: dts: exynos: Lower SD card interface voltage to 2.8v on Odroid X/X2/U3 2016-04-27 14:00 ` [PATCH v2 3/3] ARM: dts: exynos: Lower SD card interface voltage to 2.8v " Krzysztof Kozlowski @ 2016-04-27 21:30 ` Javier Martinez Canillas 0 siblings, 0 replies; 8+ messages in thread From: Javier Martinez Canillas @ 2016-04-27 21:30 UTC (permalink / raw) To: Krzysztof Kozlowski, Kukjin Kim, devicetree, linux-arm-kernel, linux-samsung-soc, linux-kernel, Tobias Jakobi, Marek Szyprowski Cc: linux-mmc, linux.amoon, Bartlomiej Zolnierkiewicz Hello On 04/27/2016 10:00 AM, Krzysztof Kozlowski wrote: > Odroid X/X2/U3 schematics say that SD card vmmc regulator > (LDO21/TFLASH) operates on 2.8 V. Mainline U-Boot uses that value as > well. 2.8 V is common on Exynos-based boards. Additionally use some > descriptive name for this regulator. > > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> > Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com> Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/3] ARM: dts: exynos: Define vqmmc for SD card and allow disabling regulators on Odroid X/X2/U3 2016-04-27 14:00 [PATCH v2 1/3] ARM: dts: exynos: Define vqmmc for SD card and allow disabling regulators on Odroid X/X2/U3 Krzysztof Kozlowski 2016-04-27 14:00 ` [PATCH v2 2/3] ARM: dts: exynos: Define vqmmc for eMMC card " Krzysztof Kozlowski 2016-04-27 14:00 ` [PATCH v2 3/3] ARM: dts: exynos: Lower SD card interface voltage to 2.8v " Krzysztof Kozlowski @ 2016-04-27 21:09 ` Javier Martinez Canillas 2 siblings, 0 replies; 8+ messages in thread From: Javier Martinez Canillas @ 2016-04-27 21:09 UTC (permalink / raw) To: Krzysztof Kozlowski, Kukjin Kim, devicetree, linux-arm-kernel, linux-samsung-soc, linux-kernel, Tobias Jakobi, Marek Szyprowski Cc: linux-mmc, linux.amoon, Bartlomiej Zolnierkiewicz Hello Krzysztof, On 04/27/2016 10:00 AM, Krzysztof Kozlowski wrote: > The SD-card vmmc-supply contained incorrectly two regulators. The second > one is ignored. Fix this by defining proper vmmc and vqmmc supplies. > Additionally these regulators do not have to be always on, so allow > disabling them to reduce energy consumption. > > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> > > --- Patch looks good to me. Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com> Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-04-28 6:21 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-04-27 14:00 [PATCH v2 1/3] ARM: dts: exynos: Define vqmmc for SD card and allow disabling regulators on Odroid X/X2/U3 Krzysztof Kozlowski 2016-04-27 14:00 ` [PATCH v2 2/3] ARM: dts: exynos: Define vqmmc for eMMC card " Krzysztof Kozlowski 2016-04-27 21:29 ` Javier Martinez Canillas 2016-04-28 5:50 ` Krzysztof Kozlowski 2016-04-28 6:21 ` Javier Martinez Canillas 2016-04-27 14:00 ` [PATCH v2 3/3] ARM: dts: exynos: Lower SD card interface voltage to 2.8v " Krzysztof Kozlowski 2016-04-27 21:30 ` Javier Martinez Canillas 2016-04-27 21:09 ` [PATCH v2 1/3] ARM: dts: exynos: Define vqmmc for SD card and allow disabling regulators " Javier Martinez Canillas
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).