On 4 January 2018 at 14:58, Michael Trimarchi
<michael@xxxxxxxxxxxxxxxxxxxx> wrote:
> mmc clock can be stopped during runtime suspend and restart during runtime
> resume if the sdio irq is not enabled. Stop sdio clock reduce EMI of
> the device when the bus is not in use.
>
> Signed-off-by: Michael Trimarchi <michael@xxxxxxxxxxxxxxxxxxxx>
> ---
> Changes V1->V2:
> - rebase to latest linux
> - address sdio irq wakeup
> - move the clock enable clk_ahb up to be balance
> with the runtime suspend function and to make
> function more clean by the end without two if
> condition
> ---
> drivers/mmc/host/sdhci-esdhc-imx.c | 20 +++++++++++++-------
> 1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index d08c21e..53cc1b6 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -193,6 +193,7 @@ struct pltfm_imx_data {
> struct clk *clk_ipg;
> struct clk *clk_ahb;
> struct clk *clk_per;
> + unsigned int actual_clock;
> enum {
> NO_CMD_PENDING, /* no multiblock command pending */
> MULTIBLK_IN_PROCESS, /* exact multiblock cmd in process */
> @@ -1396,6 +1397,8 @@ static int sdhci_esdhc_runtime_suspend(struct device *dev)
> mmc_retune_needed(host->mmc);
>
> if (!sdhci_sdio_irq_enabled(host)) {
> + imx_data->actual_clock = host->mmc->actual_clock;
> + esdhc_pltfm_set_clock(host, 0);
> clk_disable_unprepare(imx_data->clk_per);
> clk_disable_unprepare(imx_data->clk_ipg);
> }
> @@ -1411,31 +1414,34 @@ static int sdhci_esdhc_runtime_resume(struct device *dev)
> struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
> int err;
>
> + err = clk_prepare_enable(imx_data->clk_ahb);
> + if (err)
> + return err;
> +
So this changes the order of how clocks are being re-enabled. I think
the change makes sense, as normally we do things in the reverse order
as compared what is being done during ->runtime_suspend().
However, I think you should have this being a separate patch,
preceding the one that actually calls esdhc_pltfm_set_clock() in in
the runtime PM callbacks.
> if (!sdhci_sdio_irq_enabled(host)) {
> err = clk_prepare_enable(imx_data->clk_per);
> if (err)
> - return err;
> + goto disable_ahb_clk;
> err = clk_prepare_enable(imx_data->clk_ipg);
> if (err)
> goto disable_per_clk;
> + esdhc_pltfm_set_clock(host, imx_data->actual_clock);
> }
> - err = clk_prepare_enable(imx_data->clk_ahb);
> - if (err)
> - goto disable_ipg_clk;
> +
> err = sdhci_runtime_resume_host(host);
> if (err)
> - goto disable_ahb_clk;
> + goto disable_ipg_clk;
>
> return 0;
>
> -disable_ahb_clk:
> - clk_disable_unprepare(imx_data->clk_ahb);
> disable_ipg_clk:
> if (!sdhci_sdio_irq_enabled(host))
> clk_disable_unprepare(imx_data->clk_ipg);
> disable_per_clk:
> if (!sdhci_sdio_irq_enabled(host))
> clk_disable_unprepare(imx_data->clk_per);
> +disable_ahb_clk:
> + clk_disable_unprepare(imx_data->clk_ahb);
> return err;
> }
> #endif
> --
> 2.7.4
>
Otherwise this looks good to me.
Kind regards
Uffe
--
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
