[U-Boot] [PATCH 2/4] EXYNOS: clock: add the get_mmc_clk function

[Andy Fleming]

On Tue, Jul 3, 2012 at 12:58 AM, Jaehoon Chung <jh80.chung at samsung.com> wrote:
> To get more exactly sclk value, used the get_mmc_clk.
>
> Signed-off-by: Jaehoon Chung <jh80.chung at samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park at samsung.com>
> ---
>  arch/arm/cpu/armv7/exynos/clock.c      |   78 ++++++++++++++++++++++++++++++++
>  arch/arm/include/asm/arch-exynos/clk.h |    1 +


> +/* exynos4: return mmc clock frequency */
> +static unsigned long exynos4_get_mmc_clk(int dev_index)
> +{
> +       struct exynos4_clock *clk =
> +               (struct exynos4_clock *)samsung_get_base_clock();
> +       unsigned long uclk, sclk;
> +       unsigned int sel;
> +       unsigned int ratio;
> +       unsigned int pre_ratio;
> +
> +       sel = readl(&clk->src_fsys);
> +       sel = (sel >> (dev_index << 2)) & 0xf;


I think it would be more clear, here, if you did:

sel = (sel >> (4 * dev_index)) & 0xf;

It makes it obvious that you're shifting a value by 4 bits per device.


> +/* exynos5: return mmc clock frequency */
> +static unsigned long exynos5_get_mmc_clk(int dev_index)
> +{
> +       struct exynos5_clock *clk =
> +               (struct exynos5_clock *)samsung_get_base_clock();
> +       unsigned long uclk, sclk;
> +       unsigned int sel;
> +       unsigned int ratio;
> +       unsigned int pre_ratio;
> +
> +       sel = readl(&clk->src_fsys);
> +       sel = (sel >> (dev_index << 2)) & 0xf;


Same comment here about dev_index * 4


> +
> +       if (sel == 0x6)
> +               sclk = get_pll_clk(MPLL);
> +       else if (sel == 0x7)
> +               sclk = get_pll_clk(EPLL);
> +       else if (sel == 0x8)
> +               sclk = get_pll_clk(VPLL);
> +       else
> +               return 0;
> +
> +       ratio = readl(&clk->div_fsys1);
> +       ratio = (ratio >> (dev_index << 2)) & 0xf;

And here.


> +unsigned long get_mmc_clk(int dev_index)
> +{
> +       if (cpu_is_exynos5())
> +               return exynos5_get_mmc_clk(dev_index);
> +       else
> +               return exynos4_get_mmc_clk(dev_index);
> +}


This is a very generic name, and could interfere with the generic
layer if it ever has a similarly-named function.


> +
>  void set_mmc_clk(int dev_index, unsigned int div)


This isn't part of your patch, but this is *also* too generic of a name.


Andy