All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/14] mmc: mmci: Improved PM support, cleanup and bugfixes
@ 2011-12-05 17:35 ` Ulf Hansson
  0 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2011-12-05 17:35 UTC (permalink / raw)
  To: linux-mmc, linux-arm-kernel; +Cc: Russell King, Ulf Hansson, Lee Jones

This patchserie is based on Linux 3.2-rc3 and contains the following:

Patch 1 -> 4:
Minor changes for bugs and some performance optimizations.
Previously sent to the mailing list.

Patch 5 -> 7:
Minor changes affecting how to handle a levelshifter.
Previously sent to the mailing list.

Patch 8:
Bugfix for dma. Previously sent to the mailing list, ongoing discussion.

Patch 9 -> 14:
Improvement of PM support.
An earlier patch around extending the PM runtime support for mmci has been
discussed on the mailing list rather recently. Those review comment's
has been considered in these patches.


This patchstack is becomming quite big. Hopefully it should still be possible
review each piece separate. Those patches that earlier have been uploaded into
Russell's patchtracker will be replaced with these new rebased versions.


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

Ulf Hansson (13):
  mmc: mmci: Support MMC_PM_KEEP_POWER
  mmc: mmci: Fixup handling of MCI_STARTBITERR
  mmc: mmci: Increase max_segs from 16 to 128
  mmc: mmci: Do not release spinlock in request_end
  mmc: mmci: Provide option to configure bus signal direction
  mmc: mmci: Change vdd_handler to a generic ios_handler
  mmc: mmci: Fixup error handling for dma
  mmc: mmci: Change from using legacy suspend
  mmc: mmci: Cache MMCICLOCK and MMCIPOWER register
  mmc: mmci: Fixup use of runtime PM and use autosuspend
  mmc: mmci: Decrease current consumption in suspend
  mmc: mmci: Implement PM runtime callbacks to save power
  mmc: mmci: Use ios_handler to save power

 arch/arm/mach-ux500/board-mop500-sdi.c |   21 +--
 drivers/mmc/host/mmci.c                |  266 ++++++++++++++++++++++++++------
 drivers/mmc/host/mmci.h                |   15 +--
 include/linux/amba/mmci.h              |   22 +++-
 4 files changed, 252 insertions(+), 72 deletions(-)

-- 
1.7.5.4


^ permalink raw reply	[flat|nested] 52+ messages in thread

* [PATCH 00/14] mmc: mmci: Improved PM support, cleanup and bugfixes
@ 2011-12-05 17:35 ` Ulf Hansson
  0 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2011-12-05 17:35 UTC (permalink / raw)
  To: linux-arm-kernel

This patchserie is based on Linux 3.2-rc3 and contains the following:

Patch 1 -> 4:
Minor changes for bugs and some performance optimizations.
Previously sent to the mailing list.

Patch 5 -> 7:
Minor changes affecting how to handle a levelshifter.
Previously sent to the mailing list.

Patch 8:
Bugfix for dma. Previously sent to the mailing list, ongoing discussion.

Patch 9 -> 14:
Improvement of PM support.
An earlier patch around extending the PM runtime support for mmci has been
discussed on the mailing list rather recently. Those review comment's
has been considered in these patches.


This patchstack is becomming quite big. Hopefully it should still be possible
review each piece separate. Those patches that earlier have been uploaded into
Russell's patchtracker will be replaced with these new rebased versions.


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

Ulf Hansson (13):
  mmc: mmci: Support MMC_PM_KEEP_POWER
  mmc: mmci: Fixup handling of MCI_STARTBITERR
  mmc: mmci: Increase max_segs from 16 to 128
  mmc: mmci: Do not release spinlock in request_end
  mmc: mmci: Provide option to configure bus signal direction
  mmc: mmci: Change vdd_handler to a generic ios_handler
  mmc: mmci: Fixup error handling for dma
  mmc: mmci: Change from using legacy suspend
  mmc: mmci: Cache MMCICLOCK and MMCIPOWER register
  mmc: mmci: Fixup use of runtime PM and use autosuspend
  mmc: mmci: Decrease current consumption in suspend
  mmc: mmci: Implement PM runtime callbacks to save power
  mmc: mmci: Use ios_handler to save power

 arch/arm/mach-ux500/board-mop500-sdi.c |   21 +--
 drivers/mmc/host/mmci.c                |  266 ++++++++++++++++++++++++++------
 drivers/mmc/host/mmci.h                |   15 +--
 include/linux/amba/mmci.h              |   22 +++-
 4 files changed, 252 insertions(+), 72 deletions(-)

-- 
1.7.5.4

^ permalink raw reply	[flat|nested] 52+ messages in thread

* [PATCH 01/14] mmc: mmci: Support MMC_PM_KEEP_POWER
  2011-12-05 17:35 ` Ulf Hansson
@ 2011-12-05 17:35   ` Ulf Hansson
  -1 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2011-12-05 17:35 UTC (permalink / raw)
  To: linux-mmc, linux-arm-kernel; +Cc: Russell King, Ulf Hansson, Lee Jones

Add MMC_PM_KEEP_POWER to pm_caps so SDIO clients are able
to use this option to prevent power off in suspend.

Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
---
 drivers/mmc/host/mmci.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 50b5f99..1ac04a2 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -1240,6 +1240,9 @@ static int __devinit mmci_probe(struct amba_device *dev,
 		mmc->ocr_avail = plat->ocr_mask;
 	mmc->caps = plat->capabilities;
 
+	/* We support these PM capabilities. */
+	mmc->pm_caps = MMC_PM_KEEP_POWER;
+
 	/*
 	 * We can do SGIO
 	 */
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 01/14] mmc: mmci: Support MMC_PM_KEEP_POWER
@ 2011-12-05 17:35   ` Ulf Hansson
  0 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2011-12-05 17:35 UTC (permalink / raw)
  To: linux-arm-kernel

Add MMC_PM_KEEP_POWER to pm_caps so SDIO clients are able
to use this option to prevent power off in suspend.

Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
---
 drivers/mmc/host/mmci.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 50b5f99..1ac04a2 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -1240,6 +1240,9 @@ static int __devinit mmci_probe(struct amba_device *dev,
 		mmc->ocr_avail = plat->ocr_mask;
 	mmc->caps = plat->capabilities;
 
+	/* We support these PM capabilities. */
+	mmc->pm_caps = MMC_PM_KEEP_POWER;
+
 	/*
 	 * We can do SGIO
 	 */
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 02/14] mmc: mmci: Fixup handling of MCI_STARTBITERR
  2011-12-05 17:35 ` Ulf Hansson
@ 2011-12-05 17:35   ` Ulf Hansson
  -1 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2011-12-05 17:35 UTC (permalink / raw)
  To: linux-mmc, linux-arm-kernel; +Cc: Russell King, Ulf Hansson, Lee Jones

The interrupt was previously enabled and then correctly cleared.
Now we also handle it correctly.

Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/mmc/host/mmci.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 1ac04a2..2bc7b6b 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -675,7 +675,8 @@ mmci_data_irq(struct mmci_host *host, struct mmc_data *data,
 	      unsigned int status)
 {
 	/* First check for errors */
-	if (status & (MCI_DATACRCFAIL|MCI_DATATIMEOUT|MCI_TXUNDERRUN|MCI_RXOVERRUN)) {
+	if (status & (MCI_DATACRCFAIL|MCI_DATATIMEOUT|MCI_STARTBITERR|
+		      MCI_TXUNDERRUN|MCI_RXOVERRUN)) {
 		u32 remain, success;
 
 		/* Terminate the DMA transfer */
@@ -955,8 +956,9 @@ static irqreturn_t mmci_irq(int irq, void *dev_id)
 		dev_dbg(mmc_dev(host->mmc), "irq0 (data+cmd) %08x\n", status);
 
 		data = host->data;
-		if (status & (MCI_DATACRCFAIL|MCI_DATATIMEOUT|MCI_TXUNDERRUN|
-			      MCI_RXOVERRUN|MCI_DATAEND|MCI_DATABLOCKEND) && data)
+		if (status & (MCI_DATACRCFAIL|MCI_DATATIMEOUT|MCI_STARTBITERR|
+			      MCI_TXUNDERRUN|MCI_RXOVERRUN|MCI_DATAEND|
+			      MCI_DATABLOCKEND) && data)
 			mmci_data_irq(host, data, status);
 
 		cmd = host->cmd;
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 02/14] mmc: mmci: Fixup handling of MCI_STARTBITERR
@ 2011-12-05 17:35   ` Ulf Hansson
  0 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2011-12-05 17:35 UTC (permalink / raw)
  To: linux-arm-kernel

The interrupt was previously enabled and then correctly cleared.
Now we also handle it correctly.

Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/mmc/host/mmci.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 1ac04a2..2bc7b6b 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -675,7 +675,8 @@ mmci_data_irq(struct mmci_host *host, struct mmc_data *data,
 	      unsigned int status)
 {
 	/* First check for errors */
-	if (status & (MCI_DATACRCFAIL|MCI_DATATIMEOUT|MCI_TXUNDERRUN|MCI_RXOVERRUN)) {
+	if (status & (MCI_DATACRCFAIL|MCI_DATATIMEOUT|MCI_STARTBITERR|
+		      MCI_TXUNDERRUN|MCI_RXOVERRUN)) {
 		u32 remain, success;
 
 		/* Terminate the DMA transfer */
@@ -955,8 +956,9 @@ static irqreturn_t mmci_irq(int irq, void *dev_id)
 		dev_dbg(mmc_dev(host->mmc), "irq0 (data+cmd) %08x\n", status);
 
 		data = host->data;
-		if (status & (MCI_DATACRCFAIL|MCI_DATATIMEOUT|MCI_TXUNDERRUN|
-			      MCI_RXOVERRUN|MCI_DATAEND|MCI_DATABLOCKEND) && data)
+		if (status & (MCI_DATACRCFAIL|MCI_DATATIMEOUT|MCI_STARTBITERR|
+			      MCI_TXUNDERRUN|MCI_RXOVERRUN|MCI_DATAEND|
+			      MCI_DATABLOCKEND) && data)
 			mmci_data_irq(host, data, status);
 
 		cmd = host->cmd;
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 03/14] mmc: mmci: Increase max_segs from 16 to 128
  2011-12-05 17:35 ` Ulf Hansson
@ 2011-12-05 17:35   ` Ulf Hansson
  -1 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2011-12-05 17:35 UTC (permalink / raw)
  To: linux-mmc, linux-arm-kernel
  Cc: Russell King, Ulf Hansson, Lee Jones, Per Forlin

A significant increase (10-20%) in performance throughput for
USB mass storage is the reason for incrementing the value.
By some reason the USB driver allocates buffers which requires
a scattergather list to contain a lot more than 16 elements to
get optimal performance. This change sets the maximum elements
to 128.

Tests with large reads and large writes (100 MiB) show that the
throughput increase is significant for write (10% for this test)
but not for read. Tests are run on a Linux host with ext4 FS on
the gadget mass storage device. The sg-len still exceeds 16 for
the read tests but the performance gain is low or nothing.

Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
Signed-off-by: Per Forlin <per.forlin@stericsson.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/mmc/host/mmci.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 79e4143..49f153e 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -160,7 +160,7 @@
 	(MCI_RXFIFOHALFFULLMASK | MCI_RXDATAAVLBLMASK | \
 	 MCI_TXFIFOHALFEMPTYMASK)
 
-#define NR_SG		16
+#define NR_SG		128
 
 struct clk;
 struct variant_data;
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 03/14] mmc: mmci: Increase max_segs from 16 to 128
@ 2011-12-05 17:35   ` Ulf Hansson
  0 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2011-12-05 17:35 UTC (permalink / raw)
  To: linux-arm-kernel

A significant increase (10-20%) in performance throughput for
USB mass storage is the reason for incrementing the value.
By some reason the USB driver allocates buffers which requires
a scattergather list to contain a lot more than 16 elements to
get optimal performance. This change sets the maximum elements
to 128.

Tests with large reads and large writes (100 MiB) show that the
throughput increase is significant for write (10% for this test)
but not for read. Tests are run on a Linux host with ext4 FS on
the gadget mass storage device. The sg-len still exceeds 16 for
the read tests but the performance gain is low or nothing.

Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
Signed-off-by: Per Forlin <per.forlin@stericsson.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/mmc/host/mmci.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 79e4143..49f153e 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -160,7 +160,7 @@
 	(MCI_RXFIFOHALFFULLMASK | MCI_RXDATAAVLBLMASK | \
 	 MCI_TXFIFOHALFEMPTYMASK)
 
-#define NR_SG		16
+#define NR_SG		128
 
 struct clk;
 struct variant_data;
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 04/14] mmc: mmci: Do not release spinlock in request_end
  2011-12-05 17:35 ` Ulf Hansson
@ 2011-12-05 17:35   ` Ulf Hansson
  -1 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2011-12-05 17:35 UTC (permalink / raw)
  To: linux-mmc, linux-arm-kernel; +Cc: Russell King, Ulf Hansson, Lee Jones

The patch "mmc: core: move ->request() call from atomic context",
is the reason to why this change is possible. This simplifies the
error handling code execution path quite a lot and potentially also
fixes some error handling hang problems.

Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/mmc/host/mmci.c |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 2bc7b6b..409e876 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -166,14 +166,8 @@ mmci_request_end(struct mmci_host *host, struct mmc_request *mrq)
 	host->mrq = NULL;
 	host->cmd = NULL;
 
-	/*
-	 * Need to drop the host lock here; mmc_request_done may call
-	 * back into the driver...
-	 */
-	spin_unlock(&host->lock);
 	pm_runtime_put(mmc_dev(host->mmc));
 	mmc_request_done(host->mmc, mrq);
-	spin_lock(&host->lock);
 }
 
 static void mmci_set_mask1(struct mmci_host *host, unsigned int mask)
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 04/14] mmc: mmci: Do not release spinlock in request_end
@ 2011-12-05 17:35   ` Ulf Hansson
  0 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2011-12-05 17:35 UTC (permalink / raw)
  To: linux-arm-kernel

The patch "mmc: core: move ->request() call from atomic context",
is the reason to why this change is possible. This simplifies the
error handling code execution path quite a lot and potentially also
fixes some error handling hang problems.

Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/mmc/host/mmci.c |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 2bc7b6b..409e876 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -166,14 +166,8 @@ mmci_request_end(struct mmci_host *host, struct mmc_request *mrq)
 	host->mrq = NULL;
 	host->cmd = NULL;
 
-	/*
-	 * Need to drop the host lock here; mmc_request_done may call
-	 * back into the driver...
-	 */
-	spin_unlock(&host->lock);
 	pm_runtime_put(mmc_dev(host->mmc));
 	mmc_request_done(host->mmc, mrq);
-	spin_lock(&host->lock);
 }
 
 static void mmci_set_mask1(struct mmci_host *host, unsigned int mask)
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 05/14] mmc: mmci: Put power register deviations in variant data
  2011-12-05 17:35 ` Ulf Hansson
@ 2011-12-05 17:35   ` Ulf Hansson
  -1 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2011-12-05 17:35 UTC (permalink / raw)
  To: linux-mmc, linux-arm-kernel
  Cc: Russell King, Ulf Hansson, Lee Jones, Sebastian Rasmussen

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 409e876..a3912cc 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,
 };
 
 /*
@@ -1002,6 +1009,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;
@@ -1028,11 +1036,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


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 05/14] mmc: mmci: Put power register deviations in variant data
@ 2011-12-05 17:35   ` Ulf Hansson
  0 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2011-12-05 17:35 UTC (permalink / raw)
  To: linux-arm-kernel

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 409e876..a3912cc 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,
 };
 
 /*
@@ -1002,6 +1009,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;
@@ -1028,11 +1036,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

^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 06/14] mmc: mmci: Provide option to configure bus signal direction
  2011-12-05 17:35 ` Ulf Hansson
@ 2011-12-05 17:35   ` Ulf Hansson
  -1 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2011-12-05 17:35 UTC (permalink / raw)
  To: linux-mmc, linux-arm-kernel
  Cc: Russell King, Ulf Hansson, Lee Jones, Sebastian Rasmussen

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>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
 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 a3912cc..d95555e 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,
 };
 
 /*
@@ -1050,6 +1055,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 49f153e..89eb2e3 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


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 06/14] mmc: mmci: Provide option to configure bus signal direction
@ 2011-12-05 17:35   ` Ulf Hansson
  0 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2011-12-05 17:35 UTC (permalink / raw)
  To: linux-arm-kernel

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>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
 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 a3912cc..d95555e 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,
 };
 
 /*
@@ -1050,6 +1055,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 49f153e..89eb2e3 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

^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 07/14] mmc: mmci: Change vdd_handler to a generic ios_handler
  2011-12-05 17:35 ` Ulf Hansson
@ 2011-12-05 17:35   ` Ulf Hansson
  -1 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2011-12-05 17:35 UTC (permalink / raw)
  To: linux-mmc, linux-arm-kernel
  Cc: Russell King, Ulf Hansson, Lee Jones, Sebastian Rasmussen

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>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
 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 d95555e..e900f51 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -1019,6 +1019,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)
@@ -1038,10 +1042,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


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 07/14] mmc: mmci: Change vdd_handler to a generic ios_handler
@ 2011-12-05 17:35   ` Ulf Hansson
  0 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2011-12-05 17:35 UTC (permalink / raw)
  To: linux-arm-kernel

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>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
 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 d95555e..e900f51 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -1019,6 +1019,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)
@@ -1038,10 +1042,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

^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 08/14] mmc: mmci: Fixup error handling for dma
  2011-12-05 17:35 ` Ulf Hansson
@ 2011-12-05 17:35   ` Ulf Hansson
  -1 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2011-12-05 17:35 UTC (permalink / raw)
  To: linux-mmc, linux-arm-kernel
  Cc: Russell King, Ulf Hansson, Lee Jones, Per Forlin

When getting a cmd irq during an ongoing data transfer
with dma, the dma job were never terminated. This is now
corrected.

Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
Signed-off-by: Per Forlin <per.forlin@stericsson.com>
---
 drivers/mmc/host/mmci.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index e900f51..62ad649 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -761,8 +761,12 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,
 	}
 
 	if (!cmd->data || cmd->error) {
-		if (host->data)
+		if (host->data) {
+			/* Terminate the DMA transfer */
+			if (dma_inprogress(host))
+				mmci_dma_data_error(host);
 			mmci_stop_data(host);
+		}
 		mmci_request_end(host, cmd->mrq);
 	} else if (!(cmd->data->flags & MMC_DATA_READ)) {
 		mmci_start_data(host, cmd->data);
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 08/14] mmc: mmci: Fixup error handling for dma
@ 2011-12-05 17:35   ` Ulf Hansson
  0 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2011-12-05 17:35 UTC (permalink / raw)
  To: linux-arm-kernel

When getting a cmd irq during an ongoing data transfer
with dma, the dma job were never terminated. This is now
corrected.

Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
Signed-off-by: Per Forlin <per.forlin@stericsson.com>
---
 drivers/mmc/host/mmci.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index e900f51..62ad649 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -761,8 +761,12 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,
 	}
 
 	if (!cmd->data || cmd->error) {
-		if (host->data)
+		if (host->data) {
+			/* Terminate the DMA transfer */
+			if (dma_inprogress(host))
+				mmci_dma_data_error(host);
 			mmci_stop_data(host);
+		}
 		mmci_request_end(host, cmd->mrq);
 	} else if (!(cmd->data->flags & MMC_DATA_READ)) {
 		mmci_start_data(host, cmd->data);
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 09/14] mmc: mmci: Change from using legacy suspend
  2011-12-05 17:35 ` Ulf Hansson
@ 2011-12-05 17:35   ` Ulf Hansson
  -1 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2011-12-05 17:35 UTC (permalink / raw)
  To: linux-mmc, linux-arm-kernel; +Cc: Russell King, Ulf Hansson, Lee Jones

This patch switch from using the legacy suspend/resume
to the new way of registering PM callbacks. No functional
change is done.

Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
---
 drivers/mmc/host/mmci.c |   22 ++++++++++++----------
 1 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 62ad649..4560b20 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -1455,10 +1455,11 @@ static int __devexit mmci_remove(struct amba_device *dev)
 	return 0;
 }
 
-#ifdef CONFIG_PM
-static int mmci_suspend(struct amba_device *dev, pm_message_t state)
+#ifdef CONFIG_SUSPEND
+static int mmci_suspend(struct device *dev)
 {
-	struct mmc_host *mmc = amba_get_drvdata(dev);
+	struct amba_device *adev = to_amba_device(dev);
+	struct mmc_host *mmc = amba_get_drvdata(adev);
 	int ret = 0;
 
 	if (mmc) {
@@ -1472,9 +1473,10 @@ static int mmci_suspend(struct amba_device *dev, pm_message_t state)
 	return ret;
 }
 
-static int mmci_resume(struct amba_device *dev)
+static int mmci_resume(struct device *dev)
 {
-	struct mmc_host *mmc = amba_get_drvdata(dev);
+	struct amba_device *adev = to_amba_device(dev);
+	struct mmc_host *mmc = amba_get_drvdata(adev);
 	int ret = 0;
 
 	if (mmc) {
@@ -1487,11 +1489,12 @@ static int mmci_resume(struct amba_device *dev)
 
 	return ret;
 }
-#else
-#define mmci_suspend	NULL
-#define mmci_resume	NULL
 #endif
 
+static const struct dev_pm_ops mmci_dev_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(mmci_suspend, mmci_resume)
+};
+
 static struct amba_id mmci_ids[] = {
 	{
 		.id	= 0x00041180,
@@ -1535,11 +1538,10 @@ static struct amba_id mmci_ids[] = {
 static struct amba_driver mmci_driver = {
 	.drv		= {
 		.name	= DRIVER_NAME,
+		.pm	= &mmci_dev_pm_ops,
 	},
 	.probe		= mmci_probe,
 	.remove		= __devexit_p(mmci_remove),
-	.suspend	= mmci_suspend,
-	.resume		= mmci_resume,
 	.id_table	= mmci_ids,
 };
 
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 09/14] mmc: mmci: Change from using legacy suspend
@ 2011-12-05 17:35   ` Ulf Hansson
  0 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2011-12-05 17:35 UTC (permalink / raw)
  To: linux-arm-kernel

This patch switch from using the legacy suspend/resume
to the new way of registering PM callbacks. No functional
change is done.

Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
---
 drivers/mmc/host/mmci.c |   22 ++++++++++++----------
 1 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 62ad649..4560b20 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -1455,10 +1455,11 @@ static int __devexit mmci_remove(struct amba_device *dev)
 	return 0;
 }
 
-#ifdef CONFIG_PM
-static int mmci_suspend(struct amba_device *dev, pm_message_t state)
+#ifdef CONFIG_SUSPEND
+static int mmci_suspend(struct device *dev)
 {
-	struct mmc_host *mmc = amba_get_drvdata(dev);
+	struct amba_device *adev = to_amba_device(dev);
+	struct mmc_host *mmc = amba_get_drvdata(adev);
 	int ret = 0;
 
 	if (mmc) {
@@ -1472,9 +1473,10 @@ static int mmci_suspend(struct amba_device *dev, pm_message_t state)
 	return ret;
 }
 
-static int mmci_resume(struct amba_device *dev)
+static int mmci_resume(struct device *dev)
 {
-	struct mmc_host *mmc = amba_get_drvdata(dev);
+	struct amba_device *adev = to_amba_device(dev);
+	struct mmc_host *mmc = amba_get_drvdata(adev);
 	int ret = 0;
 
 	if (mmc) {
@@ -1487,11 +1489,12 @@ static int mmci_resume(struct amba_device *dev)
 
 	return ret;
 }
-#else
-#define mmci_suspend	NULL
-#define mmci_resume	NULL
 #endif
 
+static const struct dev_pm_ops mmci_dev_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(mmci_suspend, mmci_resume)
+};
+
 static struct amba_id mmci_ids[] = {
 	{
 		.id	= 0x00041180,
@@ -1535,11 +1538,10 @@ static struct amba_id mmci_ids[] = {
 static struct amba_driver mmci_driver = {
 	.drv		= {
 		.name	= DRIVER_NAME,
+		.pm	= &mmci_dev_pm_ops,
 	},
 	.probe		= mmci_probe,
 	.remove		= __devexit_p(mmci_remove),
-	.suspend	= mmci_suspend,
-	.resume		= mmci_resume,
 	.id_table	= mmci_ids,
 };
 
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 10/14] mmc: mmci: Cache MMCICLOCK and MMCIPOWER register
  2011-12-05 17:35 ` Ulf Hansson
@ 2011-12-05 17:35   ` Ulf Hansson
  -1 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2011-12-05 17:35 UTC (permalink / raw)
  To: linux-mmc, linux-arm-kernel; +Cc: Russell King, Ulf Hansson, Lee Jones

Instead of reading a register value everytime we need to
apply a new value for it, maintain a cached copy for it.
This also means we are able to skip writes that are not
needed.

Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
---
 drivers/mmc/host/mmci.c |   41 +++++++++++++++++++++++++++++------------
 drivers/mmc/host/mmci.h |    3 ++-
 2 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 4560b20..45e794a 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -121,6 +121,28 @@ static struct variant_data variant_ux500v2 = {
 /*
  * This must be called with host->lock held
  */
+static void mmci_write_clkreg(struct mmci_host *host, u32 clk)
+{
+	if (host->clk_reg != clk) {
+		host->clk_reg = clk;
+		writel(clk, host->base + MMCICLOCK);
+	}
+}
+
+/*
+ * This must be called with host->lock held
+ */
+static void mmci_write_pwrreg(struct mmci_host *host, u32 pwr)
+{
+	if (host->pwr_reg != pwr) {
+		host->pwr_reg = pwr;
+		writel(pwr, host->base + MMCIPOWER);
+	}
+}
+
+/*
+ * This must be called with host->lock held
+ */
 static void mmci_set_clkreg(struct mmci_host *host, unsigned int desired)
 {
 	struct variant_data *variant = host->variant;
@@ -165,7 +187,7 @@ static void mmci_set_clkreg(struct mmci_host *host, unsigned int desired)
 	if (host->mmc->ios.bus_width == MMC_BUS_WIDTH_8)
 		clk |= MCI_ST_8BIT_BUS;
 
-	writel(clk, host->base + MMCICLOCK);
+	mmci_write_clkreg(host, clk);
 }
 
 static void
@@ -824,14 +846,13 @@ static int mmci_pio_write(struct mmci_host *host, char *buffer, unsigned int rem
 		 */
 		if (variant->sdio &&
 		    mmc_card_sdio(host->mmc->card)) {
+			u32 clk;
 			if (count < 8)
-				writel(readl(host->base + MMCICLOCK) &
-					~variant->clkreg_enable,
-					host->base + MMCICLOCK);
+				clk = host->clk_reg & ~variant->clkreg_enable;
 			else
-				writel(readl(host->base + MMCICLOCK) |
-					variant->clkreg_enable,
-					host->base + MMCICLOCK);
+				clk = host->clk_reg | variant->clkreg_enable;
+
+			mmci_write_clkreg(host, clk);
 		}
 
 		/*
@@ -1090,11 +1111,7 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	spin_lock_irqsave(&host->lock, flags);
 
 	mmci_set_clkreg(host, ios->clock);
-
-	if (host->pwr != pwr) {
-		host->pwr = pwr;
-		writel(pwr, host->base + MMCIPOWER);
-	}
+	mmci_write_pwrreg(host, pwr);
 
 	spin_unlock_irqrestore(&host->lock, flags);
 }
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 89eb2e3..d437ccf 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -179,7 +179,8 @@ struct mmci_host {
 
 	unsigned int		mclk;
 	unsigned int		cclk;
-	u32			pwr;
+	u32			pwr_reg;
+	u32			clk_reg;
 	struct mmci_platform_data *plat;
 	struct variant_data	*variant;
 
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 10/14] mmc: mmci: Cache MMCICLOCK and MMCIPOWER register
@ 2011-12-05 17:35   ` Ulf Hansson
  0 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2011-12-05 17:35 UTC (permalink / raw)
  To: linux-arm-kernel

Instead of reading a register value everytime we need to
apply a new value for it, maintain a cached copy for it.
This also means we are able to skip writes that are not
needed.

Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
---
 drivers/mmc/host/mmci.c |   41 +++++++++++++++++++++++++++++------------
 drivers/mmc/host/mmci.h |    3 ++-
 2 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 4560b20..45e794a 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -121,6 +121,28 @@ static struct variant_data variant_ux500v2 = {
 /*
  * This must be called with host->lock held
  */
+static void mmci_write_clkreg(struct mmci_host *host, u32 clk)
+{
+	if (host->clk_reg != clk) {
+		host->clk_reg = clk;
+		writel(clk, host->base + MMCICLOCK);
+	}
+}
+
+/*
+ * This must be called with host->lock held
+ */
+static void mmci_write_pwrreg(struct mmci_host *host, u32 pwr)
+{
+	if (host->pwr_reg != pwr) {
+		host->pwr_reg = pwr;
+		writel(pwr, host->base + MMCIPOWER);
+	}
+}
+
+/*
+ * This must be called with host->lock held
+ */
 static void mmci_set_clkreg(struct mmci_host *host, unsigned int desired)
 {
 	struct variant_data *variant = host->variant;
@@ -165,7 +187,7 @@ static void mmci_set_clkreg(struct mmci_host *host, unsigned int desired)
 	if (host->mmc->ios.bus_width == MMC_BUS_WIDTH_8)
 		clk |= MCI_ST_8BIT_BUS;
 
-	writel(clk, host->base + MMCICLOCK);
+	mmci_write_clkreg(host, clk);
 }
 
 static void
@@ -824,14 +846,13 @@ static int mmci_pio_write(struct mmci_host *host, char *buffer, unsigned int rem
 		 */
 		if (variant->sdio &&
 		    mmc_card_sdio(host->mmc->card)) {
+			u32 clk;
 			if (count < 8)
-				writel(readl(host->base + MMCICLOCK) &
-					~variant->clkreg_enable,
-					host->base + MMCICLOCK);
+				clk = host->clk_reg & ~variant->clkreg_enable;
 			else
-				writel(readl(host->base + MMCICLOCK) |
-					variant->clkreg_enable,
-					host->base + MMCICLOCK);
+				clk = host->clk_reg | variant->clkreg_enable;
+
+			mmci_write_clkreg(host, clk);
 		}
 
 		/*
@@ -1090,11 +1111,7 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	spin_lock_irqsave(&host->lock, flags);
 
 	mmci_set_clkreg(host, ios->clock);
-
-	if (host->pwr != pwr) {
-		host->pwr = pwr;
-		writel(pwr, host->base + MMCIPOWER);
-	}
+	mmci_write_pwrreg(host, pwr);
 
 	spin_unlock_irqrestore(&host->lock, flags);
 }
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 89eb2e3..d437ccf 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -179,7 +179,8 @@ struct mmci_host {
 
 	unsigned int		mclk;
 	unsigned int		cclk;
-	u32			pwr;
+	u32			pwr_reg;
+	u32			clk_reg;
 	struct mmci_platform_data *plat;
 	struct variant_data	*variant;
 
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 11/14] mmc: mmci: Fixup use of runtime PM and use autosuspend
  2011-12-05 17:35 ` Ulf Hansson
@ 2011-12-05 17:35   ` Ulf Hansson
  -1 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2011-12-05 17:35 UTC (permalink / raw)
  To: linux-mmc, linux-arm-kernel; +Cc: Russell King, Ulf Hansson, Lee Jones

Added use of runtime PM autosuspend feature, with a fixed
timeout of 50 ms. This will prevent adding a latency,
although very minor, for _every_ request.

Moreover the runtime_get_sync is now also used in set_ios and
suspend since the runtime resourses are needed here as well.

Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
---
 drivers/mmc/host/mmci.c |   19 ++++++++++++++++---
 1 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 45e794a..8971157 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -200,8 +200,10 @@ mmci_request_end(struct mmci_host *host, struct mmc_request *mrq)
 	host->mrq = NULL;
 	host->cmd = NULL;
 
-	pm_runtime_put(mmc_dev(host->mmc));
 	mmc_request_done(host->mmc, mrq);
+
+	pm_runtime_mark_last_busy(mmc_dev(host->mmc));
+	pm_runtime_put_autosuspend(mmc_dev(host->mmc));
 }
 
 static void mmci_set_mask1(struct mmci_host *host, unsigned int mask)
@@ -1044,6 +1046,8 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	unsigned long flags;
 	int ret;
 
+	pm_runtime_get_sync(mmc_dev(mmc));
+
 	if (host->plat->ios_handler &&
 		host->plat->ios_handler(mmc_dev(mmc), ios))
 			dev_err(mmc_dev(mmc), "platform ios_handler failed\n");
@@ -1064,7 +1068,7 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 				 * power should be rare so we print an error
 				 * and return here.
 				 */
-				return;
+				goto out;
 			}
 		}
 		/*
@@ -1114,6 +1118,10 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	mmci_write_pwrreg(host, pwr);
 
 	spin_unlock_irqrestore(&host->lock, flags);
+
+ out:
+	pm_runtime_mark_last_busy(mmc_dev(mmc));
+	pm_runtime_put_autosuspend(mmc_dev(mmc));
 }
 
 static int mmci_get_ro(struct mmc_host *mmc)
@@ -1388,6 +1396,8 @@ static int __devinit mmci_probe(struct amba_device *dev,
 
 	mmci_dma_setup(host);
 
+	pm_runtime_set_autosuspend_delay(&dev->dev, 50);
+	pm_runtime_use_autosuspend(&dev->dev);
 	pm_runtime_put(&dev->dev);
 
 	mmc_add_host(mmc);
@@ -1483,8 +1493,10 @@ static int mmci_suspend(struct device *dev)
 		struct mmci_host *host = mmc_priv(mmc);
 
 		ret = mmc_suspend_host(mmc);
-		if (ret == 0)
+		if (ret == 0) {
+			pm_runtime_get_sync(dev);
 			writel(0, host->base + MMCIMASK0);
+		}
 	}
 
 	return ret;
@@ -1500,6 +1512,7 @@ static int mmci_resume(struct device *dev)
 		struct mmci_host *host = mmc_priv(mmc);
 
 		writel(MCI_IRQENABLE, host->base + MMCIMASK0);
+		pm_runtime_put(dev);
 
 		ret = mmc_resume_host(mmc);
 	}
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 11/14] mmc: mmci: Fixup use of runtime PM and use autosuspend
@ 2011-12-05 17:35   ` Ulf Hansson
  0 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2011-12-05 17:35 UTC (permalink / raw)
  To: linux-arm-kernel

Added use of runtime PM autosuspend feature, with a fixed
timeout of 50 ms. This will prevent adding a latency,
although very minor, for _every_ request.

Moreover the runtime_get_sync is now also used in set_ios and
suspend since the runtime resourses are needed here as well.

Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
---
 drivers/mmc/host/mmci.c |   19 ++++++++++++++++---
 1 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 45e794a..8971157 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -200,8 +200,10 @@ mmci_request_end(struct mmci_host *host, struct mmc_request *mrq)
 	host->mrq = NULL;
 	host->cmd = NULL;
 
-	pm_runtime_put(mmc_dev(host->mmc));
 	mmc_request_done(host->mmc, mrq);
+
+	pm_runtime_mark_last_busy(mmc_dev(host->mmc));
+	pm_runtime_put_autosuspend(mmc_dev(host->mmc));
 }
 
 static void mmci_set_mask1(struct mmci_host *host, unsigned int mask)
@@ -1044,6 +1046,8 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	unsigned long flags;
 	int ret;
 
+	pm_runtime_get_sync(mmc_dev(mmc));
+
 	if (host->plat->ios_handler &&
 		host->plat->ios_handler(mmc_dev(mmc), ios))
 			dev_err(mmc_dev(mmc), "platform ios_handler failed\n");
@@ -1064,7 +1068,7 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 				 * power should be rare so we print an error
 				 * and return here.
 				 */
-				return;
+				goto out;
 			}
 		}
 		/*
@@ -1114,6 +1118,10 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	mmci_write_pwrreg(host, pwr);
 
 	spin_unlock_irqrestore(&host->lock, flags);
+
+ out:
+	pm_runtime_mark_last_busy(mmc_dev(mmc));
+	pm_runtime_put_autosuspend(mmc_dev(mmc));
 }
 
 static int mmci_get_ro(struct mmc_host *mmc)
@@ -1388,6 +1396,8 @@ static int __devinit mmci_probe(struct amba_device *dev,
 
 	mmci_dma_setup(host);
 
+	pm_runtime_set_autosuspend_delay(&dev->dev, 50);
+	pm_runtime_use_autosuspend(&dev->dev);
 	pm_runtime_put(&dev->dev);
 
 	mmc_add_host(mmc);
@@ -1483,8 +1493,10 @@ static int mmci_suspend(struct device *dev)
 		struct mmci_host *host = mmc_priv(mmc);
 
 		ret = mmc_suspend_host(mmc);
-		if (ret == 0)
+		if (ret == 0) {
+			pm_runtime_get_sync(dev);
 			writel(0, host->base + MMCIMASK0);
+		}
 	}
 
 	return ret;
@@ -1500,6 +1512,7 @@ static int mmci_resume(struct device *dev)
 		struct mmci_host *host = mmc_priv(mmc);
 
 		writel(MCI_IRQENABLE, host->base + MMCIMASK0);
+		pm_runtime_put(dev);
 
 		ret = mmc_resume_host(mmc);
 	}
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 12/14] mmc: mmci: Decrease current consumption in suspend
  2011-12-05 17:35 ` Ulf Hansson
@ 2011-12-05 17:35   ` Ulf Hansson
  -1 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2011-12-05 17:35 UTC (permalink / raw)
  To: linux-mmc, linux-arm-kernel; +Cc: Russell King, Ulf Hansson, Lee Jones

To decrease current consumption in suspend state the
VCORE regulator, the MCLK and PCLK for the ARM PL18x
are now disabled.

When resuming the resourses are re-enabled and
register values for MMCICLOCK, MMCIPOWER and MMCIMASK0
are restored.

Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
---
 drivers/mmc/host/mmci.c |   64 ++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 58 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 8971157..d29b7dc 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -1483,6 +1483,60 @@ static int __devexit mmci_remove(struct amba_device *dev)
 }
 
 #ifdef CONFIG_SUSPEND
+static int mmci_save(struct amba_device *dev)
+{
+	struct mmc_host *mmc = amba_get_drvdata(dev);
+	unsigned long flags;
+
+	if (mmc) {
+		struct mmci_host *host = mmc_priv(mmc);
+
+		spin_lock_irqsave(&host->lock, flags);
+
+		/*
+		 * Make sure we do not get any interrupts when we disabled the
+		 * clock and the regulator and as well make sure to clear the
+		 * registers for clock and power.
+		 */
+		writel(0, host->base + MMCIMASK0);
+		writel(0, host->base + MMCIPOWER);
+		writel(0, host->base + MMCICLOCK);
+
+		spin_unlock_irqrestore(&host->lock, flags);
+
+		clk_unprepare(host->clk);
+		clk_disable(host->clk);
+		amba_vcore_disable(dev);
+	}
+
+	return 0;
+}
+
+static int mmci_restore(struct amba_device *dev)
+{
+	struct mmc_host *mmc = amba_get_drvdata(dev);
+	unsigned long flags;
+
+	if (mmc) {
+		struct mmci_host *host = mmc_priv(mmc);
+
+		amba_vcore_enable(dev);
+		clk_prepare(host->clk);
+		clk_enable(host->clk);
+
+		spin_lock_irqsave(&host->lock, flags);
+
+		/* Restore registers and re-enable interrupts. */
+		writel(host->clk_reg, host->base + MMCICLOCK);
+		writel(host->pwr_reg, host->base + MMCIPOWER);
+		writel(MCI_IRQENABLE, host->base + MMCIMASK0);
+
+		spin_unlock_irqrestore(&host->lock, flags);
+	}
+
+	return 0;
+}
+
 static int mmci_suspend(struct device *dev)
 {
 	struct amba_device *adev = to_amba_device(dev);
@@ -1490,12 +1544,11 @@ static int mmci_suspend(struct device *dev)
 	int ret = 0;
 
 	if (mmc) {
-		struct mmci_host *host = mmc_priv(mmc);
-
 		ret = mmc_suspend_host(mmc);
 		if (ret == 0) {
 			pm_runtime_get_sync(dev);
-			writel(0, host->base + MMCIMASK0);
+			mmci_save(adev);
+			amba_pclk_disable(adev);
 		}
 	}
 
@@ -1509,9 +1562,8 @@ static int mmci_resume(struct device *dev)
 	int ret = 0;
 
 	if (mmc) {
-		struct mmci_host *host = mmc_priv(mmc);
-
-		writel(MCI_IRQENABLE, host->base + MMCIMASK0);
+		amba_pclk_enable(adev);
+		mmci_restore(adev);
 		pm_runtime_put(dev);
 
 		ret = mmc_resume_host(mmc);
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 12/14] mmc: mmci: Decrease current consumption in suspend
@ 2011-12-05 17:35   ` Ulf Hansson
  0 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2011-12-05 17:35 UTC (permalink / raw)
  To: linux-arm-kernel

To decrease current consumption in suspend state the
VCORE regulator, the MCLK and PCLK for the ARM PL18x
are now disabled.

When resuming the resourses are re-enabled and
register values for MMCICLOCK, MMCIPOWER and MMCIMASK0
are restored.

Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
---
 drivers/mmc/host/mmci.c |   64 ++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 58 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 8971157..d29b7dc 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -1483,6 +1483,60 @@ static int __devexit mmci_remove(struct amba_device *dev)
 }
 
 #ifdef CONFIG_SUSPEND
+static int mmci_save(struct amba_device *dev)
+{
+	struct mmc_host *mmc = amba_get_drvdata(dev);
+	unsigned long flags;
+
+	if (mmc) {
+		struct mmci_host *host = mmc_priv(mmc);
+
+		spin_lock_irqsave(&host->lock, flags);
+
+		/*
+		 * Make sure we do not get any interrupts when we disabled the
+		 * clock and the regulator and as well make sure to clear the
+		 * registers for clock and power.
+		 */
+		writel(0, host->base + MMCIMASK0);
+		writel(0, host->base + MMCIPOWER);
+		writel(0, host->base + MMCICLOCK);
+
+		spin_unlock_irqrestore(&host->lock, flags);
+
+		clk_unprepare(host->clk);
+		clk_disable(host->clk);
+		amba_vcore_disable(dev);
+	}
+
+	return 0;
+}
+
+static int mmci_restore(struct amba_device *dev)
+{
+	struct mmc_host *mmc = amba_get_drvdata(dev);
+	unsigned long flags;
+
+	if (mmc) {
+		struct mmci_host *host = mmc_priv(mmc);
+
+		amba_vcore_enable(dev);
+		clk_prepare(host->clk);
+		clk_enable(host->clk);
+
+		spin_lock_irqsave(&host->lock, flags);
+
+		/* Restore registers and re-enable interrupts. */
+		writel(host->clk_reg, host->base + MMCICLOCK);
+		writel(host->pwr_reg, host->base + MMCIPOWER);
+		writel(MCI_IRQENABLE, host->base + MMCIMASK0);
+
+		spin_unlock_irqrestore(&host->lock, flags);
+	}
+
+	return 0;
+}
+
 static int mmci_suspend(struct device *dev)
 {
 	struct amba_device *adev = to_amba_device(dev);
@@ -1490,12 +1544,11 @@ static int mmci_suspend(struct device *dev)
 	int ret = 0;
 
 	if (mmc) {
-		struct mmci_host *host = mmc_priv(mmc);
-
 		ret = mmc_suspend_host(mmc);
 		if (ret == 0) {
 			pm_runtime_get_sync(dev);
-			writel(0, host->base + MMCIMASK0);
+			mmci_save(adev);
+			amba_pclk_disable(adev);
 		}
 	}
 
@@ -1509,9 +1562,8 @@ static int mmci_resume(struct device *dev)
 	int ret = 0;
 
 	if (mmc) {
-		struct mmci_host *host = mmc_priv(mmc);
-
-		writel(MCI_IRQENABLE, host->base + MMCIMASK0);
+		amba_pclk_enable(adev);
+		mmci_restore(adev);
 		pm_runtime_put(dev);
 
 		ret = mmc_resume_host(mmc);
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 13/14] mmc: mmci: Implement PM runtime callbacks to save power
  2011-12-05 17:35 ` Ulf Hansson
@ 2011-12-05 17:35   ` Ulf Hansson
  -1 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2011-12-05 17:35 UTC (permalink / raw)
  To: linux-mmc, linux-arm-kernel; +Cc: Russell King, Ulf Hansson, Lee Jones

In the runtime_suspend callback we make use of mmci_save
to disable the VCORE regulator and the MCLK to decrease
current consumption. At runtime resume, we use mmci_restore
to re-enable the resourses again.

>From now on this will mean that especially the mmci_restore
function must be fast to execute since otherwise request
latency will be introduced.

For the ARM Primcell PL180, the MMCIPOWER register is used
to control an external power supply to the card. Thus we
need to prevent the runtime callbacks from doing save and
restore, otherwise the power to card will be cut. This is
done by adding a new flag to the variant data.

Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
---
 drivers/mmc/host/mmci.c |   43 ++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 42 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index d29b7dc..c0d6026 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -55,6 +55,7 @@ static unsigned int fmax = 515633;
  * @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
+ * @pwrreg_ctrl_power: bits in MMCIPOWER register controls ext. power supply
  */
 struct variant_data {
 	unsigned int		clkreg;
@@ -67,6 +68,7 @@ struct variant_data {
 	bool			blksz_datactrl16;
 	u32			pwrreg_powerup;
 	bool			signal_direction;
+	bool			pwrreg_ctrl_power;
 };
 
 static struct variant_data variant_arm = {
@@ -74,6 +76,7 @@ static struct variant_data variant_arm = {
 	.fifohalfsize		= 8 * 4,
 	.datalength_bits	= 16,
 	.pwrreg_powerup		= MCI_PWR_UP,
+	.pwrreg_ctrl_power	= true,
 };
 
 static struct variant_data variant_arm_extended_fifo = {
@@ -81,6 +84,7 @@ static struct variant_data variant_arm_extended_fifo = {
 	.fifohalfsize		= 64 * 4,
 	.datalength_bits	= 16,
 	.pwrreg_powerup		= MCI_PWR_UP,
+	.pwrreg_ctrl_power	= true,
 };
 
 static struct variant_data variant_u300 = {
@@ -1482,7 +1486,7 @@ static int __devexit mmci_remove(struct amba_device *dev)
 	return 0;
 }
 
-#ifdef CONFIG_SUSPEND
+#if defined(CONFIG_SUSPEND) || defined(CONFIG_PM_RUNTIME)
 static int mmci_save(struct amba_device *dev)
 {
 	struct mmc_host *mmc = amba_get_drvdata(dev);
@@ -1536,7 +1540,9 @@ static int mmci_restore(struct amba_device *dev)
 
 	return 0;
 }
+#endif
 
+#ifdef CONFIG_SUSPEND
 static int mmci_suspend(struct device *dev)
 {
 	struct amba_device *adev = to_amba_device(dev);
@@ -1573,8 +1579,43 @@ static int mmci_resume(struct device *dev)
 }
 #endif
 
+#ifdef CONFIG_PM_RUNTIME
+static int mmci_runtime_suspend(struct device *dev)
+{
+	struct amba_device *adev = to_amba_device(dev);
+	struct mmc_host *mmc = amba_get_drvdata(adev);
+	int ret = 0;
+
+	if (mmc) {
+		struct mmci_host *host = mmc_priv(mmc);
+		struct variant_data *variant = host->variant;
+		if (!variant->pwrreg_ctrl_power)
+			ret = mmci_save(adev);
+	}
+
+	return ret;
+}
+
+static int mmci_runtime_resume(struct device *dev)
+{
+	struct amba_device *adev = to_amba_device(dev);
+	struct mmc_host *mmc = amba_get_drvdata(adev);
+	int ret = 0;
+
+	if (mmc) {
+		struct mmci_host *host = mmc_priv(mmc);
+		struct variant_data *variant = host->variant;
+		if (!variant->pwrreg_ctrl_power)
+			ret = mmci_restore(adev);
+	}
+
+	return ret;
+}
+#endif
+
 static const struct dev_pm_ops mmci_dev_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(mmci_suspend, mmci_resume)
+	SET_RUNTIME_PM_OPS(mmci_runtime_suspend, mmci_runtime_resume, NULL)
 };
 
 static struct amba_id mmci_ids[] = {
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 13/14] mmc: mmci: Implement PM runtime callbacks to save power
@ 2011-12-05 17:35   ` Ulf Hansson
  0 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2011-12-05 17:35 UTC (permalink / raw)
  To: linux-arm-kernel

In the runtime_suspend callback we make use of mmci_save
to disable the VCORE regulator and the MCLK to decrease
current consumption. At runtime resume, we use mmci_restore
to re-enable the resourses again.

>From now on this will mean that especially the mmci_restore
function must be fast to execute since otherwise request
latency will be introduced.

For the ARM Primcell PL180, the MMCIPOWER register is used
to control an external power supply to the card. Thus we
need to prevent the runtime callbacks from doing save and
restore, otherwise the power to card will be cut. This is
done by adding a new flag to the variant data.

Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
---
 drivers/mmc/host/mmci.c |   43 ++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 42 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index d29b7dc..c0d6026 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -55,6 +55,7 @@ static unsigned int fmax = 515633;
  * @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
+ * @pwrreg_ctrl_power: bits in MMCIPOWER register controls ext. power supply
  */
 struct variant_data {
 	unsigned int		clkreg;
@@ -67,6 +68,7 @@ struct variant_data {
 	bool			blksz_datactrl16;
 	u32			pwrreg_powerup;
 	bool			signal_direction;
+	bool			pwrreg_ctrl_power;
 };
 
 static struct variant_data variant_arm = {
@@ -74,6 +76,7 @@ static struct variant_data variant_arm = {
 	.fifohalfsize		= 8 * 4,
 	.datalength_bits	= 16,
 	.pwrreg_powerup		= MCI_PWR_UP,
+	.pwrreg_ctrl_power	= true,
 };
 
 static struct variant_data variant_arm_extended_fifo = {
@@ -81,6 +84,7 @@ static struct variant_data variant_arm_extended_fifo = {
 	.fifohalfsize		= 64 * 4,
 	.datalength_bits	= 16,
 	.pwrreg_powerup		= MCI_PWR_UP,
+	.pwrreg_ctrl_power	= true,
 };
 
 static struct variant_data variant_u300 = {
@@ -1482,7 +1486,7 @@ static int __devexit mmci_remove(struct amba_device *dev)
 	return 0;
 }
 
-#ifdef CONFIG_SUSPEND
+#if defined(CONFIG_SUSPEND) || defined(CONFIG_PM_RUNTIME)
 static int mmci_save(struct amba_device *dev)
 {
 	struct mmc_host *mmc = amba_get_drvdata(dev);
@@ -1536,7 +1540,9 @@ static int mmci_restore(struct amba_device *dev)
 
 	return 0;
 }
+#endif
 
+#ifdef CONFIG_SUSPEND
 static int mmci_suspend(struct device *dev)
 {
 	struct amba_device *adev = to_amba_device(dev);
@@ -1573,8 +1579,43 @@ static int mmci_resume(struct device *dev)
 }
 #endif
 
+#ifdef CONFIG_PM_RUNTIME
+static int mmci_runtime_suspend(struct device *dev)
+{
+	struct amba_device *adev = to_amba_device(dev);
+	struct mmc_host *mmc = amba_get_drvdata(adev);
+	int ret = 0;
+
+	if (mmc) {
+		struct mmci_host *host = mmc_priv(mmc);
+		struct variant_data *variant = host->variant;
+		if (!variant->pwrreg_ctrl_power)
+			ret = mmci_save(adev);
+	}
+
+	return ret;
+}
+
+static int mmci_runtime_resume(struct device *dev)
+{
+	struct amba_device *adev = to_amba_device(dev);
+	struct mmc_host *mmc = amba_get_drvdata(adev);
+	int ret = 0;
+
+	if (mmc) {
+		struct mmci_host *host = mmc_priv(mmc);
+		struct variant_data *variant = host->variant;
+		if (!variant->pwrreg_ctrl_power)
+			ret = mmci_restore(adev);
+	}
+
+	return ret;
+}
+#endif
+
 static const struct dev_pm_ops mmci_dev_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(mmci_suspend, mmci_resume)
+	SET_RUNTIME_PM_OPS(mmci_runtime_suspend, mmci_runtime_resume, NULL)
 };
 
 static struct amba_id mmci_ids[] = {
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 14/14] mmc: mmci: Use ios_handler to save power
  2011-12-05 17:35 ` Ulf Hansson
@ 2011-12-05 17:36   ` Ulf Hansson
  -1 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2011-12-05 17:36 UTC (permalink / raw)
  To: linux-mmc, linux-arm-kernel; +Cc: Russell King, Ulf Hansson, Lee Jones

To disable a levelshifter when we are in an idle state will
decrease current consumption. We make use of the ios_handler
at runtime suspend and at regular suspend to accomplish this.

Of course depending on the used levelshifter the decrease of
current differs. For ST6G3244ME the value is up to ~200 uA.

Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
---
 drivers/mmc/host/mmci.c |   19 ++++++++++++++++++-
 1 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index c0d6026..b98ee98 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -1491,10 +1491,22 @@ static int mmci_save(struct amba_device *dev)
 {
 	struct mmc_host *mmc = amba_get_drvdata(dev);
 	unsigned long flags;
+	struct mmc_ios ios;
+	int ret = 0;
 
 	if (mmc) {
 		struct mmci_host *host = mmc_priv(mmc);
 
+		/* Let the ios_handler act on a POWER_OFF to save power. */
+		if (host->plat->ios_handler) {
+			memcpy(&ios, &mmc->ios, sizeof(struct mmc_ios));
+			ios.power_mode = MMC_POWER_OFF;
+			ret = host->plat->ios_handler(mmc_dev(mmc),
+						      &ios);
+			if (ret)
+				return ret;
+		}
+
 		spin_lock_irqsave(&host->lock, flags);
 
 		/*
@@ -1513,7 +1525,7 @@ static int mmci_save(struct amba_device *dev)
 		amba_vcore_disable(dev);
 	}
 
-	return 0;
+	return ret;
 }
 
 static int mmci_restore(struct amba_device *dev)
@@ -1536,6 +1548,11 @@ static int mmci_restore(struct amba_device *dev)
 		writel(MCI_IRQENABLE, host->base + MMCIMASK0);
 
 		spin_unlock_irqrestore(&host->lock, flags);
+
+		/* Restore settings done by the ios_handler. */
+		if (host->plat->ios_handler)
+			host->plat->ios_handler(mmc_dev(mmc),
+						&mmc->ios);
 	}
 
 	return 0;
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 14/14] mmc: mmci: Use ios_handler to save power
@ 2011-12-05 17:36   ` Ulf Hansson
  0 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2011-12-05 17:36 UTC (permalink / raw)
  To: linux-arm-kernel

To disable a levelshifter when we are in an idle state will
decrease current consumption. We make use of the ios_handler
at runtime suspend and at regular suspend to accomplish this.

Of course depending on the used levelshifter the decrease of
current differs. For ST6G3244ME the value is up to ~200 uA.

Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
---
 drivers/mmc/host/mmci.c |   19 ++++++++++++++++++-
 1 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index c0d6026..b98ee98 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -1491,10 +1491,22 @@ static int mmci_save(struct amba_device *dev)
 {
 	struct mmc_host *mmc = amba_get_drvdata(dev);
 	unsigned long flags;
+	struct mmc_ios ios;
+	int ret = 0;
 
 	if (mmc) {
 		struct mmci_host *host = mmc_priv(mmc);
 
+		/* Let the ios_handler act on a POWER_OFF to save power. */
+		if (host->plat->ios_handler) {
+			memcpy(&ios, &mmc->ios, sizeof(struct mmc_ios));
+			ios.power_mode = MMC_POWER_OFF;
+			ret = host->plat->ios_handler(mmc_dev(mmc),
+						      &ios);
+			if (ret)
+				return ret;
+		}
+
 		spin_lock_irqsave(&host->lock, flags);
 
 		/*
@@ -1513,7 +1525,7 @@ static int mmci_save(struct amba_device *dev)
 		amba_vcore_disable(dev);
 	}
 
-	return 0;
+	return ret;
 }
 
 static int mmci_restore(struct amba_device *dev)
@@ -1536,6 +1548,11 @@ static int mmci_restore(struct amba_device *dev)
 		writel(MCI_IRQENABLE, host->base + MMCIMASK0);
 
 		spin_unlock_irqrestore(&host->lock, flags);
+
+		/* Restore settings done by the ios_handler. */
+		if (host->plat->ios_handler)
+			host->plat->ios_handler(mmc_dev(mmc),
+						&mmc->ios);
 	}
 
 	return 0;
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 52+ messages in thread

* Re: [PATCH 00/14] mmc: mmci: Improved PM support, cleanup and bugfixes
  2011-12-05 17:35 ` Ulf Hansson
@ 2011-12-07 12:06   ` Linus Walleij
  -1 siblings, 0 replies; 52+ messages in thread
From: Linus Walleij @ 2011-12-07 12:06 UTC (permalink / raw)
  To: Ulf Hansson, Russell King; +Cc: linux-mmc, linux-arm-kernel, Lee Jones

Hi Ulf,

On Mon, Dec 5, 2011 at 6:35 PM, Ulf Hansson <ulf.hansson@stericsson.com> wrote:

> Patch 1 -> 4:
> Minor changes for bugs and some performance optimizations.
> Previously sent to the mailing list.
> Patch 5 -> 7:
> Minor changes affecting how to handle a levelshifter.
> Previously sent to the mailing list.
> Patch 8:
> Bugfix for dma. Previously sent to the mailing list, ongoing discussion.
> Patch 9 -> 14:
> Improvement of PM support.

I applied all 14 patches on top of current v3.2-rc4
and regression-tested the whole shebang on RealView PB1176
and U300.

- Mounted /dev/mmcblk0p1
- Copy a kernel-sized uImage file from the card to ramdisk
- Copy the file back to the card from ramdisk
- Unmount
- Mount
- Compare the two files with diff (identical)
- Delete copy
- Unmount
- dd if=/dev/mmcblk0 of=/dev/null

(If any of you wants any more specific tests, tell me.)

Both platforms still work like a charm with these patches.
PB1176 is as slow on MMC as it always has been but dd:ed
the entire card without errors.

I have the branch here:
git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-stericsson.git mmci

Ulf you can add my Tested-by: Linus Walleij <linus.walleij@linaro.org>
for these patches.

Russell if you want a pull request for a subset of the patches I can
arrange that, just tell me.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 52+ messages in thread

* [PATCH 00/14] mmc: mmci: Improved PM support, cleanup and bugfixes
@ 2011-12-07 12:06   ` Linus Walleij
  0 siblings, 0 replies; 52+ messages in thread
From: Linus Walleij @ 2011-12-07 12:06 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Ulf,

On Mon, Dec 5, 2011 at 6:35 PM, Ulf Hansson <ulf.hansson@stericsson.com> wrote:

> Patch 1 -> 4:
> Minor changes for bugs and some performance optimizations.
> Previously sent to the mailing list.
> Patch 5 -> 7:
> Minor changes affecting how to handle a levelshifter.
> Previously sent to the mailing list.
> Patch 8:
> Bugfix for dma. Previously sent to the mailing list, ongoing discussion.
> Patch 9 -> 14:
> Improvement of PM support.

I applied all 14 patches on top of current v3.2-rc4
and regression-tested the whole shebang on RealView PB1176
and U300.

- Mounted /dev/mmcblk0p1
- Copy a kernel-sized uImage file from the card to ramdisk
- Copy the file back to the card from ramdisk
- Unmount
- Mount
- Compare the two files with diff (identical)
- Delete copy
- Unmount
- dd if=/dev/mmcblk0 of=/dev/null

(If any of you wants any more specific tests, tell me.)

Both platforms still work like a charm with these patches.
PB1176 is as slow on MMC as it always has been but dd:ed
the entire card without errors.

I have the branch here:
git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-stericsson.git mmci

Ulf you can add my Tested-by: Linus Walleij <linus.walleij@linaro.org>
for these patches.

Russell if you want a pull request for a subset of the patches I can
arrange that, just tell me.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 00/14] mmc: mmci: Improved PM support, cleanup and bugfixes
  2011-12-07 12:06   ` Linus Walleij
@ 2011-12-13 16:17     ` Ulf Hansson
  -1 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2011-12-13 16:17 UTC (permalink / raw)
  To: Linus Walleij; +Cc: Russell King, linux-mmc, linux-arm-kernel, Lee Jones

Linus Walleij wrote:
> 
> I applied all 14 patches on top of current v3.2-rc4
> and regression-tested the whole shebang on RealView PB1176
> and U300.
> 
> - Mounted /dev/mmcblk0p1
> - Copy a kernel-sized uImage file from the card to ramdisk
> - Copy the file back to the card from ramdisk
> - Unmount
> - Mount
> - Compare the two files with diff (identical)
> - Delete copy
> - Unmount
> - dd if=/dev/mmcblk0 of=/dev/null
> 
> (If any of you wants any more specific tests, tell me.)
> 
> Both platforms still work like a charm with these patches.
> PB1176 is as slow on MMC as it always has been but dd:ed
> the entire card without errors.
> 
> I have the branch here:
> git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-stericsson.git mmci
> 
> Ulf you can add my Tested-by: Linus Walleij <linus.walleij@linaro.org>
> for these patches.
> 
> Russell if you want a pull request for a subset of the patches I can
> arrange that, just tell me.
> 
> Yours,
> Linus Walleij
> 

I just uploaded these patches into Russell's patchtracker tool. All 
patches has now been rebased for Linux 3.2-rc4, which you were testing 
with Linus. Moreover, I added "Tested-by: Linus Walleij 
<linus.walleij@linaro.org>".

Br
Ulf Hansson



^ permalink raw reply	[flat|nested] 52+ messages in thread

* [PATCH 00/14] mmc: mmci: Improved PM support, cleanup and bugfixes
@ 2011-12-13 16:17     ` Ulf Hansson
  0 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2011-12-13 16:17 UTC (permalink / raw)
  To: linux-arm-kernel

Linus Walleij wrote:
> 
> I applied all 14 patches on top of current v3.2-rc4
> and regression-tested the whole shebang on RealView PB1176
> and U300.
> 
> - Mounted /dev/mmcblk0p1
> - Copy a kernel-sized uImage file from the card to ramdisk
> - Copy the file back to the card from ramdisk
> - Unmount
> - Mount
> - Compare the two files with diff (identical)
> - Delete copy
> - Unmount
> - dd if=/dev/mmcblk0 of=/dev/null
> 
> (If any of you wants any more specific tests, tell me.)
> 
> Both platforms still work like a charm with these patches.
> PB1176 is as slow on MMC as it always has been but dd:ed
> the entire card without errors.
> 
> I have the branch here:
> git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-stericsson.git mmci
> 
> Ulf you can add my Tested-by: Linus Walleij <linus.walleij@linaro.org>
> for these patches.
> 
> Russell if you want a pull request for a subset of the patches I can
> arrange that, just tell me.
> 
> Yours,
> Linus Walleij
> 

I just uploaded these patches into Russell's patchtracker tool. All 
patches has now been rebased for Linux 3.2-rc4, which you were testing 
with Linus. Moreover, I added "Tested-by: Linus Walleij 
<linus.walleij@linaro.org>".

Br
Ulf Hansson

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 02/14] mmc: mmci: Fixup handling of MCI_STARTBITERR
  2011-12-05 17:35   ` Ulf Hansson
@ 2011-12-18 23:15     ` Russell King - ARM Linux
  -1 siblings, 0 replies; 52+ messages in thread
From: Russell King - ARM Linux @ 2011-12-18 23:15 UTC (permalink / raw)
  To: Ulf Hansson; +Cc: linux-mmc, linux-arm-kernel, Lee Jones

On Mon, Dec 05, 2011 at 06:35:48PM +0100, Ulf Hansson wrote:
> The interrupt was previously enabled and then correctly cleared.
> Now we also handle it correctly.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>

I've queued this in my 'fixes' branch with a Cc to the stable team for
previous kernel versions.  I believe that to be appropriate.  Please
confirm.

^ permalink raw reply	[flat|nested] 52+ messages in thread

* [PATCH 02/14] mmc: mmci: Fixup handling of MCI_STARTBITERR
@ 2011-12-18 23:15     ` Russell King - ARM Linux
  0 siblings, 0 replies; 52+ messages in thread
From: Russell King - ARM Linux @ 2011-12-18 23:15 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Dec 05, 2011 at 06:35:48PM +0100, Ulf Hansson wrote:
> The interrupt was previously enabled and then correctly cleared.
> Now we also handle it correctly.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>

I've queued this in my 'fixes' branch with a Cc to the stable team for
previous kernel versions.  I believe that to be appropriate.  Please
confirm.

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 08/14] mmc: mmci: Fixup error handling for dma
  2011-12-05 17:35   ` Ulf Hansson
@ 2011-12-18 23:16     ` Russell King - ARM Linux
  -1 siblings, 0 replies; 52+ messages in thread
From: Russell King - ARM Linux @ 2011-12-18 23:16 UTC (permalink / raw)
  To: Ulf Hansson; +Cc: linux-mmc, linux-arm-kernel, Per Forlin, Lee Jones

On Mon, Dec 05, 2011 at 06:35:54PM +0100, Ulf Hansson wrote:
> When getting a cmd irq during an ongoing data transfer
> with dma, the dma job were never terminated. This is now
> corrected.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
> Signed-off-by: Per Forlin <per.forlin@stericsson.com>

I've queued this up in my 'fixes' branch with a Cc to the stable team
for previous kernels.  I believe this to be appropriate.  Please confirm.

^ permalink raw reply	[flat|nested] 52+ messages in thread

* [PATCH 08/14] mmc: mmci: Fixup error handling for dma
@ 2011-12-18 23:16     ` Russell King - ARM Linux
  0 siblings, 0 replies; 52+ messages in thread
From: Russell King - ARM Linux @ 2011-12-18 23:16 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Dec 05, 2011 at 06:35:54PM +0100, Ulf Hansson wrote:
> When getting a cmd irq during an ongoing data transfer
> with dma, the dma job were never terminated. This is now
> corrected.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
> Signed-off-by: Per Forlin <per.forlin@stericsson.com>

I've queued this up in my 'fixes' branch with a Cc to the stable team
for previous kernels.  I believe this to be appropriate.  Please confirm.

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 02/14] mmc: mmci: Fixup handling of MCI_STARTBITERR
  2011-12-18 23:15     ` Russell King - ARM Linux
@ 2011-12-19  8:59       ` Ulf Hansson
  -1 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2011-12-19  8:59 UTC (permalink / raw)
  To: Russell King - ARM Linux; +Cc: linux-mmc, linux-arm-kernel, Lee Jones

Russell King - ARM Linux wrote:
> On Mon, Dec 05, 2011 at 06:35:48PM +0100, Ulf Hansson wrote:
>> The interrupt was previously enabled and then correctly cleared.
>> Now we also handle it correctly.
>>
>> Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
>> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> 
> I've queued this in my 'fixes' branch with a Cc to the stable team for
> previous kernel versions.  I believe that to be appropriate.  Please
> confirm.
> 

Great!

BR
Ulf Hansson

^ permalink raw reply	[flat|nested] 52+ messages in thread

* [PATCH 02/14] mmc: mmci: Fixup handling of MCI_STARTBITERR
@ 2011-12-19  8:59       ` Ulf Hansson
  0 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2011-12-19  8:59 UTC (permalink / raw)
  To: linux-arm-kernel

Russell King - ARM Linux wrote:
> On Mon, Dec 05, 2011 at 06:35:48PM +0100, Ulf Hansson wrote:
>> The interrupt was previously enabled and then correctly cleared.
>> Now we also handle it correctly.
>>
>> Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
>> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> 
> I've queued this in my 'fixes' branch with a Cc to the stable team for
> previous kernel versions.  I believe that to be appropriate.  Please
> confirm.
> 

Great!

BR
Ulf Hansson

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 08/14] mmc: mmci: Fixup error handling for dma
  2011-12-18 23:16     ` Russell King - ARM Linux
@ 2011-12-19  8:59       ` Ulf Hansson
  -1 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2011-12-19  8:59 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: linux-mmc, linux-arm-kernel, Per FORLIN, Lee Jones

Russell King - ARM Linux wrote:
> On Mon, Dec 05, 2011 at 06:35:54PM +0100, Ulf Hansson wrote:
>> When getting a cmd irq during an ongoing data transfer
>> with dma, the dma job were never terminated. This is now
>> corrected.
>>
>> Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
>> Signed-off-by: Per Forlin <per.forlin@stericsson.com>
> 
> I've queued this up in my 'fixes' branch with a Cc to the stable team
> for previous kernels.  I believe this to be appropriate.  Please confirm.
> 

Great!

BR
Ulf Hansson


^ permalink raw reply	[flat|nested] 52+ messages in thread

* [PATCH 08/14] mmc: mmci: Fixup error handling for dma
@ 2011-12-19  8:59       ` Ulf Hansson
  0 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2011-12-19  8:59 UTC (permalink / raw)
  To: linux-arm-kernel

Russell King - ARM Linux wrote:
> On Mon, Dec 05, 2011 at 06:35:54PM +0100, Ulf Hansson wrote:
>> When getting a cmd irq during an ongoing data transfer
>> with dma, the dma job were never terminated. This is now
>> corrected.
>>
>> Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
>> Signed-off-by: Per Forlin <per.forlin@stericsson.com>
> 
> I've queued this up in my 'fixes' branch with a Cc to the stable team
> for previous kernels.  I believe this to be appropriate.  Please confirm.
> 

Great!

BR
Ulf Hansson

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 10/14] mmc: mmci: Cache MMCICLOCK and MMCIPOWER register
  2011-12-05 17:35   ` Ulf Hansson
@ 2012-01-08 10:25     ` Russell King - ARM Linux
  -1 siblings, 0 replies; 52+ messages in thread
From: Russell King - ARM Linux @ 2012-01-08 10:25 UTC (permalink / raw)
  To: Ulf Hansson; +Cc: linux-mmc, linux-arm-kernel, Lee Jones

On Mon, Dec 05, 2011 at 06:35:56PM +0100, Ulf Hansson wrote:
> Instead of reading a register value everytime we need to
> apply a new value for it, maintain a cached copy for it.
> This also means we are able to skip writes that are not
> needed.

I'm not sure this is a good idea.  The ARM Primecells require a certain
number of bus clocks and MCLK periods between writes to both these
registers, and reading them back helps to ensure that we conform to
that requirement.  Maintaining a cached copy of them allows faster
writes to these registers which could cause that requirement to be
violated.

What you could do is read the register, modify, and check whether the
modification has had any effect before writing it back.  That will
allow unnecessary writes to still be skipped.

^ permalink raw reply	[flat|nested] 52+ messages in thread

* [PATCH 10/14] mmc: mmci: Cache MMCICLOCK and MMCIPOWER register
@ 2012-01-08 10:25     ` Russell King - ARM Linux
  0 siblings, 0 replies; 52+ messages in thread
From: Russell King - ARM Linux @ 2012-01-08 10:25 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Dec 05, 2011 at 06:35:56PM +0100, Ulf Hansson wrote:
> Instead of reading a register value everytime we need to
> apply a new value for it, maintain a cached copy for it.
> This also means we are able to skip writes that are not
> needed.

I'm not sure this is a good idea.  The ARM Primecells require a certain
number of bus clocks and MCLK periods between writes to both these
registers, and reading them back helps to ensure that we conform to
that requirement.  Maintaining a cached copy of them allows faster
writes to these registers which could cause that requirement to be
violated.

What you could do is read the register, modify, and check whether the
modification has had any effect before writing it back.  That will
allow unnecessary writes to still be skipped.

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 12/14] mmc: mmci: Decrease current consumption in suspend
  2011-12-05 17:35   ` Ulf Hansson
@ 2012-01-08 10:38     ` Russell King - ARM Linux
  -1 siblings, 0 replies; 52+ messages in thread
From: Russell King - ARM Linux @ 2012-01-08 10:38 UTC (permalink / raw)
  To: Ulf Hansson; +Cc: linux-mmc, linux-arm-kernel, Lee Jones

On Mon, Dec 05, 2011 at 06:35:58PM +0100, Ulf Hansson wrote:
> To decrease current consumption in suspend state the
> VCORE regulator, the MCLK and PCLK for the ARM PL18x
> are now disabled.
> 
> When resuming the resourses are re-enabled and
> register values for MMCICLOCK, MMCIPOWER and MMCIMASK0
> are restored.

I still do not agree with this.  The MMC core driver should be shutting
down the interface (turning off the MMC clock, disabling the MMC power).
And indeed it does:

int mmc_suspend_host(struct mmc_host *host)
{
...
        if (!err && !mmc_card_keep_power(host))
                mmc_power_off(host);
}

void mmc_power_off(struct mmc_host *host)
{
...
        host->ios.power_mode = MMC_POWER_OFF;
        host->ios.bus_width = MMC_BUS_WIDTH_1;
        host->ios.timing = MMC_TIMING_LEGACY;
        mmc_set_ios(host);
...
}

On ARM primecells we can _not_ go from power off to power on without
first going through the power up state, so this simple save-and-restore
will not do - we have to go through the proper power up sequence.

Why do you think that the MMC core is not shutting down the power and
clock registers on suspend?

^ permalink raw reply	[flat|nested] 52+ messages in thread

* [PATCH 12/14] mmc: mmci: Decrease current consumption in suspend
@ 2012-01-08 10:38     ` Russell King - ARM Linux
  0 siblings, 0 replies; 52+ messages in thread
From: Russell King - ARM Linux @ 2012-01-08 10:38 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Dec 05, 2011 at 06:35:58PM +0100, Ulf Hansson wrote:
> To decrease current consumption in suspend state the
> VCORE regulator, the MCLK and PCLK for the ARM PL18x
> are now disabled.
> 
> When resuming the resourses are re-enabled and
> register values for MMCICLOCK, MMCIPOWER and MMCIMASK0
> are restored.

I still do not agree with this.  The MMC core driver should be shutting
down the interface (turning off the MMC clock, disabling the MMC power).
And indeed it does:

int mmc_suspend_host(struct mmc_host *host)
{
...
        if (!err && !mmc_card_keep_power(host))
                mmc_power_off(host);
}

void mmc_power_off(struct mmc_host *host)
{
...
        host->ios.power_mode = MMC_POWER_OFF;
        host->ios.bus_width = MMC_BUS_WIDTH_1;
        host->ios.timing = MMC_TIMING_LEGACY;
        mmc_set_ios(host);
...
}

On ARM primecells we can _not_ go from power off to power on without
first going through the power up state, so this simple save-and-restore
will not do - we have to go through the proper power up sequence.

Why do you think that the MMC core is not shutting down the power and
clock registers on suspend?

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 10/14] mmc: mmci: Cache MMCICLOCK and MMCIPOWER register
  2012-01-08 10:25     ` Russell King - ARM Linux
@ 2012-01-09 11:46       ` Ulf Hansson
  -1 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2012-01-09 11:46 UTC (permalink / raw)
  To: Russell King - ARM Linux; +Cc: linux-mmc, linux-arm-kernel, Lee Jones

Russell King - ARM Linux wrote:
> On Mon, Dec 05, 2011 at 06:35:56PM +0100, Ulf Hansson wrote:
>> Instead of reading a register value everytime we need to
>> apply a new value for it, maintain a cached copy for it.
>> This also means we are able to skip writes that are not
>> needed.
> 
> I'm not sure this is a good idea.  The ARM Primecells require a certain
> number of bus clocks and MCLK periods between writes to both these
> registers, and reading them back helps to ensure that we conform to
> that requirement.  Maintaining a cached copy of them allows faster
> writes to these registers which could cause that requirement to be
> violated.

You are definitely right. But how do we know that reading the register 
value is enough?

> 
> What you could do is read the register, modify, and check whether the
> modification has had any effect before writing it back.  That will
> allow unnecessary writes to still be skipped.
> 

That could work. Do you want me to fixup the patch to include this "read 
before write" mechanism then?

BR
Ulf Hansson

^ permalink raw reply	[flat|nested] 52+ messages in thread

* [PATCH 10/14] mmc: mmci: Cache MMCICLOCK and MMCIPOWER register
@ 2012-01-09 11:46       ` Ulf Hansson
  0 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2012-01-09 11:46 UTC (permalink / raw)
  To: linux-arm-kernel

Russell King - ARM Linux wrote:
> On Mon, Dec 05, 2011 at 06:35:56PM +0100, Ulf Hansson wrote:
>> Instead of reading a register value everytime we need to
>> apply a new value for it, maintain a cached copy for it.
>> This also means we are able to skip writes that are not
>> needed.
> 
> I'm not sure this is a good idea.  The ARM Primecells require a certain
> number of bus clocks and MCLK periods between writes to both these
> registers, and reading them back helps to ensure that we conform to
> that requirement.  Maintaining a cached copy of them allows faster
> writes to these registers which could cause that requirement to be
> violated.

You are definitely right. But how do we know that reading the register 
value is enough?

> 
> What you could do is read the register, modify, and check whether the
> modification has had any effect before writing it back.  That will
> allow unnecessary writes to still be skipped.
> 

That could work. Do you want me to fixup the patch to include this "read 
before write" mechanism then?

BR
Ulf Hansson

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 12/14] mmc: mmci: Decrease current consumption in suspend
  2012-01-08 10:38     ` Russell King - ARM Linux
@ 2012-01-09 14:12       ` Ulf Hansson
  -1 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2012-01-09 14:12 UTC (permalink / raw)
  To: Russell King - ARM Linux; +Cc: linux-mmc, linux-arm-kernel, Lee Jones

Russell King - ARM Linux wrote:
> On Mon, Dec 05, 2011 at 06:35:58PM +0100, Ulf Hansson wrote:
>> To decrease current consumption in suspend state the
>> VCORE regulator, the MCLK and PCLK for the ARM PL18x
>> are now disabled.
>>
>> When resuming the resourses are re-enabled and
>> register values for MMCICLOCK, MMCIPOWER and MMCIMASK0
>> are restored.
> 
> I still do not agree with this.  The MMC core driver should be shutting
> down the interface (turning off the MMC clock, disabling the MMC power).
> And indeed it does:
> 
> int mmc_suspend_host(struct mmc_host *host)
> {
> ...
>         if (!err && !mmc_card_keep_power(host))
>                 mmc_power_off(host);
> }
> 

If mmc_power_off gets called, this will mean cutting the power to the 
card. Either through the MMCIPOWER register(ARM rimecells) or by using 
the host->vcc regulator (ST-versions). Similar actions is done for 
MMCICLOCK register, which in the end means the clock to the card gets gated.

The MCLK (host->clk), the PCLK (amba_pclk) and the regulator 
(amba_vcore) is not disabled. This should only be handled during probe, 
suspend/resume and remove I believe.

> void mmc_power_off(struct mmc_host *host)
> {
> ...
>         host->ios.power_mode = MMC_POWER_OFF;
>         host->ios.bus_width = MMC_BUS_WIDTH_1;
>         host->ios.timing = MMC_TIMING_LEGACY;
>         mmc_set_ios(host);
> ...
> }
> 
> On ARM primecells we can _not_ go from power off to power on without
> first going through the power up state, so this simple save-and-restore
> will not do - we have to go through the proper power up sequence.

This will be handled when doing mmc_resume_host so there should be no 
problems for ARM Primecells. It might be the naming of these functions 
that kind of confuses you at this state. For runtime PM, these names 
(mmci_save and mmci_restore) make more sence. :-) Right now they are 
kind of only used for "enable"/"disable" clocks and regulators.

I can change the patch to only do the enable/disable part if you like 
that better at this stage. But since the earlier version of suspend 
function reset the MMCIMASK0 to zero, I thought reseting the other 
registers as well would be okay at this stage.

If you review the patch "mmci: Implement PM runtime callbacks to save 
power" together with this one, it should hopefully bring you more 
clarity how I have thought around this hole thing.

> 
> Why do you think that the MMC core is not shutting down the power and
> clock registers on suspend?
> 

Br
Ulf Hansson

^ permalink raw reply	[flat|nested] 52+ messages in thread

* [PATCH 12/14] mmc: mmci: Decrease current consumption in suspend
@ 2012-01-09 14:12       ` Ulf Hansson
  0 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2012-01-09 14:12 UTC (permalink / raw)
  To: linux-arm-kernel

Russell King - ARM Linux wrote:
> On Mon, Dec 05, 2011 at 06:35:58PM +0100, Ulf Hansson wrote:
>> To decrease current consumption in suspend state the
>> VCORE regulator, the MCLK and PCLK for the ARM PL18x
>> are now disabled.
>>
>> When resuming the resourses are re-enabled and
>> register values for MMCICLOCK, MMCIPOWER and MMCIMASK0
>> are restored.
> 
> I still do not agree with this.  The MMC core driver should be shutting
> down the interface (turning off the MMC clock, disabling the MMC power).
> And indeed it does:
> 
> int mmc_suspend_host(struct mmc_host *host)
> {
> ...
>         if (!err && !mmc_card_keep_power(host))
>                 mmc_power_off(host);
> }
> 

If mmc_power_off gets called, this will mean cutting the power to the 
card. Either through the MMCIPOWER register(ARM rimecells) or by using 
the host->vcc regulator (ST-versions). Similar actions is done for 
MMCICLOCK register, which in the end means the clock to the card gets gated.

The MCLK (host->clk), the PCLK (amba_pclk) and the regulator 
(amba_vcore) is not disabled. This should only be handled during probe, 
suspend/resume and remove I believe.

> void mmc_power_off(struct mmc_host *host)
> {
> ...
>         host->ios.power_mode = MMC_POWER_OFF;
>         host->ios.bus_width = MMC_BUS_WIDTH_1;
>         host->ios.timing = MMC_TIMING_LEGACY;
>         mmc_set_ios(host);
> ...
> }
> 
> On ARM primecells we can _not_ go from power off to power on without
> first going through the power up state, so this simple save-and-restore
> will not do - we have to go through the proper power up sequence.

This will be handled when doing mmc_resume_host so there should be no 
problems for ARM Primecells. It might be the naming of these functions 
that kind of confuses you at this state. For runtime PM, these names 
(mmci_save and mmci_restore) make more sence. :-) Right now they are 
kind of only used for "enable"/"disable" clocks and regulators.

I can change the patch to only do the enable/disable part if you like 
that better at this stage. But since the earlier version of suspend 
function reset the MMCIMASK0 to zero, I thought reseting the other 
registers as well would be okay at this stage.

If you review the patch "mmci: Implement PM runtime callbacks to save 
power" together with this one, it should hopefully bring you more 
clarity how I have thought around this hole thing.

> 
> Why do you think that the MMC core is not shutting down the power and
> clock registers on suspend?
> 

Br
Ulf Hansson

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 10/14] mmc: mmci: Cache MMCICLOCK and MMCIPOWER register
  2012-01-08 10:25     ` Russell King - ARM Linux
@ 2012-01-09 15:12       ` Ulf Hansson
  -1 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2012-01-09 15:12 UTC (permalink / raw)
  To: Russell King - ARM Linux; +Cc: linux-mmc, linux-arm-kernel, Lee Jones

Hi again,

Russell King - ARM Linux wrote:
> On Mon, Dec 05, 2011 at 06:35:56PM +0100, Ulf Hansson wrote:
>> Instead of reading a register value everytime we need to
>> apply a new value for it, maintain a cached copy for it.
>> This also means we are able to skip writes that are not
>> needed.
> 
> I'm not sure this is a good idea.  The ARM Primecells require a certain
> number of bus clocks and MCLK periods between writes to both these
> registers, and reading them back helps to ensure that we conform to
> that requirement.  Maintaining a cached copy of them allows faster
> writes to these registers which could cause that requirement to be
> violated.

I were just about to update my patch according to your proposal, when I 
realized the only place were the registers were previously "read before 
write", were at an SDIO corner case in pio_write.

Earlier the register values were always written, without considering the 
old value. Thus the impact with this patch is kind of only decreasing 
the number of writes and affects a corner case for SDIO.

Do you anyway prefer to add a register "read before write" even if it 
never has been needed before?

> 
> What you could do is read the register, modify, and check whether the
> modification has had any effect before writing it back.  That will
> allow unnecessary writes to still be skipped.
> 

Sorry for spamming you.

BR
Ulf Hansson


^ permalink raw reply	[flat|nested] 52+ messages in thread

* [PATCH 10/14] mmc: mmci: Cache MMCICLOCK and MMCIPOWER register
@ 2012-01-09 15:12       ` Ulf Hansson
  0 siblings, 0 replies; 52+ messages in thread
From: Ulf Hansson @ 2012-01-09 15:12 UTC (permalink / raw)
  To: linux-arm-kernel

Hi again,

Russell King - ARM Linux wrote:
> On Mon, Dec 05, 2011 at 06:35:56PM +0100, Ulf Hansson wrote:
>> Instead of reading a register value everytime we need to
>> apply a new value for it, maintain a cached copy for it.
>> This also means we are able to skip writes that are not
>> needed.
> 
> I'm not sure this is a good idea.  The ARM Primecells require a certain
> number of bus clocks and MCLK periods between writes to both these
> registers, and reading them back helps to ensure that we conform to
> that requirement.  Maintaining a cached copy of them allows faster
> writes to these registers which could cause that requirement to be
> violated.

I were just about to update my patch according to your proposal, when I 
realized the only place were the registers were previously "read before 
write", were at an SDIO corner case in pio_write.

Earlier the register values were always written, without considering the 
old value. Thus the impact with this patch is kind of only decreasing 
the number of writes and affects a corner case for SDIO.

Do you anyway prefer to add a register "read before write" even if it 
never has been needed before?

> 
> What you could do is read the register, modify, and check whether the
> modification has had any effect before writing it back.  That will
> allow unnecessary writes to still be skipped.
> 

Sorry for spamming you.

BR
Ulf Hansson

^ permalink raw reply	[flat|nested] 52+ messages in thread

end of thread, other threads:[~2012-01-09 15:13 UTC | newest]

Thread overview: 52+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-12-05 17:35 [PATCH 00/14] mmc: mmci: Improved PM support, cleanup and bugfixes Ulf Hansson
2011-12-05 17:35 ` Ulf Hansson
2011-12-05 17:35 ` [PATCH 01/14] mmc: mmci: Support MMC_PM_KEEP_POWER Ulf Hansson
2011-12-05 17:35   ` Ulf Hansson
2011-12-05 17:35 ` [PATCH 02/14] mmc: mmci: Fixup handling of MCI_STARTBITERR Ulf Hansson
2011-12-05 17:35   ` Ulf Hansson
2011-12-18 23:15   ` Russell King - ARM Linux
2011-12-18 23:15     ` Russell King - ARM Linux
2011-12-19  8:59     ` Ulf Hansson
2011-12-19  8:59       ` Ulf Hansson
2011-12-05 17:35 ` [PATCH 03/14] mmc: mmci: Increase max_segs from 16 to 128 Ulf Hansson
2011-12-05 17:35   ` Ulf Hansson
2011-12-05 17:35 ` [PATCH 04/14] mmc: mmci: Do not release spinlock in request_end Ulf Hansson
2011-12-05 17:35   ` Ulf Hansson
2011-12-05 17:35 ` [PATCH 05/14] mmc: mmci: Put power register deviations in variant data Ulf Hansson
2011-12-05 17:35   ` Ulf Hansson
2011-12-05 17:35 ` [PATCH 06/14] mmc: mmci: Provide option to configure bus signal direction Ulf Hansson
2011-12-05 17:35   ` Ulf Hansson
2011-12-05 17:35 ` [PATCH 07/14] mmc: mmci: Change vdd_handler to a generic ios_handler Ulf Hansson
2011-12-05 17:35   ` Ulf Hansson
2011-12-05 17:35 ` [PATCH 08/14] mmc: mmci: Fixup error handling for dma Ulf Hansson
2011-12-05 17:35   ` Ulf Hansson
2011-12-18 23:16   ` Russell King - ARM Linux
2011-12-18 23:16     ` Russell King - ARM Linux
2011-12-19  8:59     ` Ulf Hansson
2011-12-19  8:59       ` Ulf Hansson
2011-12-05 17:35 ` [PATCH 09/14] mmc: mmci: Change from using legacy suspend Ulf Hansson
2011-12-05 17:35   ` Ulf Hansson
2011-12-05 17:35 ` [PATCH 10/14] mmc: mmci: Cache MMCICLOCK and MMCIPOWER register Ulf Hansson
2011-12-05 17:35   ` Ulf Hansson
2012-01-08 10:25   ` Russell King - ARM Linux
2012-01-08 10:25     ` Russell King - ARM Linux
2012-01-09 11:46     ` Ulf Hansson
2012-01-09 11:46       ` Ulf Hansson
2012-01-09 15:12     ` Ulf Hansson
2012-01-09 15:12       ` Ulf Hansson
2011-12-05 17:35 ` [PATCH 11/14] mmc: mmci: Fixup use of runtime PM and use autosuspend Ulf Hansson
2011-12-05 17:35   ` Ulf Hansson
2011-12-05 17:35 ` [PATCH 12/14] mmc: mmci: Decrease current consumption in suspend Ulf Hansson
2011-12-05 17:35   ` Ulf Hansson
2012-01-08 10:38   ` Russell King - ARM Linux
2012-01-08 10:38     ` Russell King - ARM Linux
2012-01-09 14:12     ` Ulf Hansson
2012-01-09 14:12       ` Ulf Hansson
2011-12-05 17:35 ` [PATCH 13/14] mmc: mmci: Implement PM runtime callbacks to save power Ulf Hansson
2011-12-05 17:35   ` Ulf Hansson
2011-12-05 17:36 ` [PATCH 14/14] mmc: mmci: Use ios_handler " Ulf Hansson
2011-12-05 17:36   ` Ulf Hansson
2011-12-07 12:06 ` [PATCH 00/14] mmc: mmci: Improved PM support, cleanup and bugfixes Linus Walleij
2011-12-07 12:06   ` Linus Walleij
2011-12-13 16:17   ` Ulf Hansson
2011-12-13 16:17     ` Ulf Hansson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.