| Message ID | 052c21c000f64c28fd6b64c4db11e4b706bebf79.1520520655.git-series.maxime.ripard@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 8 March 2018 at 15:52, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > So far, even if our card was not in use, we didn't shut down our main > clock, which meant that it was still output on the MMC bus. > > While this obviously means that we could save some power there, it also > created issues when it comes with EMC control since we'll have a perfect > peak at the card clock rate. > > Let's implement runtime_pm with autosuspend so that we will shut down the > clock when it's not been in use for quite some time. > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> > --- > drivers/mmc/host/sunxi-mmc.c | 46 +++++++++++++++++++++++++++++++++++++- > 1 file changed, 46 insertions(+) > > diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c > index f6374066081b..0f98a5fcaade 100644 > --- a/drivers/mmc/host/sunxi-mmc.c > +++ b/drivers/mmc/host/sunxi-mmc.c > @@ -35,6 +35,7 @@ > #include <linux/of_gpio.h> > #include <linux/of_platform.h> > #include <linux/platform_device.h> > +#include <linux/pm_runtime.h> > #include <linux/regulator/consumer.h> > #include <linux/reset.h> > #include <linux/scatterlist.h> > @@ -973,6 +974,9 @@ static void sunxi_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable) > unsigned long flags; > u32 imask; > > + if (enable) > + pm_runtime_get_noresume(host->dev); > + > spin_lock_irqsave(&host->lock, flags); > > imask = mmc_readl(host, REG_IMASK); > @@ -985,6 +989,9 @@ static void sunxi_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable) > } > mmc_writel(host, REG_IMASK, imask); > spin_unlock_irqrestore(&host->lock, flags); > + > + if (!enable) > + pm_runtime_put_noidle(host->mmc->parent); > } > > static void sunxi_mmc_hw_reset(struct mmc_host *mmc) > @@ -1398,6 +1405,11 @@ static int sunxi_mmc_probe(struct platform_device *pdev) > if (ret) > goto error_free_dma; > > + pm_runtime_set_active(&pdev->dev); > + pm_runtime_set_autosuspend_delay(&pdev->dev, 50); > + pm_runtime_use_autosuspend(&pdev->dev); > + pm_runtime_enable(&pdev->dev); > + > ret = mmc_add_host(mmc); > if (ret) > goto error_free_dma; > @@ -1418,6 +1430,7 @@ static int sunxi_mmc_remove(struct platform_device *pdev) > struct sunxi_mmc_host *host = mmc_priv(mmc); > > mmc_remove_host(mmc); > + pm_runtime_force_suspend(&pdev->dev); > disable_irq(host->irq); > sunxi_mmc_disable(host); > dma_free_coherent(&pdev->dev, PAGE_SIZE, host->sg_cpu, host->sg_dma); > @@ -1426,10 +1439,43 @@ static int sunxi_mmc_remove(struct platform_device *pdev) > return 0; > } > > +static int sunxi_mmc_runtime_resume(struct device *dev) > +{ > + struct mmc_host *mmc = dev_get_drvdata(dev); > + struct sunxi_mmc_host *host = mmc_priv(mmc); > + int ret; > + > + ret = sunxi_mmc_enable(host); > + if (ret) > + return ret; > + > + sunxi_mmc_power_up(mmc, &mmc->ios); Instead of doing power up, you may need restore some ios settings, such as the clock rate for example. You may also need to restore some registers in sunxi device, in case it's possible that the controller loses context at runtime suspend. > + > + return 0; > +} > + > +static int sunxi_mmc_runtime_suspend(struct device *dev) > +{ > + struct mmc_host *mmc = dev_get_drvdata(dev); > + struct sunxi_mmc_host *host = mmc_priv(mmc); > + > + sunxi_mmc_power_off(mmc, &mmc->ios); > + sunxi_mmc_disable(host); > + > + return 0; > +} > + > +static const struct dev_pm_ops sunxi_mmc_pm_ops = { > + SET_RUNTIME_PM_OPS(sunxi_mmc_runtime_suspend, > + sunxi_mmc_runtime_resume, > + NULL) > +}; > + > static struct platform_driver sunxi_mmc_driver = { > .driver = { > .name = "sunxi-mmc", > .of_match_table = of_match_ptr(sunxi_mmc_of_match), > + .pm = &sunxi_mmc_pm_ops, > }, > .probe = sunxi_mmc_probe, > .remove = sunxi_mmc_remove, > -- > git-series 0.9.1 Otherwise, overall, this looks rather okay to me. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ulf, On Thu, Mar 15, 2018 at 10:30:54AM +0100, Ulf Hansson wrote: > On 8 March 2018 at 15:52, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > So far, even if our card was not in use, we didn't shut down our main > > clock, which meant that it was still output on the MMC bus. > > > > While this obviously means that we could save some power there, it also > > created issues when it comes with EMC control since we'll have a perfect > > peak at the card clock rate. > > > > Let's implement runtime_pm with autosuspend so that we will shut down the > > clock when it's not been in use for quite some time. > > > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> > > --- > > drivers/mmc/host/sunxi-mmc.c | 46 +++++++++++++++++++++++++++++++++++++- > > 1 file changed, 46 insertions(+) > > > > diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c > > index f6374066081b..0f98a5fcaade 100644 > > --- a/drivers/mmc/host/sunxi-mmc.c > > +++ b/drivers/mmc/host/sunxi-mmc.c > > @@ -35,6 +35,7 @@ > > #include <linux/of_gpio.h> > > #include <linux/of_platform.h> > > #include <linux/platform_device.h> > > +#include <linux/pm_runtime.h> > > #include <linux/regulator/consumer.h> > > #include <linux/reset.h> > > #include <linux/scatterlist.h> > > @@ -973,6 +974,9 @@ static void sunxi_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable) > > unsigned long flags; > > u32 imask; > > > > + if (enable) > > + pm_runtime_get_noresume(host->dev); > > + > > spin_lock_irqsave(&host->lock, flags); > > > > imask = mmc_readl(host, REG_IMASK); > > @@ -985,6 +989,9 @@ static void sunxi_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable) > > } > > mmc_writel(host, REG_IMASK, imask); > > spin_unlock_irqrestore(&host->lock, flags); > > + > > + if (!enable) > > + pm_runtime_put_noidle(host->mmc->parent); > > } > > > > static void sunxi_mmc_hw_reset(struct mmc_host *mmc) > > @@ -1398,6 +1405,11 @@ static int sunxi_mmc_probe(struct platform_device *pdev) > > if (ret) > > goto error_free_dma; > > > > + pm_runtime_set_active(&pdev->dev); > > + pm_runtime_set_autosuspend_delay(&pdev->dev, 50); > > + pm_runtime_use_autosuspend(&pdev->dev); > > + pm_runtime_enable(&pdev->dev); > > + > > ret = mmc_add_host(mmc); > > if (ret) > > goto error_free_dma; > > @@ -1418,6 +1430,7 @@ static int sunxi_mmc_remove(struct platform_device *pdev) > > struct sunxi_mmc_host *host = mmc_priv(mmc); > > > > mmc_remove_host(mmc); > > + pm_runtime_force_suspend(&pdev->dev); > > disable_irq(host->irq); > > sunxi_mmc_disable(host); > > dma_free_coherent(&pdev->dev, PAGE_SIZE, host->sg_cpu, host->sg_dma); > > @@ -1426,10 +1439,43 @@ static int sunxi_mmc_remove(struct platform_device *pdev) > > return 0; > > } > > > > +static int sunxi_mmc_runtime_resume(struct device *dev) > > +{ > > + struct mmc_host *mmc = dev_get_drvdata(dev); > > + struct sunxi_mmc_host *host = mmc_priv(mmc); > > + int ret; > > + > > + ret = sunxi_mmc_enable(host); > > + if (ret) > > + return ret; > > + > > + sunxi_mmc_power_up(mmc, &mmc->ios); > > Instead of doing power up, you may need restore some ios settings, > such as the clock rate for example. > > You may also need to restore some registers in sunxi device, in case > it's possible that the controller loses context at runtime suspend. The thing I was precisely trying to avoid was this :) I could save and restore registers when the MMC controller is put into suspend, but that's pretty much exactly the same sequence than what is done in the MMC_POWER_UP sequence in .set_ios. So it just felt cleaner to just do the power_up sequence at resume time. It avoids having to maintain the list of registers to save and restore, and it involves less code overall. It suprised me a bit that the core will not call .set_ios with MMC_POWER_UP at resume by itself, but I guess there's a good reason for that :) Thanks! Maxime
On 15 March 2018 at 11:04, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > Hi Ulf, > > On Thu, Mar 15, 2018 at 10:30:54AM +0100, Ulf Hansson wrote: >> On 8 March 2018 at 15:52, Maxime Ripard <maxime.ripard@bootlin.com> wrote: >> > So far, even if our card was not in use, we didn't shut down our main >> > clock, which meant that it was still output on the MMC bus. >> > >> > While this obviously means that we could save some power there, it also >> > created issues when it comes with EMC control since we'll have a perfect >> > peak at the card clock rate. >> > >> > Let's implement runtime_pm with autosuspend so that we will shut down the >> > clock when it's not been in use for quite some time. >> > >> > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> >> > --- >> > drivers/mmc/host/sunxi-mmc.c | 46 +++++++++++++++++++++++++++++++++++++- >> > 1 file changed, 46 insertions(+) >> > >> > diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c >> > index f6374066081b..0f98a5fcaade 100644 >> > --- a/drivers/mmc/host/sunxi-mmc.c >> > +++ b/drivers/mmc/host/sunxi-mmc.c >> > @@ -35,6 +35,7 @@ >> > #include <linux/of_gpio.h> >> > #include <linux/of_platform.h> >> > #include <linux/platform_device.h> >> > +#include <linux/pm_runtime.h> >> > #include <linux/regulator/consumer.h> >> > #include <linux/reset.h> >> > #include <linux/scatterlist.h> >> > @@ -973,6 +974,9 @@ static void sunxi_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable) >> > unsigned long flags; >> > u32 imask; >> > >> > + if (enable) >> > + pm_runtime_get_noresume(host->dev); >> > + >> > spin_lock_irqsave(&host->lock, flags); >> > >> > imask = mmc_readl(host, REG_IMASK); >> > @@ -985,6 +989,9 @@ static void sunxi_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable) >> > } >> > mmc_writel(host, REG_IMASK, imask); >> > spin_unlock_irqrestore(&host->lock, flags); >> > + >> > + if (!enable) >> > + pm_runtime_put_noidle(host->mmc->parent); >> > } >> > >> > static void sunxi_mmc_hw_reset(struct mmc_host *mmc) >> > @@ -1398,6 +1405,11 @@ static int sunxi_mmc_probe(struct platform_device *pdev) >> > if (ret) >> > goto error_free_dma; >> > >> > + pm_runtime_set_active(&pdev->dev); >> > + pm_runtime_set_autosuspend_delay(&pdev->dev, 50); >> > + pm_runtime_use_autosuspend(&pdev->dev); >> > + pm_runtime_enable(&pdev->dev); >> > + >> > ret = mmc_add_host(mmc); >> > if (ret) >> > goto error_free_dma; >> > @@ -1418,6 +1430,7 @@ static int sunxi_mmc_remove(struct platform_device *pdev) >> > struct sunxi_mmc_host *host = mmc_priv(mmc); >> > >> > mmc_remove_host(mmc); >> > + pm_runtime_force_suspend(&pdev->dev); >> > disable_irq(host->irq); >> > sunxi_mmc_disable(host); >> > dma_free_coherent(&pdev->dev, PAGE_SIZE, host->sg_cpu, host->sg_dma); >> > @@ -1426,10 +1439,43 @@ static int sunxi_mmc_remove(struct platform_device *pdev) >> > return 0; >> > } >> > >> > +static int sunxi_mmc_runtime_resume(struct device *dev) >> > +{ >> > + struct mmc_host *mmc = dev_get_drvdata(dev); >> > + struct sunxi_mmc_host *host = mmc_priv(mmc); >> > + int ret; >> > + >> > + ret = sunxi_mmc_enable(host); >> > + if (ret) >> > + return ret; >> > + >> > + sunxi_mmc_power_up(mmc, &mmc->ios); >> >> Instead of doing power up, you may need restore some ios settings, >> such as the clock rate for example. >> >> You may also need to restore some registers in sunxi device, in case >> it's possible that the controller loses context at runtime suspend. > > The thing I was precisely trying to avoid was this :) > > I could save and restore registers when the MMC controller is put into > suspend, but that's pretty much exactly the same sequence than what is > done in the MMC_POWER_UP sequence in .set_ios. Well, there may be some overlap. > > So it just felt cleaner to just do the power_up sequence at resume > time. It avoids having to maintain the list of registers to save and > restore, and it involves less code overall. I understand. > > It suprised me a bit that the core will not call .set_ios with > MMC_POWER_UP at resume by itself, but I guess there's a good reason > for that :) It does that when it runtime PM manages the mmc/sd/sdio card, don't confuse that with the mmc host. That's because it needs to follow the (e)MMC/SD/SDIO spec, which don't allows you to just cut the power to card without first informing (sending commands) the card about it. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Thu, Mar 15, 2018 at 11:24:36AM +0100, Ulf Hansson wrote: > >> > +static int sunxi_mmc_runtime_resume(struct device *dev) > >> > +{ > >> > + struct mmc_host *mmc = dev_get_drvdata(dev); > >> > + struct sunxi_mmc_host *host = mmc_priv(mmc); > >> > + int ret; > >> > + > >> > + ret = sunxi_mmc_enable(host); > >> > + if (ret) > >> > + return ret; > >> > + > >> > + sunxi_mmc_power_up(mmc, &mmc->ios); > >> > >> Instead of doing power up, you may need restore some ios settings, > >> such as the clock rate for example. > >> > >> You may also need to restore some registers in sunxi device, in case > >> it's possible that the controller loses context at runtime suspend. > > > > The thing I was precisely trying to avoid was this :) > > > > I could save and restore registers when the MMC controller is put into > > suspend, but that's pretty much exactly the same sequence than what is > > done in the MMC_POWER_UP sequence in .set_ios. > > Well, there may be some overlap. > > > > > So it just felt cleaner to just do the power_up sequence at resume > > time. It avoids having to maintain the list of registers to save and > > restore, and it involves less code overall. > > I understand. > > > > > It suprised me a bit that the core will not call .set_ios with > > MMC_POWER_UP at resume by itself, but I guess there's a good reason > > for that :) > > It does that when it runtime PM manages the mmc/sd/sdio card, don't > confuse that with the mmc host. That's because it needs to follow the > (e)MMC/SD/SDIO spec, which don't allows you to just cut the power to > card without first informing (sending commands) the card about it. Ok. So this might sound a bit trivial, but I'm confused now about what set_ios is supposed to be doing. I thought this was about the MMC controller itself, but from what you're saying, it seems that it is used for both the MMC controller and the MMC card itself, with the powermode being about the MMC card, and the rest about the MMC controller. I guess we're mixing the two then, especially the power off part that will also reset the controller, or the controller that is initialised only at power on. Other MMC controller drivers I could find were doing the MMC controller setup unconditionally, so I guess we have room for improvements here. Maxime
On 15 March 2018 at 13:04, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > Hi, > > On Thu, Mar 15, 2018 at 11:24:36AM +0100, Ulf Hansson wrote: >> >> > +static int sunxi_mmc_runtime_resume(struct device *dev) >> >> > +{ >> >> > + struct mmc_host *mmc = dev_get_drvdata(dev); >> >> > + struct sunxi_mmc_host *host = mmc_priv(mmc); >> >> > + int ret; >> >> > + >> >> > + ret = sunxi_mmc_enable(host); >> >> > + if (ret) >> >> > + return ret; >> >> > + >> >> > + sunxi_mmc_power_up(mmc, &mmc->ios); >> >> >> >> Instead of doing power up, you may need restore some ios settings, >> >> such as the clock rate for example. >> >> >> >> You may also need to restore some registers in sunxi device, in case >> >> it's possible that the controller loses context at runtime suspend. >> > >> > The thing I was precisely trying to avoid was this :) >> > >> > I could save and restore registers when the MMC controller is put into >> > suspend, but that's pretty much exactly the same sequence than what is >> > done in the MMC_POWER_UP sequence in .set_ios. >> >> Well, there may be some overlap. >> >> > >> > So it just felt cleaner to just do the power_up sequence at resume >> > time. It avoids having to maintain the list of registers to save and >> > restore, and it involves less code overall. >> >> I understand. >> >> > >> > It suprised me a bit that the core will not call .set_ios with >> > MMC_POWER_UP at resume by itself, but I guess there's a good reason >> > for that :) >> >> It does that when it runtime PM manages the mmc/sd/sdio card, don't >> confuse that with the mmc host. That's because it needs to follow the >> (e)MMC/SD/SDIO spec, which don't allows you to just cut the power to >> card without first informing (sending commands) the card about it. > > Ok. So this might sound a bit trivial, but I'm confused now about what > set_ios is supposed to be doing. > > I thought this was about the MMC controller itself, but from what > you're saying, it seems that it is used for both the MMC controller > and the MMC card itself, with the powermode being about the MMC card, > and the rest about the MMC controller. Correct. The ->set_ios() ops, is an interface called by the core at points when it wants to change some settings for the card. Those settings may or may not involve changes to internal controller registers, depending on the controller/SoC. For example, some mmc controllers have build in support for changing (dividing) the clock rate. While others may rely solely on other separate clock providers. Same goes for powering the mmc card, while I would say it more common to use external regulators controlled by the regulator APIs, some actually have internal registers needed be change to provide power to the card. You may have a look at mmci.c, it has these kind of things. > > I guess we're mixing the two then, especially the power off part that > will also reset the controller, or the controller that is initialised If you need to reset the controller at runtime resume of the controller device, that's controller and SoC specific. > only at power on. Other MMC controller drivers I could find were doing > the MMC controller setup unconditionally, so I guess we have room for > improvements here. Yeah, perhaps they are better safe than sorry. But again, it's device and SoC specific. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ulf, On Thu, Mar 15, 2018 at 01:40:41PM +0100, Ulf Hansson wrote: > On 15 March 2018 at 13:04, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > Hi, > > > > On Thu, Mar 15, 2018 at 11:24:36AM +0100, Ulf Hansson wrote: > >> >> > +static int sunxi_mmc_runtime_resume(struct device *dev) > >> >> > +{ > >> >> > + struct mmc_host *mmc = dev_get_drvdata(dev); > >> >> > + struct sunxi_mmc_host *host = mmc_priv(mmc); > >> >> > + int ret; > >> >> > + > >> >> > + ret = sunxi_mmc_enable(host); > >> >> > + if (ret) > >> >> > + return ret; > >> >> > + > >> >> > + sunxi_mmc_power_up(mmc, &mmc->ios); > >> >> > >> >> Instead of doing power up, you may need restore some ios settings, > >> >> such as the clock rate for example. > >> >> > >> >> You may also need to restore some registers in sunxi device, in case > >> >> it's possible that the controller loses context at runtime suspend. > >> > > >> > The thing I was precisely trying to avoid was this :) > >> > > >> > I could save and restore registers when the MMC controller is put into > >> > suspend, but that's pretty much exactly the same sequence than what is > >> > done in the MMC_POWER_UP sequence in .set_ios. > >> > >> Well, there may be some overlap. > >> > >> > > >> > So it just felt cleaner to just do the power_up sequence at resume > >> > time. It avoids having to maintain the list of registers to save and > >> > restore, and it involves less code overall. > >> > >> I understand. > >> > >> > > >> > It suprised me a bit that the core will not call .set_ios with > >> > MMC_POWER_UP at resume by itself, but I guess there's a good reason > >> > for that :) > >> > >> It does that when it runtime PM manages the mmc/sd/sdio card, don't > >> confuse that with the mmc host. That's because it needs to follow the > >> (e)MMC/SD/SDIO spec, which don't allows you to just cut the power to > >> card without first informing (sending commands) the card about it. > > > > Ok. So this might sound a bit trivial, but I'm confused now about what > > set_ios is supposed to be doing. > > > > I thought this was about the MMC controller itself, but from what > > you're saying, it seems that it is used for both the MMC controller > > and the MMC card itself, with the powermode being about the MMC card, > > and the rest about the MMC controller. > > Correct. > > The ->set_ios() ops, is an interface called by the core at points when > it wants to change some settings for the card. Those settings may or > may not involve changes to internal controller registers, depending on > the controller/SoC. > > For example, some mmc controllers have build in support for changing > (dividing) the clock rate. While others may rely solely on other > separate clock providers. > > Same goes for powering the mmc card, while I would say it more common > to use external regulators controlled by the regulator APIs, some > actually have internal registers needed be change to provide power to > the card. > > You may have a look at mmci.c, it has these kind of things. So I guess if I split out the MMC controller initialization into separate functions, called in both the set_ios callback and the runtime_resume path, that would be ok for you? > > > > I guess we're mixing the two then, especially the power off part that > > will also reset the controller, or the controller that is initialised > > If you need to reset the controller at runtime resume of the > controller device, that's controller and SoC specific. I was talking about: https://elixir.bootlin.com/linux/v4.16-rc6/source/drivers/mmc/host/sunxi-mmc.c#L899 Ie. in set_ios, with MMC_POWER_DOWN, not during runtime_resume. I guess from what you were telling me that it's not really advisable? > > only at power on. Other MMC controller drivers I could find were doing > > the MMC controller setup unconditionally, so I guess we have room for > > improvements here. > > Yeah, perhaps they are better safe than sorry. But again, it's device > and SoC specific. Ok. Thanks! Maxime
diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c index f6374066081b..0f98a5fcaade 100644 --- a/drivers/mmc/host/sunxi-mmc.c +++ b/drivers/mmc/host/sunxi-mmc.c @@ -35,6 +35,7 @@ #include <linux/of_gpio.h> #include <linux/of_platform.h> #include <linux/platform_device.h> +#include <linux/pm_runtime.h> #include <linux/regulator/consumer.h> #include <linux/reset.h> #include <linux/scatterlist.h> @@ -973,6 +974,9 @@ static void sunxi_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable) unsigned long flags; u32 imask; + if (enable) + pm_runtime_get_noresume(host->dev); + spin_lock_irqsave(&host->lock, flags); imask = mmc_readl(host, REG_IMASK); @@ -985,6 +989,9 @@ static void sunxi_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable) } mmc_writel(host, REG_IMASK, imask); spin_unlock_irqrestore(&host->lock, flags); + + if (!enable) + pm_runtime_put_noidle(host->mmc->parent); } static void sunxi_mmc_hw_reset(struct mmc_host *mmc) @@ -1398,6 +1405,11 @@ static int sunxi_mmc_probe(struct platform_device *pdev) if (ret) goto error_free_dma; + pm_runtime_set_active(&pdev->dev); + pm_runtime_set_autosuspend_delay(&pdev->dev, 50); + pm_runtime_use_autosuspend(&pdev->dev); + pm_runtime_enable(&pdev->dev); + ret = mmc_add_host(mmc); if (ret) goto error_free_dma; @@ -1418,6 +1430,7 @@ static int sunxi_mmc_remove(struct platform_device *pdev) struct sunxi_mmc_host *host = mmc_priv(mmc); mmc_remove_host(mmc); + pm_runtime_force_suspend(&pdev->dev); disable_irq(host->irq); sunxi_mmc_disable(host); dma_free_coherent(&pdev->dev, PAGE_SIZE, host->sg_cpu, host->sg_dma); @@ -1426,10 +1439,43 @@ static int sunxi_mmc_remove(struct platform_device *pdev) return 0; } +static int sunxi_mmc_runtime_resume(struct device *dev) +{ + struct mmc_host *mmc = dev_get_drvdata(dev); + struct sunxi_mmc_host *host = mmc_priv(mmc); + int ret; + + ret = sunxi_mmc_enable(host); + if (ret) + return ret; + + sunxi_mmc_power_up(mmc, &mmc->ios); + + return 0; +} + +static int sunxi_mmc_runtime_suspend(struct device *dev) +{ + struct mmc_host *mmc = dev_get_drvdata(dev); + struct sunxi_mmc_host *host = mmc_priv(mmc); + + sunxi_mmc_power_off(mmc, &mmc->ios); + sunxi_mmc_disable(host); + + return 0; +} + +static const struct dev_pm_ops sunxi_mmc_pm_ops = { + SET_RUNTIME_PM_OPS(sunxi_mmc_runtime_suspend, + sunxi_mmc_runtime_resume, + NULL) +}; + static struct platform_driver sunxi_mmc_driver = { .driver = { .name = "sunxi-mmc", .of_match_table = of_match_ptr(sunxi_mmc_of_match), + .pm = &sunxi_mmc_pm_ops, }, .probe = sunxi_mmc_probe, .remove = sunxi_mmc_remove,
So far, even if our card was not in use, we didn't shut down our main clock, which meant that it was still output on the MMC bus. While this obviously means that we could save some power there, it also created issues when it comes with EMC control since we'll have a perfect peak at the card clock rate. Let's implement runtime_pm with autosuspend so that we will shut down the clock when it's not been in use for quite some time. Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> --- drivers/mmc/host/sunxi-mmc.c | 46 +++++++++++++++++++++++++++++++++++++- 1 file changed, 46 insertions(+)