Re: [U-Boot] [PATCH V3 5/9] EXYNOS5: DWMMC: API to set mmc clock divisor
Hi Jaehoon, Thanks for the comments. Please find my response below. Thanks Regards Amarendra On 3 January 2013 12:15, Jaehoon Chung jh80.ch...@samsung.com wrote: Hi Amar, ..snip.. I didn't understand this function(exynos5_mmc_set_clk_div?). What purpose? I think good that proper to use the div as argument. void exynos5_mmc_set_clk(int dev_id, unsigned int div). For fixing dwmmc driver, I referred to working code from chromiumos/src/third_party/u-boot. In chromium uboot, during mhci init, the function clock_set_mshci() is called which is in arch/arm/cpu/armv7/exynos5/clock.c Our function *exynos5_mmc_set_clk_div()* is same as *clock_set_mshci().* Now coming to 'div' as argument we have the below scenarios / questions 1- What is the value of 'div' to be passed from calling function ? It will be the pre_ratio's value for FSYSx register. OK. 2- The value of 'div' needs to be computed in calling function. It will be computed before called this function. For example, in dw_mmc.c or exynos_dw_mc.c OK. 3- As per my understanding, 'div' value depends on values of MPLL clock and FSYS1/2. Right..but it will be changed the source clock. MMC/SD can use the every source clock. OK. 4- *Question:* Is it OK to put the piece of code which accesses MPLL, FSYS1, FSYS2 in drivers/mmc/exynos_dw_mmc.c. ? Sure..we can use the get_mmc_clk/set_mmc_clk. then we can get source clock value. and compute the div value for request clock. OK. I will change accordingly. 5- If we compute 'div' value in drivers/mmc/exynos_dw_mmc.c, then there will be duplication of code (Read of FSYS1/2). What code is duplication? Could you explain to me more? Only need to compute the div value in exynos_dw_mmc.c. why need to compute into here? Sorry..there is no duplication. I thought that reading fsys1/2 will be done at two places. Best Regards, Jaehoon Chung Please comment on the above. + /* get_lcd_clk: return lcd clock frequency */ static unsigned long exynos4_get_lcd_clk(void) { diff --git a/arch/arm/include/asm/arch-exynos/clk.h b/arch/arm/include/asm/arch-exynos/clk.h index 1935b0b..2fd7c3e 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); @@ -36,6 +39,7 @@ unsigned long get_pwm_clk(void); unsigned long get_uart_clk(int dev_index); unsigned long get_mmc_clk(int dev_index); void set_mmc_clk(int dev_index, unsigned int div); +int exynos5_mmc_set_clk_div(int dev_index); unsigned long get_lcd_clk(void); void set_lcd_clk(void); void set_mipi_clk(void); ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V3 5/9] EXYNOS5: DWMMC: API to set mmc clock divisor
Hi Jaehoon, Thanks for the comments. Please find my response below. Thanks Regards Amarendra On 3 January 2013 16:46, Amarendra Reddy amar.lavan...@gmail.com wrote: Hi Jaehoon, Thanks for the comments. Please find my response below. Thanks Regards Amarendra On 3 January 2013 12:15, Jaehoon Chung jh80.ch...@samsung.com wrote: Hi Amar, ..snip.. I didn't understand this function(exynos5_mmc_set_clk_div?). What purpose? I think good that proper to use the div as argument. void exynos5_mmc_set_clk(int dev_id, unsigned int div). For fixing dwmmc driver, I referred to working code from chromiumos/src/third_party/u-boot. In chromium uboot, during mhci init, the function clock_set_mshci() is called which is in arch/arm/cpu/armv7/exynos5/clock.c Our function *exynos5_mmc_set_clk_div()* is same as *clock_set_mshci().* Now coming to 'div' as argument we have the below scenarios / questions 1- What is the value of 'div' to be passed from calling function ? It will be the pre_ratio's value for FSYSx register. OK. 2- The value of 'div' needs to be computed in calling function. It will be computed before called this function. For example, in dw_mmc.c or exynos_dw_mc.c OK. 3- As per my understanding, 'div' value depends on values of MPLL clock and FSYS1/2. Right..but it will be changed the source clock. MMC/SD can use the every source clock. OK. 4- *Question:* Is it OK to put the piece of code which accesses MPLL, FSYS1, FSYS2 in drivers/mmc/exynos_dw_mmc.c. ? Sure..we can use the get_mmc_clk/set_mmc_clk. then we can get source clock value. and compute the div value for request clock. OK. I will change accordingly. Ok, using the existing function set_mmc_clk() will do the job. I will remove the new function exynos5_mmc_set_clk_div() in next patch. 5- If we compute 'div' value in drivers/mmc/exynos_dw_mmc.c, then there will be duplication of code (Read of FSYS1/2). What code is duplication? Could you explain to me more? Only need to compute the div value in exynos_dw_mmc.c. why need to compute into here? Sorry..there is no duplication. I thought that reading fsys1/2 will be done at two places. Best Regards, Jaehoon Chung Please comment on the above. + /* get_lcd_clk: return lcd clock frequency */ static unsigned long exynos4_get_lcd_clk(void) { diff --git a/arch/arm/include/asm/arch-exynos/clk.h b/arch/arm/include/asm/arch-exynos/clk.h index 1935b0b..2fd7c3e 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); @@ -36,6 +39,7 @@ unsigned long get_pwm_clk(void); unsigned long get_uart_clk(int dev_index); unsigned long get_mmc_clk(int dev_index); void set_mmc_clk(int dev_index, unsigned int div); +int exynos5_mmc_set_clk_div(int dev_index); unsigned long get_lcd_clk(void); void set_lcd_clk(void); void set_mipi_clk(void); ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V3 5/9] EXYNOS5: DWMMC: API to set mmc clock divisor
Hi Jaehoon Thanks for the comments. Please find my response below. Than On 2 January 2013 09:01, Jaehoon Chung jh80.ch...@samsung.com wrote: Hi Amar, On 12/31/2012 07:58 PM, Amar 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. Signed-off-by: Amar amarendra...@samsung.com --- arch/arm/cpu/armv7/exynos/clock.c | 38 -- arch/arm/include/asm/arch-exynos/clk.h | 4 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/arch/arm/cpu/armv7/exynos/clock.c b/arch/arm/cpu/armv7/exynos/clock.c index 973b84e..cd42689 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; @@ -659,6 +659,40 @@ static void exynos5_set_mmc_clk(int dev_index, unsigned int div) writel(val, addr); } +/* exynos5: set the mmc clock div ratio in fsys1 */ +int exynos5_mmc_set_clk_div(int dev_id) Why return the int type? Yes, not required to return. I will update. +{ + struct exynos5_clock *clk = + (struct exynos5_clock *)samsung_get_base_clock(); + unsigned int addr; + unsigned int clock; + unsigned int tmp; + unsigned int i; Can't use unsigned int addr, clock? We can save the line. Ok. + + /* get mpll clock */ + clock = get_pll_clk(MPLL) / 100; + + /* + * CLK_DIV_FSYS1 + * MMC0_PRE_RATIO [15:8], MMC0_RATIO [3:0] + * CLK_DIV_FSYS2 + * MMC2_PRE_RATIO [15:8], MMC2_RATIO [3:0] + */ + if (dev_id = PERIPH_ID_SDMMC1) + addr = (unsigned int)clk-div_fsys1; + else + addr = (unsigned int)clk-div_fsys2; + + tmp = readl(addr) ~FSYS1_MMC0_DIV_MASK; FSYS1_MMC0_DIV_MASK? then case of MMC2? It is wrong macro name..You also used the clk-div_fsys2.. Yes, will change the macro name. + for (i = 0; i = 0xf; i++) { + if ((clock / (i + 1)) = 400) { + writel(tmp | i 0, addr); + break; + } + } + return 0; +} I didn't understand this function(exynos5_mmc_set_clk_div?). What purpose? I think good that proper to use the div as argument. void exynos5_mmc_set_clk(int dev_id, unsigned int div). For fixing dwmmc driver, I referred to working code from chromiumos/src/third_party/u-boot. In chromium uboot, during mhci init, the function clock_set_mshci() is called which is in arch/arm/cpu/armv7/exynos5/clock.c Our function *exynos5_mmc_set_clk_div()* is same as *clock_set_mshci().* Now coming to 'div' as argument we have the below scenarios / questions 1- What is the value of 'div' to be passed from calling function ? 2- The value of 'div' needs to be computed in calling function. 3- As per my understanding, 'div' value depends on values of MPLL clock and FSYS1/2. 4- *Question:* Is it OK to put the piece of code which accesses MPLL, FSYS1, FSYS2 in drivers/mmc/exynos_dw_mmc.c. ? 5- If we compute 'div' value in drivers/mmc/exynos_dw_mmc.c, then there will be duplication of code (Read of FSYS1/2). Please comment on the above. + /* get_lcd_clk: return lcd clock frequency */ static unsigned long exynos4_get_lcd_clk(void) { diff --git a/arch/arm/include/asm/arch-exynos/clk.h b/arch/arm/include/asm/arch-exynos/clk.h index 1935b0b..2fd7c3e 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); @@ -36,6 +39,7 @@ unsigned long get_pwm_clk(void); unsigned long get_uart_clk(int dev_index); unsigned long get_mmc_clk(int dev_index); void set_mmc_clk(int dev_index, unsigned int div); +int exynos5_mmc_set_clk_div(int dev_index); unsigned long get_lcd_clk(void); void set_lcd_clk(void); void set_mipi_clk(void);
Re: [U-Boot] [PATCH V3 5/9] EXYNOS5: DWMMC: API to set mmc clock divisor
Hi Amar, ..snip.. I didn't understand this function(exynos5_mmc_set_clk_div?). What purpose? I think good that proper to use the div as argument. void exynos5_mmc_set_clk(int dev_id, unsigned int div). For fixing dwmmc driver, I referred to working code from chromiumos/src/third_party/u-boot. In chromium uboot, during mhci init, the function clock_set_mshci() is called which is in arch/arm/cpu/armv7/exynos5/clock.c Our function *exynos5_mmc_set_clk_div()* is same as *clock_set_mshci().* Now coming to 'div' as argument we have the below scenarios / questions 1- What is the value of 'div' to be passed from calling function ? It will be the pre_ratio's value for FSYSx register. 2- The value of 'div' needs to be computed in calling function. It will be computed before called this function. For example, in dw_mmc.c or exynos_dw_mc.c 3- As per my understanding, 'div' value depends on values of MPLL clock and FSYS1/2. Right..but it will be changed the source clock. MMC/SD can use the every source clock. 4- *Question:* Is it OK to put the piece of code which accesses MPLL, FSYS1, FSYS2 in drivers/mmc/exynos_dw_mmc.c. ? Sure..we can use the get_mmc_clk/set_mmc_clk. then we can get source clock value. and compute the div value for request clock. 5- If we compute 'div' value in drivers/mmc/exynos_dw_mmc.c, then there will be duplication of code (Read of FSYS1/2). What code is duplication? Could you explain to me more? Only need to compute the div value in exynos_dw_mmc.c. why need to compute into here? Best Regards, Jaehoon Chung Please comment on the above. + /* get_lcd_clk: return lcd clock frequency */ static unsigned long exynos4_get_lcd_clk(void) { diff --git a/arch/arm/include/asm/arch-exynos/clk.h b/arch/arm/include/asm/arch-exynos/clk.h index 1935b0b..2fd7c3e 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); @@ -36,6 +39,7 @@ unsigned long get_pwm_clk(void); unsigned long get_uart_clk(int dev_index); unsigned long get_mmc_clk(int dev_index); void set_mmc_clk(int dev_index, unsigned int div); +int exynos5_mmc_set_clk_div(int dev_index); unsigned long get_lcd_clk(void); void set_lcd_clk(void); void set_mipi_clk(void); ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V3 5/9] EXYNOS5: DWMMC: API to set mmc clock divisor
Hi Amar, On 12/31/2012 07:58 PM, Amar 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. Signed-off-by: Amar amarendra...@samsung.com --- arch/arm/cpu/armv7/exynos/clock.c | 38 -- arch/arm/include/asm/arch-exynos/clk.h | 4 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/arch/arm/cpu/armv7/exynos/clock.c b/arch/arm/cpu/armv7/exynos/clock.c index 973b84e..cd42689 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; @@ -659,6 +659,40 @@ static void exynos5_set_mmc_clk(int dev_index, unsigned int div) writel(val, addr); } +/* exynos5: set the mmc clock div ratio in fsys1 */ +int exynos5_mmc_set_clk_div(int dev_id) Why return the int type? +{ + struct exynos5_clock *clk = + (struct exynos5_clock *)samsung_get_base_clock(); + unsigned int addr; + unsigned int clock; + unsigned int tmp; + unsigned int i; Can't use unsigned int addr, clock? We can save the line. + + /* get mpll clock */ + clock = get_pll_clk(MPLL) / 100; + + /* + * CLK_DIV_FSYS1 + * MMC0_PRE_RATIO [15:8], MMC0_RATIO [3:0] + * CLK_DIV_FSYS2 + * MMC2_PRE_RATIO [15:8], MMC2_RATIO [3:0] + */ + if (dev_id = PERIPH_ID_SDMMC1) + addr = (unsigned int)clk-div_fsys1; + else + addr = (unsigned int)clk-div_fsys2; + + tmp = readl(addr) ~FSYS1_MMC0_DIV_MASK; FSYS1_MMC0_DIV_MASK? then case of MMC2? It is wrong macro name..You also used the clk-div_fsys2.. + for (i = 0; i = 0xf; i++) { + if ((clock / (i + 1)) = 400) { + writel(tmp | i 0, addr); + break; + } + } + return 0; +} I didn't understand this function(exynos5_mmc_set_clk_div?). What purpose? I think good that proper to use the div as argument. void exynos5_mmc_set_clk(int dev_id, unsigned int div). + /* get_lcd_clk: return lcd clock frequency */ static unsigned long exynos4_get_lcd_clk(void) { diff --git a/arch/arm/include/asm/arch-exynos/clk.h b/arch/arm/include/asm/arch-exynos/clk.h index 1935b0b..2fd7c3e 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); @@ -36,6 +39,7 @@ unsigned long get_pwm_clk(void); unsigned long get_uart_clk(int dev_index); unsigned long get_mmc_clk(int dev_index); void set_mmc_clk(int dev_index, unsigned int div); +int exynos5_mmc_set_clk_div(int dev_index); unsigned long get_lcd_clk(void); void set_lcd_clk(void); void set_mipi_clk(void); ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH V3 5/9] EXYNOS5: DWMMC: API to set mmc clock divisor
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. Signed-off-by: Amar amarendra...@samsung.com --- arch/arm/cpu/armv7/exynos/clock.c | 38 -- arch/arm/include/asm/arch-exynos/clk.h | 4 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/arch/arm/cpu/armv7/exynos/clock.c b/arch/arm/cpu/armv7/exynos/clock.c index 973b84e..cd42689 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; @@ -659,6 +659,40 @@ static void exynos5_set_mmc_clk(int dev_index, unsigned int div) writel(val, addr); } +/* exynos5: set the mmc clock div ratio in fsys1 */ +int exynos5_mmc_set_clk_div(int dev_id) +{ + struct exynos5_clock *clk = + (struct exynos5_clock *)samsung_get_base_clock(); + unsigned int addr; + unsigned int clock; + unsigned int tmp; + unsigned int i; + + /* get mpll clock */ + clock = get_pll_clk(MPLL) / 100; + + /* +* CLK_DIV_FSYS1 +* MMC0_PRE_RATIO [15:8], MMC0_RATIO [3:0] +* CLK_DIV_FSYS2 +* MMC2_PRE_RATIO [15:8], MMC2_RATIO [3:0] +*/ + if (dev_id = PERIPH_ID_SDMMC1) + addr = (unsigned int)clk-div_fsys1; + else + addr = (unsigned int)clk-div_fsys2; + + tmp = readl(addr) ~FSYS1_MMC0_DIV_MASK; + for (i = 0; i = 0xf; i++) { + if ((clock / (i + 1)) = 400) { + writel(tmp | i 0, addr); + break; + } + } + return 0; +} + /* get_lcd_clk: return lcd clock frequency */ static unsigned long exynos4_get_lcd_clk(void) { diff --git a/arch/arm/include/asm/arch-exynos/clk.h b/arch/arm/include/asm/arch-exynos/clk.h index 1935b0b..2fd7c3e 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_MASK0xff0f +#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); @@ -36,6 +39,7 @@ unsigned long get_pwm_clk(void); unsigned long get_uart_clk(int dev_index); unsigned long get_mmc_clk(int dev_index); void set_mmc_clk(int dev_index, unsigned int div); +int exynos5_mmc_set_clk_div(int dev_index); unsigned long get_lcd_clk(void); void set_lcd_clk(void); void set_mipi_clk(void); -- 1.8.0 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot