2010-01-08 12:05:52

by Chaithrika U S

[permalink] [raw]
Subject: [PATCH v3 2/2] davinci: MMC: updates to suspend/resume implementation

Improve the suspend and resume callbacks in DaVinci MMC host controller driver.
Modify the reset status of the contorller and clock during suspend and resume.
Also migrate the power management callbacks from platform driver to dev_pm_ops
structure.

Tested on DA850/OMAP-L138 EVM. This testing requires patches
which add suspen-to-RAM support to DA850/OMAP-L138 SoC[1].

[1]http://linux.davincidsp.com/pipermail/davinci-linux-open-source/
2009-September/016118.html

Signed-off-by: Chaithrika U S <[email protected]>
---
Applies to Linus' kernel tree.
Since the previous version, patch description has been updated.

drivers/mmc/host/davinci_mmc.c | 51 +++++++++++++++++++++++++++++++++------
1 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/davinci_mmc.c b/drivers/mmc/host/davinci_mmc.c
index 25645bf..d60f648 100644
--- a/drivers/mmc/host/davinci_mmc.c
+++ b/drivers/mmc/host/davinci_mmc.c
@@ -170,6 +170,7 @@ struct mmc_davinci_host {
#define DAVINCI_MMC_DATADIR_READ 1
#define DAVINCI_MMC_DATADIR_WRITE 2
unsigned char data_dir;
+ unsigned char suspended;

/* buffer is used during PIO of one scatterlist segment, and
* is updated along with buffer_bytes_left. bytes_left applies
@@ -1297,32 +1298,66 @@ static int __exit davinci_mmcsd_remove(struct platform_device *pdev)
}

#ifdef CONFIG_PM
-static int davinci_mmcsd_suspend(struct platform_device *pdev, pm_message_t msg)
+static int davinci_mmcsd_suspend(struct device *dev)
{
+ struct platform_device *pdev = to_platform_device(dev);
struct mmc_davinci_host *host = platform_get_drvdata(pdev);
+ struct pm_message msg = { PM_EVENT_SUSPEND };
+ int ret;

- return mmc_suspend_host(host->mmc, msg);
+ mmc_host_enable(host->mmc);
+ ret = mmc_suspend_host(host->mmc, msg);
+ if (!ret) {
+ writel(0, host->base + DAVINCI_MMCIM);
+ mmc_davinci_reset_ctrl(host, 1);
+ mmc_host_disable(host->mmc);
+ clk_disable(host->clk);
+ host->suspended = 1;
+ } else {
+ host->suspended = 0;
+ mmc_host_disable(host->mmc);
+ }
+
+ return ret;
}

-static int davinci_mmcsd_resume(struct platform_device *pdev)
+static int davinci_mmcsd_resume(struct device *dev)
{
+ struct platform_device *pdev = to_platform_device(dev);
struct mmc_davinci_host *host = platform_get_drvdata(pdev);
+ int ret;

- return mmc_resume_host(host->mmc);
+ if (!host->suspended)
+ return 0;
+
+ clk_enable(host->clk);
+ mmc_host_enable(host->mmc);
+
+ mmc_davinci_reset_ctrl(host, 0);
+ ret = mmc_resume_host(host->mmc);
+ if (!ret)
+ host->suspended = 0;
+
+ return ret;
}
+
+static struct dev_pm_ops davinci_mmcsd_pm = {
+ .suspend = davinci_mmcsd_suspend,
+ .resume = davinci_mmcsd_resume,
+};
+
+#define davinci_mmcsd_pm_ops (&davinci_mmcsd_pm)
#else
-#define davinci_mmcsd_suspend NULL
-#define davinci_mmcsd_resume NULL
+#define davinci_mmcsd_pm_ops NULL
#endif

static struct platform_driver davinci_mmcsd_driver = {
.driver = {
.name = "davinci_mmc",
.owner = THIS_MODULE,
+ .pm = davinci_mmcsd_pm_ops,
},
.remove = __exit_p(davinci_mmcsd_remove),
- .suspend = davinci_mmcsd_suspend,
- .resume = davinci_mmcsd_resume,
};

static int __init davinci_mmcsd_init(void)
--
1.5.6


2010-01-08 14:55:20

by Kevin Hilman

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] davinci: MMC: updates to suspend/resume implementation

Chaithrika U S <[email protected]> writes:

> Improve the suspend and resume callbacks in DaVinci MMC host controller driver.
> Modify the reset status of the contorller and clock during suspend and resume.
> Also migrate the power management callbacks from platform driver to dev_pm_ops
> structure.
>
> Tested on DA850/OMAP-L138 EVM. This testing requires patches
> which add suspen-to-RAM support to DA850/OMAP-L138 SoC[1].
>
> [1]http://linux.davincidsp.com/pipermail/davinci-linux-open-source/
> 2009-September/016118.html
>
> Signed-off-by: Chaithrika U S <[email protected]>

Acked-by: Kevin Hilman <[email protected]>

> ---
> Applies to Linus' kernel tree.
> Since the previous version, patch description has been updated.
>
> drivers/mmc/host/davinci_mmc.c | 51 +++++++++++++++++++++++++++++++++------
> 1 files changed, 43 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mmc/host/davinci_mmc.c b/drivers/mmc/host/davinci_mmc.c
> index 25645bf..d60f648 100644
> --- a/drivers/mmc/host/davinci_mmc.c
> +++ b/drivers/mmc/host/davinci_mmc.c
> @@ -170,6 +170,7 @@ struct mmc_davinci_host {
> #define DAVINCI_MMC_DATADIR_READ 1
> #define DAVINCI_MMC_DATADIR_WRITE 2
> unsigned char data_dir;
> + unsigned char suspended;
>
> /* buffer is used during PIO of one scatterlist segment, and
> * is updated along with buffer_bytes_left. bytes_left applies
> @@ -1297,32 +1298,66 @@ static int __exit davinci_mmcsd_remove(struct platform_device *pdev)
> }
>
> #ifdef CONFIG_PM
> -static int davinci_mmcsd_suspend(struct platform_device *pdev, pm_message_t msg)
> +static int davinci_mmcsd_suspend(struct device *dev)
> {
> + struct platform_device *pdev = to_platform_device(dev);
> struct mmc_davinci_host *host = platform_get_drvdata(pdev);
> + struct pm_message msg = { PM_EVENT_SUSPEND };
> + int ret;
>
> - return mmc_suspend_host(host->mmc, msg);
> + mmc_host_enable(host->mmc);
> + ret = mmc_suspend_host(host->mmc, msg);
> + if (!ret) {
> + writel(0, host->base + DAVINCI_MMCIM);
> + mmc_davinci_reset_ctrl(host, 1);
> + mmc_host_disable(host->mmc);
> + clk_disable(host->clk);
> + host->suspended = 1;
> + } else {
> + host->suspended = 0;
> + mmc_host_disable(host->mmc);
> + }
> +
> + return ret;
> }
>
> -static int davinci_mmcsd_resume(struct platform_device *pdev)
> +static int davinci_mmcsd_resume(struct device *dev)
> {
> + struct platform_device *pdev = to_platform_device(dev);
> struct mmc_davinci_host *host = platform_get_drvdata(pdev);
> + int ret;
>
> - return mmc_resume_host(host->mmc);
> + if (!host->suspended)
> + return 0;
> +
> + clk_enable(host->clk);
> + mmc_host_enable(host->mmc);
> +
> + mmc_davinci_reset_ctrl(host, 0);
> + ret = mmc_resume_host(host->mmc);
> + if (!ret)
> + host->suspended = 0;
> +
> + return ret;
> }
> +
> +static struct dev_pm_ops davinci_mmcsd_pm = {
> + .suspend = davinci_mmcsd_suspend,
> + .resume = davinci_mmcsd_resume,
> +};
> +
> +#define davinci_mmcsd_pm_ops (&davinci_mmcsd_pm)
> #else
> -#define davinci_mmcsd_suspend NULL
> -#define davinci_mmcsd_resume NULL
> +#define davinci_mmcsd_pm_ops NULL
> #endif
>
> static struct platform_driver davinci_mmcsd_driver = {
> .driver = {
> .name = "davinci_mmc",
> .owner = THIS_MODULE,
> + .pm = davinci_mmcsd_pm_ops,
> },
> .remove = __exit_p(davinci_mmcsd_remove),
> - .suspend = davinci_mmcsd_suspend,
> - .resume = davinci_mmcsd_resume,
> };
>
> static int __init davinci_mmcsd_init(void)
> --
> 1.5.6

2010-01-12 23:37:41

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] davinci: MMC: updates to suspend/resume implementation

On Fri, 8 Jan 2010 17:09:00 +0530
Chaithrika U S <[email protected]> wrote:

> -static int davinci_mmcsd_suspend(struct platform_device *pdev, pm_message_t msg)
> +static int davinci_mmcsd_suspend(struct device *dev)
> {
> + struct platform_device *pdev = to_platform_device(dev);
> struct mmc_davinci_host *host = platform_get_drvdata(pdev);
> + struct pm_message msg = { PM_EVENT_SUSPEND };
> + int ret;
>
> - return mmc_suspend_host(host->mmc, msg);
> + mmc_host_enable(host->mmc);
> + ret = mmc_suspend_host(host->mmc, msg);
> + if (!ret) {
> + writel(0, host->base + DAVINCI_MMCIM);
> + mmc_davinci_reset_ctrl(host, 1);
> + mmc_host_disable(host->mmc);
> + clk_disable(host->clk);
> + host->suspended = 1;
> + } else {
> + host->suspended = 0;
> + mmc_host_disable(host->mmc);
> + }
> +
> + return ret;
> }
>
> -static int davinci_mmcsd_resume(struct platform_device *pdev)
> +static int davinci_mmcsd_resume(struct device *dev)
> {
> + struct platform_device *pdev = to_platform_device(dev);
> struct mmc_davinci_host *host = platform_get_drvdata(pdev);
> + int ret;
>
> - return mmc_resume_host(host->mmc);
> + if (!host->suspended)
> + return 0;
> +
> + clk_enable(host->clk);
> + mmc_host_enable(host->mmc);
> +
> + mmc_davinci_reset_ctrl(host, 0);
> + ret = mmc_resume_host(host->mmc);
> + if (!ret)
> + host->suspended = 0;
> +
> + return ret;
> }

Was it proper to ignore the mmc_host_enable() return values?