Re: [PATCH 1/2] mmc: tmio: leave clock handling to PM if enabled

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 16 Jul 2019 at 17:01, Ulrich Hecht <uli+renesas@xxxxxxxx> wrote:
>
> This fixes a clock imbalance that occurs because the SD clock is handled
> by both PM and the hardware driver.
> See https://www.spinics.net/lists/linux-mmc/msg44431.html for details.

This is a generic problem, when a device are being attached to a genpd
and when the genpd has got the ->stop|start() callbacks assigned, as
to manage device clocks.

Can you try to describe this problem a little bit more in detail, as I
think that's important to carry in the change log.

>
> This patch removes the clock handling from the driver's PM callbacks and

runtime PM callbacks and/or system PM callbacks?

> turns the clock off after probing.
>
> Reported-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> Signed-off-by: Ulrich Hecht <uli+renesas@xxxxxxxx>
> ---
>  drivers/mmc/host/tmio_mmc_core.c | 24 ++++++------------------
>  1 file changed, 6 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
> index 31ffcc3..26dcbba 100644
> --- a/drivers/mmc/host/tmio_mmc_core.c
> +++ b/drivers/mmc/host/tmio_mmc_core.c
> @@ -1260,9 +1260,14 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host)
>         /* See if we also get DMA */
>         tmio_mmc_request_dma(_host, pdata);
>
> -       pm_runtime_set_active(&pdev->dev);
> +#ifdef CONFIG_PM
> +       /* PM handles the clock; disable it so it won't be enabled twice. */
> +       if (_host->clk_disable)
> +               _host->clk_disable(_host);

The clock managed here, is that the same clock as being managed by
genpd's ->stop|start() callbacks?

> +       pm_runtime_get_sync(&pdev->dev);
>         pm_runtime_set_autosuspend_delay(&pdev->dev, 50);
>         pm_runtime_use_autosuspend(&pdev->dev);
> +#endif

So what happens if you have CONFIG_PM set, but the device doesn't have
a genpd attached?

I am guessing the driver should handle the clock in such scenario, right?

>
>         ret = mmc_add_host(mmc);
>         if (ret)
> @@ -1302,20 +1307,6 @@ void tmio_mmc_host_remove(struct tmio_mmc_host *host)
>  EXPORT_SYMBOL_GPL(tmio_mmc_host_remove);
>
>  #ifdef CONFIG_PM
> -static int tmio_mmc_clk_enable(struct tmio_mmc_host *host)
> -{
> -       if (!host->clk_enable)
> -               return -ENOTSUPP;
> -
> -       return host->clk_enable(host);
> -}
> -
> -static void tmio_mmc_clk_disable(struct tmio_mmc_host *host)
> -{
> -       if (host->clk_disable)
> -               host->clk_disable(host);
> -}
> -
>  int tmio_mmc_host_runtime_suspend(struct device *dev)
>  {
>         struct tmio_mmc_host *host = dev_get_drvdata(dev);
> @@ -1325,8 +1316,6 @@ int tmio_mmc_host_runtime_suspend(struct device *dev)
>         if (host->clk_cache)
>                 host->set_clock(host, 0);
>
> -       tmio_mmc_clk_disable(host);
> -
>         return 0;
>  }
>  EXPORT_SYMBOL_GPL(tmio_mmc_host_runtime_suspend);
> @@ -1340,7 +1329,6 @@ int tmio_mmc_host_runtime_resume(struct device *dev)
>  {
>         struct tmio_mmc_host *host = dev_get_drvdata(dev);
>
> -       tmio_mmc_clk_enable(host);
>         tmio_mmc_hw_reset(host->mmc);
>
>         if (host->clk_cache)
> --
> 2.7.4
>

Kind regards
Uffe



[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux