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

[Michal Simek]

Hi, +Siva,

On 25.11.2016 14:59, 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>
> 
> ---
> 
> Changes in v2:
> - Remove unused index from get sdio clock function
> - Introduce common get clock function
> 
>  arch/arm/mach-zynq/clk.c              | 50 ++++++++++++++++++++++++++++-------
>  arch/arm/mach-zynq/include/mach/clk.h |  1 +
>  drivers/mmc/zynq_sdhci.c              | 33 +++++++++++++++++++++--
>  3 files changed, 72 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/arm/mach-zynq/clk.c b/arch/arm/mach-zynq/clk.c
> index e256b58..8df2de2 100644
> --- a/arch/arm/mach-zynq/clk.c
> +++ b/arch/arm/mach-zynq/clk.c
> @@ -550,20 +550,26 @@ void zynq_clk_early_init(void)
>  }
>  
>  /**
> - * get_uart_clk() - Get UART input frequency
> - * Returns UART input clock frequency in Hz.
> + * zynq_clk_get_early() - Get zynq clock frequency early
> + * Returns clock frequency in Hz.
>   *
>   * Compared to zynq_clk_get_rate() this function is designed to work before
> - * relocation and can be called when the serial UART is set up.
> + * relocation and can be called when the peripheral is set up.
>   */
> -unsigned long get_uart_clk(void)
> +unsigned long zynq_clk_get_early(u32 *addr)
>  {
> -	u32 reg = readl(&slcr_base->uart_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);
> +	u32 reg, div, srcsel;
> +	enum zynq_clk parent;
> +	u32 *pllreg;
> +	unsigned long prate;
> +
> +	reg = readl(addr);
> +	div = (reg & CLK_CTRL_DIV0_MASK) >> CLK_CTRL_DIV0_SHIFT;
> +	srcsel = (reg & CLK_CTRL_SRCSEL_MASK) >> CLK_CTRL_SRCSEL_SHIFT;
> +
> +	parent = __zynq_clk_periph_get_parent(srcsel);
> +	pllreg = clkid_2_register(parent);
> +	prate = __zynq_clk_pll_get_rate(pllreg);
>  
>  	if (!div)
>  		div = 1;
> @@ -572,6 +578,30 @@ unsigned long get_uart_clk(void)
>  }
>  
>  /**
> + * get_uart_clk() - Get UART input frequency
> + * Returns UART 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 serial UART is set up.
> + */
> +unsigned long get_uart_clk(void)
> +{
> +	return zynq_clk_get_early(&slcr_base->uart_clk_ctrl);
> +}
> +
> +/**
> + * get_sdio_clk() - Get SDIO input frequency
> + * 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(void)
> +{
> +	return zynq_clk_get_early(&slcr_base->sdio_clk_ctrl);
> +}
> +
> +/**
>   * 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 ba2210d..bed4903 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(void);
> +unsigned long get_sdio_clk(void);
>  
>  #endif
> diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c
> index 69efa38..26bd1be 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();

I don't think that this is the right way to go. What you have above is
correct but doesn't make sense to add something what we should try to
remove instead.
Moving current zynq clk driver to driver model shouldn't be that hard
because all should be in place. and then you can keep just that
if defined(CONFIG_CLK) part above.

Siva: Have you looked at moving zynq clk driver to DM?

Also please keep in your mind SPL size when we move CLK driver to DM.

Thanks,
Michal