Skip to contentSkip to conversation

sdhci-clk-gating-support

PATCH In review MADHAV SINGHCHAUHAN 3 participants 3 messages Jul 9, 2010 – Aug 27, 2010

Subsystems (2)
MMC, SD and SDIO Secure digital host controller interface (SDHCI) driver
lore
Share & tools
lore

MADHAV SINGHCHAUHAN

author
#0 lore raw

PATCH 1/1 sdhci-clk-gating-support

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(-)
drivers/mmc/host/sdhci.c +35 -0
index c6d1bd8..5ed7c50 100644
@@ -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                                                          *
drivers/mmc/host/sdhci.h +6 -0
index c846813..5031d33 100644
@@ -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);
include/linux/mmc/host.h +1 -0
index f65913c..b5c3563 100644
@@ -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 */
Signature · 1 line
1.7.0.4

Review

Quoted 2 lines
(quoted diff) +2 -0
+       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.

Signature · 2 lines
Chris Ball   <[email protected]> 
One Laptop Per Child
#1 lore raw

Hi,
Can any one comment on this patch??

Regards,
Madhav

Quoted 169 lines

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(-)
drivers/mmc/host/sdhci.c +35 -0
index c6d1bd8..5ed7c50 100644
@@ -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                                                          *
drivers/mmc/host/sdhci.h +0 -0
index c846813..5031d33 100644
@@ -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; };

drivers/mmc/host/sdhci.h +4 -0
@@ -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);
include/linux/mmc/host.h +1 -0
index f65913c..b5c3563 100644
@@ -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

Quoted 2 lines

+       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   <[email protected]>
One Laptop Per Child

#2 lore raw
In reply to MADHAV SINGHCHAUHAN

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.

Quoted 143 lines · MADHAV SINGHCHAUHAN

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(-)
drivers/mmc/host/sdhci.c +35 -0
index c6d1bd8..5ed7c50 100644
@@ -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                                                          *
drivers/mmc/host/sdhci.h +6 -0
index c846813..5031d33 100644
@@ -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);
include/linux/mmc/host.h +1 -0
index f65913c..b5c3563 100644
@@ -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 */
Signature · 1 line
1.7.0.4
Signature · 2 lines
Chris Ball   <[email protected]>   <http://printf.net/>
One Laptop Per Child