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

login
register
mail settings
Submitter Amar
Date Jan. 4, 2013, 9:34 a.m.
Message ID <1357292050-12137-6-git-send-email-amarendra.xt@samsung.com>
Download mbox | patch
Permalink /patch/209414/
State Superseded
Delegated to: Minkyu Kang
Headers show

Comments

Amar - Jan. 4, 2013, 9:34 a.m.
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(-)
Simon Glass - Jan. 10, 2013, 3:35 p.m.
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?

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

> +
>  unsigned long get_pll_clk(int pllreg);
>  unsigned long get_arm_clk(void);
>  unsigned long get_i2c_clk(void);
> --
> 1.8.0
>

Regards,
Simon
Jaehoon Chung - Jan. 11, 2013, 3:52 a.m.
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..
> 
>>
>>         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?

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
>
Amarendra Reddy - Jan. 11, 2013, 1:23 p.m.
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@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
Simon Glass - Jan. 11, 2013, 2:28 p.m.
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@lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
>
>

Patch

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;
 
 	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
+
 unsigned long get_pll_clk(int pllreg);
 unsigned long get_arm_clk(void);
 unsigned long get_i2c_clk(void);