[PATCH 01/13 v2] sh: ecovec: switch MMC power control to regulators

[PATCH 01/13 v2] sh: ecovec: switch MMC power control to regulators

[Guennadi Liakhovetski]

Power on the CN11 and CN12 SD/MMC slots on ecovec is controlled by GPIOs,
which makes it possible to use the fixed voltage regulator driver to switch
card power on and off.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---

v2: request the SD/MMC power-switching GPIO not in the board initialisation 
routine, but in the .set_pwr() callback similar to ag5evm.

 arch/sh/boards/mach-ecovec24/setup.c |  115 +++++++++++++++++++++++++++++-----
 1 files changed, 99 insertions(+), 16 deletions(-)

diff --git a/arch/sh/boards/mach-ecovec24/setup.c b/arch/sh/boards/mach-ecovec24/setup.c
index dbedfda..c17e82c 100644
--- a/arch/sh/boards/mach-ecovec24/setup.c
+++ b/arch/sh/boards/mach-ecovec24/setup.c
@@ -19,6 +19,8 @@
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/delay.h>
+#include <linux/regulator/fixed.h>
+#include <linux/regulator/machine.h>
 #include <linux/usb/r8a66597.h>
 #include <linux/usb/renesas_usbhs.h>
 #include <linux/i2c.h>
@@ -518,10 +520,86 @@ static struct i2c_board_info ts_i2c_clients = {
 	.irq		= IRQ0,
 };
 
+static struct regulator_consumer_supply cn12_power_consumers[] +{
+	REGULATOR_SUPPLY("vmmc", "sh_mmcif.0"),
+	REGULATOR_SUPPLY("vqmmc", "sh_mmcif.0"),
+	REGULATOR_SUPPLY("vmmc", "sh_mobile_sdhi.1"),
+	REGULATOR_SUPPLY("vqmmc", "sh_mobile_sdhi.1"),
+};
+
+static struct regulator_init_data cn12_power_init_data = {
+	.constraints = {
+		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
+	},
+	.num_consumer_supplies  = ARRAY_SIZE(cn12_power_consumers),
+	.consumer_supplies      = cn12_power_consumers,
+};
+
+static struct fixed_voltage_config cn12_power_info = {
+	.supply_name = "CN12 SD/MMC Vdd",
+	.microvolts = 3300000,
+	.gpio = GPIO_PTB7,
+	.enable_high = 1,
+	.init_data = &cn12_power_init_data,
+};
+
+static struct platform_device cn12_power = {
+	.name = "reg-fixed-voltage",
+	.id   = 0,
+	.dev  = {
+		.platform_data = &cn12_power_info,
+	},
+};
+
 #if defined(CONFIG_MMC_SDHI) || defined(CONFIG_MMC_SDHI_MODULE)
 /* SDHI0 */
+static struct regulator_consumer_supply sdhi0_power_consumers[] +{
+	REGULATOR_SUPPLY("vmmc", "sh_mobile_sdhi.0"),
+	REGULATOR_SUPPLY("vqmmc", "sh_mobile_sdhi.0"),
+};
+
+static struct regulator_init_data sdhi0_power_init_data = {
+	.constraints = {
+		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
+	},
+	.num_consumer_supplies  = ARRAY_SIZE(sdhi0_power_consumers),
+	.consumer_supplies      = sdhi0_power_consumers,
+};
+
+static struct fixed_voltage_config sdhi0_power_info = {
+	.supply_name = "CN11 SD/MMC Vdd",
+	.microvolts = 3300000,
+	.gpio = GPIO_PTB6,
+	.enable_high = 1,
+	.init_data = &sdhi0_power_init_data,
+};
+
+static struct platform_device sdhi0_power = {
+	.name = "reg-fixed-voltage",
+	.id   = 1,
+	.dev  = {
+		.platform_data = &sdhi0_power_info,
+	},
+};
+
 static void sdhi0_set_pwr(struct platform_device *pdev, int state)
 {
+	static int power_gpio = -EINVAL;
+
+	if (power_gpio < 0) {
+		int ret = gpio_request(GPIO_PTB6, NULL);
+		if (!ret) {
+			power_gpio = GPIO_PTB6;
+			gpio_direction_output(power_gpio, 0);
+		}
+	}
+
+	/*
+	 * Toggle the GPIO regardless, whether we managed to grab it above or
+	 * the fixed regulator driver did.
+	 */
 	gpio_set_value(GPIO_PTB6, state);
 }
 
@@ -562,13 +640,27 @@ static struct platform_device sdhi0_device = {
 	},
 };
 
-#if !defined(CONFIG_MMC_SH_MMCIF) && !defined(CONFIG_MMC_SH_MMCIF_MODULE)
-/* SDHI1 */
-static void sdhi1_set_pwr(struct platform_device *pdev, int state)
+static void cn12_set_pwr(struct platform_device *pdev, int state)
 {
+	static int power_gpio = -EINVAL;
+
+	if (power_gpio < 0) {
+		int ret = gpio_request(GPIO_PTB7, NULL);
+		if (!ret) {
+			power_gpio = GPIO_PTB7;
+			gpio_direction_output(power_gpio, 0);
+		}
+	}
+
+	/*
+	 * Toggle the GPIO regardless, whether we managed to grab it above or
+	 * the fixed regulator driver did.
+	 */
 	gpio_set_value(GPIO_PTB7, state);
 }
 
+#if !defined(CONFIG_MMC_SH_MMCIF) && !defined(CONFIG_MMC_SH_MMCIF_MODULE)
+/* SDHI1 */
 static int sdhi1_get_cd(struct platform_device *pdev)
 {
 	return !gpio_get_value(GPIO_PTW7);
@@ -579,7 +671,7 @@ static struct sh_mobile_sdhi_info sdhi1_info = {
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI1_RX,
 	.tmio_caps      = MMC_CAP_SDIO_IRQ | MMC_CAP_POWER_OFF_CARD |
 			  MMC_CAP_NEEDS_POLL,
-	.set_pwr	= sdhi1_set_pwr,
+	.set_pwr	= cn12_set_pwr,
 	.get_cd		= sdhi1_get_cd,
 };
 
@@ -899,11 +991,6 @@ static struct platform_device vou_device = {
 
 #if defined(CONFIG_MMC_SH_MMCIF) || defined(CONFIG_MMC_SH_MMCIF_MODULE)
 /* SH_MMCIF */
-static void mmcif_set_pwr(struct platform_device *pdev, int state)
-{
-	gpio_set_value(GPIO_PTB7, state);
-}
-
 static struct resource sh_mmcif_resources[] = {
 	[0] = {
 		.name	= "SH_MMCIF",
@@ -924,7 +1011,7 @@ static struct resource sh_mmcif_resources[] = {
 };
 
 static struct sh_mmcif_plat_data sh_mmcif_plat = {
-	.set_pwr	= mmcif_set_pwr,
+	.set_pwr	= cn12_set_pwr,
 	.sup_pclk	= 0, /* SH7724: Max Pclk/2 */
 	.caps		= MMC_CAP_4_BIT_DATA |
 			  MMC_CAP_8_BIT_DATA |
@@ -954,7 +1041,9 @@ static struct platform_device *ecovec_devices[] __initdata = {
 	&ceu0_device,
 	&ceu1_device,
 	&keysc_device,
+	&cn12_power,
 #if defined(CONFIG_MMC_SDHI) || defined(CONFIG_MMC_SDHI_MODULE)
+	&sdhi0_power,
 	&sdhi0_device,
 #if !defined(CONFIG_MMC_SH_MMCIF) && !defined(CONFIG_MMC_SH_MMCIF_MODULE)
 	&sdhi1_device,
@@ -1252,8 +1341,6 @@ static int __init arch_setup(void)
 	gpio_request(GPIO_FN_SDHI0D2,  NULL);
 	gpio_request(GPIO_FN_SDHI0D1,  NULL);
 	gpio_request(GPIO_FN_SDHI0D0,  NULL);
-	gpio_request(GPIO_PTB6, NULL);
-	gpio_direction_output(GPIO_PTB6, 0);
 #else
 	/* enable MSIOF0 on CN11 (needs DS2.4 set to OFF) */
 	gpio_request(GPIO_FN_MSIOF0_TXD, NULL);
@@ -1282,8 +1369,6 @@ static int __init arch_setup(void)
 	gpio_request(GPIO_FN_MMC_D0, NULL);
 	gpio_request(GPIO_FN_MMC_CLK, NULL);
 	gpio_request(GPIO_FN_MMC_CMD, NULL);
-	gpio_request(GPIO_PTB7, NULL);
-	gpio_direction_output(GPIO_PTB7, 0);
 
 	cn12_enabled = true;
 #elif defined(CONFIG_MMC_SDHI) || defined(CONFIG_MMC_SDHI_MODULE)
@@ -1295,8 +1380,6 @@ static int __init arch_setup(void)
 	gpio_request(GPIO_FN_SDHI1D2,  NULL);
 	gpio_request(GPIO_FN_SDHI1D1,  NULL);
 	gpio_request(GPIO_FN_SDHI1D0,  NULL);
-	gpio_request(GPIO_PTB7, NULL);
-	gpio_direction_output(GPIO_PTB7, 0);
 
 	/* Card-detect, used on CN12 with SDHI1 */
 	gpio_request(GPIO_PTW7, NULL);
-- 
1.7.2.5

Re: [PATCH 01/13 v2] sh: ecovec: switch MMC power control to regulators

[Paul Mundt]

On Wed, Jun 27, 2012 at 12:49:57AM +0200, Guennadi Liakhovetski wrote:
> Power on the CN11 and CN12 SD/MMC slots on ecovec is controlled by GPIOs,
> which makes it possible to use the fixed voltage regulator driver to switch
> card power on and off.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
> 
> v2: request the SD/MMC power-switching GPIO not in the board initialisation 
> routine, but in the .set_pwr() callback similar to ag5evm.
> 
That looks fine, but..

>  static void sdhi0_set_pwr(struct platform_device *pdev, int state)
>  {
> +	static int power_gpio = -EINVAL;
> +
> +	if (power_gpio < 0) {
> +		int ret = gpio_request(GPIO_PTB6, NULL);
> +		if (!ret) {
> +			power_gpio = GPIO_PTB6;
> +			gpio_direction_output(power_gpio, 0);
> +		}

You do have the platform device available here, so there's no reason to
have a NULL label for gpio_request(). Even just wrapping to dev_name()
will be more helpful.

> -#if !defined(CONFIG_MMC_SH_MMCIF) && !defined(CONFIG_MMC_SH_MMCIF_MODULE)
> -/* SDHI1 */
> -static void sdhi1_set_pwr(struct platform_device *pdev, int state)
> +static void cn12_set_pwr(struct platform_device *pdev, int state)
>  {
> +	static int power_gpio = -EINVAL;
> +
> +	if (power_gpio < 0) {
> +		int ret = gpio_request(GPIO_PTB7, NULL);

Likewise.

Re: [PATCH 01/13 v2] sh: ecovec: switch MMC power control to regulators

[Paul Mundt]

On Wed, Jun 27, 2012 at 12:49:57AM +0200, Guennadi Liakhovetski wrote:
> Power on the CN11 and CN12 SD/MMC slots on ecovec is controlled by GPIOs,
> which makes it possible to use the fixed voltage regulator driver to switch
> card power on and off.
> 
[snip]

>  #if defined(CONFIG_MMC_SH_MMCIF) || defined(CONFIG_MMC_SH_MMCIF_MODULE)
>  /* SH_MMCIF */
> -static void mmcif_set_pwr(struct platform_device *pdev, int state)
> -{
> -	gpio_set_value(GPIO_PTB7, state);
> -}
> -
>  static struct resource sh_mmcif_resources[] = {
>  	[0] = {
>  		.name	= "SH_MMCIF",
> @@ -924,7 +1011,7 @@ static struct resource sh_mmcif_resources[] = {
>  };
>  
>  static struct sh_mmcif_plat_data sh_mmcif_plat = {
> -	.set_pwr	= mmcif_set_pwr,
> +	.set_pwr	= cn12_set_pwr,
>  	.sup_pclk	= 0, /* SH7724: Max Pclk/2 */
>  	.caps		= MMC_CAP_4_BIT_DATA |
>  			  MMC_CAP_8_BIT_DATA |

This doesn't match what is in the tree, so I have no idea what tree you
are working on. It fails to apply because of the down_pwr thing upstream
which doesn't exist in your patchset, and I'm certainly not going to
track it down.

Next time you send a patch series, make sure it actually applies to the
dependencies that you have written out, instead of some hypothetical tree
combination that needs to be reverse engineered.

The rest are applied. At least now you can go back and make those changes
to the patch while figuring out what tree you're working on.