[PATCH V2 1/3] mmc: sdhci-esdhc-imx: Manage sdhci_runtime_suspend_host error code

[PATCH V2 1/3] mmc: sdhci-esdhc-imx: Manage sdhci_runtime_suspend_host error code

[Michael Trimarchi]

We need to return in case of error even if the actual implementation
of sdhci_runtime_suspend_host always return 0. We don't want to
power down the clock and the assuption is that the sdhci_runtime_suspend_host
always let the system consistent in case of failure

Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
---
Changes None

---
 drivers/mmc/host/sdhci-esdhc-imx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 85140c9..d08c21e 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -1389,6 +1389,8 @@ static int sdhci_esdhc_runtime_suspend(struct device *dev)
 	int ret;
 
 	ret = sdhci_runtime_suspend_host(host);
+	if (ret)
+		return ret;
 
 	if (host->tuning_mode != SDHCI_TUNING_MODE_3)
 		mmc_retune_needed(host->mmc);
-- 
2.7.4

[PATCH 2/3] mmc: sdhci-esdhc-imx: Changes the order of how clocks are being re-enabled

[Michael Trimarchi]

runtime_resume() should re-enable the clocks in reverse order comparing with
runtime_suspend()

Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index d08c21e..6d4e323 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -1411,31 +1411,33 @@ 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;
+
 	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;
 	}
-	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

[PATCH 3/3] mmc: sdhci-esdhc-imx: Enable/Disable mmc clock during runtime suspend

[Michael Trimarchi]

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@amarulasolutions.com>
---
Changes V2->V3:
	- move clock re-order on a separated patch

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 | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 6d4e323..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);
 	}
@@ -1422,6 +1425,7 @@ static int sdhci_esdhc_runtime_resume(struct device *dev)
 		err = clk_prepare_enable(imx_data->clk_ipg);
 		if (err)
 			goto disable_per_clk;
+		esdhc_pltfm_set_clock(host, imx_data->actual_clock);
 	}
 
 	err = sdhci_runtime_resume_host(host);
-- 
2.7.4

Re: [PATCH V2 1/3] mmc: sdhci-esdhc-imx: Manage sdhci_runtime_suspend_host error code

[Ulf Hansson]

On 4 January 2018 at 16:30, Michael Trimarchi
<michael@amarulasolutions.com> wrote:
> We need to return in case of error even if the actual implementation
> of sdhci_runtime_suspend_host always return 0. We don't want to
> power down the clock and the assuption is that the sdhci_runtime_suspend_host
> always let the system consistent in case of failure
>
> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>

Thanks, applied for next!

Adrian, please tell me if you are not happy with this.

Kind regards
Uffe

> ---
> Changes None
>
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 85140c9..d08c21e 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -1389,6 +1389,8 @@ static int sdhci_esdhc_runtime_suspend(struct device *dev)
>         int ret;
>
>         ret = sdhci_runtime_suspend_host(host);
> +       if (ret)
> +               return ret;
>
>         if (host->tuning_mode != SDHCI_TUNING_MODE_3)
>                 mmc_retune_needed(host->mmc);
> --
> 2.7.4
>

Re: [PATCH 2/3] mmc: sdhci-esdhc-imx: Changes the order of how clocks are being re-enabled

[Ulf Hansson]

On 4 January 2018 at 16:30, Michael Trimarchi
<michael@amarulasolutions.com> wrote:
> runtime_resume() should re-enable the clocks in reverse order comparing with
> runtime_suspend()
>
> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>

Thanks, applied for next!

Adrian, please tell me if you are not happy with this.

Kind regards
Uffe


> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index d08c21e..6d4e323 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -1411,31 +1411,33 @@ 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;
> +
>         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;
>         }
> -       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
>

Re: [PATCH 3/3] mmc: sdhci-esdhc-imx: Enable/Disable mmc clock during runtime suspend

[Ulf Hansson]

On 4 January 2018 at 16:30, Michael Trimarchi
<michael@amarulasolutions.com> 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@amarulasolutions.com>

Thanks, applied for next!

Adrian, please tell me if you are not happy with this.

Kind regards
Uffe


> ---
> Changes V2->V3:
>         - move clock re-order on a separated patch
>
> 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 | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 6d4e323..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);
>         }
> @@ -1422,6 +1425,7 @@ static int sdhci_esdhc_runtime_resume(struct device *dev)
>                 err = clk_prepare_enable(imx_data->clk_ipg);
>                 if (err)
>                         goto disable_per_clk;
> +               esdhc_pltfm_set_clock(host, imx_data->actual_clock);
>         }
>
>         err = sdhci_runtime_resume_host(host);
> --
> 2.7.4
>