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

Amarendra Reddy
2013-01-11T13:23:58Z

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@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@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@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
>