Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753614AbcD0V3V (ORCPT ); Wed, 27 Apr 2016 17:29:21 -0400 Received: from lists.s-osg.org ([54.187.51.154]:43166 "EHLO lists.s-osg.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752784AbcD0V3T (ORCPT ); Wed, 27 Apr 2016 17:29:19 -0400 Subject: Re: [PATCH v2 2/3] ARM: dts: exynos: Define vqmmc for eMMC card on Odroid X/X2/U3 To: Krzysztof Kozlowski , Kukjin Kim , devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org, Tobias Jakobi , Marek Szyprowski References: <1461765620-6833-1-git-send-email-k.kozlowski@samsung.com> <1461765620-6833-2-git-send-email-k.kozlowski@samsung.com> From: Javier Martinez Canillas Cc: linux-mmc@vger.kernel.org, linux.amoon@gmail.com, Bartlomiej Zolnierkiewicz Message-ID: Date: Wed, 27 Apr 2016 17:29:03 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.0 MIME-Version: 1.0 In-Reply-To: <1461765620-6833-2-git-send-email-k.kozlowski@samsung.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5216 Lines: 174 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 > > --- > > 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 [0]: http://www.spinics.net/lists/linux-omap/msg127035.html Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America