On Wed, Oct 30, 2013 at 11:49:19PM +0100, Ulf Hansson wrote:
> On 30 October 2013 15:12, Dong Aisheng <b29396@xxxxxxxxxxxxx> wrote:
> > The root clock will be disabled in runtime pm which can be used to save power.
> >
> > Signed-off-by: Dong Aisheng <b29396@xxxxxxxxxxxxx>
> > ---
> > drivers/mmc/host/sdhci-esdhc-imx.c | 53 ++++++++++++++++++++++++++++++++++-
> > 1 files changed, 51 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> > index da8026b..60201fd 100644
> > --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> > @@ -27,6 +27,7 @@
> > #include <linux/of_gpio.h>
> > #include <linux/pinctrl/consumer.h>
> > #include <linux/platform_data/mmc-esdhc-imx.h>
> > +#include <linux/pm_runtime.h>
> > #include "sdhci-pltfm.h"
> > #include "sdhci-esdhc.h"
> >
> > @@ -1117,9 +1118,17 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
> > host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;
> > }
> >
>
> Add:
> pm_runtime_set_active
>
Right.
Thanks for reminder.
BTW, it seems with adding pm_runtime_set_active, i need
put pm_runtime_* related functions right behind sdhci_add_host
since i observed that pm_runtime_set_autosuspend_delay would trigger a suspend
automatically if the device initial state is ACTIVE and during this time,
the host structure is still not initialized which will lead to a crash
when access it in sdhci_runtime_suspend_host.
> > + pm_runtime_enable(&pdev->dev);
> > + pm_runtime_set_autosuspend_delay(&pdev->dev, 50);
> > + pm_runtime_use_autosuspend(&pdev->dev);
> > + pm_suspend_ignore_children(&pdev->dev, 1);
> > +
> > err = sdhci_add_host(host);
> > - if (err)
> > + if (err) {
> > + pm_runtime_forbid(&pdev->dev);
> > + pm_runtime_get_noresume(&pdev->dev);
> > goto disable_clk;
> > + }
> >
> > return 0;
> >
> > @@ -1141,6 +1150,9 @@ static int sdhci_esdhc_imx_remove(struct platform_device *pdev)
> >
> > sdhci_remove_host(host, dead);
> >
> > + pm_runtime_dont_use_autosuspend(&pdev->dev);
> > + pm_runtime_disable(&pdev->dev);
> > +
> > clk_disable_unprepare(imx_data->clk_per);
> > clk_disable_unprepare(imx_data->clk_ipg);
> > clk_disable_unprepare(imx_data->clk_ahb);
> > @@ -1150,12 +1162,49 @@ static int sdhci_esdhc_imx_remove(struct platform_device *pdev)
> > return 0;
> > }
> >
> > +#ifdef CONFIG_PM_RUNTIME
> > +static int sdhci_esdhc_runtime_suspend(struct device *dev)
> > +{
> > + struct sdhci_host *host = dev_get_drvdata(dev);
> > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> > + struct pltfm_imx_data *imx_data = pltfm_host->priv;
> > + int ret;
> > +
> > + ret = sdhci_runtime_suspend_host(host);
> > +
> > + clk_disable_unprepare(imx_data->clk_per);
> > + clk_disable_unprepare(imx_data->clk_ipg);
> > + clk_disable_unprepare(imx_data->clk_ahb);
> > +
> > + return ret;
> > +}
> > +
> > +static int sdhci_esdhc_runtime_resume(struct device *dev)
> > +{
> > + struct sdhci_host *host = dev_get_drvdata(dev);
> > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> > + struct pltfm_imx_data *imx_data = pltfm_host->priv;
> > +
> > + clk_prepare_enable(imx_data->clk_per);
> > + clk_prepare_enable(imx_data->clk_ipg);
> > + clk_prepare_enable(imx_data->clk_ahb);
> > +
> > + return sdhci_runtime_resume_host(host);
> > +}
> > +#endif
> > +
> > +static const struct dev_pm_ops sdhci_esdhc_pmops = {
> > + SET_SYSTEM_SLEEP_PM_OPS(sdhci_pltfm_suspend, sdhci_pltfm_resume)
> > + SET_RUNTIME_PM_OPS(sdhci_esdhc_runtime_suspend,
> > + sdhci_esdhc_runtime_resume, NULL)
> > +};
> > +
> > static struct platform_driver sdhci_esdhc_imx_driver = {
> > .driver = {
> > .name = "sdhci-esdhc-imx",
> > .owner = THIS_MODULE,
> > .of_match_table = imx_esdhc_dt_ids,
> > - .pm = SDHCI_PLTFM_PMOPS,
> > + .pm = &sdhci_esdhc_pmops,
> > },
> > .id_table = imx_esdhc_devtype,
> > .probe = sdhci_esdhc_imx_probe,
> > --
> > 1.7.2.rc3
>
> Apart from the minor issue in the probe function, looks good to me.
>
> Kind regards
> Ulf Hansson
>
Thanks for the review.
Will update it in v2.
Regards
Dong Aisheng
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
