| Message ID | 1357292050-12137-6-git-send-email-amarendra.xt@samsung.com |
|---|---|
| State | Superseded |
| Delegated to: | Minkyu Kang |
| Headers | show |
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
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 >
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 >
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 > >
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);
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(-)