diff mbox

ARM: dts: exynos: Fix invalid maximum voltage for buck9 supplying SD card

Message ID 1458807125-13580-1-git-send-email-k.kozlowski@samsung.com
State Rejected
Headers show

Commit Message

Krzysztof Kozlowski March 24, 2016, 8:12 a.m. UTC
The buck9 regulator of S2MPS11 PMIC had incorrect maximum voltage -
lower by 5 mV than specified by datasheet. This buck9 provides power to
other regulators, including LDO13 and LDO19 which supply the MMC2 (SD
card).

When constraints were applied, the depending regulators were not
providing enough power, leading to SD card detection errors:
	mmc1: card never left busy state
	mmc1: error -110 whilst initialising SD card

Fixes: 86a2d2ac5e5d ("ARM: dts: Add dts file for Odroid XU3 board")
Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

---

The issue can be reproduced on next-20160324 with
bae4fdc88d7f7dda1 (regulator: core: Ensure we are at least in bounds for our constraints).
---
 arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jaehoon Chung March 24, 2016, 9:33 a.m. UTC | #1
On 03/24/2016 05:12 PM, Krzysztof Kozlowski wrote:
> The buck9 regulator of S2MPS11 PMIC had incorrect maximum voltage -
> lower by 5 mV than specified by datasheet. This buck9 provides power to
> other regulators, including LDO13 and LDO19 which supply the MMC2 (SD
> card).
> 
> When constraints were applied, the depending regulators were not
> providing enough power, leading to SD card detection errors:
> 	mmc1: card never left busy state
> 	mmc1: error -110 whilst initialising SD card
> 
> Fixes: 86a2d2ac5e5d ("ARM: dts: Add dts file for Odroid XU3 board")
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Tested-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

> 
> ---
> 
> The issue can be reproduced on next-20160324 with
> bae4fdc88d7f7dda1 (regulator: core: Ensure we are at least in bounds for our constraints).
> ---
>  arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
> index 1bd507bfa750..82b2ba632cf6 100644
> --- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
> +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
> @@ -299,7 +299,7 @@
>  			buck9_reg: BUCK9 {
>  				regulator-name = "vdd_2.8v_ldo";
>  				regulator-min-microvolt = <3000000>;
> -				regulator-max-microvolt = <3750000>;
> +				regulator-max-microvolt = <3755000>;
>  				regulator-always-on;
>  				regulator-boot-on;
>  			};
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown March 24, 2016, 9:53 a.m. UTC | #2
On Thu, Mar 24, 2016 at 05:12:05PM +0900, Krzysztof Kozlowski wrote:

>  			buck9_reg: BUCK9 {
>  				regulator-name = "vdd_2.8v_ldo";
>  				regulator-min-microvolt = <3000000>;
> -				regulator-max-microvolt = <3750000>;
> +				regulator-max-microvolt = <3755000>;
>  				regulator-always-on;
>  				regulator-boot-on;
>  			};

Why does this regulator even have voltage constraints configured?  Is
there any intention that it should ever be varied at runtime?
Krzysztof Kozlowski March 24, 2016, 10:25 a.m. UTC | #3
On Thu, Mar 24, 2016 at 6:53 PM, Mark Brown <broonie@kernel.org> wrote:
> On Thu, Mar 24, 2016 at 05:12:05PM +0900, Krzysztof Kozlowski wrote:
>
>>                       buck9_reg: BUCK9 {
>>                               regulator-name = "vdd_2.8v_ldo";
>>                               regulator-min-microvolt = <3000000>;
>> -                             regulator-max-microvolt = <3750000>;
>> +                             regulator-max-microvolt = <3755000>;
>>                               regulator-always-on;
>>                               regulator-boot-on;
>>                       };
>
> Why does this regulator even have voltage constraints configured?  Is
> there any intention that it should ever be varied at runtime?

Good question. I was also wondering that, if tetting it by 5 mV lower
causes failure, then the constraints are actually fixed at 3.755 V. I
do not know the answer. Boards schematics do not specify the
requirement here, datasheet for PMIC says default value of 3.4V. Heh,
maybe the regulator driver is missing proper starting value for 'vsel'
(datasheet says values in register start from 0x40 -> 3 V). Something
looks wrong here...

I'll look at it after weekend. For now I applied my patch to the
for-next, so the next should be fixed.

Best regards,
Krzysztof
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown March 24, 2016, 11:02 a.m. UTC | #4
On Thu, Mar 24, 2016 at 07:25:35PM +0900, Krzysztof Kozlowski wrote:
> On Thu, Mar 24, 2016 at 6:53 PM, Mark Brown <broonie@kernel.org> wrote:

> > Why does this regulator even have voltage constraints configured?  Is
> > there any intention that it should ever be varied at runtime?

> Good question. I was also wondering that, if tetting it by 5 mV lower
> causes failure, then the constraints are actually fixed at 3.755 V. I
> do not know the answer. Boards schematics do not specify the
> requirement here, datasheet for PMIC says default value of 3.4V. Heh,

For some reason people seem to frequently want to put the maximum
voltage range for the regulator into the constraints even though that is
almost certainly broken and generally nonsensical.

> maybe the regulator driver is missing proper starting value for 'vsel'
> (datasheet says values in register start from 0x40 -> 3 V). Something
> looks wrong here...

It's possible it's just broken and has never been tested.
Mark Brown March 24, 2016, 11:02 a.m. UTC | #5
On Thu, Mar 24, 2016 at 07:25:35PM +0900, Krzysztof Kozlowski wrote:

> I'll look at it after weekend. For now I applied my patch to the
> for-next, so the next should be fixed.

Oh, and please send this to Linus - the regulator change is fixing a
problem some other boards noticed.
Markus Reichl March 24, 2016, 2:10 p.m. UTC | #6
Hi Krzysztof,

with this patch on Odroid XU4 I get the following dmesg|grep vdd_2.8v_ldo:

[    1.739943] vdd_2.8v_ldo: override max_uV, 3755000 -> 3750000
[    1.740905] vdd_2.8v_ldo: 3000 <--> 3750 mV at 5000 mV 

Regards,
--
Markus Reichl
Am 24.03.2016 um 09:12 schrieb Krzysztof Kozlowski:
> The buck9 regulator of S2MPS11 PMIC had incorrect maximum voltage -
> lower by 5 mV than specified by datasheet. This buck9 provides power to
> other regulators, including LDO13 and LDO19 which supply the MMC2 (SD
> card).
> 
> When constraints were applied, the depending regulators were not
> providing enough power, leading to SD card detection errors:
> 	mmc1: card never left busy state
> 	mmc1: error -110 whilst initialising SD card
> 
> Fixes: 86a2d2ac5e5d ("ARM: dts: Add dts file for Odroid XU3 board")
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> 
> ---
> 
> The issue can be reproduced on next-20160324 with
> bae4fdc88d7f7dda1 (regulator: core: Ensure we are at least in bounds for our constraints).
> ---
>  arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
> index 1bd507bfa750..82b2ba632cf6 100644
> --- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
> +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
> @@ -299,7 +299,7 @@
>  			buck9_reg: BUCK9 {
>  				regulator-name = "vdd_2.8v_ldo";
>  				regulator-min-microvolt = <3000000>;
> -				regulator-max-microvolt = <3750000>;
> +				regulator-max-microvolt = <3755000>;
>  				regulator-always-on;
>  				regulator-boot-on;
>  			};
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Javier Martinez Canillas March 25, 2016, 5:45 a.m. UTC | #7
Hello Krzysztof,

On 03/24/2016 05:12 AM, Krzysztof Kozlowski wrote:
> The buck9 regulator of S2MPS11 PMIC had incorrect maximum voltage -
> lower by 5 mV than specified by datasheet. This buck9 provides power to
> other regulators, including LDO13 and LDO19 which supply the MMC2 (SD
> card).
> 
> When constraints were applied, the depending regulators were not
> providing enough power, leading to SD card detection errors:
> 	mmc1: card never left busy state
> 	mmc1: error -110 whilst initialising SD card
> 
> Fixes: 86a2d2ac5e5d ("ARM: dts: Add dts file for Odroid XU3 board")
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> 
> ---
> 
> The issue can be reproduced on next-20160324 with
> bae4fdc88d7f7dda1 (regulator: core: Ensure we are at least in bounds for our constraints).
> ---
>  arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
> index 1bd507bfa750..82b2ba632cf6 100644
> --- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
> +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
> @@ -299,7 +299,7 @@
>  			buck9_reg: BUCK9 {
>  				regulator-name = "vdd_2.8v_ldo";
>  				regulator-min-microvolt = <3000000>;
> -				regulator-max-microvolt = <3750000>;
> +				regulator-max-microvolt = <3755000>;
>  				regulator-always-on;
>  				regulator-boot-on;
>  			};
> 

I tested next-20160324 + $SUBJECT on an Odroid XU4 (with exynos_defconfig)
and while it's true that the patch makes the SD card error to go away, I see
that the s2mps11 regulator driver fails to probe due an -EINVAL being return
by machine_constraints_voltage() -> _regulator_do_set_voltage():

[    1.710300] vdd_2.8v_ldo: failed to apply 3755000-3755000uV constraint(-22)
[    1.716387] s2mps11-pmic s2mps11-regulator: regulator init failed for 46
[    1.734878] s2mps11-pmic: probe of s2mps11-regulator failed with error -22

I wonder how neither Markus nor you saw this issue when testing so maybe I'm
doing something wrong? Since AFAICT it's only working because the driver fails
to probe so the same effect can be achieved disabling CONFIG_REGULATOR_S2MPS11.

Best regards,
Markus Reichl March 25, 2016, 7:14 a.m. UTC | #8
Hi Javier,

Am Freitag, 25. März 2016, 02:45:20 schrieb Javier Martinez Canillas:
> Hello Krzysztof,
> 
> On 03/24/2016 05:12 AM, Krzysztof Kozlowski wrote:
> > The buck9 regulator of S2MPS11 PMIC had incorrect maximum voltage -
> > lower by 5 mV than specified by datasheet. This buck9 provides power to
> > other regulators, including LDO13 and LDO19 which supply the MMC2 (SD
> > card).
> > 
> > When constraints were applied, the depending regulators were not
> > providing enough power, leading to SD card detection errors:
> > 	mmc1: card never left busy state
> > 	mmc1: error -110 whilst initialising SD card
> > 
> > Fixes: 86a2d2ac5e5d ("ARM: dts: Add dts file for Odroid XU3 board")
> > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > 
> > ---
> > 
> > The issue can be reproduced on next-20160324 with
> > bae4fdc88d7f7dda1 (regulator: core: Ensure we are at least in bounds for our constraints).
> > ---
> >  arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
> > index 1bd507bfa750..82b2ba632cf6 100644
> > --- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
> > +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
> > @@ -299,7 +299,7 @@
> >  			buck9_reg: BUCK9 {
> >  				regulator-name = "vdd_2.8v_ldo";
> >  				regulator-min-microvolt = <3000000>;
> > -				regulator-max-microvolt = <3750000>;
> > +				regulator-max-microvolt = <3755000>;
> >  				regulator-always-on;
> >  				regulator-boot-on;
> >  			};
> > 
> 
> I tested next-20160324 + $SUBJECT on an Odroid XU4 (with exynos_defconfig)
> and while it's true that the patch makes the SD card error to go away, I see
> that the s2mps11 regulator driver fails to probe due an -EINVAL being return
> by machine_constraints_voltage() -> _regulator_do_set_voltage():
> 
> [    1.710300] vdd_2.8v_ldo: failed to apply 3755000-3755000uV constraint(-22)
> [    1.716387] s2mps11-pmic s2mps11-regulator: regulator init failed for 46
> [    1.734878] s2mps11-pmic: probe of s2mps11-regulator failed with error -22
> 
> I wonder how neither Markus nor you saw this issue when testing so maybe I'm
> doing something wrong? Since AFAICT it's only working because the driver fails
> to probe so the same effect can be achieved disabling CONFIG_REGULATOR_S2MPS11.
> 
> Best regards,
> 
My first test yesterday was with next-20160324 + $SUBJECT, too, and I encountered the same
error of the regulator system as you.
As with next-20160324 I could not get the ethernet chip running (with or without $SUBJECT) I switched back to
next-20160323 + $SUBJECT and posted the result above.

Regards,
--
Markus 
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Krzysztof Kozlowski March 28, 2016, 12:08 a.m. UTC | #9
On 25.03.2016 14:45, Javier Martinez Canillas wrote:
> Hello Krzysztof,
> 
> On 03/24/2016 05:12 AM, Krzysztof Kozlowski wrote:
>> The buck9 regulator of S2MPS11 PMIC had incorrect maximum voltage -
>> lower by 5 mV than specified by datasheet. This buck9 provides power to
>> other regulators, including LDO13 and LDO19 which supply the MMC2 (SD
>> card).
>>
>> When constraints were applied, the depending regulators were not
>> providing enough power, leading to SD card detection errors:
>> 	mmc1: card never left busy state
>> 	mmc1: error -110 whilst initialising SD card
>>
>> Fixes: 86a2d2ac5e5d ("ARM: dts: Add dts file for Odroid XU3 board")
>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>>
>> ---
>>
>> The issue can be reproduced on next-20160324 with
>> bae4fdc88d7f7dda1 (regulator: core: Ensure we are at least in bounds for our constraints).
>> ---
>>  arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
>> index 1bd507bfa750..82b2ba632cf6 100644
>> --- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
>> +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
>> @@ -299,7 +299,7 @@
>>  			buck9_reg: BUCK9 {
>>  				regulator-name = "vdd_2.8v_ldo";
>>  				regulator-min-microvolt = <3000000>;
>> -				regulator-max-microvolt = <3750000>;
>> +				regulator-max-microvolt = <3755000>;
>>  				regulator-always-on;
>>  				regulator-boot-on;
>>  			};
>>
> 
> I tested next-20160324 + $SUBJECT on an Odroid XU4 (with exynos_defconfig)
> and while it's true that the patch makes the SD card error to go away, I see
> that the s2mps11 regulator driver fails to probe due an -EINVAL being return
> by machine_constraints_voltage() -> _regulator_do_set_voltage():
> 
> [    1.710300] vdd_2.8v_ldo: failed to apply 3755000-3755000uV constraint(-22)
> [    1.716387] s2mps11-pmic s2mps11-regulator: regulator init failed for 46
> [    1.734878] s2mps11-pmic: probe of s2mps11-regulator failed with error -22

Oh, damn, so the fix was only because driver failed to probe (which I
missed). This explains Friday's next failures. I'll drop it from next
because it makes more harm than good...

BR,
Krzysztof

> I wonder how neither Markus nor you saw this issue when testing so maybe I'm
> doing something wrong? Since AFAICT it's only working because the driver fails
> to probe so the same effect can be achieved disabling CONFIG_REGULATOR_S2MPS11.
> 
> Best regards,
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
index 1bd507bfa750..82b2ba632cf6 100644
--- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
+++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
@@ -299,7 +299,7 @@ 
 			buck9_reg: BUCK9 {
 				regulator-name = "vdd_2.8v_ldo";
 				regulator-min-microvolt = <3000000>;
-				regulator-max-microvolt = <3750000>;
+				regulator-max-microvolt = <3755000>;
 				regulator-always-on;
 				regulator-boot-on;
 			};