[PATCH 0/3] ARM: ux500: Fix SD-card regression by using GPIO regulator

[PATCH 0/3] ARM: ux500: Fix SD-card regression by using GPIO regulator

[Ulf Hansson]

This patchset fixes an old stability issue for detection of SD-cards. The DT
node for the GPIO regulator which controls the I/O voltage level towards the
SD-card was disabled in below commit.

c94a4ab7af3f ("ARM: ux500: Disable the MMCI gpio-regulator by default")

The conseqeunce from that commit, was that the initially chosen voltage level
remained, even after I/O voltage switch sequence was performed for UHS-I cards,
since no GPIO regulator could be fetched.

Appearantly some SD-cards didn't have any problem to cope with that, but some
had.

Recently thorough error handling was introduced for the MMCI mmc host driver,
while fetching the regulators for the SD-card. That actually made the driver to
continously return -EPROBE_DEFER while probing, which thus meant failing all the
times instead of just failing "sometimes".

The commit that introduced the thorough error handling is:
9369c97cc7ec ("mmc: mmci: Cascade EPROBE_DEFER from regulators")


Ulf Hansson (3):
  ARM: ux500: Move GPIO regulator for SD-card into board DTSs
  ARM: ux500: Enable GPIO regulator for SD-card for HREF boards
  ARM: ux500: Enable GPIO regulator for SD-card for snowball

 arch/arm/boot/dts/ste-dbx5x0.dtsi  | 17 -----------------
 arch/arm/boot/dts/ste-href.dtsi    | 15 +++++++++++++++
 arch/arm/boot/dts/ste-snowball.dts | 13 +++++++++++++
 3 files changed, 28 insertions(+), 17 deletions(-)

-- 
1.9.1

--
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

[PATCH 1/3] ARM: ux500: Move GPIO regulator for SD-card into board DTSs

[Ulf Hansson]

The GPIO regulator for the SD-card isn't a ux500 SOC configuration, but
instead it's specific to the board. Move the definition of it, into the
board DTSs.

Fixes: c94a4ab7af3f ("ARM: ux500: Disable the MMCI gpio-regulator by default")
Signed-off-by: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 arch/arm/boot/dts/ste-dbx5x0.dtsi  | 17 -----------------
 arch/arm/boot/dts/ste-href.dtsi    | 17 +++++++++++++++++
 arch/arm/boot/dts/ste-snowball.dts | 15 +++++++++++++++
 3 files changed, 32 insertions(+), 17 deletions(-)

diff --git a/arch/arm/boot/dts/ste-dbx5x0.dtsi b/arch/arm/boot/dts/ste-dbx5x0.dtsi
index bfd3f1c..2201cd5 100644
--- a/arch/arm/boot/dts/ste-dbx5x0.dtsi
+++ b/arch/arm/boot/dts/ste-dbx5x0.dtsi
@@ -1017,23 +1017,6 @@
 			status = "disabled";
 		};
 
-		vmmci: regulator-gpio {
-			compatible = "regulator-gpio";
-
-			regulator-min-microvolt = <1800000>;
-			regulator-max-microvolt = <2900000>;
-			regulator-name = "mmci-reg";
-			regulator-type = "voltage";
-
-			startup-delay-us = <100>;
-			enable-active-high;
-
-			states = <1800000 0x1
-				  2900000 0x0>;
-
-			status = "disabled";
-		};
-
 		mcde@a0350000 {
 			compatible = "stericsson,mcde";
 			reg = <0xa0350000 0x1000>, /* MCDE */
diff --git a/arch/arm/boot/dts/ste-href.dtsi b/arch/arm/boot/dts/ste-href.dtsi
index bf8f0ed..8cf499a 100644
--- a/arch/arm/boot/dts/ste-href.dtsi
+++ b/arch/arm/boot/dts/ste-href.dtsi
@@ -111,6 +111,23 @@
 			pinctrl-1 = <&i2c3_sleep_mode>;
 		};
 
+		vmmci: regulator-gpio {
+			compatible = "regulator-gpio";
+
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <2900000>;
+			regulator-name = "mmci-reg";
+			regulator-type = "voltage";
+
+			startup-delay-us = <100>;
+			enable-active-high;
+
+			states = <1800000 0x1
+				  2900000 0x0>;
+
+			status = "disabled";
+		};
+
 		// External Micro SD slot
 		sdi0_per1@80126000 {
 			arm,primecell-periphid = <0x10480180>;
diff --git a/arch/arm/boot/dts/ste-snowball.dts b/arch/arm/boot/dts/ste-snowball.dts
index 206826a..65a7f63 100644
--- a/arch/arm/boot/dts/ste-snowball.dts
+++ b/arch/arm/boot/dts/ste-snowball.dts
@@ -146,8 +146,23 @@
 		};
 
 		vmmci: regulator-gpio {
+			compatible = "regulator-gpio";
+
 			gpios = <&gpio7 4 0x4>;
 			enable-gpio = <&gpio6 25 0x4>;
+
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <2900000>;
+			regulator-name = "mmci-reg";
+			regulator-type = "voltage";
+
+			startup-delay-us = <100>;
+			enable-active-high;
+
+			states = <1800000 0x1
+				  2900000 0x0>;
+
+			status = "disabled";
 		};
 
 		// External Micro SD slot
-- 
1.9.1

--
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

[PATCH 2/3] ARM: ux500: Enable GPIO regulator for SD-card for HREF boards

[Ulf Hansson]

Fixes: c94a4ab7af3f ("ARM: ux500: Disable the MMCI gpio-regulator by default")
Signed-off-by: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 arch/arm/boot/dts/ste-href.dtsi | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm/boot/dts/ste-href.dtsi b/arch/arm/boot/dts/ste-href.dtsi
index 8cf499a..744c1e3 100644
--- a/arch/arm/boot/dts/ste-href.dtsi
+++ b/arch/arm/boot/dts/ste-href.dtsi
@@ -124,8 +124,6 @@
 
 			states = <1800000 0x1
 				  2900000 0x0>;
-
-			status = "disabled";
 		};
 
 		// External Micro SD slot
-- 
1.9.1

--
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

[PATCH 3/3] ARM: ux500: Enable GPIO regulator for SD-card for snowball

[Ulf Hansson]

Fixes: c94a4ab7af3f ("ARM: ux500: Disable the MMCI gpio-regulator by default")
Signed-off-by: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 arch/arm/boot/dts/ste-snowball.dts | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm/boot/dts/ste-snowball.dts b/arch/arm/boot/dts/ste-snowball.dts
index 65a7f63..1bc84eb 100644
--- a/arch/arm/boot/dts/ste-snowball.dts
+++ b/arch/arm/boot/dts/ste-snowball.dts
@@ -161,8 +161,6 @@
 
 			states = <1800000 0x1
 				  2900000 0x0>;
-
-			status = "disabled";
 		};
 
 		// External Micro SD slot
-- 
1.9.1

--
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

Re: [PATCH 0/3] ARM: ux500: Fix SD-card regression by using GPIO regulator

[Bjorn Andersson]

On Mon, Apr 20, 2015 at 7:02 AM, Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> This patchset fixes an old stability issue for detection of SD-cards. The DT
> node for the GPIO regulator which controls the I/O voltage level towards the
> SD-card was disabled in below commit.
>
[..]
> The commit that introduced the thorough error handling is:
> 9369c97cc7ec ("mmc: mmci: Cascade EPROBE_DEFER from regulators")
>

Glad I could help :)

>
> Ulf Hansson (3):
>   ARM: ux500: Move GPIO regulator for SD-card into board DTSs
>   ARM: ux500: Enable GPIO regulator for SD-card for HREF boards
>   ARM: ux500: Enable GPIO regulator for SD-card for snowball
>

Looks good

Reviewed-by: Bjorn Andersson <bjorn.andersson-/MT0OVThwyLZJqsBc5GL+g@public.gmane.org>

Regards,
Bjorn
--
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

Re: [PATCH 0/3] ARM: ux500: Fix SD-card regression by using GPIO regulator

[Lee Jones]

Where did you get this email address from?

> This patchset fixes an old stability issue for detection of SD-cards. The DT
> node for the GPIO regulator which controls the I/O voltage level towards the
> SD-card was disabled in below commit.
> 
> c94a4ab7af3f ("ARM: ux500: Disable the MMCI gpio-regulator by default")
> 
> The conseqeunce from that commit, was that the initially chosen voltage level
> remained, even after I/O voltage switch sequence was performed for UHS-I cards,
> since no GPIO regulator could be fetched.
> 
> Appearantly some SD-cards didn't have any problem to cope with that, but some
> had.
> 
> Recently thorough error handling was introduced for the MMCI mmc host driver,
> while fetching the regulators for the SD-card. That actually made the driver to
> continously return -EPROBE_DEFER while probing, which thus meant failing all the
> times instead of just failing "sometimes".
> 
> The commit that introduced the thorough error handling is:
> 9369c97cc7ec ("mmc: mmci: Cascade EPROBE_DEFER from regulators")
> 
> 
> Ulf Hansson (3):
>   ARM: ux500: Move GPIO regulator for SD-card into board DTSs
>   ARM: ux500: Enable GPIO regulator for SD-card for HREF boards
>   ARM: ux500: Enable GPIO regulator for SD-card for snowball
> 
>  arch/arm/boot/dts/ste-dbx5x0.dtsi  | 17 -----------------
>  arch/arm/boot/dts/ste-href.dtsi    | 15 +++++++++++++++
>  arch/arm/boot/dts/ste-snowball.dts | 13 +++++++++++++
>  3 files changed, 28 insertions(+), 17 deletions(-)
> 
--
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

Re: [PATCH 1/3] ARM: ux500: Move GPIO regulator for SD-card into board DTSs

[Lee Jones]

On Mon, 20 Apr 2015, Ulf Hansson wrote:

> The GPIO regulator for the SD-card isn't a ux500 SOC configuration, but
> instead it's specific to the board. Move the definition of it, into the
> board DTSs.

What makes you think that?

We normally place the common pieces (of which there are many in this
node) in the highest level DTSI file, then add the platform specific
ones in the DTS files.

> Fixes: c94a4ab7af3f ("ARM: ux500: Disable the MMCI gpio-regulator by default")
> Signed-off-by: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  arch/arm/boot/dts/ste-dbx5x0.dtsi  | 17 -----------------
>  arch/arm/boot/dts/ste-href.dtsi    | 17 +++++++++++++++++
>  arch/arm/boot/dts/ste-snowball.dts | 15 +++++++++++++++
>  3 files changed, 32 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/ste-dbx5x0.dtsi b/arch/arm/boot/dts/ste-dbx5x0.dtsi
> index bfd3f1c..2201cd5 100644
> --- a/arch/arm/boot/dts/ste-dbx5x0.dtsi
> +++ b/arch/arm/boot/dts/ste-dbx5x0.dtsi
> @@ -1017,23 +1017,6 @@
>  			status = "disabled";
>  		};
>  
> -		vmmci: regulator-gpio {
> -			compatible = "regulator-gpio";
> -
> -			regulator-min-microvolt = <1800000>;
> -			regulator-max-microvolt = <2900000>;
> -			regulator-name = "mmci-reg";
> -			regulator-type = "voltage";
> -
> -			startup-delay-us = <100>;
> -			enable-active-high;
> -
> -			states = <1800000 0x1
> -				  2900000 0x0>;
> -
> -			status = "disabled";
> -		};
> -
>  		mcde@a0350000 {
>  			compatible = "stericsson,mcde";
>  			reg = <0xa0350000 0x1000>, /* MCDE */
> diff --git a/arch/arm/boot/dts/ste-href.dtsi b/arch/arm/boot/dts/ste-href.dtsi
> index bf8f0ed..8cf499a 100644
> --- a/arch/arm/boot/dts/ste-href.dtsi
> +++ b/arch/arm/boot/dts/ste-href.dtsi
> @@ -111,6 +111,23 @@
>  			pinctrl-1 = <&i2c3_sleep_mode>;
>  		};
>  
> +		vmmci: regulator-gpio {
> +			compatible = "regulator-gpio";
> +
> +			regulator-min-microvolt = <1800000>;
> +			regulator-max-microvolt = <2900000>;
> +			regulator-name = "mmci-reg";
> +			regulator-type = "voltage";
> +
> +			startup-delay-us = <100>;
> +			enable-active-high;
> +
> +			states = <1800000 0x1
> +				  2900000 0x0>;
> +
> +			status = "disabled";
> +		};
> +
>  		// External Micro SD slot
>  		sdi0_per1@80126000 {
>  			arm,primecell-periphid = <0x10480180>;
> diff --git a/arch/arm/boot/dts/ste-snowball.dts b/arch/arm/boot/dts/ste-snowball.dts
> index 206826a..65a7f63 100644
> --- a/arch/arm/boot/dts/ste-snowball.dts
> +++ b/arch/arm/boot/dts/ste-snowball.dts
> @@ -146,8 +146,23 @@
>  		};
>  
>  		vmmci: regulator-gpio {
> +			compatible = "regulator-gpio";
> +
>  			gpios = <&gpio7 4 0x4>;
>  			enable-gpio = <&gpio6 25 0x4>;
> +
> +			regulator-min-microvolt = <1800000>;
> +			regulator-max-microvolt = <2900000>;
> +			regulator-name = "mmci-reg";
> +			regulator-type = "voltage";
> +
> +			startup-delay-us = <100>;
> +			enable-active-high;
> +
> +			states = <1800000 0x1
> +				  2900000 0x0>;
> +
> +			status = "disabled";
>  		};
>  
>  		// External Micro SD slot
--
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

Re: [PATCH 2/3] ARM: ux500: Enable GPIO regulator for SD-card for HREF boards

[Lee Jones]

On Mon, 20 Apr 2015, Ulf Hansson wrote:

> Fixes: c94a4ab7af3f ("ARM: ux500: Disable the MMCI gpio-regulator by default")
> Signed-off-by: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  arch/arm/boot/dts/ste-href.dtsi | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/ste-href.dtsi b/arch/arm/boot/dts/ste-href.dtsi
> index 8cf499a..744c1e3 100644
> --- a/arch/arm/boot/dts/ste-href.dtsi
> +++ b/arch/arm/boot/dts/ste-href.dtsi
> @@ -124,8 +124,6 @@
>  
>  			states = <1800000 0x1
>  				  2900000 0x0>;
> -
> -			status = "disabled";
>  		};

Seems weird to do this in a separate patch.

>  		// External Micro SD slot
--
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

Re: [PATCH 1/3] ARM: ux500: Move GPIO regulator for SD-card into board DTSs

[Ulf Hansson]

On 20 April 2015 at 20:26, Lee Jones <lee-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Mon, 20 Apr 2015, Ulf Hansson wrote:
>
>> The GPIO regulator for the SD-card isn't a ux500 SOC configuration, but
>> instead it's specific to the board. Move the definition of it, into the
>> board DTSs.
>
> What makes you think that?

Because of how it was structured today.

ste-dbx5x0.dtsi - common for all ux500 boards, thus I considered this
as the SoC configuration.

Then below are board configs which uses the above dtsi:
ste-href.dtsi - common for href boards (used by ste-hrefprev60.dtsi
and ste-hrefv60plus.dtsi), have vmmci
ste-snowball.dts, have vmmci
ste-ccu8540.dts, don't have vmmci
ste-ccu9540.dts, don't have vmmci

>
> We normally place the common pieces (of which there are many in this
> node) in the highest level DTSI file, then add the platform specific
> ones in the DTS files.

Okay, so maybe it's due to the naming of ste-dbx5x0.dtsi, that I
thought this was intended to cover the SoC configuration and not any
of the platform specific stuff.

So what your advise of doing this?

Kind regards
Uffe

>
>> Fixes: c94a4ab7af3f ("ARM: ux500: Disable the MMCI gpio-regulator by default")
>> Signed-off-by: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>>  arch/arm/boot/dts/ste-dbx5x0.dtsi  | 17 -----------------
>>  arch/arm/boot/dts/ste-href.dtsi    | 17 +++++++++++++++++
>>  arch/arm/boot/dts/ste-snowball.dts | 15 +++++++++++++++
>>  3 files changed, 32 insertions(+), 17 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/ste-dbx5x0.dtsi b/arch/arm/boot/dts/ste-dbx5x0.dtsi
>> index bfd3f1c..2201cd5 100644
>> --- a/arch/arm/boot/dts/ste-dbx5x0.dtsi
>> +++ b/arch/arm/boot/dts/ste-dbx5x0.dtsi
>> @@ -1017,23 +1017,6 @@
>>                       status = "disabled";
>>               };
>>
>> -             vmmci: regulator-gpio {
>> -                     compatible = "regulator-gpio";
>> -
>> -                     regulator-min-microvolt = <1800000>;
>> -                     regulator-max-microvolt = <2900000>;
>> -                     regulator-name = "mmci-reg";
>> -                     regulator-type = "voltage";
>> -
>> -                     startup-delay-us = <100>;
>> -                     enable-active-high;
>> -
>> -                     states = <1800000 0x1
>> -                               2900000 0x0>;
>> -
>> -                     status = "disabled";
>> -             };
>> -
>>               mcde@a0350000 {
>>                       compatible = "stericsson,mcde";
>>                       reg = <0xa0350000 0x1000>, /* MCDE */
>> diff --git a/arch/arm/boot/dts/ste-href.dtsi b/arch/arm/boot/dts/ste-href.dtsi
>> index bf8f0ed..8cf499a 100644
>> --- a/arch/arm/boot/dts/ste-href.dtsi
>> +++ b/arch/arm/boot/dts/ste-href.dtsi
>> @@ -111,6 +111,23 @@
>>                       pinctrl-1 = <&i2c3_sleep_mode>;
>>               };
>>
>> +             vmmci: regulator-gpio {
>> +                     compatible = "regulator-gpio";
>> +
>> +                     regulator-min-microvolt = <1800000>;
>> +                     regulator-max-microvolt = <2900000>;
>> +                     regulator-name = "mmci-reg";
>> +                     regulator-type = "voltage";
>> +
>> +                     startup-delay-us = <100>;
>> +                     enable-active-high;
>> +
>> +                     states = <1800000 0x1
>> +                               2900000 0x0>;
>> +
>> +                     status = "disabled";
>> +             };
>> +
>>               // External Micro SD slot
>>               sdi0_per1@80126000 {
>>                       arm,primecell-periphid = <0x10480180>;
>> diff --git a/arch/arm/boot/dts/ste-snowball.dts b/arch/arm/boot/dts/ste-snowball.dts
>> index 206826a..65a7f63 100644
>> --- a/arch/arm/boot/dts/ste-snowball.dts
>> +++ b/arch/arm/boot/dts/ste-snowball.dts
>> @@ -146,8 +146,23 @@
>>               };
>>
>>               vmmci: regulator-gpio {
>> +                     compatible = "regulator-gpio";
>> +
>>                       gpios = <&gpio7 4 0x4>;
>>                       enable-gpio = <&gpio6 25 0x4>;
>> +
>> +                     regulator-min-microvolt = <1800000>;
>> +                     regulator-max-microvolt = <2900000>;
>> +                     regulator-name = "mmci-reg";
>> +                     regulator-type = "voltage";
>> +
>> +                     startup-delay-us = <100>;
>> +                     enable-active-high;
>> +
>> +                     states = <1800000 0x1
>> +                               2900000 0x0>;
>> +
>> +                     status = "disabled";
>>               };
>>
>>               // External Micro SD slot
--
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

Re: [PATCH 1/3] ARM: ux500: Move GPIO regulator for SD-card into board DTSs

[Lee Jones]

On Tue, 21 Apr 2015, Ulf Hansson wrote:

> On 20 April 2015 at 20:26, Lee Jones <lee-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > On Mon, 20 Apr 2015, Ulf Hansson wrote:
> >
> >> The GPIO regulator for the SD-card isn't a ux500 SOC configuration, but
> >> instead it's specific to the board. Move the definition of it, into the
> >> board DTSs.
> >
> > What makes you think that?
> 
> Because of how it was structured today.
> 
> ste-dbx5x0.dtsi - common for all ux500 boards, thus I considered this
> as the SoC configuration.

ste-dbx5x0.dtsi is common for all ux500 and ux540 boards.

> Then below are board configs which uses the above dtsi:
> ste-href.dtsi - common for href boards (used by ste-hrefprev60.dtsi
> and ste-hrefv60plus.dtsi), have vmmci
> ste-snowball.dts, have vmmci
> ste-ccu8540.dts, don't have vmmci
> ste-ccu9540.dts, don't have vmmci

Ah, got you.  In which case it doesn't belong in ste-dbx5x0.dtsi.

> > We normally place the common pieces (of which there are many in this
> > node) in the highest level DTSI file, then add the platform specific
> > ones in the DTS files.
> 
> Okay, so maybe it's due to the naming of ste-dbx5x0.dtsi, that I
> thought this was intended to cover the SoC configuration and not any
> of the platform specific stuff.

ste-dbx5x0.dtsi should cover all pieces which are common to all ux500
and ux540 devices.  Then the lower level file ste-href-ab8500.dtsi
should cover all pieces which are common to ux500 devices and finally
the DTS files should add board specific information.  Duplicate
nodes and properties are frowned upon.

> So what your advise of doing this?

So the file which covers the x500 boards is ste-href-ab8500.dtsi.  I
would move the node into there instead.  Keep it disabled and enable
the associated nodes in the 2 DTS files.

> >> Fixes: c94a4ab7af3f ("ARM: ux500: Disable the MMCI gpio-regulator by default")
> >> Signed-off-by: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> >> ---
> >>  arch/arm/boot/dts/ste-dbx5x0.dtsi  | 17 -----------------
> >>  arch/arm/boot/dts/ste-href.dtsi    | 17 +++++++++++++++++
> >>  arch/arm/boot/dts/ste-snowball.dts | 15 +++++++++++++++
> >>  3 files changed, 32 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/arch/arm/boot/dts/ste-dbx5x0.dtsi b/arch/arm/boot/dts/ste-dbx5x0.dtsi
> >> index bfd3f1c..2201cd5 100644
> >> --- a/arch/arm/boot/dts/ste-dbx5x0.dtsi
> >> +++ b/arch/arm/boot/dts/ste-dbx5x0.dtsi
> >> @@ -1017,23 +1017,6 @@
> >>                       status = "disabled";
> >>               };
> >>
> >> -             vmmci: regulator-gpio {
> >> -                     compatible = "regulator-gpio";
> >> -
> >> -                     regulator-min-microvolt = <1800000>;
> >> -                     regulator-max-microvolt = <2900000>;
> >> -                     regulator-name = "mmci-reg";
> >> -                     regulator-type = "voltage";
> >> -
> >> -                     startup-delay-us = <100>;
> >> -                     enable-active-high;
> >> -
> >> -                     states = <1800000 0x1
> >> -                               2900000 0x0>;
> >> -
> >> -                     status = "disabled";
> >> -             };
> >> -
> >>               mcde@a0350000 {
> >>                       compatible = "stericsson,mcde";
> >>                       reg = <0xa0350000 0x1000>, /* MCDE */
> >> diff --git a/arch/arm/boot/dts/ste-href.dtsi b/arch/arm/boot/dts/ste-href.dtsi
> >> index bf8f0ed..8cf499a 100644
> >> --- a/arch/arm/boot/dts/ste-href.dtsi
> >> +++ b/arch/arm/boot/dts/ste-href.dtsi
> >> @@ -111,6 +111,23 @@
> >>                       pinctrl-1 = <&i2c3_sleep_mode>;
> >>               };
> >>
> >> +             vmmci: regulator-gpio {
> >> +                     compatible = "regulator-gpio";
> >> +
> >> +                     regulator-min-microvolt = <1800000>;
> >> +                     regulator-max-microvolt = <2900000>;
> >> +                     regulator-name = "mmci-reg";
> >> +                     regulator-type = "voltage";
> >> +
> >> +                     startup-delay-us = <100>;
> >> +                     enable-active-high;
> >> +
> >> +                     states = <1800000 0x1
> >> +                               2900000 0x0>;
> >> +
> >> +                     status = "disabled";
> >> +             };
> >> +
> >>               // External Micro SD slot
> >>               sdi0_per1@80126000 {
> >>                       arm,primecell-periphid = <0x10480180>;
> >> diff --git a/arch/arm/boot/dts/ste-snowball.dts b/arch/arm/boot/dts/ste-snowball.dts
> >> index 206826a..65a7f63 100644
> >> --- a/arch/arm/boot/dts/ste-snowball.dts
> >> +++ b/arch/arm/boot/dts/ste-snowball.dts
> >> @@ -146,8 +146,23 @@
> >>               };
> >>
> >>               vmmci: regulator-gpio {
> >> +                     compatible = "regulator-gpio";
> >> +
> >>                       gpios = <&gpio7 4 0x4>;
> >>                       enable-gpio = <&gpio6 25 0x4>;
> >> +
> >> +                     regulator-min-microvolt = <1800000>;
> >> +                     regulator-max-microvolt = <2900000>;
> >> +                     regulator-name = "mmci-reg";
> >> +                     regulator-type = "voltage";
> >> +
> >> +                     startup-delay-us = <100>;
> >> +                     enable-active-high;
> >> +
> >> +                     states = <1800000 0x1
> >> +                               2900000 0x0>;
> >> +
> >> +                     status = "disabled";
> >>               };
> >>
> >>               // External Micro SD slot
--
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

Re: [PATCH 1/3] ARM: ux500: Move GPIO regulator for SD-card into board DTSs

[Ulf Hansson]

On 21 April 2015 at 09:33, Lee Jones <lee-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Tue, 21 Apr 2015, Ulf Hansson wrote:
>
>> On 20 April 2015 at 20:26, Lee Jones <lee-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> > On Mon, 20 Apr 2015, Ulf Hansson wrote:
>> >
>> >> The GPIO regulator for the SD-card isn't a ux500 SOC configuration, but
>> >> instead it's specific to the board. Move the definition of it, into the
>> >> board DTSs.
>> >
>> > What makes you think that?
>>
>> Because of how it was structured today.
>>
>> ste-dbx5x0.dtsi - common for all ux500 boards, thus I considered this
>> as the SoC configuration.
>
> ste-dbx5x0.dtsi is common for all ux500 and ux540 boards.
>
>> Then below are board configs which uses the above dtsi:
>> ste-href.dtsi - common for href boards (used by ste-hrefprev60.dtsi
>> and ste-hrefv60plus.dtsi), have vmmci
>> ste-snowball.dts, have vmmci
>> ste-ccu8540.dts, don't have vmmci
>> ste-ccu9540.dts, don't have vmmci
>
> Ah, got you.  In which case it doesn't belong in ste-dbx5x0.dtsi.
>
>> > We normally place the common pieces (of which there are many in this
>> > node) in the highest level DTSI file, then add the platform specific
>> > ones in the DTS files.
>>
>> Okay, so maybe it's due to the naming of ste-dbx5x0.dtsi, that I
>> thought this was intended to cover the SoC configuration and not any
>> of the platform specific stuff.
>
> ste-dbx5x0.dtsi should cover all pieces which are common to all ux500
> and ux540 devices.  Then the lower level file ste-href-ab8500.dtsi
> should cover all pieces which are common to ux500 devices and finally
> the DTS files should add board specific information.  Duplicate
> nodes and properties are frowned upon.
>
>> So what your advise of doing this?
>
> So the file which covers the x500 boards is ste-href-ab8500.dtsi.  I
> would move the node into there instead.  Keep it disabled and enable
> the associated nodes in the 2 DTS files.

Why ste-href-ab8500.dtsi? Isn't that suppose to cover configurations
common to the ab8500 subsystem?

The vmmci models a board specific mounted circuit (aka level-shifter).
Thus it exist on some boards but not on others.

Kind regards
Uffe
--
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

Re: [PATCH 1/3] ARM: ux500: Move GPIO regulator for SD-card into board DTSs

[Lee Jones]

On Tue, 21 Apr 2015, Ulf Hansson wrote:

> On 21 April 2015 at 09:33, Lee Jones <lee-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > On Tue, 21 Apr 2015, Ulf Hansson wrote:
> >
> >> On 20 April 2015 at 20:26, Lee Jones <lee-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> >> > On Mon, 20 Apr 2015, Ulf Hansson wrote:
> >> >
> >> >> The GPIO regulator for the SD-card isn't a ux500 SOC configuration, but
> >> >> instead it's specific to the board. Move the definition of it, into the
> >> >> board DTSs.
> >> >
> >> > What makes you think that?
> >>
> >> Because of how it was structured today.
> >>
> >> ste-dbx5x0.dtsi - common for all ux500 boards, thus I considered this
> >> as the SoC configuration.
> >
> > ste-dbx5x0.dtsi is common for all ux500 and ux540 boards.
> >
> >> Then below are board configs which uses the above dtsi:
> >> ste-href.dtsi - common for href boards (used by ste-hrefprev60.dtsi
> >> and ste-hrefv60plus.dtsi), have vmmci
> >> ste-snowball.dts, have vmmci
> >> ste-ccu8540.dts, don't have vmmci
> >> ste-ccu9540.dts, don't have vmmci
> >
> > Ah, got you.  In which case it doesn't belong in ste-dbx5x0.dtsi.
> >
> >> > We normally place the common pieces (of which there are many in this
> >> > node) in the highest level DTSI file, then add the platform specific
> >> > ones in the DTS files.
> >>
> >> Okay, so maybe it's due to the naming of ste-dbx5x0.dtsi, that I
> >> thought this was intended to cover the SoC configuration and not any
> >> of the platform specific stuff.
> >
> > ste-dbx5x0.dtsi should cover all pieces which are common to all ux500
> > and ux540 devices.  Then the lower level file ste-href-ab8500.dtsi
> > should cover all pieces which are common to ux500 devices and finally
> > the DTS files should add board specific information.  Duplicate
> > nodes and properties are frowned upon.
> >
> >> So what your advise of doing this?
> >
> > So the file which covers the x500 boards is ste-href-ab8500.dtsi.  I
> > would move the node into there instead.  Keep it disabled and enable
> > the associated nodes in the 2 DTS files.
> 
> Why ste-href-ab8500.dtsi? Isn't that suppose to cover configurations
> common to the ab8500 subsystem?

Only because up until now that has been what is a) different from the
abx5{40,05} platforms and b) common on abx500 ones.

However, the point of the DTS(I) hierarchy is to prevent duplication.
Lower level DTSI files contain nodes which are similar to a sub-set of
platforms, whereas the highest level DTSI files contain nodes which
are shared between all associated platforms.

> The vmmci models a board specific mounted circuit (aka level-shifter).
> Thus it exist on some boards but not on others.

Many of the peripherals we use on the boards are 'off-chip'.  That
does not preclude them from DTSI files if they are shared among
various platforms.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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