Re: [PATCH v3 6/7] mmc: sunxi: Add runtime_pm support

Marc Zyngier
2018-06-14T14:11:26Z

Hi Maxime,

On 16/04/18 15:23, Maxime Ripard wrote:
> So far, even if our card was not in use, we didn't shut down our MMC
> controller, which meant that it was still active and clocking the bus.
>
> While this obviously means that we could save some power there, it also
> creates issues when it comes to 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
> controller 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 | 48 +++++++++++++++++++++++++++++++++++++-
> 1 file changed, 48 insertions(+)
>
> diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
> index 0165da0d022a..0253deb153a4 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>
> @@ -969,6 +970,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);
> @@ -981,6 +985,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)
> @@ -1394,6 +1401,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;
> @@ -1414,6 +1426,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);
> @@ -1422,10 +1435,45 @@ 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_init_host(host);
> + sunxi_mmc_set_bus_width(host, mmc->ios.bus_width);
> + sunxi_mmc_set_clk(host, &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_reset_host(host);
> + 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,
>

This patch has the unfortunate impact of killing my A20 system
(cubietruck), as of 9a8e1e8cc2c02c57c4e941651a8481a633506c91:

[...]
[ 3.286649] NET: Registered protocol family 10
[ 3.291898] mmcblk0: p1
[ 3.295297] mmc1: queuing unknown CIS tuple 0x80 (2 bytes)
[ 3.302773] mmc1: queuing unknown CIS tuple 0x80 (3 bytes)
[ 3.305787] Segment Routing with IPv6
[ 3.312225] mip6: Mobile IPv6
[ 3.316166] sit: IPv6, IPv4 and MPLS over IPv4 tunneling driver
[ 3.316246] mmc1: queuing unknown CIS tuple 0x80 (3 bytes)
[ 3.323721] ip6_gre: GRE over IPv6 tunneling driver
[ 3.333954] NET: Registered protocol family 17
[ 3.338837] 9pnet: Installing 9P2000 support
[ 3.343379] NET: Registered protocol family 37
[ 3.347885] Key type dns_resolver registered
[ 3.352214] mmc1: queuing unknown CIS tuple 0x80 (7 bytes)
[ 3.352217] openvswitch: Open vSwitch switching datapath
[ 3.352620] mpls_gso: MPLS GSO support
[ 3.367001] ThumbEE CPU extension supported.

and that's where it stops. No message, just a hard lockup (I suppose
that both the CPUs are trying to access some device that is now not
clocked).

With a working kernel, I see SATA and the wifi SDIO being probed.

Happy to help testing stuff if you have any idea.

Thanks,

M.
--
Jazz is not dead. It just smells funny...