Hi Adrian, On 20/06/2017 08:36, Adrian Hunter wrote: > On 16/06/17 10:29, Quentin Schulz wrote: >> The setting of clocks and presets is currently done in probe only but >> once deep PM support is added, it'll be needed in the resume function. >> >> Let's create a function for this setting. >> >> Signed-off-by: Quentin Schulz <quentin.schulz@xxxxxxxxxxxxxxxxxx> > > Apart from cosmetic comment below: > > Acked-by: Adrian Hunter <adrian.hunter@xxxxxxxxx> > >> --- >> drivers/mmc/host/sdhci-of-at91.c | 147 ++++++++++++++++++++++----------------- >> 1 file changed, 82 insertions(+), 65 deletions(-) >> >> diff --git a/drivers/mmc/host/sdhci-of-at91.c b/drivers/mmc/host/sdhci-of-at91.c >> index 7611fd679f1a..fb8c6011f13d 100644 >> --- a/drivers/mmc/host/sdhci-of-at91.c >> +++ b/drivers/mmc/host/sdhci-of-at91.c >> @@ -128,6 +128,84 @@ static const struct of_device_id sdhci_at91_dt_match[] = { >> }; >> MODULE_DEVICE_TABLE(of, sdhci_at91_dt_match); >> >> +static int sdhci_at91_set_clks_presets(struct device *dev) >> +{ >> + struct sdhci_host *host = dev_get_drvdata(dev); >> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); >> + struct sdhci_at91_priv *priv = sdhci_pltfm_priv(pltfm_host); >> + int ret; >> + unsigned int caps0, caps1; >> + unsigned int clk_base, clk_mul; >> + unsigned int gck_rate, real_gck_rate; >> + unsigned int preset_div; > > Too much whitespace. > Simply moved some variables from the original code (see sdhci_at91_probe below). Thanks, Quentin >> + >> + /* >> + * The mult clock is provided by as a generated clock by the PMC >> + * controller. In order to set the rate of gck, we have to get the >> + * base clock rate and the clock mult from capabilities. >> + */ >> + clk_prepare_enable(priv->hclock); >> + caps0 = readl(host->ioaddr + SDHCI_CAPABILITIES); >> + caps1 = readl(host->ioaddr + SDHCI_CAPABILITIES_1); >> + clk_base = (caps0 & SDHCI_CLOCK_V3_BASE_MASK) >> SDHCI_CLOCK_BASE_SHIFT; >> + clk_mul = (caps1 & SDHCI_CLOCK_MUL_MASK) >> SDHCI_CLOCK_MUL_SHIFT; >> + gck_rate = clk_base * 1000000 * (clk_mul + 1); >> + ret = clk_set_rate(priv->gck, gck_rate); >> + if (ret < 0) { >> + dev_err(dev, "failed to set gck"); >> + clk_disable_unprepare(priv->hclock); >> + return ret; >> + } >> + /* >> + * We need to check if we have the requested rate for gck because in >> + * some cases this rate could be not supported. If it happens, the rate >> + * is the closest one gck can provide. We have to update the value >> + * of clk mul. >> + */ >> + real_gck_rate = clk_get_rate(priv->gck); >> + if (real_gck_rate != gck_rate) { >> + clk_mul = real_gck_rate / (clk_base * 1000000) - 1; >> + caps1 &= (~SDHCI_CLOCK_MUL_MASK); >> + caps1 |= ((clk_mul << SDHCI_CLOCK_MUL_SHIFT) & >> + SDHCI_CLOCK_MUL_MASK); >> + /* Set capabilities in r/w mode. */ >> + writel(SDMMC_CACR_KEY | SDMMC_CACR_CAPWREN, >> + host->ioaddr + SDMMC_CACR); >> + writel(caps1, host->ioaddr + SDHCI_CAPABILITIES_1); >> + /* Set capabilities in ro mode. */ >> + writel(0, host->ioaddr + SDMMC_CACR); >> + dev_info(dev, "update clk mul to %u as gck rate is %u Hz\n", >> + clk_mul, real_gck_rate); >> + } >> + >> + /* >> + * We have to set preset values because it depends on the clk_mul >> + * value. Moreover, SDR104 is supported in a degraded mode since the >> + * maximum sd clock value is 120 MHz instead of 208 MHz. For that >> + * reason, we need to use presets to support SDR104. >> + */ >> + preset_div = DIV_ROUND_UP(real_gck_rate, 24000000) - 1; >> + writew(SDHCI_AT91_PRESET_COMMON_CONF | preset_div, >> + host->ioaddr + SDHCI_PRESET_FOR_SDR12); >> + preset_div = DIV_ROUND_UP(real_gck_rate, 50000000) - 1; >> + writew(SDHCI_AT91_PRESET_COMMON_CONF | preset_div, >> + host->ioaddr + SDHCI_PRESET_FOR_SDR25); >> + preset_div = DIV_ROUND_UP(real_gck_rate, 100000000) - 1; >> + writew(SDHCI_AT91_PRESET_COMMON_CONF | preset_div, >> + host->ioaddr + SDHCI_PRESET_FOR_SDR50); >> + preset_div = DIV_ROUND_UP(real_gck_rate, 120000000) - 1; >> + writew(SDHCI_AT91_PRESET_COMMON_CONF | preset_div, >> + host->ioaddr + SDHCI_PRESET_FOR_SDR104); >> + preset_div = DIV_ROUND_UP(real_gck_rate, 50000000) - 1; >> + writew(SDHCI_AT91_PRESET_COMMON_CONF | preset_div, >> + host->ioaddr + SDHCI_PRESET_FOR_DDR50); >> + >> + clk_prepare_enable(priv->mainck); >> + clk_prepare_enable(priv->gck); >> + >> + return 0; >> +} >> + >> #ifdef CONFIG_PM >> static int sdhci_at91_runtime_suspend(struct device *dev) >> { >> @@ -192,11 +270,7 @@ static int sdhci_at91_probe(struct platform_device *pdev) >> struct sdhci_host *host; >> struct sdhci_pltfm_host *pltfm_host; >> struct sdhci_at91_priv *priv; >> - unsigned int caps0, caps1; >> - unsigned int clk_base, clk_mul; >> - unsigned int gck_rate, real_gck_rate; >> int ret; >> - unsigned int preset_div; >> >> match = of_match_device(sdhci_at91_dt_match, &pdev->dev); >> if (!match) >> @@ -228,66 +302,9 @@ static int sdhci_at91_probe(struct platform_device *pdev) >> return PTR_ERR(priv->gck); >> } >> >> - /* >> - * The mult clock is provided by as a generated clock by the PMC >> - * controller. In order to set the rate of gck, we have to get the >> - * base clock rate and the clock mult from capabilities. >> - */ >> - clk_prepare_enable(priv->hclock); >> - caps0 = readl(host->ioaddr + SDHCI_CAPABILITIES); >> - caps1 = readl(host->ioaddr + SDHCI_CAPABILITIES_1); >> - clk_base = (caps0 & SDHCI_CLOCK_V3_BASE_MASK) >> SDHCI_CLOCK_BASE_SHIFT; >> - clk_mul = (caps1 & SDHCI_CLOCK_MUL_MASK) >> SDHCI_CLOCK_MUL_SHIFT; >> - gck_rate = clk_base * 1000000 * (clk_mul + 1); >> - ret = clk_set_rate(priv->gck, gck_rate); >> - if (ret < 0) { >> - dev_err(&pdev->dev, "failed to set gck"); >> - goto hclock_disable_unprepare; >> - } >> - /* >> - * We need to check if we have the requested rate for gck because in >> - * some cases this rate could be not supported. If it happens, the rate >> - * is the closest one gck can provide. We have to update the value >> - * of clk mul. >> - */ >> - real_gck_rate = clk_get_rate(priv->gck); >> - if (real_gck_rate != gck_rate) { >> - clk_mul = real_gck_rate / (clk_base * 1000000) - 1; >> - caps1 &= (~SDHCI_CLOCK_MUL_MASK); >> - caps1 |= ((clk_mul << SDHCI_CLOCK_MUL_SHIFT) & SDHCI_CLOCK_MUL_MASK); >> - /* Set capabilities in r/w mode. */ >> - writel(SDMMC_CACR_KEY | SDMMC_CACR_CAPWREN, host->ioaddr + SDMMC_CACR); >> - writel(caps1, host->ioaddr + SDHCI_CAPABILITIES_1); >> - /* Set capabilities in ro mode. */ >> - writel(0, host->ioaddr + SDMMC_CACR); >> - dev_info(&pdev->dev, "update clk mul to %u as gck rate is %u Hz\n", >> - clk_mul, real_gck_rate); >> - } >> - >> - /* >> - * We have to set preset values because it depends on the clk_mul >> - * value. Moreover, SDR104 is supported in a degraded mode since the >> - * maximum sd clock value is 120 MHz instead of 208 MHz. For that >> - * reason, we need to use presets to support SDR104. >> - */ >> - preset_div = DIV_ROUND_UP(real_gck_rate, 24000000) - 1; >> - writew(SDHCI_AT91_PRESET_COMMON_CONF | preset_div, >> - host->ioaddr + SDHCI_PRESET_FOR_SDR12); >> - preset_div = DIV_ROUND_UP(real_gck_rate, 50000000) - 1; >> - writew(SDHCI_AT91_PRESET_COMMON_CONF | preset_div, >> - host->ioaddr + SDHCI_PRESET_FOR_SDR25); >> - preset_div = DIV_ROUND_UP(real_gck_rate, 100000000) - 1; >> - writew(SDHCI_AT91_PRESET_COMMON_CONF | preset_div, >> - host->ioaddr + SDHCI_PRESET_FOR_SDR50); >> - preset_div = DIV_ROUND_UP(real_gck_rate, 120000000) - 1; >> - writew(SDHCI_AT91_PRESET_COMMON_CONF | preset_div, >> - host->ioaddr + SDHCI_PRESET_FOR_SDR104); >> - preset_div = DIV_ROUND_UP(real_gck_rate, 50000000) - 1; >> - writew(SDHCI_AT91_PRESET_COMMON_CONF | preset_div, >> - host->ioaddr + SDHCI_PRESET_FOR_DDR50); >> - >> - clk_prepare_enable(priv->mainck); >> - clk_prepare_enable(priv->gck); >> + ret = sdhci_at91_set_clks_presets(&pdev->dev); >> + if (ret) >> + goto sdhci_pltfm_free; >> >> ret = mmc_of_parse(host->mmc); >> if (ret) >> @@ -335,8 +352,8 @@ static int sdhci_at91_probe(struct platform_device *pdev) >> clocks_disable_unprepare: >> clk_disable_unprepare(priv->gck); >> clk_disable_unprepare(priv->mainck); >> -hclock_disable_unprepare: >> clk_disable_unprepare(priv->hclock); >> +sdhci_pltfm_free: >> sdhci_pltfm_free(pdev); >> return ret; >> } >> > -- Quentin Schulz, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- 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