[PATCH V2 0/3] Clearify code paths for how to modify the power register

[PATCH V2 0/3] Clearify code paths for how to modify the power register

[Ulf Hansson]

In the set_ios function there has been a mess of how the power register
can be modified. One part, especially strange was related to the use of 
the vdd_handler.

V2 changes:
	-Rebased to remove dependency to other patch serie for mmci called
	"mmc: mmci: Improvements and bugfixes for SDIO".
	-Fixed minor review comments.

Sebastian Rasmussen (1):
  mmc: mmci: Put power register deviations in variant data

Ulf Hansson (2):
  mmc: mmci: Provide option to configure bus signal direction
  mmc: mmci: Change vdd_handler to a generic ios_handler

 arch/arm/mach-ux500/board-mop500-sdi.c |   21 +++++--------
 drivers/mmc/host/mmci.c                |   49 ++++++++++++++++++++++++++-----
 drivers/mmc/host/mmci.h                |   10 ------
 include/linux/amba/mmci.h              |   22 ++++++++++++--
 4 files changed, 68 insertions(+), 34 deletions(-)

-- 
1.7.5.4

[PATCH V2 1/3] mmc: mmci: Put power register deviations in variant data

[Ulf Hansson]

From: Sebastian Rasmussen <sebastian.rasmussen@stericsson.com>

Use variant data to store hardware controller deviations concerning
power registers to improve readability of the code.

Signed-off-by: Sebastian Rasmussen <sebastian.rasmussen@stericsson.com>
Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
Reviewed-by: Linus Walleij <linus.walleij@stericsson.com>
---
 drivers/mmc/host/mmci.c |   22 +++++++++++++++++-----
 1 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 50b5f99..1296719 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -53,6 +53,7 @@ static unsigned int fmax = 515633;
  * @sdio: variant supports SDIO
  * @st_clkdiv: true if using a ST-specific clock divider algorithm
  * @blksz_datactrl16: true if Block size is at b16..b30 position in datactrl register
+ * @pwrreg_powerup: power up value for MMCIPOWER register
  */
 struct variant_data {
 	unsigned int		clkreg;
@@ -63,18 +64,21 @@ struct variant_data {
 	bool			sdio;
 	bool			st_clkdiv;
 	bool			blksz_datactrl16;
+	u32			pwrreg_powerup;
 };
 
 static struct variant_data variant_arm = {
 	.fifosize		= 16 * 4,
 	.fifohalfsize		= 8 * 4,
 	.datalength_bits	= 16,
+	.pwrreg_powerup		= MCI_PWR_UP,
 };
 
 static struct variant_data variant_arm_extended_fifo = {
 	.fifosize		= 128 * 4,
 	.fifohalfsize		= 64 * 4,
 	.datalength_bits	= 16,
+	.pwrreg_powerup		= MCI_PWR_UP,
 };
 
 static struct variant_data variant_u300 = {
@@ -83,6 +87,7 @@ static struct variant_data variant_u300 = {
 	.clkreg_enable		= MCI_ST_U300_HWFCEN,
 	.datalength_bits	= 16,
 	.sdio			= true,
+	.pwrreg_powerup		= MCI_PWR_ON,
 };
 
 static struct variant_data variant_ux500 = {
@@ -93,6 +98,7 @@ static struct variant_data variant_ux500 = {
 	.datalength_bits	= 24,
 	.sdio			= true,
 	.st_clkdiv		= true,
+	.pwrreg_powerup		= MCI_PWR_ON,
 };
 
 static struct variant_data variant_ux500v2 = {
@@ -104,6 +110,7 @@ static struct variant_data variant_ux500v2 = {
 	.sdio			= true,
 	.st_clkdiv		= true,
 	.blksz_datactrl16	= true,
+	.pwrreg_powerup		= MCI_PWR_ON,
 };
 
 /*
@@ -1006,6 +1013,7 @@ static void mmci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 {
 	struct mmci_host *host = mmc_priv(mmc);
+	struct variant_data *variant = host->variant;
 	u32 pwr = 0;
 	unsigned long flags;
 	int ret;
@@ -1032,11 +1040,15 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 		if (host->plat->vdd_handler)
 			pwr |= host->plat->vdd_handler(mmc_dev(mmc), ios->vdd,
 						       ios->power_mode);
-		/* The ST version does not have this, fall through to POWER_ON */
-		if (host->hw_designer != AMBA_VENDOR_ST) {
-			pwr |= MCI_PWR_UP;
-			break;
-		}
+
+		/*
+		 * The ST Micro variant doesn't have the PL180s MCI_PWR_UP
+		 * and instead uses MCI_PWR_ON so apply whatever value is
+		 * configured in the variant data.
+		 */
+		pwr |= variant->pwrreg_powerup;
+
+		break;
 	case MMC_POWER_ON:
 		pwr |= MCI_PWR_ON;
 		break;
-- 
1.7.5.4

[PATCH V2 2/3] mmc: mmci: Provide option to configure bus signal direction

[Ulf Hansson]

The ST Micro variant supports bus signal direction indication. A new
member in the variant struct is added for this.

Moreover the actual signal direction configuration is board specific,
thus the amba mmci platform data is extended with a new member to be
able provide mmci with these specific board configurations.

This patch is based upon a patch from Sebastian Rasmussen.

Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
Signed-off-by: Sebastian Rasmussen <sebastian.rasmussen@stericsson.com>
---
 drivers/mmc/host/mmci.c   |   21 +++++++++++++++++++++
 drivers/mmc/host/mmci.h   |   10 ----------
 include/linux/amba/mmci.h |   16 ++++++++++++++++
 3 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 1296719..55e460c 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -54,6 +54,7 @@ static unsigned int fmax = 515633;
  * @st_clkdiv: true if using a ST-specific clock divider algorithm
  * @blksz_datactrl16: true if Block size is at b16..b30 position in datactrl register
  * @pwrreg_powerup: power up value for MMCIPOWER register
+ * @signal_direction: input/out direction of bus signals can be indicated
  */
 struct variant_data {
 	unsigned int		clkreg;
@@ -65,6 +66,7 @@ struct variant_data {
 	bool			st_clkdiv;
 	bool			blksz_datactrl16;
 	u32			pwrreg_powerup;
+	bool			signal_direction;
 };
 
 static struct variant_data variant_arm = {
@@ -88,6 +90,7 @@ static struct variant_data variant_u300 = {
 	.datalength_bits	= 16,
 	.sdio			= true,
 	.pwrreg_powerup		= MCI_PWR_ON,
+	.signal_direction	= true,
 };
 
 static struct variant_data variant_ux500 = {
@@ -99,6 +102,7 @@ static struct variant_data variant_ux500 = {
 	.sdio			= true,
 	.st_clkdiv		= true,
 	.pwrreg_powerup		= MCI_PWR_ON,
+	.signal_direction	= true,
 };
 
 static struct variant_data variant_ux500v2 = {
@@ -111,6 +115,7 @@ static struct variant_data variant_ux500v2 = {
 	.st_clkdiv		= true,
 	.blksz_datactrl16	= true,
 	.pwrreg_powerup		= MCI_PWR_ON,
+	.signal_direction	= true,
 };
 
 /*
@@ -1054,6 +1059,22 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 		break;
 	}
 
+	if (variant->signal_direction && ios->power_mode != MMC_POWER_OFF) {
+		/*
+		 * The ST Micro variant has some additional bits
+		 * indicating signal direction for the signals in
+		 * the SD/MMC bus and feedback-clock usage.
+		 */
+		pwr |= host->plat->sigdir;
+
+		if (ios->bus_width == MMC_BUS_WIDTH_4)
+			pwr &= ~MCI_ST_DATA74DIREN;
+		else if (ios->bus_width == MMC_BUS_WIDTH_1)
+			pwr &= (~MCI_ST_DATA74DIREN &
+				~MCI_ST_DATA31DIREN &
+				~MCI_ST_DATA2DIREN);
+	}
+
 	if (ios->bus_mode == MMC_BUSMODE_OPENDRAIN) {
 		if (host->hw_designer != AMBA_VENDOR_ST)
 			pwr |= MCI_ROD;
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 79e4143..5600755 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -13,16 +13,6 @@
 #define MCI_PWR_ON		0x03
 #define MCI_OD			(1 << 6)
 #define MCI_ROD			(1 << 7)
-/*
- * The ST Micro version does not have ROD and reuse the voltage registers
- * for direction settings
- */
-#define MCI_ST_DATA2DIREN	(1 << 2)
-#define MCI_ST_CMDDIREN		(1 << 3)
-#define MCI_ST_DATA0DIREN	(1 << 4)
-#define MCI_ST_DATA31DIREN	(1 << 5)
-#define MCI_ST_FBCLKEN		(1 << 7)
-#define MCI_ST_DATA74DIREN	(1 << 8)
 
 #define MMCICLOCK		0x004
 #define MCI_CLK_ENABLE		(1 << 8)
diff --git a/include/linux/amba/mmci.h b/include/linux/amba/mmci.h
index 2111481..8ce34e8 100644
--- a/include/linux/amba/mmci.h
+++ b/include/linux/amba/mmci.h
@@ -6,6 +6,19 @@
 
 #include <linux/mmc/host.h>
 
+
+/*
+ * These defines is places here due to access is needed from machine
+ * configuration files. The ST Micro version does not have ROD and
+ * reuse the voltage registers for direction settings.
+ */
+#define MCI_ST_DATA2DIREN	(1 << 2)
+#define MCI_ST_CMDDIREN		(1 << 3)
+#define MCI_ST_DATA0DIREN	(1 << 4)
+#define MCI_ST_DATA31DIREN	(1 << 5)
+#define MCI_ST_FBCLKEN		(1 << 7)
+#define MCI_ST_DATA74DIREN	(1 << 8)
+
 /* Just some dummy forwarding */
 struct dma_chan;
 
@@ -30,6 +43,8 @@ struct dma_chan;
  * @cd_invert: true if the gpio_cd pin value is active low
  * @capabilities: the capabilities of the block as implemented in
  * this platform, signify anything MMC_CAP_* from mmc/host.h
+ * @sigdir: a bit field indicating for what bits in the MMC bus the host
+ * should enable signal direction indication.
  * @dma_filter: function used to select an appropriate RX and TX
  * DMA channel to be used for DMA, if and only if you're deploying the
  * generic DMA engine
@@ -52,6 +67,7 @@ struct mmci_platform_data {
 	int	gpio_cd;
 	bool	cd_invert;
 	unsigned long capabilities;
+	u32 sigdir;
 	bool (*dma_filter)(struct dma_chan *chan, void *filter_param);
 	void *dma_rx_param;
 	void *dma_tx_param;
-- 
1.7.5.4

[PATCH V2 3/3] mmc: mmci: Change vdd_handler to a generic ios_handler

[Ulf Hansson]

The purpose of the vdd_handler does not make sense. We remove it
and use a generic approach instead. A new ios_handler is added, the
purpose of which e.g. can be to control GPIO pins to a levelshifter.

Previously the vdd_handler was also used for making additional
changes to the power register bits. This option is superfluous and is
therefore removed.

Adaptaptions from the old vdd_handler to the new ios_handler is done for
mach-ux500 board, which was the only one using the vdd_handler.

This patch is based upon a patch from Sebastian Rasmussen.

Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
Signed-off-by: Sebastian Rasmussen <sebastian.rasmussen@stericsson.com>
---
 arch/arm/mach-ux500/board-mop500-sdi.c |   21 ++++++++-------------
 drivers/mmc/host/mmci.c                |    8 ++++----
 include/linux/amba/mmci.h              |    6 +++---
 3 files changed, 15 insertions(+), 20 deletions(-)

diff --git a/arch/arm/mach-ux500/board-mop500-sdi.c b/arch/arm/mach-ux500/board-mop500-sdi.c
index 6826fae..3578c51 100644
--- a/arch/arm/mach-ux500/board-mop500-sdi.c
+++ b/arch/arm/mach-ux500/board-mop500-sdi.c
@@ -25,21 +25,13 @@
  * SDI 0 (MicroSD slot)
  */
 
-/* MMCIPOWER bits */
-#define MCI_DATA2DIREN		(1 << 2)
-#define MCI_CMDDIREN		(1 << 3)
-#define MCI_DATA0DIREN		(1 << 4)
-#define MCI_DATA31DIREN		(1 << 5)
-#define MCI_FBCLKEN		(1 << 7)
-
 /* GPIO pins used by the sdi0 level shifter */
 static int sdi0_en = -1;
 static int sdi0_vsel = -1;
 
-static u32 mop500_sdi0_vdd_handler(struct device *dev, unsigned int vdd,
-				   unsigned char power_mode)
+static int mop500_sdi0_ios_handler(struct device *dev, struct mmc_ios *ios)
 {
-	switch (power_mode) {
+	switch (ios->power_mode) {
 	case MMC_POWER_UP:
 	case MMC_POWER_ON:
 		/*
@@ -59,8 +51,7 @@ static u32 mop500_sdi0_vdd_handler(struct device *dev, unsigned int vdd,
 		break;
 	}
 
-	return MCI_FBCLKEN | MCI_CMDDIREN | MCI_DATA0DIREN |
-	       MCI_DATA2DIREN | MCI_DATA31DIREN;
+	return 0;
 }
 
 #ifdef CONFIG_STE_DMA40
@@ -84,13 +75,17 @@ static struct stedma40_chan_cfg mop500_sdi0_dma_cfg_tx = {
 #endif
 
 static struct mmci_platform_data mop500_sdi0_data = {
-	.vdd_handler	= mop500_sdi0_vdd_handler,
+	.ios_handler	= mop500_sdi0_ios_handler,
 	.ocr_mask	= MMC_VDD_29_30,
 	.f_max		= 50000000,
 	.capabilities	= MMC_CAP_4_BIT_DATA |
 				MMC_CAP_SD_HIGHSPEED |
 				MMC_CAP_MMC_HIGHSPEED,
 	.gpio_wp	= -1,
+	.sigdir		= MCI_ST_FBCLKEN |
+				MCI_ST_CMDDIREN |
+				MCI_ST_DATA0DIREN |
+				MCI_ST_DATA2DIREN,
 #ifdef CONFIG_STE_DMA40
 	.dma_filter	= stedma40_filter,
 	.dma_rx_param	= &mop500_sdi0_dma_cfg_rx,
diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 55e460c..3c8ad6b 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -1023,6 +1023,10 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	unsigned long flags;
 	int ret;
 
+	if (host->plat->ios_handler &&
+		host->plat->ios_handler(mmc_dev(mmc), ios))
+			dev_err(mmc_dev(mmc), "platform ios_handler failed\n");
+
 	switch (ios->power_mode) {
 	case MMC_POWER_OFF:
 		if (host->vcc)
@@ -1042,10 +1046,6 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 				return;
 			}
 		}
-		if (host->plat->vdd_handler)
-			pwr |= host->plat->vdd_handler(mmc_dev(mmc), ios->vdd,
-						       ios->power_mode);
-
 		/*
 		 * The ST Micro variant doesn't have the PL180s MCI_PWR_UP
 		 * and instead uses MCI_PWR_ON so apply whatever value is
diff --git a/include/linux/amba/mmci.h b/include/linux/amba/mmci.h
index 8ce34e8..84766fd 100644
--- a/include/linux/amba/mmci.h
+++ b/include/linux/amba/mmci.h
@@ -31,7 +31,8 @@ struct dma_chan;
  * @ocr_mask: available voltages on the 4 pins from the block, this
  * is ignored if a regulator is used, see the MMC_VDD_* masks in
  * mmc/host.h
- * @vdd_handler: a callback function to translate a MMC_VDD_*
+ * @ios_handler: a callback function to act on specfic ios changes,
+ * used for example to control a levelshifter
  * mask into a value to be binary (or set some other custom bits
  * in MMCIPWR) or:ed and written into the MMCIPWR register of the
  * block.  May also control external power based on the power_mode.
@@ -60,8 +61,7 @@ struct dma_chan;
 struct mmci_platform_data {
 	unsigned int f_max;
 	unsigned int ocr_mask;
-	u32 (*vdd_handler)(struct device *, unsigned int vdd,
-			   unsigned char power_mode);
+	int (*ios_handler)(struct device *, struct mmc_ios *);
 	unsigned int (*status)(struct device *);
 	int	gpio_wp;
 	int	gpio_cd;
-- 
1.7.5.4

[PATCH V2 1/3] mmc: mmci: Put power register deviations in variant data

[Ulf Hansson]

Is this patch and it's patchserie considered OK?

-Russell, patches are available in your patchtracker.

Br
Ulf Hansson

Ulf HANSSON wrote:
> From: Sebastian Rasmussen <sebastian.rasmussen@stericsson.com>
> 
> Use variant data to store hardware controller deviations concerning
> power registers to improve readability of the code.
> 
> Signed-off-by: Sebastian Rasmussen <sebastian.rasmussen@stericsson.com>
> Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
> Reviewed-by: Linus Walleij <linus.walleij@stericsson.com>
> ---
>  drivers/mmc/host/mmci.c |   22 +++++++++++++++++-----
>  1 files changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index 50b5f99..1296719 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -53,6 +53,7 @@ static unsigned int fmax = 515633;
>   * @sdio: variant supports SDIO
>   * @st_clkdiv: true if using a ST-specific clock divider algorithm
>   * @blksz_datactrl16: true if Block size is at b16..b30 position in datactrl register
> + * @pwrreg_powerup: power up value for MMCIPOWER register
>   */
>  struct variant_data {
>  	unsigned int		clkreg;
> @@ -63,18 +64,21 @@ struct variant_data {
>  	bool			sdio;
>  	bool			st_clkdiv;
>  	bool			blksz_datactrl16;
> +	u32			pwrreg_powerup;
>  };
>  
>  static struct variant_data variant_arm = {
>  	.fifosize		= 16 * 4,
>  	.fifohalfsize		= 8 * 4,
>  	.datalength_bits	= 16,
> +	.pwrreg_powerup		= MCI_PWR_UP,
>  };
>  
>  static struct variant_data variant_arm_extended_fifo = {
>  	.fifosize		= 128 * 4,
>  	.fifohalfsize		= 64 * 4,
>  	.datalength_bits	= 16,
> +	.pwrreg_powerup		= MCI_PWR_UP,
>  };
>  
>  static struct variant_data variant_u300 = {
> @@ -83,6 +87,7 @@ static struct variant_data variant_u300 = {
>  	.clkreg_enable		= MCI_ST_U300_HWFCEN,
>  	.datalength_bits	= 16,
>  	.sdio			= true,
> +	.pwrreg_powerup		= MCI_PWR_ON,
>  };
>  
>  static struct variant_data variant_ux500 = {
> @@ -93,6 +98,7 @@ static struct variant_data variant_ux500 = {
>  	.datalength_bits	= 24,
>  	.sdio			= true,
>  	.st_clkdiv		= true,
> +	.pwrreg_powerup		= MCI_PWR_ON,
>  };
>  
>  static struct variant_data variant_ux500v2 = {
> @@ -104,6 +110,7 @@ static struct variant_data variant_ux500v2 = {
>  	.sdio			= true,
>  	.st_clkdiv		= true,
>  	.blksz_datactrl16	= true,
> +	.pwrreg_powerup		= MCI_PWR_ON,
>  };
>  
>  /*
> @@ -1006,6 +1013,7 @@ static void mmci_request(struct mmc_host *mmc, struct mmc_request *mrq)
>  static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>  {
>  	struct mmci_host *host = mmc_priv(mmc);
> +	struct variant_data *variant = host->variant;
>  	u32 pwr = 0;
>  	unsigned long flags;
>  	int ret;
> @@ -1032,11 +1040,15 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>  		if (host->plat->vdd_handler)
>  			pwr |= host->plat->vdd_handler(mmc_dev(mmc), ios->vdd,
>  						       ios->power_mode);
> -		/* The ST version does not have this, fall through to POWER_ON */
> -		if (host->hw_designer != AMBA_VENDOR_ST) {
> -			pwr |= MCI_PWR_UP;
> -			break;
> -		}
> +
> +		/*
> +		 * The ST Micro variant doesn't have the PL180s MCI_PWR_UP
> +		 * and instead uses MCI_PWR_ON so apply whatever value is
> +		 * configured in the variant data.
> +		 */
> +		pwr |= variant->pwrreg_powerup;
> +
> +		break;
>  	case MMC_POWER_ON:
>  		pwr |= MCI_PWR_ON;
>  		break;