* [RFC PATCH 0/1] mmc: dw_mmc: Add runtime pm to dw_mmc
@ 2015-01-21 16:43 Karol Wrona
2015-01-21 16:43 ` [RFC PATCH 1/1] " Karol Wrona
0 siblings, 1 reply; 4+ messages in thread
From: Karol Wrona @ 2015-01-21 16:43 UTC (permalink / raw)
To: Seungwon Jeon, Jaehoon Chung, linux-mmc, linux-kernel, Ulf Hansson
Cc: Bartlomiej Zolnierkiewicz, Kyungmin Park, Karol Wrona, Karol Wrona
Hello,
This patch adds runtime pm for dwmmc host. Runtime PM is enabled for
dwmmc-exynos. This is RFC because I need some opinion about the host state
detection and handling and is not finished. The runtime pm suspend and resume
callback are empty for now because the main goal of this path is adding mmc
host to the pm runtime what in effect will give an information to the PM core.
As mshc will be added to some power domain. This will help to know mmc host
state by PM and enter Exynos LPD state. LPD is system level power control
mode. In LPD, the supporting it, unused blocks are power and clock gated.
I will also look something to add to runtime callbacks to decrease power
consumption by the dw_mmc host during normal power mode but this is very tight
because clk to the card is gated in some dw_mmc modes (where there is no
communication).
Thanks,
Karol
Karol Wrona (1):
mmc: dw_mmc: Add runtime pm to dw_mmc
drivers/mmc/host/dw_mmc-exynos.c | 69 ++++++++++++++++++++++++++++++++++++--
drivers/mmc/host/dw_mmc.c | 65 +++++++++++++++++++++++++++++++++--
2 files changed, 130 insertions(+), 4 deletions(-)
--
1.7.9.5
^ permalink raw reply [flat|nested] 4+ messages in thread
* [RFC PATCH 1/1] mmc: dw_mmc: Add runtime pm to dw_mmc
2015-01-21 16:43 [RFC PATCH 0/1] mmc: dw_mmc: Add runtime pm to dw_mmc Karol Wrona
@ 2015-01-21 16:43 ` Karol Wrona
2015-01-22 9:02 ` Ulf Hansson
0 siblings, 1 reply; 4+ messages in thread
From: Karol Wrona @ 2015-01-21 16:43 UTC (permalink / raw)
To: Seungwon Jeon, Jaehoon Chung, linux-mmc, linux-kernel, Ulf Hansson
Cc: Bartlomiej Zolnierkiewicz, Kyungmin Park, Karol Wrona, Karol Wrona
This patch adds runtime pm handling to dw_mmc and enables it for dw_mmc-exynos.
It mainly uses mci_request/mci_request_end for mmc host state information.
Signed-off-by: Karol Wrona <k.wrona@samsung.com>
---
drivers/mmc/host/dw_mmc-exynos.c | 69 ++++++++++++++++++++++++++++++++++++--
drivers/mmc/host/dw_mmc.c | 65 +++++++++++++++++++++++++++++++++--
2 files changed, 130 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
index 12a5eaa..7281c6f 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -17,12 +17,15 @@
#include <linux/mmc/mmc.h>
#include <linux/of.h>
#include <linux/of_gpio.h>
+#include <linux/pm_runtime.h>
#include <linux/slab.h>
#include "dw_mmc.h"
#include "dw_mmc-pltfm.h"
#include "dw_mmc-exynos.h"
+#define DWMMC_AUTOSUSPEND_DELAY 200
+
/* Variations in Exynos specific dw-mshc controller */
enum dw_mci_exynos_type {
DW_MCI_TYPE_EXYNOS4210,
@@ -97,6 +100,30 @@ static int dw_mci_exynos_setup_clock(struct dw_mci *host)
return 0;
}
+#ifdef CONFIG_PM
+static int dw_mci_exynos_runtime_suspend(struct device *dev)
+{
+ struct dw_mci *host = dev_get_drvdata(dev);
+
+ /* empty for now */
+
+ return 0;
+}
+
+static int dw_mci_exynos_runtime_resume(struct device *dev)
+{
+ struct dw_mci *host = dev_get_drvdata(dev);
+
+ /* empty for now */
+
+ return 0;
+}
+#else
+#define dw_mci_exynos_runtime_suspend NULL
+#define dw_mci_exynos_runtime_resume NULL
+
+#endif /* CONFIG_PM */
+
#ifdef CONFIG_PM_SLEEP
static int dw_mci_exynos_suspend(struct device *dev)
{
@@ -179,6 +206,8 @@ static void dw_mci_exynos_set_ios(struct dw_mci *host, struct mmc_ios *ios)
unsigned long actual;
u8 div = priv->ciu_div + 1;
+ pm_runtime_get_sync(host->dev);
+
if (ios->timing == MMC_TIMING_MMC_DDR52) {
if (priv->ctrl_type == DW_MCI_TYPE_EXYNOS7 ||
priv->ctrl_type == DW_MCI_TYPE_EXYNOS7_SMU)
@@ -196,6 +225,9 @@ static void dw_mci_exynos_set_ios(struct dw_mci *host, struct mmc_ios *ios)
mci_writel(host, CLKSEL, priv->sdr_timing);
}
+ pm_runtime_mark_last_busy(host->dev);
+ pm_runtime_put_autosuspend(host->dev);
+
/*
* Don't care if wanted clock is zero or
* ciu clock is unavailable
@@ -405,16 +437,49 @@ MODULE_DEVICE_TABLE(of, dw_mci_exynos_match);
static int dw_mci_exynos_probe(struct platform_device *pdev)
{
+ int ret;
const struct dw_mci_drv_data *drv_data;
const struct of_device_id *match;
match = of_match_node(dw_mci_exynos_match, pdev->dev.of_node);
drv_data = match->data;
- return dw_mci_pltfm_register(pdev, drv_data);
+ ret = dw_mci_pltfm_register(pdev, drv_data);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "register error\n");
+ goto err_pm_dis;
+ }
+
+ pm_runtime_enable(&pdev->dev);
+ pm_runtime_get_sync(&pdev->dev);
+ pm_runtime_set_autosuspend_delay(&pdev->dev, DWMMC_AUTOSUSPEND_DELAY);
+ pm_runtime_use_autosuspend(&pdev->dev);
+
+ pm_runtime_mark_last_busy(&pdev->dev);
+ pm_runtime_put_autosuspend(&pdev->dev);
+
+ return ret;
+
+err_pm_dis:
+ pm_runtime_disable(&pdev->dev);
+ return ret;
}
+static int __exit dw_mci_exynos_remove(struct platform_device *pdev)
+{
+ pm_runtime_get_sync(&pdev->dev);
+
+ dw_mci_pltfm_remove(pdev);
+
+ pm_runtime_put_sync(&pdev->dev);
+ pm_runtime_disable(&pdev->dev);
+
+ return 0;
+
+}
static const struct dev_pm_ops dw_mci_exynos_pmops = {
SET_SYSTEM_SLEEP_PM_OPS(dw_mci_exynos_suspend, dw_mci_exynos_resume)
+ SET_RUNTIME_PM_OPS(dw_mci_exynos_runtime_suspend,
+ dw_mci_exynos_runtime_resume, NULL)
.resume_noirq = dw_mci_exynos_resume_noirq,
.thaw_noirq = dw_mci_exynos_resume_noirq,
.restore_noirq = dw_mci_exynos_resume_noirq,
@@ -422,7 +487,7 @@ static const struct dev_pm_ops dw_mci_exynos_pmops = {
static struct platform_driver dw_mci_exynos_pltfm_driver = {
.probe = dw_mci_exynos_probe,
- .remove = __exit_p(dw_mci_pltfm_remove),
+ .remove = __exit_p(dw_mci_exynos_remove),
.driver = {
.name = "dwmmc_exynos",
.of_match_table = dw_mci_exynos_match,
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 4d2e3c2..1e66d7b 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -38,6 +38,7 @@
#include <linux/of.h>
#include <linux/of_gpio.h>
#include <linux/mmc/slot-gpio.h>
+#include <linux/pm_runtime.h>
#include "dw_mmc.h"
@@ -112,6 +113,8 @@ static int dw_mci_req_show(struct seq_file *s, void *v)
struct mmc_command *stop;
struct mmc_data *data;
+ pm_runtime_get_sync(slot->host->dev);
+
/* Make sure we get a consistent snapshot */
spin_lock_bh(&slot->host->lock);
mrq = slot->mrq;
@@ -141,6 +144,9 @@ static int dw_mci_req_show(struct seq_file *s, void *v)
spin_unlock_bh(&slot->host->lock);
+ pm_runtime_mark_last_busy(slot->host->dev);
+ pm_runtime_put_autosuspend(slot->host->dev);
+
return 0;
}
@@ -1043,6 +1049,8 @@ static void dw_mci_request(struct mmc_host *mmc, struct mmc_request *mrq)
WARN_ON(slot->mrq);
+ pm_runtime_get_sync(host->dev);
+
/*
* The check for card presence and queueing of the request must be
* atomic, otherwise the card could be removed in between and the
@@ -1054,6 +1062,9 @@ static void dw_mci_request(struct mmc_host *mmc, struct mmc_request *mrq)
spin_unlock_bh(&host->lock);
mrq->cmd->error = -ENOMEDIUM;
mmc_request_done(mmc, mrq);
+
+ pm_runtime_mark_last_busy(host->dev);
+ pm_runtime_put_autosuspend(host->dev);
return;
}
@@ -1069,6 +1080,8 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
u32 regs;
int ret;
+ pm_runtime_get_sync(slot->host->dev);
+
switch (ios->bus_width) {
case MMC_BUS_WIDTH_4:
slot->ctype = SDMMC_CTYPE_4BIT;
@@ -1116,7 +1129,7 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
dev_err(slot->host->dev,
"failed to enable vmmc regulator\n");
/*return, if failed turn on vmmc*/
- return;
+ goto _end;
}
}
set_bit(DW_MMC_CARD_NEED_INIT, &slot->flags);
@@ -1150,6 +1163,10 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
default:
break;
}
+
+_end:
+ pm_runtime_mark_last_busy(slot->host->dev);
+ pm_runtime_put_autosuspend(slot->host->dev);
}
static int dw_mci_card_busy(struct mmc_host *mmc)
@@ -1157,12 +1174,17 @@ static int dw_mci_card_busy(struct mmc_host *mmc)
struct dw_mci_slot *slot = mmc_priv(mmc);
u32 status;
+ pm_runtime_get_sync(slot->host->dev);
+
/*
* Check the busy bit which is low when DAT[3:0]
* (the data lines) are 0000
*/
status = mci_readl(slot->host, STATUS);
+ pm_runtime_mark_last_busy(slot->host->dev);
+ pm_runtime_put_autosuspend(slot->host->dev);
+
return !!(status & SDMMC_STATUS_BUSY);
}
@@ -1175,6 +1197,8 @@ static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
int min_uv, max_uv;
int ret;
+ pm_runtime_get_sync(host->dev);
+
/*
* Program the voltage. Note that some instances of dw_mmc may use
* the UHS_REG for this. For other instances (like exynos) the UHS_REG
@@ -1202,6 +1226,9 @@ static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
}
mci_writel(host, UHS_REG, uhs);
+ pm_runtime_mark_last_busy(host->dev);
+ pm_runtime_put_autosuspend(host->dev);
+
return 0;
}
@@ -1211,6 +1238,8 @@ static int dw_mci_get_ro(struct mmc_host *mmc)
struct dw_mci_slot *slot = mmc_priv(mmc);
int gpio_ro = mmc_gpio_get_ro(mmc);
+ pm_runtime_get_sync(slot->host->dev);
+
/* Use platform get_ro function, else try on board write protect */
if ((slot->quirks & DW_MCI_SLOT_QUIRK_NO_WRITE_PROTECT) ||
(slot->host->quirks & DW_MCI_QUIRK_NO_WRITE_PROTECT))
@@ -1221,6 +1250,9 @@ static int dw_mci_get_ro(struct mmc_host *mmc)
read_only =
mci_readl(slot->host, WRTPRT) & (1 << slot->id) ? 1 : 0;
+ pm_runtime_mark_last_busy(slot->host->dev);
+ pm_runtime_put_autosuspend(slot->host->dev);
+
dev_dbg(&mmc->class_dev, "card is %s\n",
read_only ? "read-only" : "read-write");
@@ -1235,6 +1267,8 @@ static int dw_mci_get_cd(struct mmc_host *mmc)
struct dw_mci *host = slot->host;
int gpio_cd = mmc_gpio_get_cd(mmc);
+ pm_runtime_get_sync(host->dev);
+
/* Use platform get_cd function, else try onboard card detect */
if (brd->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION)
present = 1;
@@ -1254,6 +1288,9 @@ static int dw_mci_get_cd(struct mmc_host *mmc)
}
spin_unlock_bh(&host->lock);
+ pm_runtime_mark_last_busy(host->dev);
+ pm_runtime_put_autosuspend(host->dev);
+
return present;
}
@@ -1262,6 +1299,8 @@ static void dw_mci_init_card(struct mmc_host *mmc, struct mmc_card *card)
struct dw_mci_slot *slot = mmc_priv(mmc);
struct dw_mci *host = slot->host;
+ pm_runtime_get_sync(host->dev);
+
/*
* Low power mode will stop the card clock when idle. According to the
* description of the CLKENA register we should disable low power mode
@@ -1289,6 +1328,9 @@ static void dw_mci_init_card(struct mmc_host *mmc, struct mmc_card *card)
SDMMC_CMD_PRV_DAT_WAIT, 0);
}
}
+
+ pm_runtime_mark_last_busy(host->dev);
+ pm_runtime_put_autosuspend(host->dev);
}
static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
@@ -1298,6 +1340,9 @@ static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
unsigned long irqflags;
u32 int_mask;
+ if (enb)
+ pm_runtime_get_sync(slot->host->dev);
+
spin_lock_irqsave(&host->irq_lock, irqflags);
/* Enable/disable Slot Specific SDIO interrupt */
@@ -1309,6 +1354,12 @@ static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
mci_writel(host, INTMASK, int_mask);
spin_unlock_irqrestore(&host->irq_lock, irqflags);
+
+ /* If interrupt is enabled leave device active. */
+ if (!enb) {
+ pm_runtime_mark_last_busy(slot->host->dev);
+ pm_runtime_put_autosuspend(slot->host->dev);
+ }
}
static int dw_mci_execute_tuning(struct mmc_host *mmc, u32 opcode)
@@ -1318,8 +1369,14 @@ static int dw_mci_execute_tuning(struct mmc_host *mmc, u32 opcode)
const struct dw_mci_drv_data *drv_data = host->drv_data;
int err = -ENOSYS;
+ pm_runtime_get_sync(host->dev);
+
if (drv_data && drv_data->execute_tuning)
err = drv_data->execute_tuning(slot);
+
+ pm_runtime_mark_last_busy(host->dev);
+ pm_runtime_put_autosuspend(host->dev);
+
return err;
}
@@ -1361,8 +1418,12 @@ static void dw_mci_request_end(struct dw_mci *host, struct mmc_request *mrq)
if (host->state == STATE_SENDING_CMD11)
host->state = STATE_WAITING_CMD11_DONE;
- else
+ else {
+ pm_runtime_mark_last_busy(host->dev);
+ pm_runtime_put_autosuspend(host->dev);
+
host->state = STATE_IDLE;
+ }
}
spin_unlock(&host->lock);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [RFC PATCH 1/1] mmc: dw_mmc: Add runtime pm to dw_mmc
2015-01-21 16:43 ` [RFC PATCH 1/1] " Karol Wrona
@ 2015-01-22 9:02 ` Ulf Hansson
2015-01-29 8:58 ` Karol Wrona
0 siblings, 1 reply; 4+ messages in thread
From: Ulf Hansson @ 2015-01-22 9:02 UTC (permalink / raw)
To: Karol Wrona
Cc: Seungwon Jeon, Jaehoon Chung, linux-mmc, linux-kernel,
Bartlomiej Zolnierkiewicz, Kyungmin Park, Karol Wrona
On 21 January 2015 at 17:43, Karol Wrona <k.wrona@samsung.com> wrote:
> This patch adds runtime pm handling to dw_mmc and enables it for dw_mmc-exynos.
> It mainly uses mci_request/mci_request_end for mmc host state information.
>
> Signed-off-by: Karol Wrona <k.wrona@samsung.com>
> ---
> drivers/mmc/host/dw_mmc-exynos.c | 69 ++++++++++++++++++++++++++++++++++++--
> drivers/mmc/host/dw_mmc.c | 65 +++++++++++++++++++++++++++++++++--
> 2 files changed, 130 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
> index 12a5eaa..7281c6f 100644
> --- a/drivers/mmc/host/dw_mmc-exynos.c
> +++ b/drivers/mmc/host/dw_mmc-exynos.c
> @@ -17,12 +17,15 @@
> #include <linux/mmc/mmc.h>
> #include <linux/of.h>
> #include <linux/of_gpio.h>
> +#include <linux/pm_runtime.h>
> #include <linux/slab.h>
>
> #include "dw_mmc.h"
> #include "dw_mmc-pltfm.h"
> #include "dw_mmc-exynos.h"
>
> +#define DWMMC_AUTOSUSPEND_DELAY 200
Normally we use 50 as default. Any reason to why you can't use that?
Hmm, maybe we should have such a default defined in a common mmc host
header file!?
> +
> /* Variations in Exynos specific dw-mshc controller */
> enum dw_mci_exynos_type {
> DW_MCI_TYPE_EXYNOS4210,
> @@ -97,6 +100,30 @@ static int dw_mci_exynos_setup_clock(struct dw_mci *host)
> return 0;
> }
>
> +#ifdef CONFIG_PM
> +static int dw_mci_exynos_runtime_suspend(struct device *dev)
> +{
> + struct dw_mci *host = dev_get_drvdata(dev);
> +
> + /* empty for now */
> +
> + return 0;
> +}
> +
> +static int dw_mci_exynos_runtime_resume(struct device *dev)
> +{
> + struct dw_mci *host = dev_get_drvdata(dev);
> +
> + /* empty for now */
> +
> + return 0;
> +}
> +#else
> +#define dw_mci_exynos_runtime_suspend NULL
> +#define dw_mci_exynos_runtime_resume NULL
> +
> +#endif /* CONFIG_PM */
I would suggest you to remove all the above code from this patch. If
you want to add the callbacks, let's anyway do that from a separate
patch.
> +
> #ifdef CONFIG_PM_SLEEP
> static int dw_mci_exynos_suspend(struct device *dev)
> {
> @@ -179,6 +206,8 @@ static void dw_mci_exynos_set_ios(struct dw_mci *host, struct mmc_ios *ios)
> unsigned long actual;
> u8 div = priv->ciu_div + 1;
>
> + pm_runtime_get_sync(host->dev);
> +
> if (ios->timing == MMC_TIMING_MMC_DDR52) {
> if (priv->ctrl_type == DW_MCI_TYPE_EXYNOS7 ||
> priv->ctrl_type == DW_MCI_TYPE_EXYNOS7_SMU)
> @@ -196,6 +225,9 @@ static void dw_mci_exynos_set_ios(struct dw_mci *host, struct mmc_ios *ios)
> mci_writel(host, CLKSEL, priv->sdr_timing);
> }
>
> + pm_runtime_mark_last_busy(host->dev);
> + pm_runtime_put_autosuspend(host->dev);
> +
> /*
> * Don't care if wanted clock is zero or
> * ciu clock is unavailable
> @@ -405,16 +437,49 @@ MODULE_DEVICE_TABLE(of, dw_mci_exynos_match);
>
> static int dw_mci_exynos_probe(struct platform_device *pdev)
> {
> + int ret;
> const struct dw_mci_drv_data *drv_data;
> const struct of_device_id *match;
>
> match = of_match_node(dw_mci_exynos_match, pdev->dev.of_node);
> drv_data = match->data;
> - return dw_mci_pltfm_register(pdev, drv_data);
> + ret = dw_mci_pltfm_register(pdev, drv_data);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "register error\n");
> + goto err_pm_dis;
> + }
> +
> + pm_runtime_enable(&pdev->dev);
> + pm_runtime_get_sync(&pdev->dev);
> + pm_runtime_set_autosuspend_delay(&pdev->dev, DWMMC_AUTOSUSPEND_DELAY);
> + pm_runtime_use_autosuspend(&pdev->dev);
> +
> + pm_runtime_mark_last_busy(&pdev->dev);
> + pm_runtime_put_autosuspend(&pdev->dev);
> +
> + return ret;
> +
> +err_pm_dis:
> + pm_runtime_disable(&pdev->dev);
> + return ret;
> }
>
> +static int __exit dw_mci_exynos_remove(struct platform_device *pdev)
> +{
> + pm_runtime_get_sync(&pdev->dev);
> +
> + dw_mci_pltfm_remove(pdev);
> +
> + pm_runtime_put_sync(&pdev->dev);
> + pm_runtime_disable(&pdev->dev);
> +
> + return 0;
> +
> +}
> static const struct dev_pm_ops dw_mci_exynos_pmops = {
> SET_SYSTEM_SLEEP_PM_OPS(dw_mci_exynos_suspend, dw_mci_exynos_resume)
> + SET_RUNTIME_PM_OPS(dw_mci_exynos_runtime_suspend,
> + dw_mci_exynos_runtime_resume, NULL)
> .resume_noirq = dw_mci_exynos_resume_noirq,
> .thaw_noirq = dw_mci_exynos_resume_noirq,
> .restore_noirq = dw_mci_exynos_resume_noirq,
> @@ -422,7 +487,7 @@ static const struct dev_pm_ops dw_mci_exynos_pmops = {
>
> static struct platform_driver dw_mci_exynos_pltfm_driver = {
> .probe = dw_mci_exynos_probe,
> - .remove = __exit_p(dw_mci_pltfm_remove),
> + .remove = __exit_p(dw_mci_exynos_remove),
> .driver = {
> .name = "dwmmc_exynos",
> .of_match_table = dw_mci_exynos_match,
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 4d2e3c2..1e66d7b 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -38,6 +38,7 @@
> #include <linux/of.h>
> #include <linux/of_gpio.h>
> #include <linux/mmc/slot-gpio.h>
> +#include <linux/pm_runtime.h>
>
> #include "dw_mmc.h"
>
> @@ -112,6 +113,8 @@ static int dw_mci_req_show(struct seq_file *s, void *v)
> struct mmc_command *stop;
> struct mmc_data *data;
>
> + pm_runtime_get_sync(slot->host->dev);
> +
> /* Make sure we get a consistent snapshot */
> spin_lock_bh(&slot->host->lock);
> mrq = slot->mrq;
> @@ -141,6 +144,9 @@ static int dw_mci_req_show(struct seq_file *s, void *v)
>
> spin_unlock_bh(&slot->host->lock);
>
> + pm_runtime_mark_last_busy(slot->host->dev);
> + pm_runtime_put_autosuspend(slot->host->dev);
> +
> return 0;
> }
>
> @@ -1043,6 +1049,8 @@ static void dw_mci_request(struct mmc_host *mmc, struct mmc_request *mrq)
>
> WARN_ON(slot->mrq);
>
> + pm_runtime_get_sync(host->dev);
> +
> /*
> * The check for card presence and queueing of the request must be
> * atomic, otherwise the card could be removed in between and the
> @@ -1054,6 +1062,9 @@ static void dw_mci_request(struct mmc_host *mmc, struct mmc_request *mrq)
> spin_unlock_bh(&host->lock);
> mrq->cmd->error = -ENOMEDIUM;
> mmc_request_done(mmc, mrq);
> +
> + pm_runtime_mark_last_busy(host->dev);
> + pm_runtime_put_autosuspend(host->dev);
> return;
> }
>
> @@ -1069,6 +1080,8 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> u32 regs;
> int ret;
>
> + pm_runtime_get_sync(slot->host->dev);
> +
> switch (ios->bus_width) {
> case MMC_BUS_WIDTH_4:
> slot->ctype = SDMMC_CTYPE_4BIT;
> @@ -1116,7 +1129,7 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> dev_err(slot->host->dev,
> "failed to enable vmmc regulator\n");
> /*return, if failed turn on vmmc*/
> - return;
> + goto _end;
> }
> }
> set_bit(DW_MMC_CARD_NEED_INIT, &slot->flags);
> @@ -1150,6 +1163,10 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> default:
> break;
> }
> +
> +_end:
> + pm_runtime_mark_last_busy(slot->host->dev);
> + pm_runtime_put_autosuspend(slot->host->dev);
> }
>
> static int dw_mci_card_busy(struct mmc_host *mmc)
> @@ -1157,12 +1174,17 @@ static int dw_mci_card_busy(struct mmc_host *mmc)
> struct dw_mci_slot *slot = mmc_priv(mmc);
> u32 status;
>
> + pm_runtime_get_sync(slot->host->dev);
> +
> /*
> * Check the busy bit which is low when DAT[3:0]
> * (the data lines) are 0000
> */
> status = mci_readl(slot->host, STATUS);
>
> + pm_runtime_mark_last_busy(slot->host->dev);
> + pm_runtime_put_autosuspend(slot->host->dev);
> +
> return !!(status & SDMMC_STATUS_BUSY);
> }
>
> @@ -1175,6 +1197,8 @@ static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
> int min_uv, max_uv;
> int ret;
>
> + pm_runtime_get_sync(host->dev);
> +
> /*
> * Program the voltage. Note that some instances of dw_mmc may use
> * the UHS_REG for this. For other instances (like exynos) the UHS_REG
> @@ -1202,6 +1226,9 @@ static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
> }
> mci_writel(host, UHS_REG, uhs);
>
> + pm_runtime_mark_last_busy(host->dev);
> + pm_runtime_put_autosuspend(host->dev);
> +
> return 0;
> }
>
> @@ -1211,6 +1238,8 @@ static int dw_mci_get_ro(struct mmc_host *mmc)
> struct dw_mci_slot *slot = mmc_priv(mmc);
> int gpio_ro = mmc_gpio_get_ro(mmc);
>
> + pm_runtime_get_sync(slot->host->dev);
> +
> /* Use platform get_ro function, else try on board write protect */
> if ((slot->quirks & DW_MCI_SLOT_QUIRK_NO_WRITE_PROTECT) ||
> (slot->host->quirks & DW_MCI_QUIRK_NO_WRITE_PROTECT))
> @@ -1221,6 +1250,9 @@ static int dw_mci_get_ro(struct mmc_host *mmc)
> read_only =
> mci_readl(slot->host, WRTPRT) & (1 << slot->id) ? 1 : 0;
>
> + pm_runtime_mark_last_busy(slot->host->dev);
> + pm_runtime_put_autosuspend(slot->host->dev);
> +
> dev_dbg(&mmc->class_dev, "card is %s\n",
> read_only ? "read-only" : "read-write");
>
> @@ -1235,6 +1267,8 @@ static int dw_mci_get_cd(struct mmc_host *mmc)
> struct dw_mci *host = slot->host;
> int gpio_cd = mmc_gpio_get_cd(mmc);
>
> + pm_runtime_get_sync(host->dev);
> +
> /* Use platform get_cd function, else try onboard card detect */
> if (brd->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION)
> present = 1;
> @@ -1254,6 +1288,9 @@ static int dw_mci_get_cd(struct mmc_host *mmc)
> }
> spin_unlock_bh(&host->lock);
>
> + pm_runtime_mark_last_busy(host->dev);
> + pm_runtime_put_autosuspend(host->dev);
> +
> return present;
> }
>
> @@ -1262,6 +1299,8 @@ static void dw_mci_init_card(struct mmc_host *mmc, struct mmc_card *card)
> struct dw_mci_slot *slot = mmc_priv(mmc);
> struct dw_mci *host = slot->host;
>
> + pm_runtime_get_sync(host->dev);
> +
> /*
> * Low power mode will stop the card clock when idle. According to the
> * description of the CLKENA register we should disable low power mode
> @@ -1289,6 +1328,9 @@ static void dw_mci_init_card(struct mmc_host *mmc, struct mmc_card *card)
> SDMMC_CMD_PRV_DAT_WAIT, 0);
> }
> }
> +
> + pm_runtime_mark_last_busy(host->dev);
> + pm_runtime_put_autosuspend(host->dev);
> }
>
> static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
> @@ -1298,6 +1340,9 @@ static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
> unsigned long irqflags;
> u32 int_mask;
>
> + if (enb)
> + pm_runtime_get_sync(slot->host->dev);
> +
> spin_lock_irqsave(&host->irq_lock, irqflags);
>
> /* Enable/disable Slot Specific SDIO interrupt */
> @@ -1309,6 +1354,12 @@ static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
> mci_writel(host, INTMASK, int_mask);
>
> spin_unlock_irqrestore(&host->irq_lock, irqflags);
> +
> + /* If interrupt is enabled leave device active. */
> + if (!enb) {
> + pm_runtime_mark_last_busy(slot->host->dev);
> + pm_runtime_put_autosuspend(slot->host->dev);
> + }
> }
>
> static int dw_mci_execute_tuning(struct mmc_host *mmc, u32 opcode)
> @@ -1318,8 +1369,14 @@ static int dw_mci_execute_tuning(struct mmc_host *mmc, u32 opcode)
> const struct dw_mci_drv_data *drv_data = host->drv_data;
> int err = -ENOSYS;
>
> + pm_runtime_get_sync(host->dev);
> +
> if (drv_data && drv_data->execute_tuning)
> err = drv_data->execute_tuning(slot);
> +
> + pm_runtime_mark_last_busy(host->dev);
> + pm_runtime_put_autosuspend(host->dev);
> +
> return err;
> }
>
> @@ -1361,8 +1418,12 @@ static void dw_mci_request_end(struct dw_mci *host, struct mmc_request *mrq)
>
> if (host->state == STATE_SENDING_CMD11)
> host->state = STATE_WAITING_CMD11_DONE;
> - else
> + else {
> + pm_runtime_mark_last_busy(host->dev);
> + pm_runtime_put_autosuspend(host->dev);
> +
> host->state = STATE_IDLE;
> + }
> }
>
> spin_unlock(&host->lock);
> --
> 1.7.9.5
>
An overall comment: I think most of the calls to pm_runtime_get|put()
should be done in the generic dw_mmc.c layer.
For example, look at the path in ->set_ios(). It's not enough that the
exynos specific callback ->set_ios() (in the struct dw_mci_drv_data)
invokes pm_runtime_get|put(), since the common dw_mci_set_ios()
function is also accessing the controller's registers.
Kind regards
Uffe
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH 1/1] mmc: dw_mmc: Add runtime pm to dw_mmc
2015-01-22 9:02 ` Ulf Hansson
@ 2015-01-29 8:58 ` Karol Wrona
0 siblings, 0 replies; 4+ messages in thread
From: Karol Wrona @ 2015-01-29 8:58 UTC (permalink / raw)
To: Ulf Hansson
Cc: Seungwon Jeon, Jaehoon Chung, linux-mmc, linux-kernel,
Bartlomiej Zolnierkiewicz, Kyungmin Park, Karol Wrona
On 01/22/2015 10:02 AM, Ulf Hansson wrote:
> On 21 January 2015 at 17:43, Karol Wrona <k.wrona@samsung.com> wrote:
>> This patch adds runtime pm handling to dw_mmc and enables it for dw_mmc-exynos.
>> It mainly uses mci_request/mci_request_end for mmc host state information.
>>
>> Signed-off-by: Karol Wrona <k.wrona@samsung.com>
>> ---
>> drivers/mmc/host/dw_mmc-exynos.c | 69 ++++++++++++++++++++++++++++++++++++--
>> drivers/mmc/host/dw_mmc.c | 65 +++++++++++++++++++++++++++++++++--
>> 2 files changed, 130 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
>> index 12a5eaa..7281c6f 100644
>> --- a/drivers/mmc/host/dw_mmc-exynos.c
>> +++ b/drivers/mmc/host/dw_mmc-exynos.c
>> @@ -17,12 +17,15 @@
[...]
>>
>> +#define DWMMC_AUTOSUSPEND_DELAY 200
>
> Normally we use 50 as default. Any reason to why you can't use that?
>
Thanks for looking at that.
No special reason. I will check lower delay value.
> Hmm, maybe we should have such a default defined in a common mmc host
> header file!?
Will do.
>
>> +
>> /* Variations in Exynos specific dw-mshc controller */
>> enum dw_mci_exynos_type {
>> DW_MCI_TYPE_EXYNOS4210,
>> @@ -97,6 +100,30 @@ static int dw_mci_exynos_setup_clock(struct dw_mci *host)
>> return 0;
>> }
>>
>> +#ifdef CONFIG_PM
>> +static int dw_mci_exynos_runtime_suspend(struct device *dev)
>> +{
>> + struct dw_mci *host = dev_get_drvdata(dev);
>> +
>> + /* empty for now */
>> +
>> + return 0;
>> +}
>> +
>> +static int dw_mci_exynos_runtime_resume(struct device *dev)
>> +{
>> + struct dw_mci *host = dev_get_drvdata(dev);
>> +
>> + /* empty for now */
>> +
>> + return 0;
>> +}
>> +#else
>> +#define dw_mci_exynos_runtime_suspend NULL
>> +#define dw_mci_exynos_runtime_resume NULL
>> +
>> +#endif /* CONFIG_PM */
>
> I would suggest you to remove all the above code from this patch. If
> you want to add the callbacks, let's anyway do that from a separate
> patch.
Will do.
Thanks,
Karol
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-01-29 8:58 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-21 16:43 [RFC PATCH 0/1] mmc: dw_mmc: Add runtime pm to dw_mmc Karol Wrona
2015-01-21 16:43 ` [RFC PATCH 1/1] " Karol Wrona
2015-01-22 9:02 ` Ulf Hansson
2015-01-29 8:58 ` Karol Wrona
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).