On 24/03/17 09:50, Shawn Lin wrote:
> Currently the get_timeout_clock callback doesn't clearly
> have a statement that it needs the variant drivers to return
> the timeout clock rate in kHz if the SDHCI_TIMEOUT_CLK_UNIT
> isn't present, otherwise the variant drivers should return it
> in MHz. It's also very likely that further variant drivers which
> are going to use this callback will be confused by this situation.
> Given the fact that moderm sdhci variant hosts are very prone to get
> the timeout clock from common clock framework (actually the only three
> users did that), it's more natural to return the value in Hz and we
> make an explicit comment there. Then we put the unit conversion inside
> the sdhci core. Thus will improve the code and prevent further misuses.
>
> Cc: Adrian Hunter <adrian.hunter@xxxxxxxxx>
Can be:
Acked-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
> Reported-by: Anssi Hannula <anssi.hannula@xxxxxxxxxx>
> Signed-off-by: Shawn Lin <shawn.lin@xxxxxxxxxxxxxx>
> Acked-by: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx>
> ---
>
> Changes in v6:
> - rebase on Ulf's next
> - improve some commit msg and add Anssi's tag
>
> Changes in v5:
> - fix typo
> - remove unlikely
>
> Changes in v4:
> - rebased on -next
> - add Masahiro's ack for sdhci-cadence
>
> Changes in v3:
> - squash all the patches into one to keep bisectability
> - fix wrong return value of sdhci-cadence
> - slight improve the condition check to avoid the complaint of
> checkpatch
>
> Changes in v2:
> - fix the comment and make it return in Hz
>
> drivers/mmc/host/sdhci-cadence.c | 4 ++--
> drivers/mmc/host/sdhci-of-arasan.c | 17 +----------------
> drivers/mmc/host/sdhci.c | 16 +++++++++-------
> drivers/mmc/host/sdhci.h | 1 +
> 4 files changed, 13 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c
> index 48f6419..07a27dc 100644
> --- a/drivers/mmc/host/sdhci-cadence.c
> +++ b/drivers/mmc/host/sdhci-cadence.c
> @@ -105,9 +105,9 @@ static unsigned int sdhci_cdns_get_timeout_clock(struct sdhci_host *host)
> {
> /*
> * Cadence's spec says the Timeout Clock Frequency is the same as the
> - * Base Clock Frequency. Divide it by 1000 to return a value in kHz.
> + * Base Clock Frequency.
> */
> - return host->max_clk / 1000;
> + return host->max_clk;
> }
>
> static void sdhci_cdns_set_emmc_mode(struct sdhci_cdns_priv *priv, u32 mode)
> diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
> index 61accf0..ea6b36c 100644
> --- a/drivers/mmc/host/sdhci-of-arasan.c
> +++ b/drivers/mmc/host/sdhci-of-arasan.c
> @@ -157,21 +157,6 @@ static int sdhci_arasan_syscon_write(struct sdhci_host *host,
> return ret;
> }
>
> -static unsigned int sdhci_arasan_get_timeout_clock(struct sdhci_host *host)
> -{
> - unsigned long freq;
> - struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> -
> - /* SDHCI timeout clock is in kHz */
> - freq = DIV_ROUND_UP(clk_get_rate(pltfm_host->clk), 1000);
> -
> - /* or in MHz */
> - if (host->caps & SDHCI_TIMEOUT_CLK_UNIT)
> - freq = DIV_ROUND_UP(freq, 1000);
> -
> - return freq;
> -}
> -
> static void sdhci_arasan_set_clock(struct sdhci_host *host, unsigned int clock)
> {
> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> @@ -280,7 +265,7 @@ static int sdhci_arasan_voltage_switch(struct mmc_host *mmc,
> static struct sdhci_ops sdhci_arasan_ops = {
> .set_clock = sdhci_arasan_set_clock,
> .get_max_clock = sdhci_pltfm_clk_get_max_clock,
> - .get_timeout_clock = sdhci_arasan_get_timeout_clock,
> + .get_timeout_clock = sdhci_pltfm_clk_get_max_clock,
> .set_bus_width = sdhci_set_bus_width,
> .reset = sdhci_arasan_reset,
> .set_uhs_signaling = sdhci_set_uhs_signaling,
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index a33102f..0d4485d 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -3390,20 +3390,22 @@ int sdhci_setup_host(struct sdhci_host *host)
> if (!(host->quirks & SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK)) {
> host->timeout_clk = (host->caps & SDHCI_TIMEOUT_CLK_MASK) >>
> SDHCI_TIMEOUT_CLK_SHIFT;
> +
> + if (host->caps & SDHCI_TIMEOUT_CLK_UNIT)
> + host->timeout_clk *= 1000;
> +
> if (host->timeout_clk == 0) {
> - if (host->ops->get_timeout_clock) {
> - host->timeout_clk =
> - host->ops->get_timeout_clock(host);
> - } else {
> + if (!host->ops->get_timeout_clock) {
> pr_err("%s: Hardware doesn't specify timeout clock frequency.\n",
> mmc_hostname(mmc));
> ret = -ENODEV;
> goto undma;
> }
> - }
>
> - if (host->caps & SDHCI_TIMEOUT_CLK_UNIT)
> - host->timeout_clk *= 1000;
> + host->timeout_clk =
> + DIV_ROUND_UP(host->ops->get_timeout_clock(host),
> + 1000);
> + }
>
> if (override_timeout_clk)
> host->timeout_clk = override_timeout_clk;
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index 35b41da..fdb5d7e 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -561,6 +561,7 @@ struct sdhci_ops {
> int (*enable_dma)(struct sdhci_host *host);
> unsigned int (*get_max_clock)(struct sdhci_host *host);
> unsigned int (*get_min_clock)(struct sdhci_host *host);
> + /* get_timeout_clock should return clk rate in unit of Hz */
> unsigned int (*get_timeout_clock)(struct sdhci_host *host);
> unsigned int (*get_max_timeout_count)(struct sdhci_host *host);
> void (*set_timeout)(struct sdhci_host *host,
>
--
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
