[U-Boot] [PATCH 2/3] mmc: zynq: Determine base clock frequency via clock framework

[Jaehoon Chung]

On 11/22/2016 12:26 AM, Stefan Herbrechtsmeier wrote:
> The zynq_sdhci controller driver use CONFIG_ZYNQ_SDHCI_MAX_FREQ as base
> clock frequency but this clock is not fixed and depends on the hardware
> configuration. Additionally the value of CONFIG_ZYNQ_SDHCI_MAX_FREQ
> doesn't match the real base clock frequency of SDIO_FREQ. Use the clock
> framework to determine the frequency at run time.
> 
> Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier at weidmueller.de>
> ---
> 
>  arch/arm/mach-zynq/clk.c              | 23 +++++++++++++++++++++++
>  arch/arm/mach-zynq/include/mach/clk.h |  1 +
>  drivers/mmc/zynq_sdhci.c              | 33 +++++++++++++++++++++++++++++++--
>  3 files changed, 55 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-zynq/clk.c b/arch/arm/mach-zynq/clk.c
> index 40383c1..0129549 100644
> --- a/arch/arm/mach-zynq/clk.c
> +++ b/arch/arm/mach-zynq/clk.c
> @@ -573,6 +573,29 @@ unsigned long get_uart_clk(int dev_index)
>  }
>  
>  /**
> + * get_sdio_clk() - Get SDIO input frequency
> + * @dev_index:	SDIO ID
> + * Returns SDIO input clock frequency in Hz.
> + *
> + * Compared to zynq_clk_get_rate() this function is designed to work before
> + * relocation and can be called when the SDIO is set up.
> + */
> +unsigned long get_sdio_clk(int dev_index)

Where did "dev_index" use?

> +{
> +	u32 reg = readl(&slcr_base->sdio_clk_ctrl);
> +	u32 div = (reg & CLK_CTRL_DIV0_MASK) >> CLK_CTRL_DIV0_SHIFT;
> +	u32 srcsel = (reg & CLK_CTRL_SRCSEL_MASK) >> CLK_CTRL_SRCSEL_SHIFT;
> +	enum zynq_clk parent = __zynq_clk_periph_get_parent(srcsel);
> +	u32 *pllreg = clkid_2_register(parent);
> +	unsigned long prate = __zynq_clk_pll_get_rate(pllreg);

Could you clean these codes? it's not readable..

> +
> +	if (!div)
> +		div = 1;
> +
> +	return DIV_ROUND_CLOSEST(prate, div);
> +}
> +
> +/**
>   * set_cpu_clk_info() - Initialize clock framework
>   * Always returns zero.
>   *
> diff --git a/arch/arm/mach-zynq/include/mach/clk.h b/arch/arm/mach-zynq/include/mach/clk.h
> index 250c5bc..fcb7842 100644
> --- a/arch/arm/mach-zynq/include/mach/clk.h
> +++ b/arch/arm/mach-zynq/include/mach/clk.h
> @@ -25,5 +25,6 @@ int zynq_clk_set_rate(enum zynq_clk clk, unsigned long rate);
>  unsigned long zynq_clk_get_rate(enum zynq_clk clk);
>  const char *zynq_clk_get_name(enum zynq_clk clk);
>  unsigned long get_uart_clk(int dev_id);
> +unsigned long get_sdio_clk(int dev_id);
>  
>  #endif
> diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c
> index 69efa38..1b75c5a 100644
> --- a/drivers/mmc/zynq_sdhci.c
> +++ b/drivers/mmc/zynq_sdhci.c
> @@ -6,6 +6,7 @@
>   * SPDX-License-Identifier:	GPL-2.0+
>   */
>  
> +#include <clk.h>
>  #include <common.h>
>  #include <dm.h>
>  #include <fdtdec.h>
> @@ -13,6 +14,8 @@
>  #include <malloc.h>
>  #include <sdhci.h>
>  
> +#include <asm/arch/clk.h>
> +
>  #ifndef CONFIG_ZYNQ_SDHCI_MIN_FREQ
>  # define CONFIG_ZYNQ_SDHCI_MIN_FREQ	0
>  #endif
> @@ -27,8 +30,34 @@ static int arasan_sdhci_probe(struct udevice *dev)
>  	struct arasan_sdhci_plat *plat = dev_get_platdata(dev);
>  	struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
>  	struct sdhci_host *host = dev_get_priv(dev);
> +	unsigned long clock;
>  	int ret;
>  
> +#if defined(CONFIG_CLK) || defined(CONFIG_SPL_CLK)
> +	struct clk clk;
> +
> +	ret = clk_get_by_index(dev, 0, &clk);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to get clock\n");
> +		return ret;
> +	}
> +
> +	clock = clk_get_rate(&clk);
> +	if (IS_ERR_VALUE(clock)) {
> +		dev_err(dev, "failed to get rate\n");
> +		return clock;
> +	}
> +	debug("%s: CLK %ld\n", __func__, clock);
> +
> +	ret = clk_enable(&clk);
> +	if (ret && ret != -ENOSYS) {
> +		dev_err(dev, "failed to enable clock\n");
> +		return ret;
> +	}
> +#else
> +	clock = get_sdio_clk(0);

I already mentioned..0 is meaningless.

> +#endif
> +
>  	host->quirks = SDHCI_QUIRK_WAIT_SEND_CMD |
>  		       SDHCI_QUIRK_BROKEN_R1B;
>  
> @@ -36,9 +65,9 @@ static int arasan_sdhci_probe(struct udevice *dev)
>  	host->quirks |= SDHCI_QUIRK_NO_HISPD_BIT;
>  #endif
>  
> -	host->max_clk = CONFIG_ZYNQ_SDHCI_MAX_FREQ;
> +	host->max_clk = clock;
>  
> -	ret = sdhci_setup_cfg(&plat->cfg, host, 0,
> +	ret = sdhci_setup_cfg(&plat->cfg, host, CONFIG_ZYNQ_SDHCI_MAX_FREQ,
>  			      CONFIG_ZYNQ_SDHCI_MIN_FREQ);

It's strange..in PATCH[1/3], you changed from CONFIG_ZYNQ_SDHCI_MAX_FREQ to 0.
why did you change again?

Best Regards,
Jaehoon Chung

>  	host->mmc = &plat->mmc;
>  	if (ret)
>