[PATCH 2/2] Modify MMCI/PL180 to handle agressive clocking v7

[PATCH 2/2] Modify MMCI/PL180 to handle agressive clocking v7

[Linus Walleij]

This removes the default clocking for the MMCI controller so that
the external MCI card clock does not activate until the first
.set_ios() call is issued. It will further handle the transitions
from a clock != 0 to 0 and vice versa by gating/ungating the
clock with clk_disable()/clk_enable().

This assures that the MCI clock will not be active unless there
is a card in the MMC slot.

By default the MMC core will not gate off the clock to a card
once it's enabled, but with the separate patch for aggressive
clocking this can optionally be enabled for the system.

Signed-off-by: Linus Walleij <linus.walleij@stericsson.com>
---
This complements the agressive clocking patch. The patches are
actually independent and this patch is good on its own since it
removes the unnecessary clocking of the MCI clock to slots with
no cards in them, but it also has the side effect of enabling
aggressive clocking introduced with a separate patch.
---
 drivers/mmc/host/mmci.c |   35 +++++++++++++++++++++++------------
 drivers/mmc/host/mmci.h |    1 +
 2 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 743d497..3fa4c4f 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -924,7 +924,23 @@ 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);
+	/*
+	 * Turn on clock whenever ios->clock transitions
+	 * from 0 to !=0 and gate it off whenever ios->clock
+	 * transitions from !=0 to 0.
+	 */
+	if (host->iosclock == 0 && ios->clock != 0) {
+		dev_dbg(mmc_dev(mmc), "enable clock f=%d\n", ios->clock);
+		clk_enable(host->clk);
+		mmci_set_clkreg(host, ios->clock);
+	} else if (host->iosclock != 0 && ios->clock == 0) {
+		dev_dbg(mmc_dev(mmc), "disable clock\n");
+		clk_disable(host->clk);
+	} else if (ios->clock != 0) {
+		mmci_set_clkreg(host, ios->clock);
+		dev_dbg(mmc_dev(mmc), "set clock f=%d\n", ios->clock);
+	}
+	host->iosclock = ios->clock;
 
 	if (host->pwr != pwr) {
 		host->pwr = pwr;
@@ -1009,6 +1025,8 @@ static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 
 	host = mmc_priv(mmc);
 	host->mmc = mmc;
+	host->plat = plat;
+	host->variant = variant;
 
 	host->gpio_wp = -ENOSYS;
 	host->gpio_cd = -ENOSYS;
@@ -1019,19 +1037,14 @@ static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 	dev_dbg(mmc_dev(mmc), "designer ID = 0x%02x\n", host->hw_designer);
 	dev_dbg(mmc_dev(mmc), "revision = 0x%01x\n", host->hw_revision);
 
+	/* This clock will be enabled/disabled by set_ios() calls later */
 	host->clk = clk_get(&dev->dev, NULL);
 	if (IS_ERR(host->clk)) {
 		ret = PTR_ERR(host->clk);
 		host->clk = NULL;
 		goto host_free;
 	}
-
-	ret = clk_enable(host->clk);
-	if (ret)
-		goto clk_free;
-
-	host->plat = plat;
-	host->variant = variant;
+	host->iosclock = 0;
 	host->mclk = clk_get_rate(host->clk);
 	/*
 	 * According to the spec, mclk is max 100 MHz,
@@ -1041,7 +1054,7 @@ static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 	if (host->mclk > 100000000) {
 		ret = clk_set_rate(host->clk, 100000000);
 		if (ret < 0)
-			goto clk_disable;
+			goto clk_free;
 		host->mclk = clk_get_rate(host->clk);
 		dev_dbg(mmc_dev(mmc), "eventual mclk rate: %u Hz\n",
 			host->mclk);
@@ -1050,7 +1063,7 @@ static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 	host->base = ioremap(dev->res.start, resource_size(&dev->res));
 	if (!host->base) {
 		ret = -ENOMEM;
-		goto clk_disable;
+		goto clk_free;
 	}
 
 	mmc->ops = &mmci_ops;
@@ -1203,8 +1216,6 @@ static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 		gpio_free(host->gpio_cd);
  err_gpio_cd:
 	iounmap(host->base);
- clk_disable:
-	clk_disable(host->clk);
  clk_free:
 	clk_put(host->clk);
  host_free:
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index bc3b71e..5701d33 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -170,6 +170,7 @@ struct mmci_host {
 
 	unsigned int		mclk;
 	unsigned int		cclk;
+	unsigned int		iosclock;
 	u32			pwr;
 	struct mmci_platform_data *plat;
 	struct variant_data	*variant;
-- 
1.7.2.3

RE: [PATCH 2/2] Modify MMCI/PL180 to handle agressive clocking v7

[Madhusudhan]

> -----Original Message-----
> From: linux-arm-kernel-bounces@lists.arm.linux.org.uk [mailto:linux-arm-
> kernel-bounces@lists.arm.linux.org.uk] On Behalf Of Linus Walleij
> Sent: Monday, July 20, 2009 5:27 PM
> To: Andrew Morton; linux-kernel@vger.kernel.org
> Cc: Pierre Ossman; linux-arm-kernel@lists.arm.linux.org.uk; Linus Walleij
> Subject: [PATCH 2/2] Modify MMCI/PL180 to handle agressive clocking v7
> 
> This adds a few clk_enable()/clk_disable() calls to the MMCI
> driver so as to handle an set_ios() request with clock
> frequency set to 0 as an instruction to gate the block clock.
> This also breaks out the clock setting code into its own
> function to simplify the set_ios() function a bit.

Is it possible to share the WRITE/READ throughput numbers with and without
clock gating enabled? It could be good info.

Regards,
Madhu
> 
> Signed-off-by: Linus Walleij <linus.walleij@stericsson.com>
> ---
>  drivers/mmc/host/mmci.c |   86 +++++++++++++++++++++++++++++++++++-------
> ----
>  drivers/mmc/host/mmci.h |    3 +-
>  2 files changed, 67 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index e1aa847..db09b69 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -37,8 +37,34 @@
> 
>  static unsigned int fmax = 515633;
> 
> +/*
> + * This must be called with host->lock held
> + */
> +static void mmci_set_clkreg(struct mmci_host *host, unsigned int desired)
> +{
> +	u32 clk = 0;
> +
> +	if (desired) {
> +		if (desired >= host->mclk) {
> +			clk = MCI_CLK_BYPASS;
> +			host->cclk = host->mclk;
> +			DBG(host, "MMCI: bypass clock divider, cclk = %u
Hz\n",
> host->cclk);
> +		} else {
> +			clk = host->mclk / (2 * desired) - 1;
> +			if (clk >= 256)
> +				clk = 255;
> +			host->cclk = host->mclk / (2 * (clk + 1));
> +			DBG(host, "MMCI: divide cclk to %u Hz (divide %u by
> %u)\n", host->cclk, host->mclk, clk);
> +		}
> +		if (host->hw_designer == 0x80)
> +			clk |= MCI_FCEN; /* Bug fix in ST IP block */
> +		clk |= MCI_CLK_ENABLE;
> +	}
> +	writel(clk, host->base + MMCICLOCK);
> +}
> +
>  static void
> -mmci_request_end(struct mmci_host *host, struct mmc_request *mrq)
> + mmci_request_end(struct mmci_host *host, struct mmc_request *mrq)
>  {
>  	writel(0, host->base + MMCICOMMAND);
> 
> @@ -418,22 +444,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);
> -	u32 clk = 0, pwr = 0;
> -
> -	if (ios->clock) {
> -		if (ios->clock >= host->mclk) {
> -			clk = MCI_CLK_BYPASS;
> -			host->cclk = host->mclk;
> -		} else {
> -			clk = host->mclk / (2 * ios->clock) - 1;
> -			if (clk >= 256)
> -				clk = 255;
> -			host->cclk = host->mclk / (2 * (clk + 1));
> -		}
> -		if (host->hw_designer == 0x80)
> -			clk |= MCI_FCEN; /* Bug fix in ST IP block */
> -		clk |= MCI_CLK_ENABLE;
> -	}
> +	u32 pwr = 0;
> 
>  	if (host->plat->translate_vdd)
>  		pwr |= host->plat->translate_vdd(mmc_dev(mmc), ios->vdd);
> @@ -464,12 +475,30 @@ static void mmci_set_ios(struct mmc_host *mmc,
> struct mmc_ios *ios)
>  		}
>  	}
> 
> -	writel(clk, host->base + MMCICLOCK);
> +	spin_lock(&host->lock);
> +
> +	/*
> +	 * Turn on clock whenever ios->clock transitions
> +	 * from 0 to !=0 and gate it off whenever ios->clock
> +	 * transitions from !=0 to 0.
> +	 */
> +	if (host->iosclock == 0 && ios->clock != 0) {
> +		clk_enable(host->clk);
> +		mmci_set_clkreg(host, ios->clock);
> +	} else if (host->iosclock != 0 && ios->clock == 0) {
> +		clk_disable(host->clk);
> +	}
> +	host->iosclock = ios->clock;
> 
>  	if (host->pwr != pwr) {
>  		host->pwr = pwr;
> +		/* Clock may be gated (ios->clock still 0) */
> +		clk_enable(host->clk);
>  		writel(pwr, host->base + MMCIPOWER);
> +		clk_disable(host->clk);
>  	}
> +
> +	spin_unlock(&host->lock);
>  }
> 
>  static const struct mmc_host_ops mmci_ops = {
> @@ -546,6 +575,7 @@ static int __devinit mmci_probe(struct amba_device
> *dev, struct amba_id *id)
>  		host->mclk = clk_get_rate(host->clk);
>  		DBG(host, "eventual mclk rate: %u Hz\n", host->mclk);
>  	}
> +	host->iosclock = 0;
>  	host->base = ioremap(dev->res.start, resource_size(&dev->res));
>  	if (!host->base) {
>  		ret = -ENOMEM;
> @@ -615,6 +645,8 @@ static int __devinit mmci_probe(struct amba_device
> *dev, struct amba_id *id)
>  	host->timer.expires = jiffies + HZ;
>  	add_timer(&host->timer);
> 
> +	/* The first IOS will turn the clock on again */
> +	clk_disable(host->clk);
>  	return 0;
> 
>   irq0_free:
> @@ -646,17 +678,22 @@ static int __devexit mmci_remove(struct amba_device
> *dev)
> 
>  		mmc_remove_host(mmc);
> 
> +		/* framework may have gated the block clock */
> +		clk_enable(host->clk);
> +
>  		writel(0, host->base + MMCIMASK0);
>  		writel(0, host->base + MMCIMASK1);
> 
>  		writel(0, host->base + MMCICOMMAND);
>  		writel(0, host->base + MMCIDATACTRL);
> 
> +		clk_disable(host->clk);
> +
>  		free_irq(dev->irq[0], host);
>  		free_irq(dev->irq[1], host);
> 
>  		iounmap(host->base);
> -		clk_disable(host->clk);
> +
>  		clk_put(host->clk);
> 
>  		mmc_free_host(mmc);
> @@ -677,8 +714,11 @@ static int mmci_suspend(struct amba_device *dev,
> pm_message_t state)
>  		struct mmci_host *host = mmc_priv(mmc);
> 
>  		ret = mmc_suspend_host(mmc, state);
> -		if (ret == 0)
> +		if (ret == 0) {
> +			clk_enable(host->clk);
>  			writel(0, host->base + MMCIMASK0);
> +			clk_disable(host->clk);
> +		}
>  	}
> 
>  	return ret;
> @@ -692,7 +732,11 @@ static int mmci_resume(struct amba_device *dev)
>  	if (mmc) {
>  		struct mmci_host *host = mmc_priv(mmc);
> 
> -		writel(MCI_IRQENABLE, host->base + MMCIMASK0);
> +		if (ret == 0) {
> +			clk_enable(host->clk);
> +			writel(MCI_IRQENABLE, host->base + MMCIMASK0);
> +			clk_disable(host->clk);
> +		}
> 
>  		ret = mmc_resume_host(mmc);
>  	}
> diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
> index 0441bac..796db94 100644
> --- a/drivers/mmc/host/mmci.h
> +++ b/drivers/mmc/host/mmci.h
> @@ -137,7 +137,7 @@
>   * The size of the FIFO in bytes.
>   */
>  #define MCI_FIFOSIZE	(16*4)
> -
> +
>  #define MCI_FIFOHALFSIZE (MCI_FIFOSIZE / 2)
> 
>  #define NR_SG		16
> @@ -158,6 +158,7 @@ struct mmci_host {
> 
>  	unsigned int		mclk;
>  	unsigned int		cclk;
> +	unsigned int		iosclock;
>  	u32			pwr;
>  	struct mmc_platform_data *plat;
> 
> --
> 1.6.2.5
> 
> 
> -------------------------------------------------------------------
> List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-
> kernel
> FAQ:        http://www.arm.linux.org.uk/mailinglists/faq.php
> Etiquette:  http://www.arm.linux.org.uk/mailinglists/etiquette.php

[PATCH 2/2] Modify MMCI/PL180 to handle agressive clocking v7

[Linus Walleij]

This adds a few clk_enable()/clk_disable() calls to the MMCI
driver so as to handle an set_ios() request with clock
frequency set to 0 as an instruction to gate the block clock.
This also breaks out the clock setting code into its own
function to simplify the set_ios() function a bit.

Signed-off-by: Linus Walleij <linus.walleij@stericsson.com>
---
 drivers/mmc/host/mmci.c |   86 +++++++++++++++++++++++++++++++++++-----------
 drivers/mmc/host/mmci.h |    3 +-
 2 files changed, 67 insertions(+), 22 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index e1aa847..db09b69 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -37,8 +37,34 @@
 
 static unsigned int fmax = 515633;
 
+/*
+ * This must be called with host->lock held
+ */
+static void mmci_set_clkreg(struct mmci_host *host, unsigned int desired)
+{
+	u32 clk = 0;
+
+	if (desired) {
+		if (desired >= host->mclk) {
+			clk = MCI_CLK_BYPASS;
+			host->cclk = host->mclk;
+			DBG(host, "MMCI: bypass clock divider, cclk = %u Hz\n", host->cclk);
+		} else {
+			clk = host->mclk / (2 * desired) - 1;
+			if (clk >= 256)
+				clk = 255;
+			host->cclk = host->mclk / (2 * (clk + 1));
+			DBG(host, "MMCI: divide cclk to %u Hz (divide %u by %u)\n", host->cclk, host->mclk, clk);
+		}
+		if (host->hw_designer == 0x80)
+			clk |= MCI_FCEN; /* Bug fix in ST IP block */
+		clk |= MCI_CLK_ENABLE;
+	}
+	writel(clk, host->base + MMCICLOCK);
+}
+
 static void
-mmci_request_end(struct mmci_host *host, struct mmc_request *mrq)
+ mmci_request_end(struct mmci_host *host, struct mmc_request *mrq)
 {
 	writel(0, host->base + MMCICOMMAND);
 
@@ -418,22 +444,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);
-	u32 clk = 0, pwr = 0;
-
-	if (ios->clock) {
-		if (ios->clock >= host->mclk) {
-			clk = MCI_CLK_BYPASS;
-			host->cclk = host->mclk;
-		} else {
-			clk = host->mclk / (2 * ios->clock) - 1;
-			if (clk >= 256)
-				clk = 255;
-			host->cclk = host->mclk / (2 * (clk + 1));
-		}
-		if (host->hw_designer == 0x80)
-			clk |= MCI_FCEN; /* Bug fix in ST IP block */
-		clk |= MCI_CLK_ENABLE;
-	}
+	u32 pwr = 0;
 
 	if (host->plat->translate_vdd)
 		pwr |= host->plat->translate_vdd(mmc_dev(mmc), ios->vdd);
@@ -464,12 +475,30 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 		}
 	}
 
-	writel(clk, host->base + MMCICLOCK);
+	spin_lock(&host->lock);
+
+	/*
+	 * Turn on clock whenever ios->clock transitions
+	 * from 0 to !=0 and gate it off whenever ios->clock
+	 * transitions from !=0 to 0.
+	 */
+	if (host->iosclock == 0 && ios->clock != 0) {
+		clk_enable(host->clk);
+		mmci_set_clkreg(host, ios->clock);
+	} else if (host->iosclock != 0 && ios->clock == 0) {
+		clk_disable(host->clk);
+	}
+	host->iosclock = ios->clock;
 
 	if (host->pwr != pwr) {
 		host->pwr = pwr;
+		/* Clock may be gated (ios->clock still 0) */
+		clk_enable(host->clk);
 		writel(pwr, host->base + MMCIPOWER);
+		clk_disable(host->clk);
 	}
+
+	spin_unlock(&host->lock);
 }
 
 static const struct mmc_host_ops mmci_ops = {
@@ -546,6 +575,7 @@ static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 		host->mclk = clk_get_rate(host->clk);
 		DBG(host, "eventual mclk rate: %u Hz\n", host->mclk);
 	}
+	host->iosclock = 0;
 	host->base = ioremap(dev->res.start, resource_size(&dev->res));
 	if (!host->base) {
 		ret = -ENOMEM;
@@ -615,6 +645,8 @@ static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 	host->timer.expires = jiffies + HZ;
 	add_timer(&host->timer);
 
+	/* The first IOS will turn the clock on again */
+	clk_disable(host->clk);
 	return 0;
 
  irq0_free:
@@ -646,17 +678,22 @@ static int __devexit mmci_remove(struct amba_device *dev)
 
 		mmc_remove_host(mmc);
 
+		/* framework may have gated the block clock */
+		clk_enable(host->clk);
+
 		writel(0, host->base + MMCIMASK0);
 		writel(0, host->base + MMCIMASK1);
 
 		writel(0, host->base + MMCICOMMAND);
 		writel(0, host->base + MMCIDATACTRL);
 
+		clk_disable(host->clk);
+
 		free_irq(dev->irq[0], host);
 		free_irq(dev->irq[1], host);
 
 		iounmap(host->base);
-		clk_disable(host->clk);
+
 		clk_put(host->clk);
 
 		mmc_free_host(mmc);
@@ -677,8 +714,11 @@ static int mmci_suspend(struct amba_device *dev, pm_message_t state)
 		struct mmci_host *host = mmc_priv(mmc);
 
 		ret = mmc_suspend_host(mmc, state);
-		if (ret == 0)
+		if (ret == 0) {
+			clk_enable(host->clk);
 			writel(0, host->base + MMCIMASK0);
+			clk_disable(host->clk);
+		}
 	}
 
 	return ret;
@@ -692,7 +732,11 @@ static int mmci_resume(struct amba_device *dev)
 	if (mmc) {
 		struct mmci_host *host = mmc_priv(mmc);
 
-		writel(MCI_IRQENABLE, host->base + MMCIMASK0);
+		if (ret == 0) {
+			clk_enable(host->clk);
+			writel(MCI_IRQENABLE, host->base + MMCIMASK0);
+			clk_disable(host->clk);
+		}
 
 		ret = mmc_resume_host(mmc);
 	}
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 0441bac..796db94 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -137,7 +137,7 @@
  * The size of the FIFO in bytes.
  */
 #define MCI_FIFOSIZE	(16*4)
-	
+
 #define MCI_FIFOHALFSIZE (MCI_FIFOSIZE / 2)
 
 #define NR_SG		16
@@ -158,6 +158,7 @@ struct mmci_host {
 
 	unsigned int		mclk;
 	unsigned int		cclk;
+	unsigned int		iosclock;
 	u32			pwr;
 	struct mmc_platform_data *plat;
 
-- 
1.6.2.5