* [PATCH FINAL] sdhci-clk-gating-support
@ 2010-07-09 9:29 MADHAV SINGHCHAUHAN
2010-07-13 8:46 ` Madhav Chauhan
2010-08-27 20:26 ` Chris Ball
0 siblings, 2 replies; 3+ messages in thread
From: MADHAV SINGHCHAUHAN @ 2010-07-09 9:29 UTC (permalink / raw)
To: Chris Ball; +Cc: linux-mmc@vger.kernel.org, Kyungmin Park
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@samsung.com>
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@samsung.com>, Nitish Ambastha <nitish.a@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
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
> + host->user = 0; /*For Clock gating*/
> + host->clk_restore = 0;
Also, it's not obvious to me why two variables are necessary. What's
wrong with just setting host->clk_restore when disabling clock, and
testing whether it's non-zero instead of testing host->user?
(host->user would need a more descriptive name, too.)
Thanks,
- Chris.
--
Chris Ball <cjb@laptop.org>
One Laptop Per Child
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH FINAL] sdhci-clk-gating-support
2010-07-09 9:29 [PATCH FINAL] sdhci-clk-gating-support MADHAV SINGHCHAUHAN
@ 2010-07-13 8:46 ` Madhav Chauhan
2010-08-27 20:26 ` Chris Ball
1 sibling, 0 replies; 3+ messages in thread
From: Madhav Chauhan @ 2010-07-13 8:46 UTC (permalink / raw)
To: singh.madhav; +Cc: Chris Ball, linux-mmc@vger.kernel.org, Kyungmin Park
Hi,
Can any one comment on this patch??
Regards,
Madhav
> 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@samsung.com>
> 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@samsung.com>, Nitish Ambastha <nitish.a@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
> 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
>
>
>
>
>> + host->user = 0; /*For Clock gating*/
>> + host->clk_restore = 0;
>
> Also, it's not obvious to me why two variables are necessary. What's
> wrong with just setting host->clk_restore when disabling clock, and
> testing whether it's non-zero instead of testing host->user?
> (host->user would need a more descriptive name, too.)
>
> Thanks,
>
> - Chris.
> --
> Chris Ball <cjb@laptop.org>
> One Laptop Per Child
>
>
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH FINAL] sdhci-clk-gating-support
2010-07-09 9:29 [PATCH FINAL] sdhci-clk-gating-support MADHAV SINGHCHAUHAN
2010-07-13 8:46 ` Madhav Chauhan
@ 2010-08-27 20:26 ` Chris Ball
1 sibling, 0 replies; 3+ messages in thread
From: Chris Ball @ 2010-08-27 20:26 UTC (permalink / raw)
To: MADHAV SINGHCHAUHAN; +Cc: linux-mmc@vger.kernel.org, Kyungmin Park
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@samsung.com>
> 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@samsung.com>, Nitish Ambastha <nitish.a@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
> 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@laptop.org> <http://printf.net/>
One Laptop Per Child
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-08-27 20:26 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-07-09 9:29 [PATCH FINAL] sdhci-clk-gating-support MADHAV SINGHCHAUHAN
2010-07-13 8:46 ` Madhav Chauhan
2010-08-27 20:26 ` Chris Ball
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).