RE: [PATCH] mmc: renesas-sdhi: Disable clock after tuning reset when possible

["Jaehoon Chung" <jh80.chung@samsung.com>]

> -----Original Message-----
> From: Marek Vasut <marek.vasut+renesas@mailbox.org>
> Sent: Monday, November 6, 2023 7:43 AM
> To: u-boot@lists.denx.de
> Cc: Marek Vasut <marek.vasut+renesas@mailbox.org>; Jaehoon Chung <jh80.chung@samsung.com>; Nobuhiro
> Iwamatsu <iwamatsu@nigauri.org>; Paul Barker <paul.barker.ct@bp.renesas.com>; Peng Fan
> <peng.fan@nxp.com>
> Subject: [PATCH] mmc: renesas-sdhi: Disable clock after tuning reset when possible
> 
> Currently the renesas_sdhi_reset_tuning() unconditionally leaves SDHI
> clock enabled after the tuning reset. This is not always necessary.
> 
> After the driver performed tuning reset at the end of probe function,
> or in the unlikely case that tuning failed during regular operation,
> the SDHI clock can be disabled after the tuning reset. The following
> set_ios call would reconfigure the clock as needed.
> 
> In case of regular set_ios call which requires a tuning reset, keep
> the clock enabled or disabled according to the mmc->clk_disable state.
> 
> With this in place, the controllers which have not been accessed via
> block subsystem after boot are left in quiescent state. However, if an
> MMC device is used e.g. for environment storage, that controller would
> be accessed during the environment load and left active, including its
> clock which would still be generated. This is due to the design of the
> MMC subsystem, which does not deinit a controller after it was started
> once, the controller is only deinited in case of mmc rescan, or before
> OS boot.
> 
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>

Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

> ---
> Note: To address the part where MMC device has been inited once and
>       is never deinited until rescan or OS boot, it would likely be
>       necessary to implement something like runtime PM, possibly
>       based on the cyclic framework. Basically, keep track of when
>       the MMC was accessed last, and if certain time elapsed, deinit
>       the MMC. This could also be used to handle card detect polling
>       at the same time.
> ---
> Cc: Jaehoon Chung <jh80.chung@samsung.com>
> Cc: Nobuhiro Iwamatsu <iwamatsu@nigauri.org>
> Cc: Paul Barker <paul.barker.ct@bp.renesas.com>
> Cc: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/mmc/renesas-sdhi.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/renesas-sdhi.c b/drivers/mmc/renesas-sdhi.c
> index 8cd501c5f7c..97aaf1e4ec3 100644
> --- a/drivers/mmc/renesas-sdhi.c
> +++ b/drivers/mmc/renesas-sdhi.c
> @@ -318,7 +318,7 @@ static unsigned int renesas_sdhi_init_tuning(struct tmio_sd_priv *priv)
>  		RENESAS_SDHI_SCC_DTCNTL_TAPNUM_MASK;
>  }
> 
> -static void renesas_sdhi_reset_tuning(struct tmio_sd_priv *priv)
> +static void renesas_sdhi_reset_tuning(struct tmio_sd_priv *priv, bool clk_disable)
>  {
>  	u32 reg;
> 
> @@ -350,6 +350,12 @@ static void renesas_sdhi_reset_tuning(struct tmio_sd_priv *priv)
>  	reg = tmio_sd_readl(priv, RENESAS_SDHI_SCC_RVSCNTL);
>  	reg &= ~RENESAS_SDHI_SCC_RVSCNTL_RVSEN;
>  	tmio_sd_writel(priv, reg, RENESAS_SDHI_SCC_RVSCNTL);
> +
> +	if (clk_disable) {
> +		reg = tmio_sd_readl(priv, TMIO_SD_CLKCTL);
> +		reg &= ~TMIO_SD_CLKCTL_SCLKEN;
> +		tmio_sd_writel(priv, reg, TMIO_SD_CLKCTL);
> +	}
>  }
> 
>  static int renesas_sdhi_hs400(struct udevice *dev)
> @@ -629,7 +635,7 @@ int renesas_sdhi_execute_tuning(struct udevice *dev, uint opcode)
>  out:
>  	if (ret < 0) {
>  		dev_warn(dev, "Tuning procedure failed\n");
> -		renesas_sdhi_reset_tuning(priv);
> +		renesas_sdhi_reset_tuning(priv, true);
>  	}
> 
>  	return ret;
> @@ -668,7 +674,7 @@ static int renesas_sdhi_set_ios(struct udevice *dev)
>  	    (mmc->selected_mode != UHS_SDR104) &&
>  	    (mmc->selected_mode != MMC_HS_200) &&
>  	    (mmc->selected_mode != MMC_HS_400)) {
> -		renesas_sdhi_reset_tuning(priv);
> +		renesas_sdhi_reset_tuning(priv, mmc->clk_disable);
>  	}
>  #endif
> 
> @@ -1095,7 +1101,7 @@ static int renesas_sdhi_probe(struct udevice *dev)
>      CONFIG_IS_ENABLED(MMC_HS200_SUPPORT) || \
>      CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
>  	if (priv->caps & TMIO_SD_CAP_RCAR_UHS)
> -		renesas_sdhi_reset_tuning(priv);
> +		renesas_sdhi_reset_tuning(priv, true);
>  #endif
>  	return 0;
> 
> --
> 2.42.0