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

[Madhusudhan]

> -----Original Message-----
> From: linux-arm-kernel-bounces@xxxxxxxxxxxxxxxxxxxxxx [mailto:linux-arm-
> kernel-bounces@xxxxxxxxxxxxxxxxxxxxxx] On Behalf Of Linus Walleij
> Sent: Monday, July 20, 2009 5:27 PM
> To: Andrew Morton; linux-kernel@xxxxxxxxxxxxxxx
> Cc: Pierre Ossman; linux-arm-kernel@xxxxxxxxxxxxxxxxxxxxxx; 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@xxxxxxxxxxxxxx>
> ---
>  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


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/