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

[Simon Glass]

Hi Amarendra,

On Fri, Jan 11, 2013 at 5:23 AM, Amarendra Reddy
<amar.lavanuru at gmail.com> wrote:
> 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".

OK good.

>>
>> >
>> >>
>> >>         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".

OK I see. I suppose they could move to that patch, but I suppose it
isn't important so long as the patches go in in the right order.

Regards,
Simon

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