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

Simon Glass
2013-01-11T14:28:56Z

Hi Amarendra,

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

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