[U-Boot] [PATCH V4 5/9] EXYNOS5: DWMMC: API to set mmc clock divisor

[Amarendra Reddy]

Hi Jaehoon / Simon,

Thanks for review comments.
Please find my responses below.

Thanks & Regards
Amarendra Reddy

On 11 January 2013 09:22, Jaehoon Chung <jh80.chung at samsung.com> wrote:

>  On 01/11/2013 12:35 AM, Simon Glass wrote:
> > Hi Amar,
> >
> > On Fri, Jan 4, 2013 at 1:34 AM, Amar <amarendra.xt at samsung.com> wrote:
> >> This API computes the divisor value based on MPLL clock and
> >> writes it into the FSYS1 register.
> >>
> >> Changes from V1:
> >>         1)Updated the function exynos5_mmc_set_clk_div() to receive
> >>         'device_i'd as input parameter instead of 'index'.
> >>
> >> Changes from V2:
> >>         1)Updation of commit message and resubmition of proper patch
> set.
> >>
> >> Changes from V3:
> >>         1)Removed the new API exynos5_mmc_set_clk_div() from clock.c,
> >>         because existing API set_mmc_clk() can be used to set mmc clock.
> >>
> >> Signed-off-by: Amar <amarendra.xt at samsung.com>
> >> ---
> >>  arch/arm/cpu/armv7/exynos/clock.c      | 4 ++--
> >>  arch/arm/include/asm/arch-exynos/clk.h | 3 +++
> >>  2 files changed, 5 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/arch/arm/cpu/armv7/exynos/clock.c
> b/arch/arm/cpu/armv7/exynos/clock.c
> >> index 973b84e..89574ba 100644
> >> --- a/arch/arm/cpu/armv7/exynos/clock.c
> >> +++ b/arch/arm/cpu/armv7/exynos/clock.c
> >> @@ -490,7 +490,7 @@ static unsigned long exynos4_get_mmc_clk(int
> dev_index)
> >>                 (struct exynos4_clock *)samsung_get_base_clock();
> >>         unsigned long uclk, sclk;
> >>         unsigned int sel, ratio, pre_ratio;
> >> -       int shift;
> >> +       int shift = 0;
> >
> > Is this fixing a warning?
> Maybe..fix the compiler warning..
>

As 'shift' was uninitialised, it had garbage value which was causing a
problem when "dev_index = 0 or 2".

> >
> >>
> >>         sel = readl(&clk->src_fsys);
> >>         sel = (sel >> (dev_index << 2)) & 0xf;
> >> @@ -539,7 +539,7 @@ static unsigned long exynos5_get_mmc_clk(int
> dev_index)
> >>                 (struct exynos5_clock *)samsung_get_base_clock();
> >>         unsigned long uclk, sclk;
> >>         unsigned int sel, ratio, pre_ratio;
> >> -       int shift;
> >> +       int shift = 0;
> >>
> >>         sel = readl(&clk->src_fsys);
> >>         sel = (sel >> (dev_index << 2)) & 0xf;
> >> diff --git a/arch/arm/include/asm/arch-exynos/clk.h
> b/arch/arm/include/asm/arch-exynos/clk.h
> >> index 1935b0b..a4d5b4e 100644
> >> --- a/arch/arm/include/asm/arch-exynos/clk.h
> >> +++ b/arch/arm/include/asm/arch-exynos/clk.h
> >> @@ -29,6 +29,9 @@
> >>  #define VPLL   4
> >>  #define BPLL   5
> >>
> >> +#define FSYS1_MMC0_DIV_MASK    0xff0f
> >> +#define FSYS1_MMC0_DIV_VAL     0x0701
> >
> > What is this used for? I don't see it in this patch.
> >
> > Overall it is not clear what this patch is for.
> This define didn't need. That value is not static value, isn't?
>
In fact, V2 of this patch had a new function (which I added). That new
function was using the #define values.
But later in V4 the new function has been removed.

As of now the #define values are used in the file
board/samsung/smdk5250/clock_init.c.
 The values are used during "booting from EMMC".


> Best Regards,
> Jaehoon Chung
>  >
> >> +
> >>  unsigned long get_pll_clk(int pllreg);
> >>  unsigned long get_arm_clk(void);
> >>  unsigned long get_i2c_clk(void);
> >> --
> >> 1.8.0
> >>
> >
> > Regards,
> > Simon
> >
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>