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 <[email protected]> > 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 <[email protected]>, Nitish Ambastha > <[email protected]> > Signed-off-by: Kyungmin Park <[email protected]> > --- > 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 <[email protected]> <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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
