Hi,
I reviewed this patch, but still feel uneasy about merging it -- it's
hard to know if any controllers will handle this badly, or suffer
performance problems if their clock takes a long time to stabilize.
Is pushing to -mm a reasonable thing to do if we want more testing, or
should we hold off? I could test on some of the hardware collection at
OLPC report any performance/power changes I see, if that would help.
Does anyone else have feedback on the general approach?
- Chris.
On Fri, Jul 09, 2010 at 09:29:23AM +0000, MADHAV SINGHCHAUHAN wrote:
> Hi Chris ,
> I have taken up you suggestion and coming with new patch.Also we have done perfromance analysis
> using fileop/iozone and performance is not effected by this patch,regarding the power consumption,
> after applying patch it saves 15 mA.
>
> From 3663ee85ce718c9607bc0968a5143e01d86919ed Mon Sep 17 00:00:00 2001
> From: Madhav Chauhan <singh.madhav@xxxxxxxxxxx>
> Date: Fri, 9 Jul 2010 20:02:06 +0530
> Subject: [PATCH] sdhci-clk-gating-support
>
> This patch implements clock gating support in sdhci layer. It will
> enable the clock when host controller start sending request
> to attached device and will disable it once it finish the command.
> Now this patch provides option so that some host controllers
> can be avoided using this functionality using mmc_host caps.
>
> Signed-off-by: Madhav Chauhan <singh.madhav@xxxxxxxxxxx>, Nitish Ambastha <nitish.a@xxxxxxxxxxx>
> Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
> ---
> drivers/mmc/host/sdhci.c | 35 +++++++++++++++++++++++++++++++++++
> drivers/mmc/host/sdhci.h | 6 ++++++
> include/linux/mmc/host.h | 1 +
> 3 files changed, 42 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index c6d1bd8..5ed7c50 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1103,6 +1103,9 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
>
> spin_lock_irqsave(&host->lock, flags);
>
> + if (host->mmc->caps & MMC_CAP_CLOCK_GATING)
> + sdhci_clk_enable(host); /*Enable clock as transfer starts now*/
> +
> WARN_ON(host->mrq != NULL);
>
> #ifndef SDHCI_USE_LEDS_CLASS
> @@ -1310,6 +1313,10 @@ static void sdhci_tasklet_finish(unsigned long param)
> sdhci_reset(host, SDHCI_RESET_DATA);
> }
>
> + if (host->mmc->caps & MMC_CAP_CLOCK_GATING)
> + /*Disable clock as command has been processed*/
> + sdhci_clk_disable(host);
> +
> host->mrq = NULL;
> host->cmd = NULL;
> host->data = NULL;
> @@ -1597,6 +1604,9 @@ int sdhci_suspend_host(struct sdhci_host *host, pm_message_t state)
>
> sdhci_disable_card_detection(host);
>
> + if (host->mmc->caps & MMC_CAP_CLOCK_GATING)
> + sdhci_clk_enable(host); /* Enable clock */
> +
> ret = mmc_suspend_host(host->mmc);
> if (ret)
> return ret;
> @@ -1626,6 +1636,11 @@ int sdhci_resume_host(struct sdhci_host *host)
> mmiowb();
>
> ret = mmc_resume_host(host->mmc);
> +
> + if (host->mmc->caps & MMC_CAP_CLOCK_GATING)
> + /*Now device has wake up disable it*/
> + sdhci_clk_disable(host);
> +
> sdhci_enable_card_detection(host);
>
> return ret;
> @@ -1654,6 +1669,9 @@ struct sdhci_host *sdhci_alloc_host(struct device *dev,
> return ERR_PTR(-ENOMEM);
>
> host = mmc_priv(mmc);
> +
> + host->clk_restore = 0; /* For clock gating */
> +
> host->mmc = mmc;
>
> return host;
> @@ -1984,6 +2002,23 @@ void sdhci_free_host(struct sdhci_host *host)
>
> EXPORT_SYMBOL_GPL(sdhci_free_host);
>
> +void sdhci_clk_enable(struct sdhci_host *host)
> +{
> + if (host->clk_restore && host->clock == 0)
> + sdhci_set_clock(host, host->clk_restore);
> +}
> +EXPORT_SYMBOL_GPL(sdhci_clk_enable);
> +
> +void sdhci_clk_disable(struct sdhci_host *host)
> +{
> + if (host->clock != 0) {
> + host->clk_restore = host->clock;
> + sdhci_set_clock(host, 0);
> + }
> +}
> +EXPORT_SYMBOL_GPL(sdhci_clk_disable);
> +
> +
> /*****************************************************************************\
> * *
> * Driver init/exit *
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index c846813..5031d33 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -292,6 +292,8 @@ struct sdhci_host {
>
> struct timer_list timer; /* Timer for timeouts */
>
> + unsigned int clk_restore; /* For Clock Gating */
> +
> unsigned long private[0] ____cacheline_aligned;
> };
>
> @@ -410,6 +412,10 @@ static inline void *sdhci_priv(struct sdhci_host *host)
> extern int sdhci_add_host(struct sdhci_host *host);
> extern void sdhci_remove_host(struct sdhci_host *host, int dead);
>
> +/*For Clock Gating*/
> +extern void sdhci_clk_enable(struct sdhci_host *host);
> +extern void sdhci_clk_disable(struct sdhci_host *host);
> +
> #ifdef CONFIG_PM
> extern int sdhci_suspend_host(struct sdhci_host *host, pm_message_t state);
> extern int sdhci_resume_host(struct sdhci_host *host);
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index f65913c..b5c3563 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -155,6 +155,7 @@ struct mmc_host {
> #define MMC_CAP_DISABLE (1 << 7) /* Can the host be disabled */
> #define MMC_CAP_NONREMOVABLE (1 << 8) /* Nonremovable e.g. eMMC */
> #define MMC_CAP_WAIT_WHILE_BUSY (1 << 9) /* Waits while card is busy */
> +#define MMC_CAP_CLOCK_GATING (1 << 10) /* Clock Gating feature */
>
> mmc_pm_flag_t pm_caps; /* supported pm features */
>
> --
> 1.7.0.4
--
Chris Ball <cjb@xxxxxxxxxx> <http://printf.net/>
One Laptop Per Child
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
