[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 --- 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;
-----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
On 11/6/23 11:24, Jaehoon Chung wrote:
-----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
Thanks. Do you want to take this via mmc tree or shall I take it via sh tree ? I wouldn't mind the later, since it is isolated to renesas platforms.
On Sun, Nov 05, 2023 at 11:42:45PM +0100, Marek Vasut wrote:
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
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;
Reviewed-by: Paul Barker paul.barker.ct@bp.renesas.com Tested-by: Paul Barker paul.barker.ct@bp.renesas.com (on RZ/G2L SMARC EVK)
Thanks, Paul
Hello Marek-san,
From: U-Boot On Behalf Of Marek Vasut, Sent: Monday, November 6, 2023 7:43 AM
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
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
Thank you for the patch! This patch looks good to me. So,
Reviewed-by: Yoshihiro Shimoda yoshihiro.shimoda.uh@renesas.com
Also, our outsourcing member tested this patch on R-Car H3 Salvator-XS And it works without any regression. So,
Tested-by: Thuan Nguyen Hong thuan.nguyen-hong@banvien.com.vn
Best regards, Yoshihiro Shimoda
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
participants (5)
-
Jaehoon Chung -
Marek Vasut -
Marek Vasut -
Paul Barker -
Yoshihiro Shimoda