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

On Tue, Jul 3, 2012 at 12:58 AM, Jaehoon Chung jh80.chung@samsung.com wrote:

To get more exactly sclk value, used the get_mmc_clk.

Signed-off-by: Jaehoon Chung jh80.chung@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@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