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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
>
> ---
Patch looks good to me.
Reviewed-by: Javier Martinez Canillas <[email protected]>
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
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 <[email protected]>
>
> ---
>
> 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 <[email protected]>
[0]: http://www.spinics.net/lists/linux-omap/msg127035.html
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
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 <[email protected]>
>
Reviewed-by: Javier Martinez Canillas <[email protected]>
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
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 <[email protected]>
>>
>> ---
>>
>> 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 <[email protected]>
>
> [0]: http://www.spinics.net/lists/linux-omap/msg127035.html
Thanks for feedback!
Krzysztof
Hello Krzysztof,
On Thu, Apr 28, 2016 at 1:50 AM, Krzysztof Kozlowski
<[email protected]> 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 <[email protected]>
>>>
>>> ---
>>>
>>> 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 <[email protected]>
Best regards,
Javier