[PATCH 00/12] Functionality add and bug-fixes related to MMCI regulators

[PATCH 00/12] Functionality add and bug-fixes related to MMCI regulators

[Lee Jones]

The main purpose of this patch-set is to move the MMCI's secondary
(platform specific) regulator implementation out of platform code
and into use the regulator frame-work. A few bugs were encountered
along the way, so we've fixed those too and bundled them in for
completeness.

 arch/arm/boot/dts/dbx5x0.dtsi          |    7 ++--
 arch/arm/boot/dts/href.dtsi            |    1 +
 arch/arm/boot/dts/hrefprev60.dts       |    2 +-
 arch/arm/mach-ux500/board-mop500-sdi.c |   52 ---------------------------
 arch/arm/mach-ux500/board-mop500.c     |   61 ++++++++++++++++++++++++++++++++
 drivers/mmc/host/mmci.c                |   18 ++++++++--
 drivers/mmc/host/mmci.h                |    1 +
 drivers/regulator/gpio-regulator.c     |   17 ++++++---
 8 files changed, 96 insertions(+), 63 deletions(-)

[PATCH 01/12] regulator: gpio-regulator: Demote GPIO Regulator driver to start later

[Lee Jones]

We have quite a complicated hurdle we need to over-come, and this
patch aims to rectify it the best way possible. We're attempting
to move some MMC related voltage level-shifters out of platform
code and over to the new GPIO Regulator framework. The aim of this
change is to void the requirement for two separate call-backs; one
from the TC35892 GPIO controller which sets up MMC level-shifter
GPIOs and another from the MMCI driver to toggle the lines at the
appropriate times.

The issues come from device bring-up order during boot, and
-EPROBE_DEFER cannot help for this particular use-case. In its
current configuration the GPIO Regulator starts first. It parses
the Device Tree for 'enable' and 'voltage_select' GPIOs, then
requests them. However, the TC35892 GPIO controller isn't up yet
so it defers probe(). By the time it re-probes, the MMCI driver
has finished its probe and should have toggled the 'enable' and
'voltage_select' lines a few times already by now.

The normal course of action would be to defer the MMCI driver too,
but these IOS level-shifter regulators aren't present on all
platforms, so deferring until one is found would be incorrect.

So the best solution is to demote the GPIO Regulator driver, so
it starts later than the TC35892 GPIO controller, which is also
configured to start at subsys_initcall() time, but before deferred
probing time, which starts at late_initcall(), after many of the
drivers requiring these regulators would have already started.

Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/regulator/gpio-regulator.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/regulator/gpio-regulator.c b/drivers/regulator/gpio-regulator.c
index 3ee79c8..1a71be2 100644
--- a/drivers/regulator/gpio-regulator.c
+++ b/drivers/regulator/gpio-regulator.c
@@ -383,7 +383,7 @@ static int __init gpio_regulator_init(void)
 {
 	return platform_driver_register(&gpio_regulator_driver);
 }
-subsys_initcall(gpio_regulator_init);
+fs_initcall(gpio_regulator_init);
 
 static void __exit gpio_regulator_exit(void)
 {
-- 
1.7.9.5

[PATCH 01/12] regulator: gpio-regulator: Demote GPIO Regulator driver to start later

[Mark Brown]

On Mon, Dec 10, 2012 at 08:55:50AM +0000, Lee Jones wrote:

> The normal course of action would be to defer the MMCI driver too,
> but these IOS level-shifter regulators aren't present on all
> platforms, so deferring until one is found would be incorrect.

> So the best solution is to demote the GPIO Regulator driver, so
> it starts later than the TC35892 GPIO controller, which is also
> configured to start at subsys_initcall() time, but before deferred
> probing time, which starts at late_initcall(), after many of the
> drivers requiring these regulators would have already started.

This really isn't a good solution, especially not for a system that's DT
based - on a DT system we can tell if there should be a GPIO present so
we should be able to defer only when there's something that might
provide the GPIO later on.

[PATCH 01/12] regulator: gpio-regulator: Demote GPIO Regulator driver to start later

[Lee Jones]

On Mon, 10 Dec 2012, Mark Brown wrote:

> On Mon, Dec 10, 2012 at 08:55:50AM +0000, Lee Jones wrote:
> 
> > The normal course of action would be to defer the MMCI driver too,
> > but these IOS level-shifter regulators aren't present on all
> > platforms, so deferring until one is found would be incorrect.
> 
> > So the best solution is to demote the GPIO Regulator driver, so
> > it starts later than the TC35892 GPIO controller, which is also
> > configured to start at subsys_initcall() time, but before deferred
> > probing time, which starts at late_initcall(), after many of the
> > drivers requiring these regulators would have already started.
> 
> This really isn't a good solution, especially not for a system that's DT
> based - on a DT system we can tell if there should be a GPIO present so
> we should be able to defer only when there's something that might
> provide the GPIO later on.

Understood, but what's the solution for non-DT systems?

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 01/12] regulator: gpio-regulator: Demote GPIO Regulator driver to start later

[Mark Brown]

On Mon, Dec 10, 2012 at 02:28:36PM +0000, Lee Jones wrote:
> On Mon, 10 Dec 2012, Mark Brown wrote:

> > This really isn't a good solution, especially not for a system that's DT
> > based - on a DT system we can tell if there should be a GPIO present so
> > we should be able to defer only when there's something that might
> > provide the GPIO later on.

> Understood, but what's the solution for non-DT systems?

Provide a fixed regulator or something, perhaps we need a "definitely
does not exist" regulator to help with this.  For every board you help
with a sequencing bodge you're probably going to break another that
needs different sequencing; for that matter it's not like GPIO
controlled regulators are exclusively used for MMC, or that MMC
exclusively uses GPIO - doing this for only one regulator is a bit of a
red flag.

[PATCH 01/12] regulator: gpio-regulator: Demote GPIO Regulator driver to start later

[Lee Jones]

On Mon, 10 Dec 2012, Mark Brown wrote:

> On Mon, Dec 10, 2012 at 02:28:36PM +0000, Lee Jones wrote:
> > On Mon, 10 Dec 2012, Mark Brown wrote:
> 
> > > This really isn't a good solution, especially not for a system that's DT
> > > based - on a DT system we can tell if there should be a GPIO present so
> > > we should be able to defer only when there's something that might
> > > provide the GPIO later on.
> 
> > Understood, but what's the solution for non-DT systems?
> 
> Provide a fixed regulator or something, perhaps we need a "definitely
> does not exist" regulator to help with this.  For every board you help
> with a sequencing bodge you're probably going to break another that
> needs different sequencing; for that matter it's not like GPIO
> controlled regulators are exclusively used for MMC, or that MMC
> exclusively uses GPIO - doing this for only one regulator is a bit of a
> red flag.

I understand your logic, hence why I wrote such a lengthy commit
message. However, I'm not sure I see a logical way around it. Asking
all users of MMCI to provide a not-regulator to declare that a
secondary regulator isn't available seems a little unreasonable to me.

Is there anything else we can do?

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog 

[PATCH 01/12] regulator: gpio-regulator: Demote GPIO Regulator driver to start later

[Mark Brown]

On Thu, Dec 13, 2012 at 11:55:24AM +0000, Lee Jones wrote:

> I understand your logic, hence why I wrote such a lengthy commit
> message. However, I'm not sure I see a logical way around it. Asking
> all users of MMCI to provide a not-regulator to declare that a
> secondary regulator isn't available seems a little unreasonable to me.

> Is there anything else we can do?

Have all the people setting up this secondary regulator explicitly
declare it?

[PATCH 02/12] regulator: gpio-regulator: Only read GPIO [dis|en]able pin if not always-on

[Lee Jones]

If a regulator is specified as always-on, then it can't have an
enable/disable pin, as it can't be turned off.

Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/regulator/gpio-regulator.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/gpio-regulator.c b/drivers/regulator/gpio-regulator.c
index 1a71be2..3afa46a 100644
--- a/drivers/regulator/gpio-regulator.c
+++ b/drivers/regulator/gpio-regulator.c
@@ -160,7 +160,14 @@ of_get_gpio_regulator_config(struct device *dev, struct device_node *np)
 
 	of_property_read_u32(np, "startup-delay-us", &config->startup_delay);
 
-	config->enable_gpio = of_get_named_gpio(np, "enable-gpio", 0);
+	if (!config->init_data->constraints.always_on) {
+		config->enable_gpio = of_get_named_gpio(np, "enable-gpio", 0);
+		if (config->enable_gpio < 0) {
+			dev_err(dev, "Couldn't get 'enable-gpio' (%d)\n",
+				config->enable_gpio);
+			return ERR_PTR(config->enable_gpio);
+		}
+	}
 
 	/* Fetch GPIOs. */
 	for (i = 0; ; i++)
-- 
1.7.9.5

[PATCH 02/12] regulator: gpio-regulator: Only read GPIO [dis|en]able pin if not always-on

[Mark Brown]

On Mon, Dec 10, 2012 at 08:55:51AM +0000, Lee Jones wrote:
> If a regulator is specified as always-on, then it can't have an
> enable/disable pin, as it can't be turned off.

Sometimes always on gets set for regulators which do have a physical
control wired up - the control might exist for use in suspend mode for
example.  Is the ability to specify an enable pin causing a practical
problem for systems?  If it is we should fix that.

[PATCH 02/12] regulator: gpio-regulator: Only read GPIO [dis|en]able pin if not always-on

[Lee Jones]

On Mon, 10 Dec 2012, Mark Brown wrote:

> On Mon, Dec 10, 2012 at 08:55:51AM +0000, Lee Jones wrote:
> > If a regulator is specified as always-on, then it can't have an
> > enable/disable pin, as it can't be turned off.
> 
> Sometimes always on gets set for regulators which do have a physical
> control wired up - the control might exist for use in suspend mode for
> example.  Is the ability to specify an enable pin causing a practical
> problem for systems?  If it is we should fix that.

I'm not sure I understand.

My logic is that there is no point in requesting a pin which can
disable a regulator that can't be disabled. Then we can follow
on from that logic and say that if a regulator is _not_ always on
this we _require_ a way to disable it, thus we insist on an enable
GPIO pin.

With me?

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 02/12] regulator: gpio-regulator: Only read GPIO [dis|en]able pin if not always-on

[Mark Brown]

On Thu, Dec 13, 2012 at 11:48:18AM +0000, Lee Jones wrote:
> On Mon, 10 Dec 2012, Mark Brown wrote:
> > On Mon, Dec 10, 2012 at 08:55:51AM +0000, Lee Jones wrote:
> > > If a regulator is specified as always-on, then it can't have an
> > > enable/disable pin, as it can't be turned off.

> > Sometimes always on gets set for regulators which do have a physical
> > control wired up - the control might exist for use in suspend mode for
> > example.  Is the ability to specify an enable pin causing a practical
> > problem for systems?  If it is we should fix that.

> My logic is that there is no point in requesting a pin which can
> disable a regulator that can't be disabled. Then we can follow
> on from that logic and say that if a regulator is _not_ always on
> this we _require_ a way to disable it, thus we insist on an enable
> GPIO pin.

> With me?

No.  Making the enable pin optional for always on regulators is fine,
forbidding it is not - that won't work for things like the suspend case
I mentioned.

[PATCH 03/12] regulator: gpio-regulator: Fix logical error in for() loop

[Lee Jones]

The cond-statement of this particular for() loop will always be
true as long as at least one voltage-shifting GPIO is present.
If it wasn't for the break below, we'd be stuck in a forever loop.
This patch inserts the correct cond-statement into the statement.

Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/regulator/gpio-regulator.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/regulator/gpio-regulator.c b/drivers/regulator/gpio-regulator.c
index 3afa46a..5462c28 100644
--- a/drivers/regulator/gpio-regulator.c
+++ b/drivers/regulator/gpio-regulator.c
@@ -181,7 +181,7 @@ of_get_gpio_regulator_config(struct device *dev, struct device_node *np)
 	if (!config->gpios)
 		return ERR_PTR(-ENOMEM);
 
-	for (i = 0; config->nr_gpios; i++) {
+	for (i = 0; i < config->nr_gpios; i++) {
 		gpio = of_get_named_gpio(np, "gpios", i);
 		if (gpio < 0)
 			break;
-- 
1.7.9.5

[PATCH 03/12] regulator: gpio-regulator: Fix logical error in for() loop

[Mark Brown]

On Mon, Dec 10, 2012 at 08:55:52AM +0000, Lee Jones wrote:
> The cond-statement of this particular for() loop will always be
> true as long as at least one voltage-shifting GPIO is present.
> If it wasn't for the break below, we'd be stuck in a forever loop.
> This patch inserts the correct cond-statement into the statement.

Applied, thanks.

[PATCH 04/12] regulator: gpio-regulator: gpio_set_value should use cansleep

[Lee Jones]

If it's possible for gpio_set_value to sleep, we should be using
the *_cansleep call instead. This patch fixes multiple warnings
from gpiolib.

Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/regulator/gpio-regulator.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/gpio-regulator.c b/drivers/regulator/gpio-regulator.c
index 5462c28..90f5221 100644
--- a/drivers/regulator/gpio-regulator.c
+++ b/drivers/regulator/gpio-regulator.c
@@ -82,7 +82,7 @@ static int gpio_regulator_set_voltage(struct regulator_dev *dev,
 
 	for (ptr = 0; ptr < data->nr_gpios; ptr++) {
 		state = (target & (1 << ptr)) >> ptr;
-		gpio_set_value(data->gpios[ptr].gpio, state);
+		gpio_set_value_cansleep(data->gpios[ptr].gpio, state);
 	}
 	data->state = target;
 
@@ -119,7 +119,7 @@ static int gpio_regulator_set_current_limit(struct regulator_dev *dev,
 
 	for (ptr = 0; ptr < data->nr_gpios; ptr++) {
 		state = (target & (1 << ptr)) >> ptr;
-		gpio_set_value(data->gpios[ptr].gpio, state);
+		gpio_set_value_cansleep(data->gpios[ptr].gpio, state);
 	}
 	data->state = target;
 
-- 
1.7.9.5

[PATCH 04/12] regulator: gpio-regulator: gpio_set_value should use cansleep

[Mark Brown]

On Mon, Dec 10, 2012 at 08:55:53AM +0000, Lee Jones wrote:
> If it's possible for gpio_set_value to sleep, we should be using
> the *_cansleep call instead. This patch fixes multiple warnings
> from gpiolib.

Applied, thanks.

[PATCH 05/12] mmc: mmci: Move ios_handler functionality into the driver

[Lee Jones]

There are currently two instances of the ios_handler being used.
Both of which mearly toy with some regulator settings. Now there
is a GPIO regulator API, we can use that instead, and lessen the
per platform burden. By doing this, we also become more Device
Tree compatible.

Cc: Chris Ball <cjb@laptop.org>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Acked-by: Ulf Hansson <ulf.hansson@stericsson.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mmc/host/mmci.c |   22 ++++++++++++++++++++++
 drivers/mmc/host/mmci.h |    1 +
 2 files changed, 23 insertions(+)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index edc3e9b..d45c931 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -1091,6 +1091,16 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	case MMC_POWER_OFF:
 		if (host->vcc)
 			ret = mmc_regulator_set_ocr(mmc, host->vcc, 0);
+
+		if (host->vqmmc) {
+			if (regulator_is_enabled(host->vqmmc)) {
+				ret = regulator_disable(host->vqmmc);
+				if (ret < 0)
+					dev_warn(mmc_dev(mmc),
+						 "unable to disable vmmc-ios\n");
+			}
+		}
+
 		break;
 	case MMC_POWER_UP:
 		if (host->vcc) {
@@ -1115,6 +1125,14 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 
 		break;
 	case MMC_POWER_ON:
+		if (host->vqmmc)
+			if (!regulator_is_enabled(host->vqmmc)) {
+				ret = regulator_enable(host->vqmmc);
+				if (ret < 0)
+					dev_warn(mmc_dev(mmc),
+						 "unable to enable vmmc-ios\n");
+			}
+
 		pwr |= MCI_PWR_ON;
 		break;
 	}
@@ -1379,6 +1397,10 @@ static int __devinit mmci_probe(struct amba_device *dev,
 				 "(using regulator instead)\n");
 		}
 	}
+
+	host->vqmmc = regulator_get(&dev->dev, "vqmmc");
+	if (IS_ERR(host->vqmmc))
+		host->vqmmc = NULL;
 #endif
 	/* Fall back to platform data if no regulator is found */
 	if (host->vcc == NULL)
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index d437ccf..b87d9e2 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -194,6 +194,7 @@ struct mmci_host {
 	struct sg_mapping_iter	sg_miter;
 	unsigned int		size;
 	struct regulator	*vcc;
+	struct regulator	*vqmmc;
 
 #ifdef CONFIG_DMA_ENGINE
 	/* DMA stuff */
-- 
1.7.9.5

[PATCH 06/12] ARM: ux500: Set correct MMCI regulator voltages in the ux5x0 Device Tree

[Lee Jones]

The voltages for the MMCI GPIO controlled voltage regulation differ
depending on who you believe and where you pick the information up
from. More recent internal code suggests that the true upper voltage
from the shifter is actually v3.3, instead of the previously
indicated v2.9. Let's mirror that change in the Device Tree file.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/boot/dts/dbx5x0.dtsi |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/dbx5x0.dtsi b/arch/arm/boot/dts/dbx5x0.dtsi
index 9f55363..bbea89a 100644
--- a/arch/arm/boot/dts/dbx5x0.dtsi
+++ b/arch/arm/boot/dts/dbx5x0.dtsi
@@ -634,12 +634,12 @@
 			compatible = "regulator-gpio";
 
 			regulator-min-microvolt = <1800000>;
-			regulator-max-microvolt = <2600000>;
+			regulator-max-microvolt = <3300000>;
 			regulator-name = "mmci-reg";
 			regulator-type = "voltage";
 
 			states = <1800000 0x1
-				  2900000 0x0>;
+				  3300000 0x0>;
 
 			status = "disabled";
 		};
-- 
1.7.9.5

[PATCH 06/12] ARM: ux500: Set correct MMCI regulator voltages in the ux5x0 Device Tree

[Linus Walleij]

On Mon, Dec 10, 2012 at 9:55 AM, Lee Jones <lee.jones@linaro.org> wrote:

> The voltages for the MMCI GPIO controlled voltage regulation differ
> depending on who you believe and where you pick the information up
> from. More recent internal code suggests that the true upper voltage
> from the shifter is actually v3.3, instead of the previously
> indicated v2.9. Let's mirror that change in the Device Tree file.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 06/12] ARM: ux500: Set correct MMCI regulator voltages in the ux5x0 Device Tree

[Ulf Hansson]

On 11 December 2012 10:17, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Mon, Dec 10, 2012 at 9:55 AM, Lee Jones <lee.jones@linaro.org> wrote:
>
>> The voltages for the MMCI GPIO controlled voltage regulation differ
>> depending on who you believe and where you pick the information up
>> from. More recent internal code suggests that the true upper voltage
>> from the shifter is actually v3.3, instead of the previously
>> indicated v2.9. Let's mirror that change in the Device Tree file.

This is wrong. The value should be 2.9V nothing else!

>>
>> Signed-off-by: Lee Jones <lee.jones@linaro.org>
>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
>
> Yours,
> Linus Walleij

Kind regards
Ulf Hansson

[PATCH 06/12] ARM: ux500: Set correct MMCI regulator voltages in the ux5x0 Device Tree

[Lee Jones]

On Tue, 11 Dec 2012, Ulf Hansson wrote:

> On 11 December 2012 10:17, Linus Walleij <linus.walleij@linaro.org> wrote:
> > On Mon, Dec 10, 2012 at 9:55 AM, Lee Jones <lee.jones@linaro.org> wrote:
> >
> >> The voltages for the MMCI GPIO controlled voltage regulation differ
> >> depending on who you believe and where you pick the information up
> >> from. More recent internal code suggests that the true upper voltage
> >> from the shifter is actually v3.3, instead of the previously
> >> indicated v2.9. Let's mirror that change in the Device Tree file.
> 
> This is wrong. The value should be 2.9V nothing else!

Don't panic. I'm already on this. :)

> >> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> >
> > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> >
> > Yours,
> > Linus Walleij
> 
> Kind regards
> Ulf Hansson

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 07/12] ARM: ux500: Specify the ux5x0 MMCI regulator's on/off GPIO as high-enable

[Lee Jones]

If not specified, the GPIO control bit is inverted by default i.e.
low-enable and high-disable. This is not the case with the MMCI
regulator, hence it will turn on during a disable and off when
regulator_enable() is invoked.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/boot/dts/dbx5x0.dtsi |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/dbx5x0.dtsi b/arch/arm/boot/dts/dbx5x0.dtsi
index bbea89a..0523681 100644
--- a/arch/arm/boot/dts/dbx5x0.dtsi
+++ b/arch/arm/boot/dts/dbx5x0.dtsi
@@ -638,6 +638,8 @@
 			regulator-name = "mmci-reg";
 			regulator-type = "voltage";
 
+			enable-active-high;
+
 			states = <1800000 0x1
 				  3300000 0x0>;
 
-- 
1.7.9.5

[PATCH 07/12] ARM: ux500: Specify the ux5x0 MMCI regulator's on/off GPIO as high-enable

[Linus Walleij]

On Mon, Dec 10, 2012 at 9:55 AM, Lee Jones <lee.jones@linaro.org> wrote:

> If not specified, the GPIO control bit is inverted by default i.e.
> low-enable and high-disable. This is not the case with the MMCI
> regulator, hence it will turn on during a disable and off when
> regulator_enable() is invoked.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 08/12] ARM: ux500: Specify which IOS regulator to use for MMCI

[Lee Jones]

In an effort to move platform specific GPIO controlled regulators
out from platform code we've created a new mechanism to specify
them from within the MMCI driver using the supply name 'vmmc-ios'.
For that to happen when booting device tree, we need to supply it
in the MMCI (SDI) node.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/boot/dts/href.dtsi |    1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/href.dtsi b/arch/arm/boot/dts/href.dtsi
index 592fb9d..f2c0f66 100644
--- a/arch/arm/boot/dts/href.dtsi
+++ b/arch/arm/boot/dts/href.dtsi
@@ -87,6 +87,7 @@
 			mmc-cap-sd-highspeed;
 			mmc-cap-mmc-highspeed;
 			vmmc-supply = <&ab8500_ldo_aux3_reg>;
+			vqmmc-supply = <&vmmci>;
 
 			cd-gpios  = <&tc3589x_gpio 3 0x4>;
 
-- 
1.7.9.5

[PATCH 08/12] ARM: ux500: Specify which IOS regulator to use for MMCI

[Linus Walleij]

On Mon, Dec 10, 2012 at 9:55 AM, Lee Jones <lee.jones@linaro.org> wrote:

> In an effort to move platform specific GPIO controlled regulators
> out from platform code we've created a new mechanism to specify
> them from within the MMCI driver using the supply name 'vmmc-ios'.
> For that to happen when booting device tree, we need to supply it
> in the MMCI (SDI) node.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 09/12] ARM: ux500: Use the correct name when supplying a GPIO enable pin

[Lee Jones]

Correct a typo in the Device Tree source file, where instead of
specifying property 'enable-gpio', which the driver is expecting
we specified 'gpio-enable' instead.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/boot/dts/hrefprev60.dts |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/hrefprev60.dts b/arch/arm/boot/dts/hrefprev60.dts
index eec29c4..9194fb6 100644
--- a/arch/arm/boot/dts/hrefprev60.dts
+++ b/arch/arm/boot/dts/hrefprev60.dts
@@ -40,7 +40,7 @@
 
 		vmmci: regulator-gpio {
 			gpios = <&tc3589x_gpio 18 0x4>;
-			gpio-enable = <&tc3589x_gpio 17 0x4>;
+			enable-gpio = <&tc3589x_gpio 17 0x4>;
 
 			status = "okay";
 		};
-- 
1.7.9.5

[PATCH 09/12] ARM: ux500: Use the correct name when supplying a GPIO enable pin

[Linus Walleij]

On Mon, Dec 10, 2012 at 9:55 AM, Lee Jones <lee.jones@linaro.org> wrote:

> Correct a typo in the Device Tree source file, where instead of
> specifying property 'enable-gpio', which the driver is expecting
> we specified 'gpio-enable' instead.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 10/12] ARM: ux500: Setup correct settling time for the MMCI regulator

[Lee Jones]

The GPIO controlled MMCI regulator used on the ux5x0 boards takes
100us to settle. There's already a binding to provide such
information. Let's make use of it.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/boot/dts/dbx5x0.dtsi |    1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/dbx5x0.dtsi b/arch/arm/boot/dts/dbx5x0.dtsi
index 0523681..2337b74 100644
--- a/arch/arm/boot/dts/dbx5x0.dtsi
+++ b/arch/arm/boot/dts/dbx5x0.dtsi
@@ -638,6 +638,7 @@
 			regulator-name = "mmci-reg";
 			regulator-type = "voltage";
 
+			startup-delay-us = <100>;
 			enable-active-high;
 
 			states = <1800000 0x1
-- 
1.7.9.5

[PATCH 10/12] ARM: ux500: Setup correct settling time for the MMCI regulator

[Linus Walleij]

On Mon, Dec 10, 2012 at 9:55 AM, Lee Jones <lee.jones@linaro.org> wrote:

> The GPIO controlled MMCI regulator used on the ux5x0 boards takes
> 100us to settle. There's already a binding to provide such
> information. Let's make use of it.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 11/12] ARM: ux500: Use the GPIO regulator framework for SDI0's 'en' and 'vsel'

[Lee Jones]

To prevent lots of unnecessary call-backs into platform code, we're
now using the GPIO regulator framework to control the 'enable' (en)
and 'voltage select' (vsel) GPIO pins which in turn control the
MMCI's secondary regulator settings. This already works with Device
Tree, but when booting with ATAGs we need to register it as a
platform device.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/mach-ux500/board-mop500.c |   61 ++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/arch/arm/mach-ux500/board-mop500.c b/arch/arm/mach-ux500/board-mop500.c
index daa4237..9f9fe7f 100644
--- a/arch/arm/mach-ux500/board-mop500.c
+++ b/arch/arm/mach-ux500/board-mop500.c
@@ -24,6 +24,8 @@
 #include <linux/mfd/abx500/ab8500.h>
 #include <linux/regulator/ab8500.h>
 #include <linux/regulator/fixed.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/gpio-regulator.h>
 #include <linux/mfd/tc3589x.h>
 #include <linux/mfd/tps6105x.h>
 #include <linux/mfd/abx500/ab8500-gpio.h>
@@ -91,6 +93,54 @@ static struct platform_device snowball_gpio_en_3v3_regulator_dev = {
        },
 };
 
+static struct regulator_consumer_supply sdi0_reg_consumers[] = {
+        REGULATOR_SUPPLY("vqmmc", NULL),
+};
+
+static struct regulator_init_data sdi0_reg_init_data = {
+        .constraints = {
+                .min_uV         = 1800000,
+                .max_uV         = 3300000,
+                .valid_ops_mask = REGULATOR_CHANGE_VOLTAGE|REGULATOR_CHANGE_STATUS,
+        },
+        .num_consumer_supplies  = ARRAY_SIZE(sdi0_reg_consumers),
+        .consumer_supplies      = sdi0_reg_consumers,
+};
+
+/* Dynamically populated. */
+static struct gpio sdi0_reg_gpios[] = {
+        { 0, GPIOF_OUT_INIT_LOW, "mmci_vsel" },
+};
+
+static struct gpio_regulator_state sdi0_reg_states[] = {
+        { .value = 3300000, .gpios = (0 << 0) },
+        { .value = 1800000, .gpios = (1 << 0) },
+};
+
+static struct gpio_regulator_config sdi0_reg_info = {
+        .supply_name = "mmci-reg",
+
+        .enable_high = 1,
+        .enabled_at_boot = 0,
+
+        .gpios = sdi0_reg_gpios,
+        .nr_gpios = ARRAY_SIZE(sdi0_reg_gpios),
+
+        .states = sdi0_reg_states,
+        .nr_states = ARRAY_SIZE(sdi0_reg_states),
+
+        .type = REGULATOR_VOLTAGE,
+        .init_data = &sdi0_reg_init_data,
+};
+
+static struct platform_device sdi0_regulator = {
+        .name = "gpio-regulator",
+        .id   = -1,
+        .dev  = {
+                .platform_data = &sdi0_reg_info,
+        },
+};
+
 static struct ab8500_gpio_platform_data ab8500_gpio_pdata = {
 	.gpio_base		= MOP500_AB8500_PIN_GPIO(1),
 	.irq_base		= MOP500_AB8500_VIR_GPIO_IRQ_BASE,
@@ -440,6 +490,7 @@ static struct hash_platform_data u8500_hash1_platform_data = {
 /* add any platform devices here - TODO */
 static struct platform_device *mop500_platform_devs[] __initdata = {
 	&mop500_gpio_keys_device,
+	&sdi0_regulator,
 };
 
 #ifdef CONFIG_STE_DMA40
@@ -581,6 +632,7 @@ static struct platform_device *snowball_platform_devs[] __initdata = {
 	&snowball_key_dev,
 	&snowball_sbnet_dev,
 	&snowball_gpio_en_3v3_regulator_dev,
+	&sdi0_regulator,
 };
 
 static void __init mop500_init_machine(void)
@@ -591,6 +643,9 @@ static void __init mop500_init_machine(void)
 
 	mop500_gpio_keys[0].gpio = GPIO_PROX_SENSOR;
 
+	sdi0_reg_info.enable_gpio = GPIO_SDMMC_EN;
+	sdi0_reg_info.gpios[0].gpio = GPIO_SDMMC_1V8_3V_SEL;
+
 	mop500_pinmaps_init();
 	parent = u8500_init_devices(&ab8500_platdata);
 
@@ -623,6 +678,9 @@ static void __init snowball_init_machine(void)
 	struct device *parent = NULL;
 	int i;
 
+	sdi0_reg_info.enable_gpio = SNOWBALL_SDMMC_EN_GPIO;
+	sdi0_reg_info.gpios[0].gpio = SNOWBALL_SDMMC_1V8_3V_GPIO;
+
 	snowball_pinmaps_init();
 	parent = u8500_init_devices(&ab8500_platdata);
 
@@ -655,6 +713,9 @@ static void __init hrefv60_init_machine(void)
 	 */
 	mop500_gpio_keys[0].gpio = HREFV60_PROX_SENSE_GPIO;
 
+	sdi0_reg_info.enable_gpio = HREFV60_SDMMC_EN_GPIO;
+	sdi0_reg_info.gpios[0].gpio = HREFV60_SDMMC_1V8_3V_GPIO;
+
 	hrefv60_pinmaps_init();
 	parent = u8500_init_devices(&ab8500_platdata);
 
-- 
1.7.9.5

[PATCH 11/12] ARM: ux500: Use the GPIO regulator framework for SDI0's 'en' and 'vsel'

[Ulf Hansson]

On 10 December 2012 09:56, Lee Jones <lee.jones@linaro.org> wrote:
> To prevent lots of unnecessary call-backs into platform code, we're
> now using the GPIO regulator framework to control the 'enable' (en)
> and 'voltage select' (vsel) GPIO pins which in turn control the
> MMCI's secondary regulator settings. This already works with Device
> Tree, but when booting with ATAGs we need to register it as a
> platform device.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  arch/arm/mach-ux500/board-mop500.c |   61 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 61 insertions(+)
>
> diff --git a/arch/arm/mach-ux500/board-mop500.c b/arch/arm/mach-ux500/board-mop500.c
> index daa4237..9f9fe7f 100644
> --- a/arch/arm/mach-ux500/board-mop500.c
> +++ b/arch/arm/mach-ux500/board-mop500.c
> @@ -24,6 +24,8 @@
>  #include <linux/mfd/abx500/ab8500.h>
>  #include <linux/regulator/ab8500.h>
>  #include <linux/regulator/fixed.h>
> +#include <linux/regulator/driver.h>
> +#include <linux/regulator/gpio-regulator.h>
>  #include <linux/mfd/tc3589x.h>
>  #include <linux/mfd/tps6105x.h>
>  #include <linux/mfd/abx500/ab8500-gpio.h>
> @@ -91,6 +93,54 @@ static struct platform_device snowball_gpio_en_3v3_regulator_dev = {
>         },
>  };
>
> +static struct regulator_consumer_supply sdi0_reg_consumers[] = {
> +        REGULATOR_SUPPLY("vqmmc", NULL),

REGULATOR_SUPPLY("vqmmc", "sdi0"),

> +};
> +
> +static struct regulator_init_data sdi0_reg_init_data = {

The levelshifter uses the ab8500-ext-supply3. Reflect that here.

.supply_regulator = "ab8500-ext-supply3"


> +        .constraints = {
> +                .min_uV         = 1800000,

2.9V, not 3.3V

> +                .max_uV         = 3300000,
> +                .valid_ops_mask = REGULATOR_CHANGE_VOLTAGE|REGULATOR_CHANGE_STATUS,
> +        },
> +        .num_consumer_supplies  = ARRAY_SIZE(sdi0_reg_consumers),
> +        .consumer_supplies      = sdi0_reg_consumers,
> +};
> +
> +/* Dynamically populated. */
> +static struct gpio sdi0_reg_gpios[] = {
> +        { 0, GPIOF_OUT_INIT_LOW, "mmci_vsel" },
> +};
> +
> +static struct gpio_regulator_state sdi0_reg_states[] = {
> +        { .value = 3300000, .gpios = (0 << 0) },

2.9V, not 3.3V

> +        { .value = 1800000, .gpios = (1 << 0) },
> +};
> +
> +static struct gpio_regulator_config sdi0_reg_info = {
> +        .supply_name = "mmci-reg",
> +
> +        .enable_high = 1,
> +        .enabled_at_boot = 0,
> +
> +        .gpios = sdi0_reg_gpios,
> +        .nr_gpios = ARRAY_SIZE(sdi0_reg_gpios),
> +
> +        .states = sdi0_reg_states,
> +        .nr_states = ARRAY_SIZE(sdi0_reg_states),
> +
> +        .type = REGULATOR_VOLTAGE,
> +        .init_data = &sdi0_reg_init_data,
> +};
> +
> +static struct platform_device sdi0_regulator = {
> +        .name = "gpio-regulator",
> +        .id   = -1,
> +        .dev  = {
> +                .platform_data = &sdi0_reg_info,
> +        },
> +};
> +
>  static struct ab8500_gpio_platform_data ab8500_gpio_pdata = {
>         .gpio_base              = MOP500_AB8500_PIN_GPIO(1),
>         .irq_base               = MOP500_AB8500_VIR_GPIO_IRQ_BASE,
> @@ -440,6 +490,7 @@ static struct hash_platform_data u8500_hash1_platform_data = {
>  /* add any platform devices here - TODO */
>  static struct platform_device *mop500_platform_devs[] __initdata = {
>         &mop500_gpio_keys_device,
> +       &sdi0_regulator,
>  };
>
>  #ifdef CONFIG_STE_DMA40
> @@ -581,6 +632,7 @@ static struct platform_device *snowball_platform_devs[] __initdata = {
>         &snowball_key_dev,
>         &snowball_sbnet_dev,
>         &snowball_gpio_en_3v3_regulator_dev,
> +       &sdi0_regulator,
>  };
>
>  static void __init mop500_init_machine(void)
> @@ -591,6 +643,9 @@ static void __init mop500_init_machine(void)
>
>         mop500_gpio_keys[0].gpio = GPIO_PROX_SENSOR;
>
> +       sdi0_reg_info.enable_gpio = GPIO_SDMMC_EN;
> +       sdi0_reg_info.gpios[0].gpio = GPIO_SDMMC_1V8_3V_SEL;
> +
>         mop500_pinmaps_init();
>         parent = u8500_init_devices(&ab8500_platdata);
>
> @@ -623,6 +678,9 @@ static void __init snowball_init_machine(void)
>         struct device *parent = NULL;
>         int i;
>
> +       sdi0_reg_info.enable_gpio = SNOWBALL_SDMMC_EN_GPIO;
> +       sdi0_reg_info.gpios[0].gpio = SNOWBALL_SDMMC_1V8_3V_GPIO;
> +
>         snowball_pinmaps_init();
>         parent = u8500_init_devices(&ab8500_platdata);
>
> @@ -655,6 +713,9 @@ static void __init hrefv60_init_machine(void)
>          */
>         mop500_gpio_keys[0].gpio = HREFV60_PROX_SENSE_GPIO;
>
> +       sdi0_reg_info.enable_gpio = HREFV60_SDMMC_EN_GPIO;
> +       sdi0_reg_info.gpios[0].gpio = HREFV60_SDMMC_1V8_3V_GPIO;
> +
>         hrefv60_pinmaps_init();
>         parent = u8500_init_devices(&ab8500_platdata);
>
> --
> 1.7.9.5
>

Finally, I suppose regulator inits i located in
board-mop500-regulators.c, so I would suggest to move most of this
code in there instead.

Kind regards
Ulf Hansson

[PATCH 11/12] ARM: ux500: Use the GPIO regulator framework for SDI0's 'en' and 'vsel'

[Ulf Hansson]

On 10 December 2012 11:18, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 10 December 2012 09:56, Lee Jones <lee.jones@linaro.org> wrote:
>> To prevent lots of unnecessary call-backs into platform code, we're
>> now using the GPIO regulator framework to control the 'enable' (en)
>> and 'voltage select' (vsel) GPIO pins which in turn control the
>> MMCI's secondary regulator settings. This already works with Device
>> Tree, but when booting with ATAGs we need to register it as a
>> platform device.
>>
>> Signed-off-by: Lee Jones <lee.jones@linaro.org>
>> ---
>>  arch/arm/mach-ux500/board-mop500.c |   61 ++++++++++++++++++++++++++++++++++++
>>  1 file changed, 61 insertions(+)
>>
>> diff --git a/arch/arm/mach-ux500/board-mop500.c b/arch/arm/mach-ux500/board-mop500.c
>> index daa4237..9f9fe7f 100644
>> --- a/arch/arm/mach-ux500/board-mop500.c
>> +++ b/arch/arm/mach-ux500/board-mop500.c
>> @@ -24,6 +24,8 @@
>>  #include <linux/mfd/abx500/ab8500.h>
>>  #include <linux/regulator/ab8500.h>
>>  #include <linux/regulator/fixed.h>
>> +#include <linux/regulator/driver.h>
>> +#include <linux/regulator/gpio-regulator.h>
>>  #include <linux/mfd/tc3589x.h>
>>  #include <linux/mfd/tps6105x.h>
>>  #include <linux/mfd/abx500/ab8500-gpio.h>
>> @@ -91,6 +93,54 @@ static struct platform_device snowball_gpio_en_3v3_regulator_dev = {
>>         },
>>  };
>>
>> +static struct regulator_consumer_supply sdi0_reg_consumers[] = {
>> +        REGULATOR_SUPPLY("vqmmc", NULL),
>
> REGULATOR_SUPPLY("vqmmc", "sdi0"),
>
>> +};
>> +
>> +static struct regulator_init_data sdi0_reg_init_data = {
>
> The levelshifter uses the ab8500-ext-supply3. Reflect that here.
>
> .supply_regulator = "ab8500-ext-supply3"
>
>
>> +        .constraints = {
>> +                .min_uV         = 1800000,
>
> 2.9V, not 3.3V
>
>> +                .max_uV         = 3300000,
>> +                .valid_ops_mask = REGULATOR_CHANGE_VOLTAGE|REGULATOR_CHANGE_STATUS,
>> +        },
>> +        .num_consumer_supplies  = ARRAY_SIZE(sdi0_reg_consumers),
>> +        .consumer_supplies      = sdi0_reg_consumers,
>> +};
>> +
>> +/* Dynamically populated. */
>> +static struct gpio sdi0_reg_gpios[] = {
>> +        { 0, GPIOF_OUT_INIT_LOW, "mmci_vsel" },
>> +};
>> +
>> +static struct gpio_regulator_state sdi0_reg_states[] = {
>> +        { .value = 3300000, .gpios = (0 << 0) },
>
> 2.9V, not 3.3V
>
>> +        { .value = 1800000, .gpios = (1 << 0) },
>> +};
>> +
>> +static struct gpio_regulator_config sdi0_reg_info = {
>> +        .supply_name = "mmci-reg",
>> +
>> +        .enable_high = 1,
>> +        .enabled_at_boot = 0,
>> +
>> +        .gpios = sdi0_reg_gpios,
>> +        .nr_gpios = ARRAY_SIZE(sdi0_reg_gpios),
>> +

I think the "settling time" is missing here. 100us according to
levelshifter spec.

>> +        .states = sdi0_reg_states,
>> +        .nr_states = ARRAY_SIZE(sdi0_reg_states),
>> +
>> +        .type = REGULATOR_VOLTAGE,
>> +        .init_data = &sdi0_reg_init_data,
>> +};
>> +
>> +static struct platform_device sdi0_regulator = {
>> +        .name = "gpio-regulator",
>> +        .id   = -1,
>> +        .dev  = {
>> +                .platform_data = &sdi0_reg_info,
>> +        },
>> +};
>> +
>>  static struct ab8500_gpio_platform_data ab8500_gpio_pdata = {
>>         .gpio_base              = MOP500_AB8500_PIN_GPIO(1),
>>         .irq_base               = MOP500_AB8500_VIR_GPIO_IRQ_BASE,
>> @@ -440,6 +490,7 @@ static struct hash_platform_data u8500_hash1_platform_data = {
>>  /* add any platform devices here - TODO */
>>  static struct platform_device *mop500_platform_devs[] __initdata = {
>>         &mop500_gpio_keys_device,
>> +       &sdi0_regulator,
>>  };
>>
>>  #ifdef CONFIG_STE_DMA40
>> @@ -581,6 +632,7 @@ static struct platform_device *snowball_platform_devs[] __initdata = {
>>         &snowball_key_dev,
>>         &snowball_sbnet_dev,
>>         &snowball_gpio_en_3v3_regulator_dev,
>> +       &sdi0_regulator,
>>  };
>>
>>  static void __init mop500_init_machine(void)
>> @@ -591,6 +643,9 @@ static void __init mop500_init_machine(void)
>>
>>         mop500_gpio_keys[0].gpio = GPIO_PROX_SENSOR;
>>
>> +       sdi0_reg_info.enable_gpio = GPIO_SDMMC_EN;
>> +       sdi0_reg_info.gpios[0].gpio = GPIO_SDMMC_1V8_3V_SEL;
>> +
>>         mop500_pinmaps_init();
>>         parent = u8500_init_devices(&ab8500_platdata);
>>
>> @@ -623,6 +678,9 @@ static void __init snowball_init_machine(void)
>>         struct device *parent = NULL;
>>         int i;
>>
>> +       sdi0_reg_info.enable_gpio = SNOWBALL_SDMMC_EN_GPIO;
>> +       sdi0_reg_info.gpios[0].gpio = SNOWBALL_SDMMC_1V8_3V_GPIO;
>> +
>>         snowball_pinmaps_init();
>>         parent = u8500_init_devices(&ab8500_platdata);
>>
>> @@ -655,6 +713,9 @@ static void __init hrefv60_init_machine(void)
>>          */
>>         mop500_gpio_keys[0].gpio = HREFV60_PROX_SENSE_GPIO;
>>
>> +       sdi0_reg_info.enable_gpio = HREFV60_SDMMC_EN_GPIO;
>> +       sdi0_reg_info.gpios[0].gpio = HREFV60_SDMMC_1V8_3V_GPIO;
>> +
>>         hrefv60_pinmaps_init();
>>         parent = u8500_init_devices(&ab8500_platdata);
>>
>> --
>> 1.7.9.5
>>
>
> Finally, I suppose regulator inits i located in
> board-mop500-regulators.c, so I would suggest to move most of this
> code in there instead.
>
> Kind regards
> Ulf Hansson

[PATCH 11/12] ARM: ux500: Use the GPIO regulator framework for SDI0's 'en' and 'vsel'

[Lee Jones]

> > +        .constraints = {
> > +                .min_uV         = 1800000,
> 
> 2.9V, not 3.3V

> > +static struct gpio_regulator_state sdi0_reg_states[] = {
> > +        { .value = 3300000, .gpios = (0 << 0) },
> 
> 2.9V, not 3.3V

I'm still a little unsure about this. I know the actual
voltage is v2.9, but the supported/requested MMC voltage
from the driver is v3.3.

Will it still work if I set it all up as v2.9?

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 11/12] ARM: ux500: Use the GPIO regulator framework for SDI0's 'en' and 'vsel'

[Ulf Hansson]

On 10 December 2012 11:30, Lee Jones <lee.jones@linaro.org> wrote:
>> > +        .constraints = {
>> > +                .min_uV         = 1800000,
>>
>> 2.9V, not 3.3V
>
>> > +static struct gpio_regulator_state sdi0_reg_states[] = {
>> > +        { .value = 3300000, .gpios = (0 << 0) },
>>
>> 2.9V, not 3.3V
>
> I'm still a little unsure about this. I know the actual
> voltage is v2.9, but the supported/requested MMC voltage
> from the driver is v3.3.
>
> Will it still work if I set it all up as v2.9?
>

So that is if course important to consider. Although the regulator
must nu lie about what it actually supports.

In this case mmci driver will have to decide what to do when 3.3V is
requested, but only 2.9 is supported.

> --
> Lee Jones
> Linaro ST-Ericsson Landing Team Lead
> Linaro.org ? Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog

Kind regards
Ulf Hansson

[PATCH 11/12 v2] ARM: ux500: Use the GPIO regulator framework for SDI0's 'en' and 'vsel'

[Lee Jones]

To prevent lots of unnecessary call-backs into platform code, we're
now using the GPIO regulator framework to control the 'enable' (en)
and 'voltage select' (vsel) GPIO pins which in turn control the
MMCI's secondary regulator settings. This already works with Device
Tree, but when booting with ATAGs we need to register it as a
platform device.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/mach-ux500/board-mop500-regulators.c |   14 ++++++++
 arch/arm/mach-ux500/board-mop500-regulators.h |    1 +
 arch/arm/mach-ux500/board-mop500.c            |   44 +++++++++++++++++++++++++
 3 files changed, 59 insertions(+)

diff --git a/arch/arm/mach-ux500/board-mop500-regulators.c b/arch/arm/mach-ux500/board-mop500-regulators.c
index 2a17bc5..cb75405 100644
--- a/arch/arm/mach-ux500/board-mop500-regulators.c
+++ b/arch/arm/mach-ux500/board-mop500-regulators.c
@@ -28,6 +28,20 @@ struct regulator_init_data gpio_en_3v3_regulator = {
        .consumer_supplies = gpio_en_3v3_consumers,
 };
 
+static struct regulator_consumer_supply sdi0_reg_consumers[] = {
+        REGULATOR_SUPPLY("vqmmc", "sdi0"),
+};
+
+struct regulator_init_data sdi0_reg_init_data = {
+        .constraints = {
+                .min_uV         = 1800000,
+                .max_uV         = 2900000,
+                .valid_ops_mask = REGULATOR_CHANGE_VOLTAGE|REGULATOR_CHANGE_STATUS,
+        },
+        .num_consumer_supplies  = ARRAY_SIZE(sdi0_reg_consumers),
+        .consumer_supplies      = sdi0_reg_consumers,
+};
+
 /*
  * TPS61052 regulator
  */
diff --git a/arch/arm/mach-ux500/board-mop500-regulators.h b/arch/arm/mach-ux500/board-mop500-regulators.h
index 78a0642..0c79d90 100644
--- a/arch/arm/mach-ux500/board-mop500-regulators.h
+++ b/arch/arm/mach-ux500/board-mop500-regulators.h
@@ -19,5 +19,6 @@ ab8500_regulator_reg_init[AB8500_NUM_REGULATOR_REGISTERS];
 extern struct regulator_init_data ab8500_regulators[AB8500_NUM_REGULATORS];
 extern struct regulator_init_data tps61052_regulator;
 extern struct regulator_init_data gpio_en_3v3_regulator;
+extern struct regulator_init_data sdi0_reg_init_data;
 
 #endif
diff --git a/arch/arm/mach-ux500/board-mop500.c b/arch/arm/mach-ux500/board-mop500.c
index daa4237..1d721ea 100644
--- a/arch/arm/mach-ux500/board-mop500.c
+++ b/arch/arm/mach-ux500/board-mop500.c
@@ -24,6 +24,8 @@
 #include <linux/mfd/abx500/ab8500.h>
 #include <linux/regulator/ab8500.h>
 #include <linux/regulator/fixed.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/gpio-regulator.h>
 #include <linux/mfd/tc3589x.h>
 #include <linux/mfd/tps6105x.h>
 #include <linux/mfd/abx500/ab8500-gpio.h>
@@ -91,6 +93,37 @@ static struct platform_device snowball_gpio_en_3v3_regulator_dev = {
        },
 };
 
+/* Dynamically populated. */
+static struct gpio sdi0_reg_gpios[] = {
+        { 0, GPIOF_OUT_INIT_LOW, "mmci_vsel" },
+};
+
+static struct gpio_regulator_state sdi0_reg_states[] = {
+        { .value = 2900000, .gpios = (0 << 0) },
+        { .value = 1800000, .gpios = (1 << 0) },
+};
+
+static struct gpio_regulator_config sdi0_reg_info = {
+	.supply_name           = "ab8500-ext-supply3",
+	.gpios                 = sdi0_reg_gpios,
+	.nr_gpios              = ARRAY_SIZE(sdi0_reg_gpios),
+	.states                = sdi0_reg_states,
+	.nr_states             = ARRAY_SIZE(sdi0_reg_states),
+	.type                  = REGULATOR_VOLTAGE,
+	.enable_high           = 1,
+	.enabled_at_boot       = 0,
+	.init_data             = &sdi0_reg_init_data,
+	.startup_delay         = 100,
+};
+
+static struct platform_device sdi0_regulator = {
+        .name = "gpio-regulator",
+        .id   = -1,
+        .dev  = {
+                .platform_data = &sdi0_reg_info,
+        },
+};
+
 static struct ab8500_gpio_platform_data ab8500_gpio_pdata = {
 	.gpio_base		= MOP500_AB8500_PIN_GPIO(1),
 	.irq_base		= MOP500_AB8500_VIR_GPIO_IRQ_BASE,
@@ -440,6 +473,7 @@ static struct hash_platform_data u8500_hash1_platform_data = {
 /* add any platform devices here - TODO */
 static struct platform_device *mop500_platform_devs[] __initdata = {
 	&mop500_gpio_keys_device,
+	&sdi0_regulator,
 };
 
 #ifdef CONFIG_STE_DMA40
@@ -581,6 +615,7 @@ static struct platform_device *snowball_platform_devs[] __initdata = {
 	&snowball_key_dev,
 	&snowball_sbnet_dev,
 	&snowball_gpio_en_3v3_regulator_dev,
+	&sdi0_regulator,
 };
 
 static void __init mop500_init_machine(void)
@@ -591,6 +626,9 @@ static void __init mop500_init_machine(void)
 
 	mop500_gpio_keys[0].gpio = GPIO_PROX_SENSOR;
 
+	sdi0_reg_info.enable_gpio = GPIO_SDMMC_EN;
+	sdi0_reg_info.gpios[0].gpio = GPIO_SDMMC_1V8_3V_SEL;
+
 	mop500_pinmaps_init();
 	parent = u8500_init_devices(&ab8500_platdata);
 
@@ -623,6 +661,9 @@ static void __init snowball_init_machine(void)
 	struct device *parent = NULL;
 	int i;
 
+	sdi0_reg_info.enable_gpio = SNOWBALL_SDMMC_EN_GPIO;
+	sdi0_reg_info.gpios[0].gpio = SNOWBALL_SDMMC_1V8_3V_GPIO;
+
 	snowball_pinmaps_init();
 	parent = u8500_init_devices(&ab8500_platdata);
 
@@ -655,6 +696,9 @@ static void __init hrefv60_init_machine(void)
 	 */
 	mop500_gpio_keys[0].gpio = HREFV60_PROX_SENSE_GPIO;
 
+	sdi0_reg_info.enable_gpio = HREFV60_SDMMC_EN_GPIO;
+	sdi0_reg_info.gpios[0].gpio = HREFV60_SDMMC_1V8_3V_GPIO;
+
 	hrefv60_pinmaps_init();
 	parent = u8500_init_devices(&ab8500_platdata);
 
-- 
1.7.9.5

[PATCH 11/12 v2] ARM: ux500: Use the GPIO regulator framework for SDI0's 'en' and 'vsel'

[Ulf Hansson]

On 10 December 2012 12:08, Lee Jones <lee.jones@linaro.org> wrote:
> To prevent lots of unnecessary call-backs into platform code, we're
> now using the GPIO regulator framework to control the 'enable' (en)
> and 'voltage select' (vsel) GPIO pins which in turn control the
> MMCI's secondary regulator settings. This already works with Device
> Tree, but when booting with ATAGs we need to register it as a
> platform device.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  arch/arm/mach-ux500/board-mop500-regulators.c |   14 ++++++++
>  arch/arm/mach-ux500/board-mop500-regulators.h |    1 +
>  arch/arm/mach-ux500/board-mop500.c            |   44 +++++++++++++++++++++++++
>  3 files changed, 59 insertions(+)
>
> diff --git a/arch/arm/mach-ux500/board-mop500-regulators.c b/arch/arm/mach-ux500/board-mop500-regulators.c
> index 2a17bc5..cb75405 100644
> --- a/arch/arm/mach-ux500/board-mop500-regulators.c
> +++ b/arch/arm/mach-ux500/board-mop500-regulators.c
> @@ -28,6 +28,20 @@ struct regulator_init_data gpio_en_3v3_regulator = {
>         .consumer_supplies = gpio_en_3v3_consumers,
>  };
>
> +static struct regulator_consumer_supply sdi0_reg_consumers[] = {
> +        REGULATOR_SUPPLY("vqmmc", "sdi0"),
> +};
> +
> +struct regulator_init_data sdi0_reg_init_data = {

You missed this:

The levelshifter uses the ab8500-ext-supply3. Reflect that here.

.supply_regulator = "ab8500-ext-supply3"

> +        .constraints = {
> +                .min_uV         = 1800000,
> +                .max_uV         = 2900000,
> +                .valid_ops_mask = REGULATOR_CHANGE_VOLTAGE|REGULATOR_CHANGE_STATUS,
> +        },
> +        .num_consumer_supplies  = ARRAY_SIZE(sdi0_reg_consumers),
> +        .consumer_supplies      = sdi0_reg_consumers,
> +};
> +
>  /*
>   * TPS61052 regulator
>   */
> diff --git a/arch/arm/mach-ux500/board-mop500-regulators.h b/arch/arm/mach-ux500/board-mop500-regulators.h
> index 78a0642..0c79d90 100644
> --- a/arch/arm/mach-ux500/board-mop500-regulators.h
> +++ b/arch/arm/mach-ux500/board-mop500-regulators.h
> @@ -19,5 +19,6 @@ ab8500_regulator_reg_init[AB8500_NUM_REGULATOR_REGISTERS];
>  extern struct regulator_init_data ab8500_regulators[AB8500_NUM_REGULATORS];
>  extern struct regulator_init_data tps61052_regulator;
>  extern struct regulator_init_data gpio_en_3v3_regulator;
> +extern struct regulator_init_data sdi0_reg_init_data;
>
>  #endif
> diff --git a/arch/arm/mach-ux500/board-mop500.c b/arch/arm/mach-ux500/board-mop500.c
> index daa4237..1d721ea 100644
> --- a/arch/arm/mach-ux500/board-mop500.c
> +++ b/arch/arm/mach-ux500/board-mop500.c
> @@ -24,6 +24,8 @@
>  #include <linux/mfd/abx500/ab8500.h>
>  #include <linux/regulator/ab8500.h>
>  #include <linux/regulator/fixed.h>
> +#include <linux/regulator/driver.h>
> +#include <linux/regulator/gpio-regulator.h>
>  #include <linux/mfd/tc3589x.h>
>  #include <linux/mfd/tps6105x.h>
>  #include <linux/mfd/abx500/ab8500-gpio.h>
> @@ -91,6 +93,37 @@ static struct platform_device snowball_gpio_en_3v3_regulator_dev = {
>         },
>  };
>
> +/* Dynamically populated. */
> +static struct gpio sdi0_reg_gpios[] = {t
> +        { 0, GPIOF_OUT_INIT_LOW, "mmci_vsel" },
> +};
> +
> +static struct gpio_regulator_state sdi0_reg_states[] = {
> +        { .value = 2900000, .gpios = (0 << 0) },
> +        { .value = 1800000, .gpios = (1 << 0) },
> +};
> +
> +static struct gpio_regulator_config sdi0_reg_info = {
> +       .supply_name           = "ab8500-ext-supply3",

No, this is not what I think you want. This regulator can be called
something with sdcard or levelshifter...

> +       .gpios                 = sdi0_reg_gpios,
> +       .nr_gpios              = ARRAY_SIZE(sdi0_reg_gpios),
> +       .states                = sdi0_reg_states,
> +       .nr_states             = ARRAY_SIZE(sdi0_reg_states),
> +       .type                  = REGULATOR_VOLTAGE,
> +       .enable_high           = 1,
> +       .enabled_at_boot       = 0,
> +       .init_data             = &sdi0_reg_init_data,
> +       .startup_delay         = 100,
> +};
> +
> +static struct platform_device sdi0_regulator = {
> +        .name = "gpio-regulator",
> +        .id   = -1,
> +        .dev  = {
> +                .platform_data = &sdi0_reg_info,
> +        },
> +};
> +
>  static struct ab8500_gpio_platform_data ab8500_gpio_pdata = {
>         .gpio_base              = MOP500_AB8500_PIN_GPIO(1),
>         .irq_base               = MOP500_AB8500_VIR_GPIO_IRQ_BASE,
> @@ -440,6 +473,7 @@ static struct hash_platform_data u8500_hash1_platform_data = {
>  /* add any platform devices here - TODO */
>  static struct platform_device *mop500_platform_devs[] __initdata = {
>         &mop500_gpio_keys_device,
> +       &sdi0_regulator,
>  };
>
>  #ifdef CONFIG_STE_DMA40
> @@ -581,6 +615,7 @@ static struct platform_device *snowball_platform_devs[] __initdata = {
>         &snowball_key_dev,
>         &snowball_sbnet_dev,
>         &snowball_gpio_en_3v3_regulator_dev,
> +       &sdi0_regulator,
>  };
>
>  static void __init mop500_init_machine(void)
> @@ -591,6 +626,9 @@ static void __init mop500_init_machine(void)
>
>         mop500_gpio_keys[0].gpio = GPIO_PROX_SENSOR;
>
> +       sdi0_reg_info.enable_gpio = GPIO_SDMMC_EN;
> +       sdi0_reg_info.gpios[0].gpio = GPIO_SDMMC_1V8_3V_SEL;
> +
>         mop500_pinmaps_init();
>         parent = u8500_init_devices(&ab8500_platdata);
>
> @@ -623,6 +661,9 @@ static void __init snowball_init_machine(void)
>         struct device *parent = NULL;
>         int i;
>
> +       sdi0_reg_info.enable_gpio = SNOWBALL_SDMMC_EN_GPIO;
> +       sdi0_reg_info.gpios[0].gpio = SNOWBALL_SDMMC_1V8_3V_GPIO;
> +
>         snowball_pinmaps_init();
>         parent = u8500_init_devices(&ab8500_platdata);
>
> @@ -655,6 +696,9 @@ static void __init hrefv60_init_machine(void)
>          */
>         mop500_gpio_keys[0].gpio = HREFV60_PROX_SENSE_GPIO;
>
> +       sdi0_reg_info.enable_gpio = HREFV60_SDMMC_EN_GPIO;
> +       sdi0_reg_info.gpios[0].gpio = HREFV60_SDMMC_1V8_3V_GPIO;
> +
>         hrefv60_pinmaps_init();
>         parent = u8500_init_devices(&ab8500_platdata);
>
> --
> 1.7.9.5

[PATCH 12/12] ARM: ux500: Remove traces of the ios_handler from platform code

[Lee Jones]

Now MMCI on/off functionality is using the regulator framework
from the MMCI driver, there is no need to keep the ios_handler
laying around, duplicating functionality. So we're removing it.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/mach-ux500/board-mop500-sdi.c |   52 --------------------------------
 1 file changed, 52 deletions(-)

diff --git a/arch/arm/mach-ux500/board-mop500-sdi.c b/arch/arm/mach-ux500/board-mop500-sdi.c
index 9c8e4a9..5a798d6 100644
--- a/arch/arm/mach-ux500/board-mop500-sdi.c
+++ b/arch/arm/mach-ux500/board-mop500-sdi.c
@@ -31,35 +31,6 @@
  * SDI 0 (MicroSD slot)
  */
 
-/* GPIO pins used by the sdi0 level shifter */
-static int sdi0_en = -1;
-static int sdi0_vsel = -1;
-
-static int mop500_sdi0_ios_handler(struct device *dev, struct mmc_ios *ios)
-{
-	switch (ios->power_mode) {
-	case MMC_POWER_UP:
-	case MMC_POWER_ON:
-		/*
-		 * Level shifter voltage should depend on vdd to when deciding
-		 * on either 1.8V or 2.9V. Once the decision has been made the
-		 * level shifter must be disabled and re-enabled with a changed
-		 * select signal in order to switch the voltage. Since there is
-		 * no framework support yet for indicating 1.8V in vdd, use the
-		 * default 2.9V.
-		 */
-		gpio_direction_output(sdi0_vsel, 0);
-		gpio_direction_output(sdi0_en, 1);
-		break;
-	case MMC_POWER_OFF:
-		gpio_direction_output(sdi0_vsel, 0);
-		gpio_direction_output(sdi0_en, 0);
-		break;
-	}
-
-	return 0;
-}
-
 #ifdef CONFIG_STE_DMA40
 struct stedma40_chan_cfg mop500_sdi0_dma_cfg_rx = {
 	.mode = STEDMA40_MODE_LOGICAL,
@@ -81,7 +52,6 @@ static struct stedma40_chan_cfg mop500_sdi0_dma_cfg_tx = {
 #endif
 
 struct mmci_platform_data mop500_sdi0_data = {
-	.ios_handler	= mop500_sdi0_ios_handler,
 	.ocr_mask	= MMC_VDD_29_30,
 	.f_max		= 50000000,
 	.capabilities	= MMC_CAP_4_BIT_DATA |
@@ -101,22 +71,6 @@ struct mmci_platform_data mop500_sdi0_data = {
 
 static void sdi0_configure(struct device *parent)
 {
-	int ret;
-
-	ret = gpio_request(sdi0_en, "level shifter enable");
-	if (!ret)
-		ret = gpio_request(sdi0_vsel,
-				   "level shifter 1v8-3v select");
-
-	if (ret) {
-		pr_warning("unable to config sdi0 gpios for level shifter.\n");
-		return;
-	}
-
-	/* Select the default 2.9V and enable level shifter */
-	gpio_direction_output(sdi0_vsel, 0);
-	gpio_direction_output(sdi0_en, 1);
-
 	/* Add the device, force v2 to subrevision 1 */
 	db8500_add_sdi0(parent, &mop500_sdi0_data, U8500_SDI_V2_PERIPHID);
 }
@@ -124,8 +78,6 @@ static void sdi0_configure(struct device *parent)
 void mop500_sdi_tc35892_init(struct device *parent)
 {
 	mop500_sdi0_data.gpio_cd = GPIO_SDMMC_CD;
-	sdi0_en = GPIO_SDMMC_EN;
-	sdi0_vsel = GPIO_SDMMC_1V8_3V_SEL;
 	sdi0_configure(parent);
 }
 
@@ -264,8 +216,6 @@ void __init snowball_sdi_init(struct device *parent)
 	/* External Micro SD slot */
 	mop500_sdi0_data.gpio_cd = SNOWBALL_SDMMC_CD_GPIO;
 	mop500_sdi0_data.cd_invert = true;
-	sdi0_en = SNOWBALL_SDMMC_EN_GPIO;
-	sdi0_vsel = SNOWBALL_SDMMC_1V8_3V_GPIO;
 	sdi0_configure(parent);
 }
 
@@ -277,8 +227,6 @@ void __init hrefv60_sdi_init(struct device *parent)
 	db8500_add_sdi4(parent, &mop500_sdi4_data, U8500_SDI_V2_PERIPHID);
 	/* External Micro SD slot */
 	mop500_sdi0_data.gpio_cd = HREFV60_SDMMC_CD_GPIO;
-	sdi0_en = HREFV60_SDMMC_EN_GPIO;
-	sdi0_vsel = HREFV60_SDMMC_1V8_3V_GPIO;
 	sdi0_configure(parent);
 	/* WLAN SDIO channel */
 	db8500_add_sdi1(parent, &mop500_sdi1_data, U8500_SDI_V2_PERIPHID);
-- 
1.7.9.5

[PATCH 12/12] ARM: ux500: Remove traces of the ios_handler from platform code

[Linus Walleij]

On Mon, Dec 10, 2012 at 9:56 AM, Lee Jones <lee.jones@linaro.org> wrote:

> Now MMCI on/off functionality is using the regulator framework
> from the MMCI driver, there is no need to keep the ios_handler
> laying around, duplicating functionality. So we're removing it.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Acked-by: Linus Walleij <linus.walleij@linaro.org>
(Provided the rest is merged first.)

Yours,
Linus Walleij