Skip to contentSkip to conversation

sdhci-clk-gating-support

PATCH RESEND In review MADHAV SINGHCHAUHAN 2 participants 3 messages Jul 2, 2010 – Jul 6, 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 RESEND 1/1 sdhci-clk-gating-support

I fixed the all issues after running checkpatch.pl and resending it.

From 1fae693a2eef37d878066b09c89f95f57f945f64 Mon Sep 17 00:00:00 2001
From: Madhav Singh <[email protected]>
Date: Fri, 2 Jul 2010 19:33:19 +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.

Signed-off-by Madhav Chauhan <[email protected]> , Nitish Ambastha <[email protected]>
Signed-off-by Kyungmin Park <[email protected]>
---
 drivers/mmc/host/sdhci.c |   34 ++++++++++++++++++++++++++++++++++
 drivers/mmc/host/sdhci.h |    7 +++++++
 2 files changed, 41 insertions(+), 0 deletions(-)
drivers/mmc/host/sdhci.c +34 -0
index c6d1bd8..37ed265 100644
@@ -1103,6 +1103,8 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 
 	spin_lock_irqsave(&host->lock, flags);
 
+	sdhci_clk_enable(host);	/*Enable clock as transfer starts now*/
+
 	WARN_ON(host->mrq != NULL);
 
 #ifndef SDHCI_USE_LEDS_CLASS

@@ -1310,6 +1312,8 @@ static void sdhci_tasklet_finish(unsigned long param)
 		sdhci_reset(host, SDHCI_RESET_DATA);
 	}
 
+	sdhci_clk_disable(host);	/*Disable clock as command has been processed*/
+
 	host->mrq = NULL;
 	host->cmd = NULL;
 	host->data = NULL;

@@ -1597,6 +1601,8 @@ int sdhci_suspend_host(struct sdhci_host *host, pm_message_t state)
 
 	sdhci_disable_card_detection(host);
 
+	sdhci_clk_enable(host);	/* Enable clock */
+
 	ret = mmc_suspend_host(host->mmc);
 	if (ret)
 		return ret;

@@ -1626,6 +1632,9 @@ int sdhci_resume_host(struct sdhci_host *host)
 	mmiowb();
 
 	ret = mmc_resume_host(host->mmc);
+
+	sdhci_clk_disable(host);	/*Now device has wake up disable it*/
+
 	sdhci_enable_card_detection(host);
 
 	return ret;

@@ -1654,6 +1663,9 @@ struct sdhci_host *sdhci_alloc_host(struct device *dev,
 		return ERR_PTR(-ENOMEM);
 
 	host = mmc_priv(mmc);
+	host->user = 0;		/*For Clock gating*/
+	host->clk_restore = 0;
+
 	host->mmc = mmc;
 
 	return host;

@@ -1984,6 +1996,28 @@ void sdhci_free_host(struct sdhci_host *host)
 
 EXPORT_SYMBOL_GPL(sdhci_free_host);
 
+void sdhci_clk_enable(struct sdhci_host *host)
+{
+	if (!host->user) {
+		if (host->clk_restore && host->clock == 0)
+			sdhci_set_clock(host, host->clk_restore);
+	}
+	host->user++;
+}
+EXPORT_SYMBOL_GPL(sdhci_clk_enable);
+
+void sdhci_clk_disable(struct sdhci_host *host)
+{
+	host->user--;
+	if (!host->user)
+		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 +7 -0
index c846813..5912e28 100644
@@ -292,6 +292,9 @@ struct sdhci_host {
 
 	struct timer_list	timer;		/* Timer for timeouts */
 
+	unsigned int		clk_restore;	/*For Clock Gating*/
+	unsigned int		user;
+
 	unsigned long		private[0] ____cacheline_aligned;
 };
 

@@ -410,6 +413,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);
Signature · 1 line
1.6.0.4
#1 lore raw

Hi Madhav,

drivers/mmc/host/sdhci.c struct sdhci_host *sdhci_alloc_host(struct device *dev, · line 1667

Quoted 2 lines
drivers/mmc/host/sdhci.c +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
author
#2 lore raw
Quoted 8 lines

Hi Madhav,

drivers/mmc/host/sdhci.c struct sdhci_host *sdhci_alloc_host(struct device *dev, · line 1667

Quoted 2 lines
drivers/mmc/host/sdhci.c +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.)

we used host->user for making enable disable in a proper sequence i.e. enable-disable-enable and were facing some issues as we were using workqueue.So in current version we tested the patch without host->user and it works fine so we can remove it.

Quoted 3 lines

Some general points:
* have you measured whether there's a performance impact from bringing the clock up and down for every request?

I will test this and update you soon.

Quoted 2 lines

* I'd expect this to be implemented as an MMC capability, so that it can be avoided on specific controllers.

Yes we can do this just we need to add one macro for clock gating capability in struct mmc_host and also looking for any other good alternative for the same. Do we have such host controllers for them we should avoid this??

Quoted 7 lines

Thanks,

- Chris.

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