On Fri, May 15, 2020 at 04:04:59PM +0200, Ulf Hansson wrote:
> If the tmio device is attached to a genpd (PM domain), that genpd may have
> ->start|stop() callback assigned to it. To make sure the device is
> accessible during ->probe(), genpd's ->start() callback must be invoked,
> which is currently managed by tmio_mmc_host_probe(). This is very likely to
> be too late for some cases, as registers may be read and written way before
> that.
>
> To fix this behaviour, let's drop the call to dev_pm_domain_start() from
> tmio_mmc_host_probe() - and let the tmio clients manage this instead.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
Okay, this seems to work on Gen3.
> @@ -909,6 +910,8 @@ int renesas_sdhi_probe(struct platform_device *pdev,
> if (ret)
> goto efree;
>
> + dev_pm_domain_start(&pdev->dev);
> +
Can't we put it before the custom clk_enable()? And then clean up
further like this to have the main clock only controlled via RPM?
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -83,16 +83,11 @@ static int renesas_sdhi_clk_enable(struct tmio_mmc_host *host)
{
struct mmc_host *mmc = host->mmc;
struct renesas_sdhi *priv = host_to_priv(host);
- int ret = clk_prepare_enable(priv->clk);
-
- if (ret < 0)
- return ret;
+ int ret;
ret = clk_prepare_enable(priv->clk_cd);
- if (ret < 0) {
- clk_disable_unprepare(priv->clk);
+ if (ret < 0)
return ret;
- }
/*
* The clock driver may not know what maximum frequency
@@ -198,7 +193,6 @@ static void renesas_sdhi_clk_disable(struct tmio_mmc_host *host)
{
struct renesas_sdhi *priv = host_to_priv(host);
- clk_disable_unprepare(priv->clk);
clk_disable_unprepare(priv->clk_cd);
}
@@ -906,12 +900,12 @@ int renesas_sdhi_probe(struct platform_device *pdev,
/* All SDHI have SDIO status bits which must be 1 */
mmc_data->flags |= TMIO_MMC_SDIO_STATUS_SETBITS;
+ dev_pm_domain_start(&pdev->dev);
+
ret = renesas_sdhi_clk_enable(host);
if (ret)
goto efree;
- dev_pm_domain_start(&pdev->dev);
-
ver = sd_ctrl_read16(host, CTL_VERSION);
/* GEN2_SDR104 is first known SDHI to use 32bit block count */
if (ver < SDHI_VER_GEN2_SDR104 && mmc_data->max_blk_count > U16_MAX)
Again, this is only tested on Gen3. I will check Gen2 tomorrow.
> diff --git a/drivers/mmc/host/tmio_mmc.c b/drivers/mmc/host/tmio_mmc.c
> index 93e83ad25976..b8f5687e10be 100644
> --- a/drivers/mmc/host/tmio_mmc.c
> +++ b/drivers/mmc/host/tmio_mmc.c
> @@ -17,6 +17,7 @@
> #include <linux/mmc/host.h>
> #include <linux/module.h>
> #include <linux/pagemap.h>
> +#include <linux/pm_domain.h>
> #include <linux/scatterlist.h>
>
> #include "tmio_mmc.h"
> @@ -172,6 +173,8 @@ static int tmio_mmc_probe(struct platform_device *pdev)
> host->mmc->f_max = pdata->hclk;
> host->mmc->f_min = pdata->hclk / 512;
>
> + dev_pm_domain_start(&pdev->dev);
> +
I am quite sure tmio_mmc won't need this, but better safe than sorry.
Thanks, Ulf!
Attachment:
signature.asc
Description: PGP signature
