ARM: S5PV210: allow clk to use clksrc as parents
Subsystems (3)
PATCH 1/1 ARM: S5PV210: allow clk to use clksrc as parents
This patch allows clk in init_clocks[] and init_clocks_disable[] to use clksrc_clk.clk in clksrcs[].
For example, clk "lcd" may need to set "sclk-fimd" as a parent if the div and src settting of sclk-fimd changes and a user of "lcd" wants to get the exact information about the rate. If "sclk-fimd" is set as a parent of "lcd", then the clk framework can provide the wanted information; otherwise, the change in "sclk-fimd" does not update "lcd" properly.
To do so, we have relocated clksrcs[] and put enum indexes to clksrcs in order to allow clk in init_clocks[] and init_clocks_disable[] may refer one of clksrcs[] as its parent. (enum clksrc_refer)
For example, "lcd" may set "sclk-fimd" as a parent if the following line replaces the line 878:
.parent = &clk_hclk_dsys.clk,
to
.parent = &clksrcs[_clksrc_sclk_fimd],
--- arch/arm/mach-s5pv210/clock.c | 986 ++++++++++++++++++++++------------------- 1 files changed, 520 insertions(+), 466 deletions(-)
arch/arm/mach-s5pv210/clock.c +520 -466
@@ -264,6 +264,526 @@ static struct clksrc_clk clk_sclk_vpll = { .reg_src = { .reg = S5P_CLK_SRC0, .shift = 12, .size = 1 }, }; +static struct clk *clkset_uart_list[] = { + [6 ] = &clk_mout_mpll.clk, + [7 ] = &clk_mout_epll.clk, +}; + +static struct clksrc_sources clkset_uart = { + .sources = clkset_uart_list, + .nr_sources =ARRAY_SIZE (clkset_uart_list), +}; + +static struct clk *clkset_group1_list[] = { + [0 ] = &clk_sclk_a2m.clk, + [1 ] = &clk_mout_mpll.clk, + [2 ] = &clk_mout_epll.clk, + [3 ] = &clk_sclk_vpll.clk, +}; + +static struct clksrc_sources clkset_group1 = { + .sources = clkset_group1_list, + .nr_sources =ARRAY_SIZE (clkset_group1_list), +}; + +static struct clk *clkset_sclk_onenand_list[] = { + [0 ] = &clk_hclk_psys.clk, + [1 ] = &clk_hclk_dsys.clk, +}; + +static struct clksrc_sources clkset_sclk_onenand = { + .sources = clkset_sclk_onenand_list, + .nr_sources =ARRAY_SIZE (clkset_sclk_onenand_list), +}; + +static struct clk *clkset_sclk_dac_list[] = { + [0 ] = &clk_sclk_vpll.clk, + [1 ] = &clk_sclk_hdmiphy, +}; + +static struct clksrc_sources clkset_sclk_dac = { + .sources = clkset_sclk_dac_list, + .nr_sources =ARRAY_SIZE (clkset_sclk_dac_list), +}; + +static struct clksrc_clk clk_sclk_dac = { + .clk = { + .name ="sclk_dac" , + .id =-1 , + .ctrlbit = (1 <<2 ), + .enable = s5pv210_clk_mask0_ctrl, + }, + .sources = &clkset_sclk_dac, + .reg_src = { .reg =S5P_CLK_SRC1 , .shift =8 , .size =1 }, +}; + +static struct clksrc_clk clk_sclk_pixel = { + .clk = { + .name ="sclk_pixel" , + .id =-1 , + .parent = &clk_sclk_vpll.clk, + }, + .reg_div = { .reg =S5P_CLK_DIV1 , .shift =0 , .size =4 }, +}; + +static struct clk *clkset_sclk_hdmi_list[] = { + [0 ] = &clk_sclk_pixel.clk, + [1 ] = &clk_sclk_hdmiphy, +}; + +static struct clksrc_sources clkset_sclk_hdmi = { + .sources = clkset_sclk_hdmi_list, + .nr_sources =ARRAY_SIZE (clkset_sclk_hdmi_list), +}; + +static struct clksrc_clk clk_sclk_hdmi = { + .clk = { + .name ="sclk_hdmi" , + .id =-1 , + .ctrlbit = (1 <<0 ), + .enable = s5pv210_clk_mask0_ctrl, + }, + .sources = &clkset_sclk_hdmi, + .reg_src = { .reg =S5P_CLK_SRC1 , .shift =0 , .size =1 }, +}; + +static struct clk *clkset_sclk_mixer_list[] = { + [0 ] = &clk_sclk_dac.clk, + [1 ] = &clk_sclk_hdmi.clk, +}; + +static struct clksrc_sources clkset_sclk_mixer = { + .sources = clkset_sclk_mixer_list, + .nr_sources =ARRAY_SIZE (clkset_sclk_mixer_list), +}; + +static struct clk *clkset_sclk_audio0_list[] = { + [0 ] = &clk_ext_xtal_mux, + [1 ] = &clk_pcmcdclk0, + [2 ] = &clk_sclk_hdmi27m, + [3 ] = &clk_sclk_usbphy0, + [4 ] = &clk_sclk_usbphy1, + [5 ] = &clk_sclk_hdmiphy, + [6 ] = &clk_mout_mpll.clk, + [7 ] = &clk_mout_epll.clk, + [8 ] = &clk_sclk_vpll.clk, +}; + +static struct clksrc_sources clkset_sclk_audio0 = { + .sources = clkset_sclk_audio0_list, + .nr_sources =ARRAY_SIZE (clkset_sclk_audio0_list), +}; + +static struct clksrc_clk clk_sclk_audio0 = { + .clk = { + .name ="sclk_audio" , + .id =0 , + .enable = s5pv210_clk_mask0_ctrl, + .ctrlbit = (1 <<24 ), + }, + .sources = &clkset_sclk_audio0, + .reg_src = { .reg =S5P_CLK_SRC6 , .shift =0 , .size =4 }, + .reg_div = { .reg =S5P_CLK_DIV6 , .shift =0 , .size =4 }, +}; + +static struct clk *clkset_sclk_audio1_list[] = { + [0 ] = &clk_ext_xtal_mux, + [1 ] = &clk_pcmcdclk1, + [2 ] = &clk_sclk_hdmi27m, + [3 ] = &clk_sclk_usbphy0, + [4 ] = &clk_sclk_usbphy1, + [5 ] = &clk_sclk_hdmiphy, + [6 ] = &clk_mout_mpll.clk, + [7 ] = &clk_mout_epll.clk, + [8 ] = &clk_sclk_vpll.clk, +}; + +static struct clksrc_sources clkset_sclk_audio1 = { + .sources = clkset_sclk_audio1_list, + .nr_sources =ARRAY_SIZE (clkset_sclk_audio1_list), +}; + +static struct clksrc_clk clk_sclk_audio1 = { + .clk = { + .name ="sclk_audio" , + .id =1 , + .enable = s5pv210_clk_mask0_ctrl, + .ctrlbit = (1 <<25 ), + }, + .sources = &clkset_sclk_audio1, + .reg_src = { .reg =S5P_CLK_SRC6 , .shift =4 , .size =4 }, + .reg_div = { .reg =S5P_CLK_DIV6 , .shift =4 , .size =4 }, +}; + +static struct clk *clkset_sclk_audio2_list[] = { + [0 ] = &clk_ext_xtal_mux, + [1 ] = &clk_pcmcdclk0, + [2 ] = &clk_sclk_hdmi27m, + [3 ] = &clk_sclk_usbphy0, + [4 ] = &clk_sclk_usbphy1, + [5 ] = &clk_sclk_hdmiphy, + [6 ] = &clk_mout_mpll.clk, + [7 ] = &clk_mout_epll.clk, + [8 ] = &clk_sclk_vpll.clk, +}; + +static struct clksrc_sources clkset_sclk_audio2 = { + .sources = clkset_sclk_audio2_list, + .nr_sources =ARRAY_SIZE (clkset_sclk_audio2_list), +}; + +static struct clksrc_clk clk_sclk_audio2 = { + .clk = { + .name ="sclk_audio" , + .id =2 , + .enable = s5pv210_clk_mask0_ctrl, + .ctrlbit = (1 <<26 ), + }, + .sources = &clkset_sclk_audio2, + .reg_src = { .reg =S5P_CLK_SRC6 , .shift =8 , .size =4 }, + .reg_div = { .reg =S5P_CLK_DIV6 , .shift =8 , .size =4 }, +}; + +static struct clk *clkset_sclk_spdif_list[] = { + [0 ] = &clk_sclk_audio0.clk, + [1 ] = &clk_sclk_audio1.clk, + [2 ] = &clk_sclk_audio2.clk, +}; + +static struct clksrc_sources clkset_sclk_spdif = { + .sources = clkset_sclk_spdif_list, + .nr_sources =ARRAY_SIZE (clkset_sclk_spdif_list), +}; + +static struct clk *clkset_group2_list[] = { + [0 ] = &clk_ext_xtal_mux, + [1 ] = &clk_xusbxti, + [2 ] = &clk_sclk_hdmi27m, + [3 ] = &clk_sclk_usbphy0, + [4 ] = &clk_sclk_usbphy1, + [5 ] = &clk_sclk_hdmiphy, + [6 ] = &clk_mout_mpll.clk, + [7 ] = &clk_mout_epll.clk, + [8 ] = &clk_sclk_vpll.clk, +}; + +static struct clksrc_sources clkset_group2 = { + .sources = clkset_group2_list, + .nr_sources =ARRAY_SIZE (clkset_group2_list), +}; + +enum clksrc_refer { + _clksrc_sclk_dmc, + _clksrc_sclk_onenand, + _clksrc_uclk1_0, + _clksrc_uclk1_1, + _clksrc_uclk1_2, + _clksrc_uclk1_3, + _clksrc_sclk_mixer, + _clksrc_sclk_spdif, + _clksrc_sclk_fimc_0, + _clksrc_sclk_fimc_1, + _clksrc_sclk_fimc_2, + _clksrc_sclk_cam_0, + _clksrc_sclk_cam_1, + _clksrc_sclk_fimd, + _clksrc_mmc_bus_0, + _clksrc_mmc_bus_1, + _clksrc_mmc_bus_2, + _clksrc_mmc_bus_3, + _clksrc_sclk_mfc, + _clksrc_sclk_g2d, + _clksrc_sclk_g3d, + _clksrc_sclk_csis, + _clksrc_sclk_spi_0, + _clksrc_sclk_spi_1, + _clksrc_sclk_pwi, + _clksrc_sclk_pwm, +}; + +static struct clksrc_clk clksrcs[] = { + [_clksrc_sclk_dmc] = { + .clk = { + .name ="sclk_dmc" , + .id =-1 , + }, + .sources = &clkset_group1, + .reg_src = { .reg =S5P_CLK_SRC6 , .shift =24 , .size =2 }, + .reg_div = { .reg =S5P_CLK_DIV6 , .shift =28 , .size =4 }, + }, + [_clksrc_sclk_onenand] = { + .clk = { + .name ="sclk_onenand" , + .id =-1 , + }, + .sources = &clkset_sclk_onenand, + .reg_src = { .reg =S5P_CLK_SRC0 , .shift =28 , .size =1 }, + .reg_div = { .reg =S5P_CLK_DIV6 , .shift =12 , .size =3 }, + }, + [_clksrc_uclk1_0] = { + .clk = { + .name ="uclk1" , + .id =0 , + .enable = s5pv210_clk_mask0_ctrl, + .ctrlbit = (1 <<12 ), + }, + .sources = &clkset_uart, + .reg_src = { .reg =S5P_CLK_SRC4 , .shift =16 , .size =4 }, + .reg_div = { .reg =S5P_CLK_DIV4 , .shift =16 , .size =4 }, + }, + [_clksrc_uclk1_1] = { + .clk = { + .name ="uclk1" , + .id =1 , + .enable = s5pv210_clk_mask0_ctrl, + .ctrlbit = (1 <<13 ), + }, + .sources = &clkset_uart, + .reg_src = { .reg =S5P_CLK_SRC4 , .shift =20 , .size =4 }, + .reg_div = { .reg =S5P_CLK_DIV4 , .shift =20 , .size =4 }, + }, + [_clksrc_uclk1_2] = { + .clk = { + .name ="uclk1" , + .id =2 , + .enable = s5pv210_clk_mask0_ctrl, + .ctrlbit = (1 <<14 ), + }, + .sources = &clkset_uart, + .reg_src = { .reg =S5P_CLK_SRC4 , .shift =24 , .size =4 }, + .reg_div = { .reg =S5P_CLK_DIV4 , .shift =24 , .size =4 }, + }, + [_clksrc_uclk1_3] = { + .clk = { + .name ="uclk1" , + .id =3 , + .enable = s5pv210_clk_mask0_ctrl, + .ctrlbit = (1 <<15 ), + }, + .sources = &clkset_uart, + .reg_src = { .reg =S5P_CLK_SRC4 , .shift =28 , .size =4 }, + .reg_div = { .reg =S5P_CLK_DIV4 , .shift =28 , .size =4 }, + }, + [_clksrc_sclk_mixer] = { + .clk = { + .name ="sclk_mixer" , + .id =-1 , + .enable = s5pv210_clk_mask0_ctrl, + .ctrlbit = (1 <<1 ), + }, + .sources = &clkset_sclk_mixer, + .reg_src = { .reg =S5P_CLK_SRC1 , .shift =4 , .size =1 }, + }, + [_clksrc_sclk_spdif] = { + .clk = { + .name ="sclk_spdif" , + .id =-1 , + .enable = s5pv210_clk_mask0_ctrl, + .ctrlbit = (1 <<27 ), + }, + .sources = &clkset_sclk_spdif, + .reg_src = { .reg =S5P_CLK_SRC6 , .shift =12 , .size =2 }, + }, + [_clksrc_sclk_fimc_0] = { + .clk = { + .name ="sclk_fimc" , + .id =0 , + .enable = s5pv210_clk_mask1_ctrl, + .ctrlbit = (1 <<2 ), + }, + .sources = &clkset_group2, + .reg_src = { .reg =S5P_CLK_SRC3 , .shift =12 , .size =4 }, + .reg_div = { .reg =S5P_CLK_DIV3 , .shift =12 , .size =4 }, + }, + [_clksrc_sclk_fimc_1] = { + .clk = { + .name ="sclk_fimc" , + .id =1 , + .enable = s5pv210_clk_mask1_ctrl, + .ctrlbit = (1 <<3 ), + }, + .sources = &clkset_group2, + .reg_src = { .reg =S5P_CLK_SRC3 , .shift =16 , .size =4 }, + .reg_div = { .reg =S5P_CLK_DIV3 , .shift =16 , .size =4 }, + }, + [_clksrc_sclk_fimc_2] = { + .clk = { + .name ="sclk_fimc" , + .id =2 , + .enable = s5pv210_clk_mask1_ctrl, + .ctrlbit = (1 <<4 ), + }, + .sources = &clkset_group2, + .reg_src = { .reg =S5P_CLK_SRC3 , .shift =20 , .size =4 }, + .reg_div = { .reg =S5P_CLK_DIV3 , .shift =20 , .size =4 }, + }, + [_clksrc_sclk_cam_0] = { + .clk = { + .name ="sclk_cam" , + .id =0 , + .enable = s5pv210_clk_mask0_ctrl, + .ctrlbit = (1 <<3 ), + }, + .sources = &clkset_group2, + .reg_src = { .reg =S5P_CLK_SRC1 , .shift =12 , .size =4 }, + .reg_div = { .reg =S5P_CLK_DIV1 , .shift =12 , .size =4 }, + }, + [_clksrc_sclk_cam_1] = { + .clk = { + .name ="sclk_cam" , + .id =1 , + .enable = s5pv210_clk_mask0_ctrl, + .ctrlbit = (1 <<4 ), + }, + .sources = &clkset_group2, + .reg_src = { .reg =S5P_CLK_SRC1 , .shift =16 , .size =4 }, + .reg_div = { .reg =S5P_CLK_DIV1 , .shift =16 , .size =4 }, + }, + [_clksrc_sclk_fimd] = { + .clk = { + .name ="sclk_fimd" , + .id =-1 , + .enable = s5pv210_clk_mask0_ctrl, + .ctrlbit = (1 <<5 ), + }, + .sources = &clkset_group2, + .reg_src = { .reg =S5P_CLK_SRC1 , .shift =20 , .size =4 }, + .reg_div = { .reg =S5P_CLK_DIV1 , .shift =20 , .size =4 }, + }, + [_clksrc_mmc_bus_0] = { + .clk = { + .name ="sclk_mmc" , + .id =0 , + .enable = s5pv210_clk_mask0_ctrl, + .ctrlbit = (1 <<8 ), + }, + .sources = &clkset_group2, + .reg_src = { .reg =S5P_CLK_SRC4 , .shift =0 , .size =4 }, + .reg_div = { .reg =S5P_CLK_DIV4 , .shift =0 , .size =4 }, + }, + [_clksrc_mmc_bus_1] = { + .clk = { + .name ="sclk_mmc" , + .id =1 , + .enable = s5pv210_clk_mask0_ctrl, + .ctrlbit = (1 <<9 ), + }, + .sources = &clkset_group2, + .reg_src = { .reg =S5P_CLK_SRC4 , .shift =4 , .size =4 }, + .reg_div = { .reg =S5P_CLK_DIV4 , .shift =4 , .size =4 }, + }, + [_clksrc_mmc_bus_2] = { + .clk = { + .name ="sclk_mmc" , + .id =2 , + .enable = s5pv210_clk_mask0_ctrl, + .ctrlbit = (1 <<10 ), + }, + .sources = &clkset_group2, + .reg_src = { .reg =S5P_CLK_SRC4 , .shift =8 , .size =4 }, + .reg_div = { .reg =S5P_CLK_DIV4 , .shift =8 , .size =4 }, + }, + [_clksrc_mmc_bus_3] = { + .clk = { + .name ="sclk_mmc" , + .id =3 , + .enable = s5pv210_clk_mask0_ctrl, + .ctrlbit = (1 <<11 ), + }, + .sources = &clkset_group2, + .reg_src = { .reg =S5P_CLK_SRC4 , .shift =12 , .size =4 }, + .reg_div = { .reg =S5P_CLK_DIV4 , .shift =12 , .size =4 }, + }, + [_clksrc_sclk_mfc] = { + .clk = { + .name ="sclk_mfc" , + .id =-1 , + .enable = s5pv210_clk_ip0_ctrl, + .ctrlbit = (1 <<16 ), + }, + .sources = &clkset_group1, + .reg_src = { .reg =S5P_CLK_SRC2 , .shift =4 , .size =2 }, + .reg_div = { .reg =S5P_CLK_DIV2 , .shift =4 , .size =4 }, + }, + [_clksrc_sclk_g2d] = { + .clk = { + .name ="sclk_g2d" , + .id =-1 , + .enable = s5pv210_clk_ip0_ctrl, + .ctrlbit = (1 <<12 ), + }, + .sources = &clkset_group1, + .reg_src = { .reg =S5P_CLK_SRC2 , .shift =8 , .size =2 }, + .reg_div = { .reg =S5P_CLK_DIV2 , .shift =8 , .size =4 }, + }, + [_clksrc_sclk_g3d] = { + .clk = { + .name ="sclk_g3d" , + .id =-1 , + .enable = s5pv210_clk_ip0_ctrl, + .ctrlbit = (1 <<8 ), + }, + .sources = &clkset_group1, + .reg_src = { .reg =S5P_CLK_SRC2 , .shift =0 , .size =2 }, + .reg_div = { .reg =S5P_CLK_DIV2 , .shift =0 , .size =4 }, + }, + [_clksrc_sclk_csis] = { + .clk = { + .name ="sclk_csis" , + .id =-1 , + .enable = s5pv210_clk_mask0_ctrl, + .ctrlbit = (1 <<6 ), + }, + .sources = &clkset_group2, + .reg_src = { .reg =S5P_CLK_SRC1 , .shift =24 , .size =4 }, + .reg_div = { .reg =S5P_CLK_DIV1 , .shift =28 , .size =4 }, + }, + [_clksrc_sclk_spi_0] = { + .clk = { + .name ="sclk_spi" , + .id =0 , + .enable = s5pv210_clk_mask0_ctrl, + .ctrlbit = (1 <<16 ), + }, + .sources = &clkset_group2, + .reg_src = { .reg =S5P_CLK_SRC5 , .shift =0 , .size =4 }, + .reg_div = { .reg =S5P_CLK_DIV5 , .shift =0 , .size =4 }, + }, + [_clksrc_sclk_spi_1] = { + .clk = { + .name ="sclk_spi" , + .id =1 , + .enable = s5pv210_clk_mask0_ctrl, + .ctrlbit = (1 <<17 ), + }, + .sources = &clkset_group2, + .reg_src = { .reg =S5P_CLK_SRC5 , .shift =4 , .size =4 }, + .reg_div = { .reg =S5P_CLK_DIV5 , .shift =4 , .size =4 }, + }, + [_clksrc_sclk_pwi] = { + .clk = { + .name ="sclk_pwi" , + .id =-1 , + .enable = s5pv210_clk_mask0_ctrl, + .ctrlbit = (1 <<29 ), + }, + .sources = &clkset_group2, + .reg_src = { .reg =S5P_CLK_SRC6 , .shift =20 , .size =4 }, + .reg_div = { .reg =S5P_CLK_DIV6 , .shift =24 , .size =4 }, + }, + [_clksrc_sclk_pwm] = { + .clk = { + .name ="sclk_pwm" , + .id =-1 , + .enable = s5pv210_clk_mask0_ctrl, + .ctrlbit = (1 <<19 ), + }, + .sources = &clkset_group2, + .reg_src = { .reg =S5P_CLK_SRC5 , .shift =12 , .size =4 }, + .reg_div = { .reg =S5P_CLK_DIV5 , .shift =12 , .size =4 }, + }, +}; +static unsigned long s5pv210_clk_imem_get_rate (struct clk *clk) {return clk_get_rate (clk->parent) /2 ;
@@ -752,472 +1272,6 @@ static struct clk init_clocks[] = { }, }; -static struct clk *clkset_uart_list[] = { - [6 ] = &clk_mout_mpll.clk, - [7 ] = &clk_mout_epll.clk, -}; - -static struct clksrc_sources clkset_uart = { - .sources = clkset_uart_list, - .nr_sources =ARRAY_SIZE (clkset_uart_list), -}; - -static struct clk *clkset_group1_list[] = { - [0 ] = &clk_sclk_a2m.clk, - [1 ] = &clk_mout_mpll.clk, - [2 ] = &clk_mout_epll.clk, - [3 ] = &clk_sclk_vpll.clk, -}; - -static struct clksrc_sources clkset_group1 = { - .sources = clkset_group1_list, - .nr_sources =ARRAY_SIZE (clkset_group1_list), -}; - -static struct clk *clkset_sclk_onenand_list[] = { - [0 ] = &clk_hclk_psys.clk, - [1 ] = &clk_hclk_dsys.clk, -}; - -static struct clksrc_sources clkset_sclk_onenand = { - .sources = clkset_sclk_onenand_list, - .nr_sources =ARRAY_SIZE (clkset_sclk_onenand_list), -}; - -static struct clk *clkset_sclk_dac_list[] = { - [0 ] = &clk_sclk_vpll.clk, - [1 ] = &clk_sclk_hdmiphy, -}; - -static struct clksrc_sources clkset_sclk_dac = { - .sources = clkset_sclk_dac_list, - .nr_sources =ARRAY_SIZE (clkset_sclk_dac_list), -}; - -static struct clksrc_clk clk_sclk_dac = { - .clk = { - .name ="sclk_dac" , - .id =-1 , - .ctrlbit = (1 <<2 ), - .enable = s5pv210_clk_mask0_ctrl, - }, - .sources = &clkset_sclk_dac, - .reg_src = { .reg =S5P_CLK_SRC1 , .shift =8 , .size =1 }, -}; - -static struct clksrc_clk clk_sclk_pixel = { - .clk = { - .name ="sclk_pixel" , - .id =-1 , - .parent = &clk_sclk_vpll.clk, - }, - .reg_div = { .reg =S5P_CLK_DIV1 , .shift =0 , .size =4 }, -}; - -static struct clk *clkset_sclk_hdmi_list[] = { - [0 ] = &clk_sclk_pixel.clk, - [1 ] = &clk_sclk_hdmiphy, -}; - -static struct clksrc_sources clkset_sclk_hdmi = { - .sources = clkset_sclk_hdmi_list, - .nr_sources =ARRAY_SIZE (clkset_sclk_hdmi_list), -}; - -static struct clksrc_clk clk_sclk_hdmi = { - .clk = { - .name ="sclk_hdmi" , - .id =-1 , - .ctrlbit = (1 <<0 ), - .enable = s5pv210_clk_mask0_ctrl, - }, - .sources = &clkset_sclk_hdmi, - .reg_src = { .reg =S5P_CLK_SRC1 , .shift =0 , .size =1 }, -}; - -static struct clk *clkset_sclk_mixer_list[] = { - [0 ] = &clk_sclk_dac.clk, - [1 ] = &clk_sclk_hdmi.clk, -}; - -static struct clksrc_sources clkset_sclk_mixer = { - .sources = clkset_sclk_mixer_list, - .nr_sources =ARRAY_SIZE (clkset_sclk_mixer_list), -}; - -static struct clk *clkset_sclk_audio0_list[] = { - [0 ] = &clk_ext_xtal_mux, - [1 ] = &clk_pcmcdclk0, - [2 ] = &clk_sclk_hdmi27m, - [3 ] = &clk_sclk_usbphy0, - [4 ] = &clk_sclk_usbphy1, - [5 ] = &clk_sclk_hdmiphy, - [6 ] = &clk_mout_mpll.clk, - [7 ] = &clk_mout_epll.clk, - [8 ] = &clk_sclk_vpll.clk, -}; - -static struct clksrc_sources clkset_sclk_audio0 = { - .sources = clkset_sclk_audio0_list, - .nr_sources =ARRAY_SIZE (clkset_sclk_audio0_list), -}; - -static struct clksrc_clk clk_sclk_audio0 = { - .clk = { - .name ="sclk_audio" , - .id =0 , - .enable = s5pv210_clk_mask0_ctrl, - .ctrlbit = (1 <<24 ), - }, - .sources = &clkset_sclk_audio0, - .reg_src = { .reg =S5P_CLK_SRC6 , .shift =0 , .size =4 }, - .reg_div = { .reg =S5P_CLK_DIV6 , .shift =0 , .size =4 }, -}; - -static struct clk *clkset_sclk_audio1_list[] = { - [0 ] = &clk_ext_xtal_mux, - [1 ] = &clk_pcmcdclk1, - [2 ] = &clk_sclk_hdmi27m, - [3 ] = &clk_sclk_usbphy0, - [4 ] = &clk_sclk_usbphy1, - [5 ] = &clk_sclk_hdmiphy, - [6 ] = &clk_mout_mpll.clk, - [7 ] = &clk_mout_epll.clk, - [8 ] = &clk_sclk_vpll.clk, -}; - -static struct clksrc_sources clkset_sclk_audio1 = { - .sources = clkset_sclk_audio1_list, - .nr_sources =ARRAY_SIZE (clkset_sclk_audio1_list), -}; - -static struct clksrc_clk clk_sclk_audio1 = { - .clk = { - .name ="sclk_audio" , - .id =1 , - .enable = s5pv210_clk_mask0_ctrl, - .ctrlbit = (1 <<25 ), - }, - .sources = &clkset_sclk_audio1, - .reg_src = { .reg =S5P_CLK_SRC6 , .shift =4 , .size =4 }, - .reg_div = { .reg =S5P_CLK_DIV6 , .shift =4 , .size =4 }, -}; - -static struct clk *clkset_sclk_audio2_list[] = { - [0 ] = &clk_ext_xtal_mux, - [1 ] = &clk_pcmcdclk0, - [2 ] = &clk_sclk_hdmi27m, - [3 ] = &clk_sclk_usbphy0, - [4 ] = &clk_sclk_usbphy1, - [5 ] = &clk_sclk_hdmiphy, - [6 ] = &clk_mout_mpll.clk, - [7 ] = &clk_mout_epll.clk, - [8 ] = &clk_sclk_vpll.clk, -}; - -static struct clksrc_sources clkset_sclk_audio2 = { - .sources = clkset_sclk_audio2_list, - .nr_sources =ARRAY_SIZE (clkset_sclk_audio2_list), -}; - -static struct clksrc_clk clk_sclk_audio2 = { - .clk = { - .name ="sclk_audio" , - .id =2 , - .enable = s5pv210_clk_mask0_ctrl, - .ctrlbit = (1 <<26 ), - }, - .sources = &clkset_sclk_audio2, - .reg_src = { .reg =S5P_CLK_SRC6 , .shift =8 , .size =4 }, - .reg_div = { .reg =S5P_CLK_DIV6 , .shift =8 , .size =4 }, -}; - -static struct clk *clkset_sclk_spdif_list[] = { - [0 ] = &clk_sclk_audio0.clk, - [1 ] = &clk_sclk_audio1.clk, - [2 ] = &clk_sclk_audio2.clk, -}; - -static struct clksrc_sources clkset_sclk_spdif = { - .sources = clkset_sclk_spdif_list, - .nr_sources =ARRAY_SIZE (clkset_sclk_spdif_list), -}; - -static struct clk *clkset_group2_list[] = { - [0 ] = &clk_ext_xtal_mux, - [1 ] = &clk_xusbxti, - [2 ] = &clk_sclk_hdmi27m, - [3 ] = &clk_sclk_usbphy0, - [4 ] = &clk_sclk_usbphy1, - [5 ] = &clk_sclk_hdmiphy, - [6 ] = &clk_mout_mpll.clk, - [7 ] = &clk_mout_epll.clk, - [8 ] = &clk_sclk_vpll.clk, -}; - -static struct clksrc_sources clkset_group2 = { - .sources = clkset_group2_list, - .nr_sources =ARRAY_SIZE (clkset_group2_list), -}; - -static struct clksrc_clk clksrcs[] = { - { - .clk = { - .name ="sclk_dmc" , - .id =-1 , - }, - .sources = &clkset_group1, - .reg_src = { .reg =S5P_CLK_SRC6 , .shift =24 , .size =2 }, - .reg_div = { .reg =S5P_CLK_DIV6 , .shift =28 , .size =4 }, - }, { - .clk = { - .name ="sclk_onenand" , - .id =-1 , - }, - .sources = &clkset_sclk_onenand, - .reg_src = { .reg =S5P_CLK_SRC0 , .shift =28 , .size =1 }, - .reg_div = { .reg =S5P_CLK_DIV6 , .shift =12 , .size =3 }, - }, { - .clk = { - .name ="uclk1" , - .id =0 , - .enable = s5pv210_clk_mask0_ctrl, - .ctrlbit = (1 <<12 ), - }, - .sources = &clkset_uart, - .reg_src = { .reg =S5P_CLK_SRC4 , .shift =16 , .size =4 }, - .reg_div = { .reg =S5P_CLK_DIV4 , .shift =16 , .size =4 }, - }, { - .clk = { - .name ="uclk1" , - .id =1 , - .enable = s5pv210_clk_mask0_ctrl, - .ctrlbit = (1 <<13 ), - }, - .sources = &clkset_uart, - .reg_src = { .reg =S5P_CLK_SRC4 , .shift =20 , .size =4 }, - .reg_div = { .reg =S5P_CLK_DIV4 , .shift =20 , .size =4 }, - }, { - .clk = { - .name ="uclk1" , - .id =2 , - .enable = s5pv210_clk_mask0_ctrl, - .ctrlbit = (1 <<14 ), - }, - .sources = &clkset_uart, - .reg_src = { .reg =S5P_CLK_SRC4 , .shift =24 , .size =4 }, - .reg_div = { .reg =S5P_CLK_DIV4 , .shift =24 , .size =4 }, - }, { - .clk = { - .name ="uclk1" , - .id =3 , - .enable = s5pv210_clk_mask0_ctrl, - .ctrlbit = (1 <<15 ), - }, - .sources = &clkset_uart, - .reg_src = { .reg =S5P_CLK_SRC4 , .shift =28 , .size =4 }, - .reg_div = { .reg =S5P_CLK_DIV4 , .shift =28 , .size =4 }, - }, { - .clk = { - .name ="sclk_mixer" , - .id =-1 , - .enable = s5pv210_clk_mask0_ctrl, - .ctrlbit = (1 <<1 ), - }, - .sources = &clkset_sclk_mixer, - .reg_src = { .reg =S5P_CLK_SRC1 , .shift =4 , .size =1 }, - }, { - .clk = { - .name ="sclk_spdif" , - .id =-1 , - .enable = s5pv210_clk_mask0_ctrl, - .ctrlbit = (1 <<27 ), - }, - .sources = &clkset_sclk_spdif, - .reg_src = { .reg =S5P_CLK_SRC6 , .shift =12 , .size =2 }, - }, { - .clk = { - .name ="sclk_fimc" , - .id =0 , - .enable = s5pv210_clk_mask1_ctrl, - .ctrlbit = (1 <<2 ), - }, - .sources = &clkset_group2, - .reg_src = { .reg =S5P_CLK_SRC3 , .shift =12 , .size =4 }, - .reg_div = { .reg =S5P_CLK_DIV3 , .shift =12 , .size =4 }, - }, { - .clk = { - .name ="sclk_fimc" , - .id =1 , - .enable = s5pv210_clk_mask1_ctrl, - .ctrlbit = (1 <<3 ), - }, - .sources = &clkset_group2, - .reg_src = { .reg =S5P_CLK_SRC3 , .shift =16 , .size =4 }, - .reg_div = { .reg =S5P_CLK_DIV3 , .shift =16 , .size =4 }, - }, { - .clk = { - .name ="sclk_fimc" , - .id =2 , - .enable = s5pv210_clk_mask1_ctrl, - .ctrlbit = (1 <<4 ), - }, - .sources = &clkset_group2, - .reg_src = { .reg =S5P_CLK_SRC3 , .shift =20 , .size =4 }, - .reg_div = { .reg =S5P_CLK_DIV3 , .shift =20 , .size =4 }, - }, { - .clk = { - .name ="sclk_cam" , - .id =0 , - .enable = s5pv210_clk_mask0_ctrl, - .ctrlbit = (1 <<3 ), - }, - .sources = &clkset_group2, - .reg_src = { .reg =S5P_CLK_SRC1 , .shift =12 , .size =4 }, - .reg_div = { .reg =S5P_CLK_DIV1 , .shift =12 , .size =4 }, - }, { - .clk = { - .name ="sclk_cam" , - .id =1 , - .enable = s5pv210_clk_mask0_ctrl, - .ctrlbit = (1 <<4 ), - }, - .sources = &clkset_group2, - .reg_src = { .reg =S5P_CLK_SRC1 , .shift =16 , .size =4 }, - .reg_div = { .reg =S5P_CLK_DIV1 , .shift =16 , .size =4 }, - }, { - .clk = { - .name ="sclk_fimd" , - .id =-1 , - .enable = s5pv210_clk_mask0_ctrl, - .ctrlbit = (1 <<5 ), - }, - .sources = &clkset_group2, - .reg_src = { .reg =S5P_CLK_SRC1 , .shift =20 , .size =4 }, - .reg_div = { .reg =S5P_CLK_DIV1 , .shift =20 , .size =4 }, - }, { - .clk = { - .name ="sclk_mmc" , - .id =0 , - .enable = s5pv210_clk_mask0_ctrl, - .ctrlbit = (1 <<8 ), - }, - .sources = &clkset_group2, - .reg_src = { .reg =S5P_CLK_SRC4 , .shift =0 , .size =4 }, - .reg_div = { .reg =S5P_CLK_DIV4 , .shift =0 , .size =4 }, - }, { - .clk = { - .name ="sclk_mmc" , - .id =1 , - .enable = s5pv210_clk_mask0_ctrl, - .ctrlbit = (1 <<9 ), - }, - .sources = &clkset_group2, - .reg_src = { .reg =S5P_CLK_SRC4 , .shift =4 , .size =4 }, - .reg_div = { .reg =S5P_CLK_DIV4 , .shift =4 , .size =4 }, - }, { - .clk = { - .name ="sclk_mmc" , - .id =2 , - .enable = s5pv210_clk_mask0_ctrl, - .ctrlbit = (1 <<10 ), - }, - .sources = &clkset_group2, - .reg_src = { .reg =S5P_CLK_SRC4 , .shift =8 , .size =4 }, - .reg_div = { .reg =S5P_CLK_DIV4 , .shift =8 , .size =4 }, - }, { - .clk = { - .name ="sclk_mmc" , - .id =3 , - .enable = s5pv210_clk_mask0_ctrl, - .ctrlbit = (1 <<11 ), - }, - .sources = &clkset_group2, - .reg_src = { .reg =S5P_CLK_SRC4 , .shift =12 , .size =4 }, - .reg_div = { .reg =S5P_CLK_DIV4 , .shift =12 , .size =4 }, - }, { - .clk = { - .name ="sclk_mfc" , - .id =-1 , - .enable = s5pv210_clk_ip0_ctrl, - .ctrlbit = (1 <<16 ), - }, - .sources = &clkset_group1, - .reg_src = { .reg =S5P_CLK_SRC2 , .shift =4 , .size =2 }, - .reg_div = { .reg =S5P_CLK_DIV2 , .shift =4 , .size =4 }, - }, { - .clk = { - .name ="sclk_g2d" , - .id =-1 , - .enable = s5pv210_clk_ip0_ctrl, - .ctrlbit = (1 <<12 ), - }, - .sources = &clkset_group1, - .reg_src = { .reg =S5P_CLK_SRC2 , .shift =8 , .size =2 }, - .reg_div = { .reg =S5P_CLK_DIV2 , .shift =8 , .size =4 }, - }, { - .clk = { - .name ="sclk_g3d" , - .id =-1 , - .enable = s5pv210_clk_ip0_ctrl, - .ctrlbit = (1 <<8 ), - }, - .sources = &clkset_group1, - .reg_src = { .reg =S5P_CLK_SRC2 , .shift =0 , .size =2 }, - .reg_div = { .reg =S5P_CLK_DIV2 , .shift =0 , .size =4 }, - }, { - .clk = { - .name ="sclk_csis" , - .id =-1 , - .enable = s5pv210_clk_mask0_ctrl, - .ctrlbit = (1 <<6 ), - }, - .sources = &clkset_group2, - .reg_src = { .reg =S5P_CLK_SRC1 , .shift =24 , .size =4 }, - .reg_div = { .reg =S5P_CLK_DIV1 , .shift =28 , .size =4 }, - }, { - .clk = { - .name ="sclk_spi" , - .id =0 , - .enable = s5pv210_clk_mask0_ctrl, - .ctrlbit = (1 <<16 ), - }, - .sources = &clkset_group2, - .reg_src = { .reg =S5P_CLK_SRC5 , .shift =0 , .size =4 }, - .reg_div = { .reg =S5P_CLK_DIV5 , .shift =0 , .size =4 }, - }, { - .clk = { - .name ="sclk_spi" , - .id =1 , - .enable = s5pv210_clk_mask0_ctrl, - .ctrlbit = (1 <<17 ), - }, - .sources = &clkset_group2, - .reg_src = { .reg =S5P_CLK_SRC5 , .shift =4 , .size =4 }, - .reg_div = { .reg =S5P_CLK_DIV5 , .shift =4 , .size =4 }, - }, { - .clk = { - .name ="sclk_pwi" , - .id =-1 , - .enable = s5pv210_clk_mask0_ctrl, - .ctrlbit = (1 <<29 ), - }, - .sources = &clkset_group2, - .reg_src = { .reg =S5P_CLK_SRC6 , .shift =20 , .size =4 }, - .reg_div = { .reg =S5P_CLK_DIV6 , .shift =24 , .size =4 }, - }, { - .clk = { - .name ="sclk_pwm" , - .id =-1 , - .enable = s5pv210_clk_mask0_ctrl, - .ctrlbit = (1 <<19 ), - }, - .sources = &clkset_group2, - .reg_src = { .reg =S5P_CLK_SRC5 , .shift =12 , .size =4 }, - .reg_div = { .reg =S5P_CLK_DIV5 , .shift =12 , .size =4 }, - }, -}; -/* Clock initialisation code */ static struct clksrc_clk *sysclks[] = { &clk_mout_apll,
Signature · 1 line
1.6.3.3
PATCH 1/1 ARM: S5PV210: allow clk to use clksrc as parents
Quoted 1 line · MyungJoo Ham
Hi,
Cc'ed Ben Dooks.
Quoted 10 lines
This patch allows clk in init_clocks[] and init_clocks_disable[] to use clksrc_clk.clk in clksrcs[].
For example, clk "lcd" may need to set "sclk-fimd" as a parent if the div and src settting of sclk-fimd changes and a user of "lcd" wants to get the exact information about the rate. If "sclk-fimd" is set as a parent of "lcd", then the clk framework can provide the wanted information; otherwise, the change in "sclk-fimd" does not update "lcd" properly.
I don't think so.
This can be problem because the parent clock of LCD(FIMD) is not always SCLK_FIMD. And LCD block is on DSYS clock domain, so the parent clock of 'lcd' should be dsys clock, not sclk_fimd.
Basically used dsys bus clock as LCD IP clock and can be used sclk_fimd or not. So should separate them and need to separate control.
As I still saying, the CLK_GATE_IP1[0] for LCD block can gate clock of SCLK_ACLK and SCLK_FIMD which can be source of LCD block, and CLK_SRC_MASK0[5] can gate only SCLK_FIMD, output clock of MUX FIMD.
Quoted 11 lines
To do so, we have relocated clksrcs[] and put enum indexes to clksrcs in order to allow clk in init_clocks[] and init_clocks_disable[] may refer one of clksrcs[] as its parent. (enum
clksrc_refer)For example, "lcd" may set "sclk-fimd" as a parent if the following line replaces the line 878:
.parent = &clk_hclk_dsys.clk,to
Quoted 8 lines
.parent = &clksrcs[_clksrc_sclk_fimd],Signed-off-by MyungJoo Ham <[email protected]>Signed-off-by Kyungmin Park <[email protected]>--- arch/arm/mach-s5pv210/clock.c | 986
++++++++++++++++++++++-------------------
arch/arm/mach-s5pv210/clock.c static struct clksrc_clk clk_sclk_vpll = { · line 266
Quoted 10 lines
1 files changed, 520 insertions(+), 466 deletions(-)arch/arm/mach-s5pv210/clock.c +0
-0@@ -264,6 +264,526 @@ static struct clksrc_clk clk_sclk_vpll = { .reg_src = { .reg = S5P_CLK_SRC0, .shift = 12, .size = 1 }, };
And...hmm...maybe made this patch with wrong way...
arch/arm/mach-s5pv210/clock.c static struct clksrc_clk clk_sclk_vpll = { · line 271
Quoted 5 lines
arch/arm/mach-s5pv210/clock.c +5
-0+static struct clk *clkset_uart_list[] = { + [6 ] = &clk_mout_mpll.clk, + [7 ] = &clk_mout_epll.clk, +}; +
Following is from this patch...
-static struct clk *clkset_uart_list[] = {
- [6] = &clk_mout_mpll.clk,
- [7] = &clk_mout_epll.clk,
-};
-
Same? :-(
Why did you remove and add same code?
(snip)
And...see the below.
Added like below...
Quoted 9 lines
arch/arm/mach-s5pv210/clock.c +9
-0+static struct clksrc_clk clksrcs[] = { + [_clksrc_sclk_dmc] = { + .clk = { + .name ="sclk_dmc" , + .id =-1 , + }, + .sources = &clkset_group1, + .reg_src = { .reg =S5P_CLK_SRC6 , .shift =24 , .size =2 }, + .reg_div = { .reg =S5P_CLK_DIV6 , .shift =28 , .size =4 },
arch/arm/mach-s5pv210/clock.c static struct clksrc_clk clk_sclk_vpll = { · line 522
Quoted 10 lines
arch/arm/mach-s5pv210/clock.c +10
-0+ }, + [_clksrc_sclk_onenand] = { + .clk = { + .name = "sclk_onenand", + .id = -1, + }, + .sources = &clkset_sclk_onenand, + .reg_src = { .reg = S5P_CLK_SRC0, .shift = 28, .size = 1 }, + .reg_div = { .reg = S5P_CLK_DIV6, .shift = 12, .size = 3 }, + },
(snip)
And removed like below...
Quoted 8 lines
arch/arm/mach-s5pv210/clock.c +0
-8-static struct clksrc_clk clksrcs[] = { - { - .clk = { - .name ="sclk_dmc" , - .id =-1 , - }, - .sources = &clkset_group1, - .reg_src = { .reg =S5P_CLK_SRC6 , .shift =24 , .size =2 },
arch/arm/mach-s5pv210/clock.c static struct clk init_clocks[] = { · line 980
Quoted 10 lines
arch/arm/mach-s5pv210/clock.c +0
-10- .reg_div = { .reg = S5P_CLK_DIV6, .shift = 28, .size = 4 }, - }, { - .clk = { - .name = "sclk_onenand", - .id = -1, - }, - .sources = &clkset_sclk_onenand, - .reg_src = { .reg = S5P_CLK_SRC0, .shift = 28, .size = 1 }, - .reg_div = { .reg = S5P_CLK_DIV6, .shift = 12, .size = 3 }, - }, {
But only changed small lines not every.
Following is from my local machine.
@@ -665,7 +665,7 @@ static struct clksrc_sources clkset_group2 = {
};
static struct clksrc_clk clksrcs[] = {
- {
+ [_clksrc_sclk_dmc] = {
.clk = {
.name = "sclk_dmc",
.id = -1,
@@ -673,7 +673,8 @@ static struct clksrc_clk clksrcs[] = {
.sources = &clkset_group1,
.reg_src = { .reg = S5P_CLK_SRC6, .shift = 24, .size = 2 },
.reg_div = { .reg = S5P_CLK_DIV6, .shift = 28, .size = 4 },
- }, {
+ },
+ [_clksrc_sclk_onenand] = { .clk = {
.name = "sclk_onenand",
.id = -1,
(snip)
Please make sure your patch has no problem before submitting.
Thanks.
Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.
PATCH 1/1 ARM: S5PV210: allow clk to use clksrc as parents
Hello,
Quoted 16 lines · Kukjin Kim
Quoted 1 line · MyungJoo Ham
Hi,
Cc'ed Ben Dooks.
Quoted 10 lines
This patch allows clk in init_clocks[] and init_clocks_disable[] to use clksrc_clk.clk in clksrcs[].
For example, clk "lcd" may need to set "sclk-fimd" as a parent if the div and src settting of sclk-fimd changes and a user of "lcd" wants to get the exact information about the rate. If "sclk-fimd" is set as a parent of "lcd", then the clk framework can provide the wanted information; otherwise, the change in "sclk-fimd" does not update "lcd" properly.
Quoted 7 lines
I don't think so.
This can be problem because the parent clock of LCD(FIMD) is not always
SCLK_FIMD. And LCD block is on DSYS clock domain, so the parent clock of 'lcd' should be dsys clock, notsclk_fimd.
Basically used dsys bus clock as LCD IP clock and can be usedsclk_fimdor not. So should separate them and need to separate control.
Well, yes SCLK_FIMD is not ALWAYS the parent of LCD(FIMD), but it is one of the possible parents of LCD(FIMD); thus, SCLK_FIMD should be able to become the parent. I'm not saying that SCLK_FIMD should be the parent, but SCLK_FIMD should be ABLE to be the parent. That's why I made it just "possible', not making it an actual parent.
However, in order to make it look better, we may need to make LCD(FIMD) easier to switch between SCLK_ACLK and SCLK_FIMD; e.g., by making it another clksrc_clk with source clocks of SCLK_ACLK and SCLK_FIMD controlling with CLK_GATE_IP.
Quoted 52 lines
As I still saying, the CLK_GATE_IP1[0] for LCD block can gate clock of
SCLK_ACLKandSCLK_FIMDwhich can be source of LCD block, and CLK_SRC_MASK0[5] can gate onlySCLK_FIMD, output clock of MUX FIMD.Quoted 11 lines
To do so, we have relocated clksrcs[] and put enum indexes to clksrcs in order to allow clk in init_clocks[] and init_clocks_disable[] may refer one of clksrcs[] as its parent. (enum
clksrc_refer)For example, "lcd" may set "sclk-fimd" as a parent if the following line replaces the line 878:
? ? ? ? ? ? ? .parent ? ? ? ? = &clk_hclk_dsys.clk,
to
Quoted 8 lines
? ? ? ? ? ? ? .parent ? ? ? ? = &clksrcs[_clksrc_sclk_fimd],
Signed-off-by MyungJoo Ham <[email protected]>Signed-off-by Kyungmin Park <[email protected]>---
?arch/arm/mach-s5pv210/clock.c | ?986++++++++++++++++++++++-------------------
arch/arm/mach-s5pv210/clock.c static struct clksrc_clk clk_sclk_vpll = { · line 266
Quoted 10 lines
?1 files changed, 520 insertions(+), 466 deletions(-)
arch/arm/mach-s5pv210/clock.c +0
-0@@ -264,6 +264,526 @@ static struct clksrc_clk clk_sclk_vpll = { ? ? ? .reg_src ? ? ? ?= { .reg = S5P_CLK_SRC0, .shift = 12, .size = 1 }, ?};And...hmm...maybe made this patch with wrong way...
arch/arm/mach-s5pv210/clock.c static struct clksrc_clk clk_sclk_vpll = { · line 271
Quoted 5 lines
arch/arm/mach-s5pv210/clock.c +5
-0+static struct clk *clkset_uart_list[] = { + ? ? [6] = &clk_mout_mpll.clk, + ? ? [7] = &clk_mout_epll.clk, +}; +Following is from this patch...
arch/arm/mach-s5pv210/clock.c +0
-5-static struct clk *clkset_uart_list[] = { - ? ? ? [6] = &clk_mout_mpll.clk, - ? ? ? [7] = &clk_mout_epll.clk, -}; -
Quoted 2 lines
Same? :-(
Why did you remove and add same code?
It is done so in order to make clksrc_clk to be able to be parents of clk. (same with the
later parts)
To do so, I moved clksrc_clk to the front of clk definitions, which in turn moved
clock source list to the front of clksrc_clk.
Or, in order to reduce the changed lines number, would it be better to declare the list of struct clksrc_clk before and keep the locations of the lists?
Quoted 85 lines
(snip)
And...see the below.
Added like below...
Quoted 9 lines
arch/arm/mach-s5pv210/clock.c +9
-0+static struct clksrc_clk clksrcs[] = { + ? ? [_clksrc_sclk_dmc] = { + ? ? ? ? ? ? .clk ? ?= { + ? ? ? ? ? ? ? ? ? ? .name ? ? ? ? ? = "sclk_dmc", + ? ? ? ? ? ? ? ? ? ? .id ? ? ? ? ? ? = -1, + ? ? ? ? ? ? }, + ? ? ? ? ? ? .sources = &clkset_group1, + ? ? ? ? ? ? .reg_src = { .reg = S5P_CLK_SRC6, .shift = 24, .size = 2 }, + ? ? ? ? ? ? .reg_div = { .reg = S5P_CLK_DIV6, .shift = 28, .size = 4 },arch/arm/mach-s5pv210/clock.c static struct clksrc_clk clk_sclk_vpll = { · line 290
Quoted 10 lines
arch/arm/mach-s5pv210/clock.c +10
-0+ ? ? }, + ? ? [_clksrc_sclk_onenand] = { + ? ? ? ? ? ? .clk ? ?= { + ? ? ? ? ? ? ? ? ? ? .name ? ? ? ? ? = "sclk_onenand", + ? ? ? ? ? ? ? ? ? ? .id ? ? ? ? ? ? = -1, + ? ? ? ? ? ? }, + ? ? ? ? ? ? .sources = &clkset_sclk_onenand, + ? ? ? ? ? ? .reg_src = { .reg = S5P_CLK_SRC0, .shift = 28, .size = 1 }, + ? ? ? ? ? ? .reg_div = { .reg = S5P_CLK_DIV6, .shift = 12, .size = 3 }, + ? ? },(snip)
And removed like below...
Quoted 8 lines
arch/arm/mach-s5pv210/clock.c +0
-8-static struct clksrc_clk clksrcs[] = { - ? ? { - ? ? ? ? ? ? .clk ? ?= { - ? ? ? ? ? ? ? ? ? ? .name ? ? ? ? ? = "sclk_dmc", - ? ? ? ? ? ? ? ? ? ? .id ? ? ? ? ? ? = -1, - ? ? ? ? ? ? }, - ? ? ? ? ? ? .sources = &clkset_group1, - ? ? ? ? ? ? .reg_src = { .reg = S5P_CLK_SRC6, .shift = 24, .size = 2 },arch/arm/mach-s5pv210/clock.c static struct clksrc_clk clk_sclk_vpll = {
Quoted 10 lines
arch/arm/mach-s5pv210/clock.c +0
-10- ? ? ? ? ? ? .reg_div = { .reg = S5P_CLK_DIV6, .shift = 28, .size = 4 }, - ? ? }, { - ? ? ? ? ? ? .clk ? ?= { - ? ? ? ? ? ? ? ? ? ? .name ? ? ? ? ? = "sclk_onenand", - ? ? ? ? ? ? ? ? ? ? .id ? ? ? ? ? ? = -1, - ? ? ? ? ? ? }, - ? ? ? ? ? ? .sources = &clkset_sclk_onenand, - ? ? ? ? ? ? .reg_src = { .reg = S5P_CLK_SRC0, .shift = 28, .size = 1 }, - ? ? ? ? ? ? .reg_div = { .reg = S5P_CLK_DIV6, .shift = 12, .size = 3 }, - ? ? }, {But only changed small lines not every.
Following is from my local machine.(quoted diff) +3
-2@@ -665,7 +665,7 @@ static struct clksrc_sources clkset_group2 = { ?}; ?static struct clksrc_clk clksrcs[] = { - ? ? ? { + ? ? ? [_clksrc_sclk_dmc] = { ? ? ? ? ? ? ? ?.clk ? ?= { ? ? ? ? ? ? ? ? ? ? ? ?.name ? ? ? ? ? = "sclk_dmc", ? ? ? ? ? ? ? ? ? ? ? ?.id ? ? ? ? ? ? = -1,@@ -673,7 +673,8 @@ static struct clksrc_clk clksrcs[] = { ? ? ? ? ? ? ? ?.sources = &clkset_group1, ? ? ? ? ? ? ? ?.reg_src = { .reg = S5P_CLK_SRC6, .shift = 24, .size = 2 }, ? ? ? ? ? ? ? ?.reg_div = { .reg = S5P_CLK_DIV6, .shift = 28, .size = 4 }, - ? ? ? }, { + ? ? ? }, + ? ? ? [_clksrc_sclk_onenand] = { ? ? ? ? ? ? ? ?.clk ? ?= { ? ? ? ? ? ? ? ? ? ? ? ?.name ? ? ? ? ? = "sclk_onenand", ? ? ? ? ? ? ? ? ? ? ? ?.id ? ? ? ? ? ? = -1,?(snip)
Please make sure your patch has no problem before submitting.
Thanks.
Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.
Signature · 5 lines
MyungJoo Ham (???), Ph.D. Mobile Software Platform Lab, Digital Media and Communications (DMC) Business Samsung Electronics cell: 82-10-6714-2858
PATCH 1/1 ARM: S5PV210: allow clk to use clksrc as parents
Quoted 28 lines · MyungJoo Ham
Hello,
Quoted 16 lines · Kukjin Kim
Quoted 1 line · MyungJoo Ham
Hi,
Cc'ed Ben Dooks.
Quoted 10 lines
This patch allows clk in init_clocks[] and init_clocks_disable[] to use clksrc_clk.clk in clksrcs[].
For example, clk "lcd" may need to set "sclk-fimd" as a parent if the div and src settting of sclk-fimd changes and a user of "lcd" wants to get the exact information about the rate. If "sclk-fimd" is set as a parent of "lcd", then the clk framework can provide the wanted information; otherwise, the change in "sclk-fimd" does not update "lcd" properly.
Quoted 7 lines
I don't think so.
This can be problem because the parent clock of LCD(FIMD) is not always
SCLK_FIMD. And LCD block is on DSYS clock domain, so the parent clock of 'lcd' should be dsys clock, notsclk_fimd.
Basically used dsys bus clock as LCD IP clock and can be usedsclk_fimdor not. So should separate them and need to separate control.
Quoted 7 lines
Well, yes
SCLK_FIMDis not ALWAYS the parent of LCD(FIMD), but it is one of the possible parents of LCD(FIMD); thus,SCLK_FIMDshould be able to become the parent. I'm not saying thatSCLK_FIMDshould be the parent, butSCLK_FIMDshould
be ABLE to be the parent. That's why I made it just "possible', not making it an actual parent.
More correctly, the clk 'lcd' means clock that should be used basically at FIMD block. So it means used clock on system bus, should be SCLK_ACLK.
Actually, if disabled clk 'lcd', we can't access FIMD block such as its register.
As I say again, clk 'lcd' is on dsys clock domain, because of these meaning, so the parent clock of 'lcd' should be dsys clock.
Quoted 150 lines
However, in order to make it look better, we may need to make LCD(FIMD) easier to switch between
SCLK_ACLKandSCLK_FIMD; e.g., by making it anotherclksrc_clk
with source clocks ofSCLK_ACLKandSCLK_FIMDcontrolling withCLK_GATE_IP.Quoted 52 lines
As I still saying, the CLK_GATE_IP1[0] for LCD block can gate clock of
SCLK_ACLKandSCLK_FIMDwhich can be source of LCD block, and CLK_SRC_MASK0[5] can gate onlySCLK_FIMD, output clock of MUX FIMD.Quoted 11 lines
To do so, we have relocated clksrcs[] and put enum indexes to clksrcs in order to allow clk in init_clocks[] and init_clocks_disable[] may refer one of clksrcs[] as its parent. (enum
clksrc_refer)For example, "lcd" may set "sclk-fimd" as a parent if the following line replaces the line 878:
.parent = &clk_hclk_dsys.clk,
to
Quoted 8 lines
.parent = &clksrcs[_clksrc_sclk_fimd],
Signed-off-by MyungJoo Ham <[email protected]>Signed-off-by Kyungmin Park <[email protected]>--- arch/arm/mach-s5pv210/clock.c | 986++++++++++++++++++++++-------------------
arch/arm/mach-s5pv210/clock.c static struct clksrc_clk clk_sclk_vpll = { · line 266
Quoted 10 lines
1 files changed, 520 insertions(+), 466 deletions(-)arch/arm/mach-s5pv210/clock.c +0
-0@@ -264,6 +264,526 @@ static struct clksrc_clk clk_sclk_vpll = { .reg_src = { .reg = S5P_CLK_SRC0, .shift = 12, .size = 1 }, };And...hmm...maybe made this patch with wrong way...
arch/arm/mach-s5pv210/clock.c static struct clksrc_clk clk_sclk_vpll = { · line 271
Quoted 5 lines
arch/arm/mach-s5pv210/clock.c +5
-0+static struct clk *clkset_uart_list[] = { + [6 ] = &clk_mout_mpll.clk, + [7 ] = &clk_mout_epll.clk, +}; +Following is from this patch...
arch/arm/mach-s5pv210/clock.c +0
-5-static struct clk *clkset_uart_list[] = { - [6 ] = &clk_mout_mpll.clk, - [7 ] = &clk_mout_epll.clk, -}; -Quoted 2 lines
Same? :-(
Why did you remove and add same code?It is done so in order to make
clksrc_clkto be able to be parents of clk. (same with the
later parts)To do so, I moved
clksrc_clkto the front of clk definitions, which in turn moved
clock source list to the front ofclksrc_clk.Or, in order to reduce the changed lines number, would it be better to declare the list of struct
clksrc_clkbefore and keep the locations of the lists?Quoted 78 lines
(snip)
And...see the below.
Added like below...
Quoted 9 lines
arch/arm/mach-s5pv210/clock.c +9
-0+static struct clksrc_clk clksrcs[] = { + [_clksrc_sclk_dmc] = { + .clk = { + .name ="sclk_dmc" , + .id =-1 , + }, + .sources = &clkset_group1, + .reg_src = { .reg =S5P_CLK_SRC6 , .shift =24 , .size =2 }, + .reg_div = { .reg =S5P_CLK_DIV6 , .shift =28 , .size =4 },arch/arm/mach-s5pv210/clock.c static struct clksrc_clk clk_sclk_vpll = { · line 522
Quoted 10 lines
arch/arm/mach-s5pv210/clock.c +10
-0+ }, + [_clksrc_sclk_onenand] = { + .clk = { + .name = "sclk_onenand", + .id = -1, + }, + .sources = &clkset_sclk_onenand, + .reg_src = { .reg = S5P_CLK_SRC0, .shift = 28, .size = 1 }, + .reg_div = { .reg = S5P_CLK_DIV6, .shift = 12, .size = 3 }, + },(snip)
And removed like below...
Quoted 8 lines
arch/arm/mach-s5pv210/clock.c +0
-8-static struct clksrc_clk clksrcs[] = { - { - .clk = { - .name ="sclk_dmc" , - .id =-1 , - }, - .sources = &clkset_group1, - .reg_src = { .reg =S5P_CLK_SRC6 , .shift =24 , .size =2 },arch/arm/mach-s5pv210/clock.c static struct clk init_clocks[] = { · line 980
Quoted 10 lines
arch/arm/mach-s5pv210/clock.c +0
-10- .reg_div = { .reg = S5P_CLK_DIV6, .shift = 28, .size = 4 }, - }, { - .clk = { - .name = "sclk_onenand", - .id = -1, - }, - .sources = &clkset_sclk_onenand, - .reg_src = { .reg = S5P_CLK_SRC0, .shift = 28, .size = 1 }, - .reg_div = { .reg = S5P_CLK_DIV6, .shift = 12, .size = 3 }, - }, {But only changed small lines not every.
Following is from my local machine.(quoted diff) +3
-2@@ -665,7 +665,7 @@ static struct clksrc_sources clkset_group2 = { }; static struct clksrc_clk clksrcs[] = { - { + [_clksrc_sclk_dmc] = { .clk = { .name = "sclk_dmc", .id = -1,@@ -673,7 +673,8 @@ static struct clksrc_clk clksrcs[] = { .sources = &clkset_group1, .reg_src = { .reg = S5P_CLK_SRC6, .shift = 24, .size = 2 }, .reg_div = { .reg = S5P_CLK_DIV6, .shift = 28, .size = 4 }, - }, { + }, + [_clksrc_sclk_onenand] = { .clk = { .name = "sclk_onenand", .id = -1,Please make sure your patch has no problem before submitting.
Thanks.
Quoted 7 lines
Quoted 7 lines
Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.
Thanks.
Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.
PATCH 1/1 ARM: S5PV210: allow clk to use clksrc as parents
Hello,
Quoted 36 lines · Kukjin Kim
Quoted 28 lines · MyungJoo Ham
Hello,
Quoted 16 lines · Kukjin Kim
Quoted 1 line · MyungJoo Ham
Hi,
Cc'ed Ben Dooks.
Quoted 10 lines
This patch allows clk in init_clocks[] and init_clocks_disable[] to use clksrc_clk.clk in clksrcs[].
For example, clk "lcd" may need to set "sclk-fimd" as a parent if the div and src settting of sclk-fimd changes and a user of "lcd" wants to get the exact information about the rate. If "sclk-fimd" is set as a parent of "lcd", then the clk framework can provide the wanted information; otherwise, the change in "sclk-fimd" does not update "lcd" properly.
Quoted 7 lines
I don't think so.
This can be problem because the parent clock of LCD(FIMD) is not always
SCLK_FIMD. And LCD block is on DSYS clock domain, so the parent clock of 'lcd' should be dsys clock, notsclk_fimd.
Basically used dsys bus clock as LCD IP clock and can be usedsclk_fimdor not. So should separate them and need to separate control.Quoted 7 lines
Well, yes
SCLK_FIMDis not ALWAYS the parent of LCD(FIMD), but it is one of the possible parents of LCD(FIMD); thus,SCLK_FIMDshould be able to become the parent. I'm not saying thatSCLK_FIMDshould be the parent, butSCLK_FIMDshould
be ABLE to be the parent. That's why I made it just "possible', not making it an actual parent.
Quoted 6 lines
More correctly, the clk 'lcd' means clock that should be used basically at FIMD block. So it means used clock on system bus, should be
SCLK_ACLK.Actually, if disabled clk 'lcd', we can't access FIMD block such as its register.
As I say again, clk 'lcd' is on dsys clock domain, because of these meaning, so the parent clock of 'lcd' should be dsys clock.
Ok, then, clk_get_rate(lcd) returns 166MHz, always, which in turn, makes it more difficult
for lcd clock users to get the correct current rate when sclk_fimd's DIV changes. When we need to show that lcd is in dsys domain, isn't it enough to set dsys is an ancestor (grandparent) of lcd?
I thought it'd contain more semantics if lcd's parent is fimd and fimd's parent is dsys; i.e., dsys is still an ancestor of lcd and clk_get_rate(lcd) may return the clock value of lcd while DIV/SRC changes. When lcd does not use sclk_fimd anymore, we can set_parent(lcd, something) anytime (to do this more easily, we may need to define lcd within clksrc_clk).
Besides, we may need to allow clock-clksrc.c's s3c_register_clksrc function to use the default .parent value to set the SRC register accordingly. For example (a patch under construction, not uploaded yet and requires patches submitted before):
Author: MyungJoo Ham <[email protected]> Date: Thu Jul 8 18:34:01 2010 +0900
ARM: SAMSUNG SoC: clksrc register uses parent value to set default src.
If .parent field is filled when a struct clksrc_clk is registered, s3c_register_clksrc sets the source clock register accordingly to the parent clock.
In the previous versions, the parent value was "ignored" because the value of default register value overwrites the parent field.
(quoted diff) +0 -0
b/arch/arm/plat-samsung/clock-clksrc.c
index ae8b850..236fb59 100644
arch/arm/plat-samsung/clock-clksrc.c +1 -1
@@ -177,7 +177,7 @@ static struct clk_ops clksrc_ops_nosrc = {void __init s3c_register_clksrc (struct clksrc_clk *clksrc,int size) { -int ret; +int ret, err =0 ;for (; size >0 ; size--, clksrc++) {if (!clksrc->reg_div.reg && !clksrc->reg_src.reg)
@@ -195,12 +195,25 @@ void __init s3c_register_clksrc(struct
clksrc_clk *clksrc, int size)
clksrc->clk.ops = &clksrc_ops;
}+ if (clksrc->clk.parent && clksrc->sources &&
+ clksrc->reg_src.reg) {
+ err = s3c_setparent_clksrc(&clksrc->clk,
+ clksrc->clk.parent);
+ if (err < 0)
+ printk(KERN_ERR "[%s:%d] setparent error@reg"
+ "ister_clksrc %s p = %s...\n", + __FILE__, __LINE__, + clksrc->clk.name, + clksrc->clk.parent->name);
+ }
+
/* setup the clocksource, but do not announce it
* as it may be re-set by the setup routines
* called after the rest of the clocks have been
* registered
*/
- s3c_set_clksrc(clksrc, false);
+ if (err < 0)
+ s3c_set_clksrc(clksrc, false);
ret = s3c24xx_register_clock(&clksrc->clk);
Quoted 174 lines
Quoted 151 lines
However, in order to make it look better, we may need to make LCD(FIMD) easier to switch between
SCLK_ACLKandSCLK_FIMD; e.g., by making it anotherclksrc_clk
with source clocks ofSCLK_ACLKandSCLK_FIMDcontrolling withCLK_GATE_IP.Quoted 52 lines
As I still saying, the CLK_GATE_IP1[0] for LCD block can gate clock of
SCLK_ACLKandSCLK_FIMDwhich can be source of LCD block, and CLK_SRC_MASK0[5] can gate onlySCLK_FIMD, output clock of MUX FIMD.Quoted 11 lines
To do so, we have relocated clksrcs[] and put enum indexes to clksrcs in order to allow clk in init_clocks[] and init_clocks_disable[] may refer one of clksrcs[] as its parent. (enum
clksrc_refer)For example, "lcd" may set "sclk-fimd" as a parent if the following line replaces the line 878:
? ? ? ? ? ? ? .parent ? ? ? ? = &clk_hclk_dsys.clk,
to
Quoted 8 lines
? ? ? ? ? ? ? .parent ? ? ? ? = &clksrcs[_clksrc_sclk_fimd],
Signed-off-by MyungJoo Ham <[email protected]>Signed-off-by Kyungmin Park <[email protected]>---
?arch/arm/mach-s5pv210/clock.c | ?986++++++++++++++++++++++-------------------
arch/arm/mach-s5pv210/clock.c static struct clksrc_clk clk_sclk_vpll = { · line 266
Quoted 10 lines
?1 files changed, 520 insertions(+), 466 deletions(-)
arch/arm/mach-s5pv210/clock.c +0
-0@@ -264,6 +264,526 @@ static struct clksrc_clk clk_sclk_vpll = { ? ? ? .reg_src ? ? ? ?= { .reg = S5P_CLK_SRC0, .shift = 12, .size = 1 }, ?};And...hmm...maybe made this patch with wrong way...
arch/arm/mach-s5pv210/clock.c static struct clksrc_clk clk_sclk_vpll = { · line 271
Quoted 5 lines
arch/arm/mach-s5pv210/clock.c +5
-0+static struct clk *clkset_uart_list[] = { + ? ? [6] = &clk_mout_mpll.clk, + ? ? [7] = &clk_mout_epll.clk, +}; +Following is from this patch...
(quoted diff) +0
-5-static struct clk *clkset_uart_list[] = { - ? ? ? [6] = &clk_mout_mpll.clk, - ? ? ? [7] = &clk_mout_epll.clk, -}; -Quoted 2 lines
Same? :-(
Why did you remove and add same code?It is done so in order to make
clksrc_clkto be able to be parents of clk. (same with the
later parts)To do so, I moved
clksrc_clkto the front of clk definitions, which in turn moved
clock source list to the front ofclksrc_clk.Or, in order to reduce the changed lines number, would it be better to declare the list of struct
clksrc_clkbefore and keep the locations of the lists?Quoted 78 lines
(snip)
And...see the below.
Added like below...
Quoted 9 lines
arch/arm/mach-s5pv210/clock.c +9
-0+static struct clksrc_clk clksrcs[] = { + ? ? [_clksrc_sclk_dmc] = { + ? ? ? ? ? ? .clk ? ?= { + ? ? ? ? ? ? ? ? ? ? .name ? ? ? ? ? = "sclk_dmc", + ? ? ? ? ? ? ? ? ? ? .id ? ? ? ? ? ? = -1, + ? ? ? ? ? ? }, + ? ? ? ? ? ? .sources = &clkset_group1, + ? ? ? ? ? ? .reg_src = { .reg = S5P_CLK_SRC6, .shift = 24, .size = 2 }, + ? ? ? ? ? ? .reg_div = { .reg = S5P_CLK_DIV6, .shift = 28, .size = 4 },arch/arm/mach-s5pv210/clock.c static struct clksrc_clk clk_sclk_vpll = { · line 290
Quoted 10 lines
arch/arm/mach-s5pv210/clock.c +10
-0+ ? ? }, + ? ? [_clksrc_sclk_onenand] = { + ? ? ? ? ? ? .clk ? ?= { + ? ? ? ? ? ? ? ? ? ? .name ? ? ? ? ? = "sclk_onenand", + ? ? ? ? ? ? ? ? ? ? .id ? ? ? ? ? ? = -1, + ? ? ? ? ? ? }, + ? ? ? ? ? ? .sources = &clkset_sclk_onenand, + ? ? ? ? ? ? .reg_src = { .reg = S5P_CLK_SRC0, .shift = 28, .size = 1 }, + ? ? ? ? ? ? .reg_div = { .reg = S5P_CLK_DIV6, .shift = 12, .size = 3 }, + ? ? },(snip)
And removed like below...
Quoted 8 lines
arch/arm/mach-s5pv210/clock.c +0
-8-static struct clksrc_clk clksrcs[] = { - ? ? { - ? ? ? ? ? ? .clk ? ?= { - ? ? ? ? ? ? ? ? ? ? .name ? ? ? ? ? = "sclk_dmc", - ? ? ? ? ? ? ? ? ? ? .id ? ? ? ? ? ? = -1, - ? ? ? ? ? ? }, - ? ? ? ? ? ? .sources = &clkset_group1, - ? ? ? ? ? ? .reg_src = { .reg = S5P_CLK_SRC6, .shift = 24, .size = 2 },arch/arm/mach-s5pv210/clock.c static struct clksrc_clk clk_sclk_vpll = {
Quoted 10 lines
arch/arm/mach-s5pv210/clock.c +0
-10- ? ? ? ? ? ? .reg_div = { .reg = S5P_CLK_DIV6, .shift = 28, .size = 4 }, - ? ? }, { - ? ? ? ? ? ? .clk ? ?= { - ? ? ? ? ? ? ? ? ? ? .name ? ? ? ? ? = "sclk_onenand", - ? ? ? ? ? ? ? ? ? ? .id ? ? ? ? ? ? = -1, - ? ? ? ? ? ? }, - ? ? ? ? ? ? .sources = &clkset_sclk_onenand, - ? ? ? ? ? ? .reg_src = { .reg = S5P_CLK_SRC0, .shift = 28, .size = 1 }, - ? ? ? ? ? ? .reg_div = { .reg = S5P_CLK_DIV6, .shift = 12, .size = 3 }, - ? ? }, {But only changed small lines not every.
Following is from my local machine.(quoted diff) +3
-2@@ -665,7 +665,7 @@ static struct clksrc_sources clkset_group2 = { ?}; ?static struct clksrc_clk clksrcs[] = { - ? ? ? { + ? ? ? [_clksrc_sclk_dmc] = { ? ? ? ? ? ? ? ?.clk ? ?= { ? ? ? ? ? ? ? ? ? ? ? ?.name ? ? ? ? ? = "sclk_dmc", ? ? ? ? ? ? ? ? ? ? ? ?.id ? ? ? ? ? ? = -1,@@ -673,7 +673,8 @@ static struct clksrc_clk clksrcs[] = { ? ? ? ? ? ? ? ?.sources = &clkset_group1, ? ? ? ? ? ? ? ?.reg_src = { .reg = S5P_CLK_SRC6, .shift = 24, .size = 2 }, ? ? ? ? ? ? ? ?.reg_div = { .reg = S5P_CLK_DIV6, .shift = 28, .size = 4 }, - ? ? ? }, { + ? ? ? }, + ? ? ? [_clksrc_sclk_onenand] = { ? ? ? ? ? ? ? ?.clk ? ?= { ? ? ? ? ? ? ? ? ? ? ? ?.name ? ? ? ? ? = "sclk_onenand", ? ? ? ? ? ? ? ? ? ? ? ?.id ? ? ? ? ? ? = -1,?(snip)
Please make sure your patch has no problem before submitting.
Thanks.
Quoted 7 lines
Quoted 7 lines
Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.Thanks.
Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd._______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Signature · 5 lines
MyungJoo Ham (???), Ph.D. Mobile Software Platform Lab, Digital Media and Communications (DMC) Business Samsung Electronics cell: 82-10-6714-2858
PATCH 1/1 ARM: S5PV210: allow clk to use clksrc as parents
Quoted 58 lines · MyungJoo Ham
Hello,
Quoted 21 lines · Kukjin Kim
Quoted 20 lines · MyungJoo Ham
Hello,
Quoted 16 lines · Kukjin Kim
Quoted 1 line · MyungJoo Ham
Hi,
Cc'ed Ben Dooks.
Quoted 10 lines
This patch allows clk in init_clocks[] and init_clocks_disable[] to use clksrc_clk.clk in clksrcs[].
For example, clk "lcd" may need to set "sclk-fimd" as a parent if the div and src settting of sclk-fimd changes and a user of "lcd" wants to get the exact information about the rate. If "sclk-fimd" is set as a parent of "lcd", then the clk framework can provide the wanted information; otherwise, the change in "sclk-fimd" does not update "lcd" properly.
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
I don't think so.
This can be problem because the parent clock of LCD(FIMD) is not always
SCLK_FIMD. And LCD block is on DSYS clock domain, so the parent clockof
Quoted 5 lines
Quoted 5 lines
Quoted 3 lines
'lcd' should be dsys clock, not
sclk_fimd.
Basically used dsys bus clock as LCD IP clock and can be usedsclk_fimdor not. So should separate them and need to separate control.Well, yes
SCLK_FIMDis not ALWAYS the parent of LCD(FIMD), but it is one ofthe
Quoted 1 line
Quoted 1 line
possible parents of LCD(FIMD); thus,
SCLK_FIMDshould be able to becomethe
Quoted 6 lines
Quoted 5 lines
parent. I'm not saying that
SCLK_FIMDshould be the parent, butSCLK_FIMDshould
be ABLE to be the parent. That's why I made it just "possible', not making it an actual parent.More correctly, the clk 'lcd' means clock that should be used basically at FIMD
block.
Quoted 5 lines
So it means used clock on system bus, should be
SCLK_ACLK.Actually, if disabled clk 'lcd', we can't access FIMD block such as its register.
As I say again, clk 'lcd' is on dsys clock domain, because of these meaning, so
the parent clock of 'lcd' should be dsys clock.
Ok, then,
clk_get_rate(lcd)returns 166MHz, always, which in turn, makes it more difficult
for lcd clock users to get the correct current rate when sclk_fimd's DIV changes. When we need to show that lcd is in dsys domain, isn't it enough to set dsys is an ancestor (grandparent) of lcd?
Quoted 8 lines
I thought it'd contain more semantics if lcd's parent is fimd and fimd's parent is dsys; i.e., dsys is still an ancestor of lcd and
clk_get_rate(lcd)may return the clock value of lcd while DIV/SRC changes. When lcd does not usesclk_fimdanymore, we canset_parent(lcd, something)anytime (to do this more easily, we may need to define lcd withinclksrc_clk).
Please refer to below diagram.
------------------------------------
dsys bus
----------------+-------------------
|
|1.clk 'lcd'
|
+---+-----------+
| | FIMD block|
| +-+ |
| | |
| | |\ |
| +-|m| |
2.SCLK_FIMD | |u|----+ |
------------+---|x| | |
| |/ | |
| | |
+----------+----+
|
inside of SoC |
-----------------------+--------------------------
outside of SoC |
| 3.clk?
|
+--------------+
| LCD module |
+--------------+In clock.c the clk 'lcd' means #1 in above diagram. So you mean clk 'lcd' is #3', maybe confused :-(
Quoted 66 lines
Besides, we may need to allow clock-clksrc.c's
s3c_register_clksrcfunction to use the default .parent value to set the SRC register accordingly. For example (a patch under construction, not uploaded yet and requires patches submitted before):Author: MyungJoo Ham <[email protected]> Date: Thu Jul 8 18:34:01 2010 +0900ARM: SAMSUNG SoC: clksrc register uses parent value to set default src.
If .parent field is filled when a struct
clksrc_clkis registered,s3c_register_clksrcsets the source clock register accordingly to the parent clock.In the previous versions, the parent value was "ignored" because the value of default register value overwrites the parent field.
arch/arm/plat-samsung/clock-clksrc.c +7
-1@@ -177,7 +177,7 @@ static struct clk_ops clksrc_ops_nosrc = {void __init s3c_register_clksrc (struct clksrc_clk *clksrc,int size) { -int ret; +int ret, err =0 ;for (; size >0 ; size--, clksrc++) {if (!clksrc->reg_div.reg && !clksrc->reg_src.reg)@@ -195,12 +195,25 @@ void __init s3c_register_clksrc(struct clksrc_clk *clksrc, int size) clksrc->clk.ops = &clksrc_ops; } + if (clksrc->clk.parent && clksrc->sources && + clksrc->reg_src.reg) { + err = s3c_setparent_clksrc(&clksrc->clk, + clksrc->clk.parent); + if (err < 0) + printk(KERN_ERR "[%s:%d] setparent errorat reg"
+ "ister_clksrc%s p
= %s...\n",arch/arm/plat-samsung/clock-clksrc.c +7
-1+ __FILE__, __LINE__, + clksrc->clk.name, + clksrc->clk.parent->name); + } +/* setup the clocksource, but do not announce it * as it may be re-set by the setup routines * called after the rest of the clocks have been * registered */ -s3c_set_clksrc (clksrc, false); +if (err <0 ) +s3c_set_clksrc (clksrc, false); ret =s3c24xx_register_clock (&clksrc->clk);Quoted 5 lines
Quoted 160 lines
Quoted 1 line
Quoted 1 line
However, in order to make it look better, we may need to make LCD(FIMD)
easier
Quoted 3 lines
Quoted 3 lines
to switch between
SCLK_ACLKandSCLK_FIMD; e.g., by making it anotherclksrc_clk
with source clocks ofSCLK_ACLKandSCLK_FIMDcontrolling with
CLK_GATE_IP.Quoted 154 lines
Quoted 153 lines
Quoted 53 lines
As I still saying, the CLK_GATE_IP1[0] for LCD block can gate clock of
SCLK_ACLKandSCLK_FIMDwhich can be source of LCD block, and CLK_SRC_MASK0[5] can gate onlySCLK_FIMD, output clock of MUX FIMD.Quoted 11 lines
To do so, we have relocated clksrcs[] and put enum indexes to clksrcs in order to allow clk in init_clocks[] and init_clocks_disable[] may refer one of clksrcs[] as its parent. (enum
clksrc_refer)For example, "lcd" may set "sclk-fimd" as a parent if the following line replaces the line 878:
.parent = &clk_hclk_dsys.clk,
to
Quoted 8 lines
.parent = &clksrcs[_clksrc_sclk_fimd],
Signed-off-by MyungJoo Ham <[email protected]>Signed-off-by Kyungmin Park <[email protected]>--- arch/arm/mach-s5pv210/clock.c | 986++++++++++++++++++++++-------------------
arch/arm/mach-s5pv210/clock.c static struct clksrc_clk clk_sclk_vpll = { · line 266
Quoted 10 lines
1 files changed, 520 insertions(+), 466 deletions(-)arch/arm/mach-s5pv210/clock.c +0
-0@@ -264,6 +264,526 @@ static struct clksrc_clk clk_sclk_vpll = { .reg_src = { .reg = S5P_CLK_SRC0, .shift = 12, .size = 1 }, };And...hmm...maybe made this patch with wrong way...
arch/arm/mach-s5pv210/clock.c static struct clksrc_clk clk_sclk_vpll = { · line 271
Quoted 5 lines
arch/arm/mach-s5pv210/clock.c +5
-0+static struct clk *clkset_uart_list[] = { + [6 ] = &clk_mout_mpll.clk, + [7 ] = &clk_mout_epll.clk, +}; +Following is from this patch...
(quoted diff) +0
-5-static struct clk *clkset_uart_list[] = { - [6] = &clk_mout_mpll.clk, - [7] = &clk_mout_epll.clk, -}; -Quoted 2 lines
Same? :-(
Why did you remove and add same code?It is done so in order to make
clksrc_clkto be able to be parents of clk. (same with the
later parts)To do so, I moved
clksrc_clkto the front of clk definitions, which in turn moved
clock source list to the front ofclksrc_clk.Or, in order to reduce the changed lines number, would it be better to declare the list of struct
clksrc_clkbefore and keep the locations of the lists?Quoted 85 lines
(snip)
And...see the below.
Added like below...
Quoted 9 lines
arch/arm/mach-s5pv210/clock.c +9
-0+static struct clksrc_clk clksrcs[] = { + [_clksrc_sclk_dmc] = { + .clk = { + .name ="sclk_dmc" , + .id =-1 , + }, + .sources = &clkset_group1, + .reg_src = { .reg =S5P_CLK_SRC6 , .shift =24 , .size =2 }, + .reg_div = { .reg =S5P_CLK_DIV6 , .shift =28 , .size =4 },arch/arm/mach-s5pv210/clock.c static struct clksrc_clk clk_sclk_vpll = { · line 290
Quoted 10 lines
arch/arm/mach-s5pv210/clock.c +10
-0+ }, + [_clksrc_sclk_onenand] = { + .clk = { + .name = "sclk_onenand", + .id = -1, + }, + .sources = &clkset_sclk_onenand, + .reg_src = { .reg = S5P_CLK_SRC0, .shift = 28, .size = 1 }, + .reg_div = { .reg = S5P_CLK_DIV6, .shift = 12, .size = 3 }, + },(snip)
And removed like below...
Quoted 8 lines
arch/arm/mach-s5pv210/clock.c +0
-8-static struct clksrc_clk clksrcs[] = { - { - .clk = { - .name ="sclk_dmc" , - .id =-1 , - }, - .sources = &clkset_group1, - .reg_src = { .reg =S5P_CLK_SRC6 , .shift =24 , .size =2 },arch/arm/mach-s5pv210/clock.c static struct clksrc_clk clk_sclk_vpll = {
Quoted 10 lines
arch/arm/mach-s5pv210/clock.c +0
-10- .reg_div = { .reg = S5P_CLK_DIV6, .shift = 28, .size = 4 }, - }, { - .clk = { - .name = "sclk_onenand", - .id = -1, - }, - .sources = &clkset_sclk_onenand, - .reg_src = { .reg = S5P_CLK_SRC0, .shift = 28, .size = 1 }, - .reg_div = { .reg = S5P_CLK_DIV6, .shift = 12, .size = 3 }, - }, {But only changed small lines not every.
Following is from my local machine.(quoted diff) +3
-2@@ -665,7 +665,7 @@ static struct clksrc_sources clkset_group2 = { }; static struct clksrc_clk clksrcs[] = { - { + [_clksrc_sclk_dmc] = { .clk = { .name = "sclk_dmc", .id = -1,@@ -673,7 +673,8 @@ static struct clksrc_clk clksrcs[] = { .sources = &clkset_group1, .reg_src = { .reg = S5P_CLK_SRC6, .shift = 24, .size = 2 }, .reg_div = { .reg = S5P_CLK_DIV6, .shift = 28, .size = 4 }, - }, { + }, + [_clksrc_sclk_onenand] = { .clk = { .name = "sclk_onenand", .id = -1,Please make sure your patch has no problem before submitting.
Thanks.
Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.
Thanks.
Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.
PATCH 1/1 ARM: S5PV210: allow clk to use clksrc as parents
Quoted 96 lines · Kukjin Kim
Quoted 58 lines · MyungJoo Ham
Hello,
Quoted 21 lines · Kukjin Kim
Quoted 20 lines · MyungJoo Ham
Hello,
Quoted 16 lines · Kukjin Kim
Quoted 1 line · MyungJoo Ham
Hi,
Cc'ed Ben Dooks.
Quoted 10 lines
This patch allows clk in init_clocks[] and init_clocks_disable[] to use clksrc_clk.clk in clksrcs[].
For example, clk "lcd" may need to set "sclk-fimd" as a parent if the div and src settting of sclk-fimd changes and a user of "lcd" wants to get the exact information about the rate. If "sclk-fimd" is set as a parent of "lcd", then the clk framework can provide the wanted information; otherwise, the change in "sclk-fimd" does not update "lcd" properly.
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
I don't think so.
This can be problem because the parent clock of LCD(FIMD) is not always
SCLK_FIMD. And LCD block is on DSYS clock domain, so the parent clockof
Quoted 5 lines
Quoted 5 lines
Quoted 3 lines
'lcd' should be dsys clock, not
sclk_fimd.
Basically used dsys bus clock as LCD IP clock and can be usedsclk_fimdor not. So should separate them and need to separate control.Well, yes
SCLK_FIMDis not ALWAYS the parent of LCD(FIMD), but it is one ofthe
Quoted 1 line
Quoted 1 line
possible parents of LCD(FIMD); thus,
SCLK_FIMDshould be able to becomethe
Quoted 6 lines
Quoted 5 lines
parent. I'm not saying that
SCLK_FIMDshould be the parent, butSCLK_FIMDshould
be ABLE to be the parent. That's why I made it just "possible', not making it an actual parent.More correctly, the clk 'lcd' means clock that should be used basically at FIMD
block.
Quoted 5 lines
So it means used clock on system bus, should be
SCLK_ACLK.Actually, if disabled clk 'lcd', we can't access FIMD block such as its register.
As I say again, clk 'lcd' is on dsys clock domain, because of these meaning, so
the parent clock of 'lcd' should be dsys clock.
Ok, then,
clk_get_rate(lcd)returns 166MHz, always, which in turn, makes it more difficult
for lcd clock users to get the correct current rate when sclk_fimd's DIV changes. When we need to show that lcd is in dsys domain, isn't it enough to set dsys is an ancestor (grandparent) of lcd?Quoted 8 lines
I thought it'd contain more semantics if lcd's parent is fimd and fimd's parent is dsys; i.e., dsys is still an ancestor of lcd and
clk_get_rate(lcd)may return the clock value of lcd while DIV/SRC changes. When lcd does not usesclk_fimdanymore, we canset_parent(lcd, something)anytime (to do this more easily, we may need to define lcd withinclksrc_clk).Please refer to below diagram.
------------------------------------
? ? ? ? ? dsys bus
----------------+-------------------? ? ? ? ? ? ? ?| ? ? ? ? ? ? ? ?|1.clk 'lcd' ? ? ? ? ? ? ? ?| ? ? ? ? ? ?+---+-----------+ ? ? ? ? ? ?| ? | FIMD block| ? ? ? ? ? ?| +-+ ? ? ? ? ? | ? ? ? ? ? ?| | ? ? ? ? ? ? | ? ? ? ? ? ?| | |\ ? ? ? ? ?| ? ? ? ? ? ?| +-|m| ? ? ? ? | 2.SCLK_FIMD | ? |u|----+ ? ?| ------------+---|x| ? ?| ? ?| ? ? ? ? ? ?| ? |/ ? ? | ? ?| ? ? ? ? ? ?| ? ? ? ? ?| ? ?|? ? ? ? ? ?+----------+----+
? ? ? ? ? ? ? ? ? ? ? |
inside of SoC ? ? ? ? ?|
-----------------------+--------------------------
outside of SoC ? ? ? ? |
? ? ? ? ? ? ? ? ? ? ? | 3.clk?
? ? ? ? ? ? ? ? ? ? ? |
? ? ? ? ? ? ? +--------------+
? ? ? ? ? ? ? | LCD module ? |
? ? ? ? ? ? ? +--------------+
Quoted 3 lines
In
clock.cthe clk 'lcd' means #1 in above diagram. So you mean clk 'lcd' is #3', maybe confused :-(
Uh.. yes, I meant #3 by the clk LCD.
This CLK_GATE_IP - FIMD gates both ACLK_FIMD and SCLK_FIMD at the same time; thus, this should be over the MUX of SCLK_FIMD, I thought. (User
Manual: Clock Controler - Clock Gating Control Register (CLK_GATE_IP1)) If we should have a struct clk that represents ACLK_FIMD, the clock representing ACLK_FIMD should not directly control CLK_GATE_IP1 as CLK_GATE_IP1 gates SCLK_FIMD as well if the User Manual is correct.
If we really need to represent ACLK_FIMD separately, why don't we create "aclk_fimd" and let "fimd" or "lcd" become a struct clksrc_clk that chooses between aclk_fimd and sclk_fimd? How about this? (and other similar clocks)
Anyway, with this feature, we may now think about calling "clk_disable(parent)" when a clk is doing "clk_set_parent(another_parent)" if the clk is "clk_enable"d. I'd be happy to hear any suggestions and comments on this.
E.g.,
struct clk *a = clk_get("A"); clk_set_parent(a, clk_A);clk_enable(a);
some ops with a;
/* we need another clock speed with different source */ clk_set_parent(a, clk_B);
some ops with b;
In this example, we, at least, need to enable clk_B when clk_set_parent is called and we'd better clk_disable(clk_A) if clk_enable(clk_A) is called at clk_enable(a);
Quoted 242 lines
Quoted 66 lines
Besides, we may need to allow clock-clksrc.c's
s3c_register_clksrcfunction to use the default .parent value to set the SRC register accordingly. For example (a patch under construction, not uploaded yet and requires patches submitted before):Author: MyungJoo Ham <[email protected]>
Date: ? Thu Jul 8 18:34:01 2010 +0900? ? ARM: SAMSUNG SoC: clksrc register uses parent value to set default src.
? ? If .parent field is filled when a struct
clksrc_clkis registered, ? ?s3c_register_clksrcsets the source clock register accordingly to the ? ? parent clock.? ? In the previous versions, the parent value was "ignored" because the ? ? value of default register value overwrites the parent field.
arch/arm/plat-samsung/clock-clksrc.c +7
-1@@ -177,7 +177,7 @@ static struct clk_ops clksrc_ops_nosrc = { ?void __init s3c_register_clksrc(struct clksrc_clk *clksrc, int size) ?{ - ? ? ? int ret; + ? ? ? int ret, err = 0; ? ? ? ? for (; size > 0; size--, clksrc++) { ? ? ? ? ? ? ? ? if (!clksrc->reg_div.reg && !clksrc->reg_src.reg)@@ -195,12 +195,25 @@ void __init s3c_register_clksrc(struct clksrc_clk *clksrc, int size) ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? clksrc->clk.ops = &clksrc_ops; ? ? ? ? ? ? ? ? } + ? ? ? ? ? ? ? if (clksrc->clk.parent && clksrc->sources && + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? clksrc->reg_src.reg) { + ? ? ? ? ? ? ? ? ? ? ? err = s3c_setparent_clksrc(&clksrc->clk, + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? clksrc->clk.parent); + ? ? ? ? ? ? ? ? ? ? ? if (err < 0) + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? printk(KERN_ERR "[%s:%d] setparent errorat reg"
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "ister_clksrc%s p = %s...\n",
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?__FILE__,__LINE__,
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?clksrc->clk.name,
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?clksrc->clk.parent->name);
+ ? ? ? ? ? ? ? }
+
? ? ? ? ? ? ? ? /* setup the clocksource, but do not announce it ? ? ? ? ? ? ? ? ?* as it may be re-set by the setup routines
? ? ? ? ? ? ? ? ?* called after the rest of the clocks have been ? ? ? ? ? ? ? ? ?* registered
? ? ? ? ? ? ? ? ?*/
- ? ? ? ? ? ? ?s3c_set_clksrc(clksrc, false);
+ ? ? ? ? ? ? ? if (err < 0)
+ ? ? ? ? ? ? ? ? ? ? ?s3c_set_clksrc(clksrc, false);? ? ? ? ? ? ? ? ret =
s3c24xx_register_clock(&clksrc->clk);Quoted 1 line
Quoted 160 lines
Quoted 1 line
Quoted 1 line
However, in order to make it look better, we may need to make LCD(FIMD)
easier
Quoted 3 lines
Quoted 3 lines
to switch between
SCLK_ACLKandSCLK_FIMD; e.g., by making it anotherclksrc_clk
with source clocks ofSCLK_ACLKandSCLK_FIMDcontrolling with
CLK_GATE_IP.Quoted 154 lines
Quoted 153 lines
Quoted 53 lines
As I still saying, the CLK_GATE_IP1[0] for LCD block can gate clock of
SCLK_ACLKandSCLK_FIMDwhich can be source of LCD block, and CLK_SRC_MASK0[5] can gate onlySCLK_FIMD, output clock of MUX FIMD.Quoted 11 lines
To do so, we have relocated clksrcs[] and put enum indexes to clksrcs in order to allow clk in init_clocks[] and init_clocks_disable[] may refer one of clksrcs[] as its parent. (enum
clksrc_refer)For example, "lcd" may set "sclk-fimd" as a parent if the following line replaces the line 878:
? ? ? ? ? ? ? .parent ? ? ? ? = &clk_hclk_dsys.clk,
to
Quoted 8 lines
? ? ? ? ? ? ? .parent ? ? ? ? = &clksrcs[_clksrc_sclk_fimd],
Signed-off-by MyungJoo Ham <[email protected]>Signed-off-by Kyungmin Park <[email protected]>---
?arch/arm/mach-s5pv210/clock.c | ?986++++++++++++++++++++++-------------------
arch/arm/mach-s5pv210/clock.c static struct clksrc_clk clk_sclk_vpll = { · line 266
Quoted 10 lines
?1 files changed, 520 insertions(+), 466 deletions(-)
arch/arm/mach-s5pv210/clock.c +0
-0@@ -264,6 +264,526 @@ static struct clksrc_clk clk_sclk_vpll = { ? ? ? .reg_src ? ? ? ?= { .reg = S5P_CLK_SRC0, .shift = 12, .size = 1 }, ?};And...hmm...maybe made this patch with wrong way...
arch/arm/mach-s5pv210/clock.c static struct clksrc_clk clk_sclk_vpll = { · line 271
Quoted 5 lines
arch/arm/mach-s5pv210/clock.c +5
-0+static struct clk *clkset_uart_list[] = { + ? ? [6] = &clk_mout_mpll.clk, + ? ? [7] = &clk_mout_epll.clk, +}; +Following is from this patch...
(quoted diff) +0
-5-static struct clk *clkset_uart_list[] = { - ? ? ? [6] = &clk_mout_mpll.clk, - ? ? ? [7] = &clk_mout_epll.clk, -}; -Quoted 2 lines
Same? :-(
Why did you remove and add same code?It is done so in order to make
clksrc_clkto be able to be parents of clk. (same with the
later parts)To do so, I moved
clksrc_clkto the front of clk definitions, which in turn moved
clock source list to the front ofclksrc_clk.Or, in order to reduce the changed lines number, would it be better to declare the list of struct
clksrc_clkbefore and keep the locations of the lists?Quoted 85 lines
(snip)
And...see the below.
Added like below...
Quoted 9 lines
arch/arm/mach-s5pv210/clock.c +9
-0+static struct clksrc_clk clksrcs[] = { + ? ? [_clksrc_sclk_dmc] = { + ? ? ? ? ? ? .clk ? ?= { + ? ? ? ? ? ? ? ? ? ? .name ? ? ? ? ? = "sclk_dmc", + ? ? ? ? ? ? ? ? ? ? .id ? ? ? ? ? ? = -1, + ? ? ? ? ? ? }, + ? ? ? ? ? ? .sources = &clkset_group1, + ? ? ? ? ? ? .reg_src = { .reg = S5P_CLK_SRC6, .shift = 24, .size = 2 }, + ? ? ? ? ? ? .reg_div = { .reg = S5P_CLK_DIV6, .shift = 28, .size = 4 },arch/arm/mach-s5pv210/clock.c static struct clksrc_clk clk_sclk_vpll = { · line 290
Quoted 10 lines
arch/arm/mach-s5pv210/clock.c +10
-0+ ? ? }, + ? ? [_clksrc_sclk_onenand] = { + ? ? ? ? ? ? .clk ? ?= { + ? ? ? ? ? ? ? ? ? ? .name ? ? ? ? ? = "sclk_onenand", + ? ? ? ? ? ? ? ? ? ? .id ? ? ? ? ? ? = -1, + ? ? ? ? ? ? }, + ? ? ? ? ? ? .sources = &clkset_sclk_onenand, + ? ? ? ? ? ? .reg_src = { .reg = S5P_CLK_SRC0, .shift = 28, .size = 1 }, + ? ? ? ? ? ? .reg_div = { .reg = S5P_CLK_DIV6, .shift = 12, .size = 3 }, + ? ? },(snip)
And removed like below...
Quoted 8 lines
arch/arm/mach-s5pv210/clock.c +0
-8-static struct clksrc_clk clksrcs[] = { - ? ? { - ? ? ? ? ? ? .clk ? ?= { - ? ? ? ? ? ? ? ? ? ? .name ? ? ? ? ? = "sclk_dmc", - ? ? ? ? ? ? ? ? ? ? .id ? ? ? ? ? ? = -1, - ? ? ? ? ? ? }, - ? ? ? ? ? ? .sources = &clkset_group1, - ? ? ? ? ? ? .reg_src = { .reg = S5P_CLK_SRC6, .shift = 24, .size = 2 },arch/arm/mach-s5pv210/clock.c static struct clksrc_clk clk_sclk_vpll = {
Quoted 10 lines
arch/arm/mach-s5pv210/clock.c +0
-10- ? ? ? ? ? ? .reg_div = { .reg = S5P_CLK_DIV6, .shift = 28, .size = 4 }, - ? ? }, { - ? ? ? ? ? ? .clk ? ?= { - ? ? ? ? ? ? ? ? ? ? .name ? ? ? ? ? = "sclk_onenand", - ? ? ? ? ? ? ? ? ? ? .id ? ? ? ? ? ? = -1, - ? ? ? ? ? ? }, - ? ? ? ? ? ? .sources = &clkset_sclk_onenand, - ? ? ? ? ? ? .reg_src = { .reg = S5P_CLK_SRC0, .shift = 28, .size = 1 }, - ? ? ? ? ? ? .reg_div = { .reg = S5P_CLK_DIV6, .shift = 12, .size = 3 }, - ? ? }, {But only changed small lines not every.
Following is from my local machine.(quoted diff) +3
-2@@ -665,7 +665,7 @@ static struct clksrc_sources clkset_group2 = { ?}; ?static struct clksrc_clk clksrcs[] = { - ? ? ? { + ? ? ? [_clksrc_sclk_dmc] = { ? ? ? ? ? ? ? ?.clk ? ?= { ? ? ? ? ? ? ? ? ? ? ? ?.name ? ? ? ? ? = "sclk_dmc", ? ? ? ? ? ? ? ? ? ? ? ?.id ? ? ? ? ? ? = -1,@@ -673,7 +673,8 @@ static struct clksrc_clk clksrcs[] = { ? ? ? ? ? ? ? ?.sources = &clkset_group1, ? ? ? ? ? ? ? ?.reg_src = { .reg = S5P_CLK_SRC6, .shift = 24, .size = 2 }, ? ? ? ? ? ? ? ?.reg_div = { .reg = S5P_CLK_DIV6, .shift = 28, .size = 4 }, - ? ? ? }, { + ? ? ? }, + ? ? ? [_clksrc_sclk_onenand] = { ? ? ? ? ? ? ? ?.clk ? ?= { ? ? ? ? ? ? ? ? ? ? ? ?.name ? ? ? ? ? = "sclk_onenand", ? ? ? ? ? ? ? ? ? ? ? ?.id ? ? ? ? ? ? = -1,?(snip)
Please make sure your patch has no problem before submitting.
Thanks.
Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.Thanks.
Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd._______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Signature · 5 lines
MyungJoo Ham (???), Ph.D. Mobile Software Platform Lab, Digital Media and Communications (DMC) Business Samsung Electronics cell: 82-10-6714-2858
PATCH 1/1 ARM: S5PV210: allow clk to use clksrc as parents
Quoted 1 line · MyungJoo Ham
(snip)
Quoted 49 lines
Quoted 30 lines
Quoted 1 line
Please refer to below diagram.
------------------------------------
dsys bus
----------------+-------------------| |1.clk 'lcd' | +---+-----------+ | | FIMD block| | +-+ | | | | | | |\ | | +-|m| | 2.SCLK_FIMD | |u|----+ | ------------+---|x| | | | |/ | | | | | +----------+----+ | inside of SoC | -----------------------+-------------------------- outside of SoC | | 3.clk? | +--------------+ | LCD module | +--------------+Quoted 3 lines
In
clock.cthe clk 'lcd' means #1 in above diagram. So you mean clk 'lcd' is #3', maybe confused :-(Uh.. yes, I meant #3 by the clk LCD.
This
CLK_GATE_IP- FIMD gates bothACLK_FIMDandSCLK_FIMDat the same time; thus, this should be over the MUX ofSCLK_FIMD, I thought. (User
Manual: Clock Controler - Clock Gating Control Register (CLK_GATE_IP1)) If we should have a struct clk that representsACLK_FIMD, the clock representingACLK_FIMDshould not directly controlCLK_GATE_IP1asCLK_GATE_IP1gatesSCLK_FIMDas well if the User Manual is correct.If we really need to represent
ACLK_FIMDseparately, why don't we create "aclk_fimd" and let "fimd" or "lcd" become a structclksrc_clkthat chooses betweenaclk_fimdandsclk_fimd? How about this? (and other similar clocks)
Quoted 4 lines
Anyway, with this feature, we may now think about calling "
clk_disable(parent)" when a clk is doing "clk_set_parent(another_parent)" if the clk is "clk_enable"d. I'd be happy to hear any suggestions and comments on this.
Basically, each clock which is used in each device driver such as lcd module clock, camemra module clock should be controlled in its device driver.
Let's say, in the case of camera module clock, FIMC IP has a specific divider which is used clock dividing for camera module clock. And the divider which is in FIMC IP has own rate for camera module at that time. So cannot/no need to define it in the common clock part such as clock.c.
In other words, can't implement set_rate() or get_rate() for camera module clock in the common clock part, because FIMC IP block has own specific divider for it.
Quoted 11 lines
E.g.,
struct clk *a =
clk_get("A");clk_set_parent(a, clk_A);clk_enable(a);
some ops with a;
/* we need another clock speed with different source */clk_set_parent(a, clk_B);
some ops with b;
Quoted 5 lines
In this example, we, at least, need to enable
clk_Bwhenclk_set_parentis called and we'd betterclk_disable(clk_A)ifclk_enable(clk_A)is called atclk_enable(a);
Thanks.
Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.
(snip)
PATCH 1/1 ARM: S5PV210: allow clk to use clksrc as parents
Quoted 63 lines · Kukjin Kim
Quoted 1 line · MyungJoo Ham
?(snip)
Quoted 49 lines
Quoted 30 lines
Quoted 1 line
Please refer to below diagram.
------------------------------------
? ? ? ? ? dsys bus
----------------+-------------------? ? ? ? ? ? ? ?| ? ? ? ? ? ? ? ?|1.clk 'lcd' ? ? ? ? ? ? ? ?| ? ? ? ? ? ?+---+-----------+ ? ? ? ? ? ?| ? | FIMD block| ? ? ? ? ? ?| +-+ ? ? ? ? ? | ? ? ? ? ? ?| | ? ? ? ? ? ? | ? ? ? ? ? ?| | |\ ? ? ? ? ?| ? ? ? ? ? ?| +-|m| ? ? ? ? | 2.SCLK_FIMD | ? |u|----+ ? ?| ------------+---|x| ? ?| ? ?| ? ? ? ? ? ?| ? |/ ? ? | ? ?| ? ? ? ? ? ?| ? ? ? ? ?| ? ?|? ? ? ? ? ?+----------+----+
? ? ? ? ? ? ? ? ? ? ? |
inside of SoC ? ? ? ? ?|
-----------------------+--------------------------
outside of SoC ? ? ? ? |
? ? ? ? ? ? ? ? ? ? ? | 3.clk?
? ? ? ? ? ? ? ? ? ? ? |
? ? ? ? ? ? ? +--------------+
? ? ? ? ? ? ? | LCD module ? |
? ? ? ? ? ? ? +--------------+Quoted 3 lines
In
clock.cthe clk 'lcd' means #1 in above diagram. So you mean clk 'lcd' is #3', maybe confused :-(Uh.. yes, I meant #3 by the clk LCD.
This
CLK_GATE_IP- FIMD gates bothACLK_FIMDandSCLK_FIMDat the same time; thus, this should be over the MUX ofSCLK_FIMD, I thought. (User
Manual: Clock Controler - Clock Gating Control Register (CLK_GATE_IP1)) If we should have a struct clk that representsACLK_FIMD, the clock representingACLK_FIMDshould not directly controlCLK_GATE_IP1asCLK_GATE_IP1gatesSCLK_FIMDas well if the User Manual is correct.If we really need to represent
ACLK_FIMDseparately, why don't we create "aclk_fimd" and let "fimd" or "lcd" become a structclksrc_clkthat chooses betweenaclk_fimdandsclk_fimd? How about this? (and other similar clocks)Quoted 4 lines
Anyway, with this feature, we may now think about calling "
clk_disable(parent)" when a clk is doing "clk_set_parent(another_parent)" if the clk is "clk_enable"d. I'd be happy to hear any suggestions and comments on this.Basically, each clock which is used in each device driver such as lcd module clock, camemra module clock should be controlled in its device driver.
Quoted 8 lines
Let's say, in the case of camera module clock, FIMC IP has a specific divider which is used clock dividing for camera module clock. And the divider which is in FIMC IP has own rate for camera module at that time. So cannot/no need to define it in the common clock part such as
clock.c.In other words, can't implement
set_rate()orget_rate()for camera module clock in the common clock part, because FIMC IP block has own specific divider for it.
To Pawel,
Can you reply his comments? usage of camera clock.
Thank you,
Kyungmin Park
Quoted 29 lines
Quoted 12 lines
E.g.,
struct clk *a =
clk_get("A");clk_set_parent(a, clk_A);clk_enable(a);
some ops with a;
/* we need another clock speed with different source */clk_set_parent(a, clk_B);
some ops with b;Quoted 5 lines
In this example, we, at least, need to enable
clk_Bwhenclk_set_parentis called and we'd betterclk_disable(clk_A)ifclk_enable(clk_A)is called atclk_enable(a);Thanks.
Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.(snip)
PATCH 1/1 ARM: S5PV210: allow clk to use clksrc as parents
In reply to Kukjin KimHello,
Quoted 5 lines · Kukjin Kim
Quoted 2 lines · MyungJoo Ham
(snip)
[snip]
Quoted 9 lines
Basically, each clock which is used in each device driver such as lcd module clock, camemra module clock should be controlled in its device driver.
Let's say, in the case of camera module clock, FIMC IP has a specific divider which is used clock dividing for camera module clock. And the divider which is in FIMC IP has own rate for camera module at that time. So cannot/no need to define it in the common clock part such as
clock.c.
IMO FIMC clock divider control registers are located in the clock controller IO memory, not in the FIMC IP. But possibly you meant something different than SCLK_CAM0/1 here.
So if we are considering SCLK_CAM0/1 the FIMC driver should be able to configure dividers corresponding to these clock outputs through the clk API.
There is core clock multiplexer in FIMC IP in some SoCs though, so in general you are right, it would not be possible to control all clocks with clk API as for now, since the dividers are located in specific IPs (e.g. SDHCI controller, FIMD).
Regards,
Sylwester
Quoted 6 lines
In other words, can't implement
set_rate()orget_rate()for camera module clock in the common clock part, because FIMC IP block has own specific divider for it.
[snip]
Quoted 4 lines
Thanks.
Best regards,
Kgene.
PATCH 1/1 ARM: S5PV210: allow clk to use clksrc as parents
In reply to Kukjin KimHello,
Quoted 59 lines · Kukjin Kim
MyungJoo Ham wrote:
(snip)Quoted 48 lines
Quoted 29 lines
Please refer to below diagram.
------------------------------------
dsys bus
----------------+-------------------| |1.clk 'lcd' | +---+-----------+ | | FIMD block| | +-+ | | | | | | |\ | | +-|m| | 2.SCLK_FIMD | |u|----+ | ------------+---|x| | | | |/ | | | | | +----------+----+ | inside of SoC | -----------------------+-------------------------- outside of SoC | | 3.clk? | +--------------+ | LCD module | +--------------+Quoted 3 lines
In
clock.cthe clk 'lcd' means #1 in above diagram. So you mean clk 'lcd' is #3', maybe confused :-(Uh.. yes, I meant #3 by the clk LCD.
This
CLK_GATE_IP- FIMD gates bothACLK_FIMDandSCLK_FIMDat the same time; thus, this should be over the MUX ofSCLK_FIMD, I thought. (User
Manual: Clock Controler - Clock Gating Control Register (CLK_GATE_IP1)) If we should have a struct clk that representsACLK_FIMD, the clock representingACLK_FIMDshould not directly controlCLK_GATE_IP1asCLK_GATE_IP1gatesSCLK_FIMDas well if the User Manual is correct.If we really need to represent
ACLK_FIMDseparately, why don't we create "aclk_fimd" and let "fimd" or "lcd" become a structclksrc_clkthat chooses betweenaclk_fimdandsclk_fimd? How about this? (and other similar clocks)Quoted 4 lines
Anyway, with this feature, we may now think about calling "
clk_disable(parent)" when a clk is doing "clk_set_parent(another_parent)" if the clk is "clk_enable"d. I'd be happy to hear any suggestions and comments on this.Basically, each clock which is used in each device driver such as lcd module clock, camera module clock should be controlled in its device driver.
Quoted 8 lines
Let's say, in the case of camera module clock, FIMC IP has a specific divider which is used clock dividing for camera module clock. And the divider which is in FIMC IP has own rate for camera module at that time. So cannot/no need to define it in the common clock part such as
clock.c.In other words, can't implement
set_rate()orget_rate()for camera module clock in the common clock part, because FIMC IP block has own specific divider for it.
Well, this 'problem' occurs at least in the following IP blocks: FIMD, FIMC and SDHCI. It looks that some common solution is required for these drivers. In all these 3 IPs the clock we are considering is an 'external' clock - it is wired out of the IP block to its external peripheral device (lcd display, camera sensor or mmc/sd card).
In the current drivers that are in mainline, only SDHCI driver changes the parent clock source and/or the rate. FIMD (s3c-fb) is hardcoded to use host bus (dsys) clock only (what is a limitation that might be an issue in some cases).
We can notice that the code for selecting the parent clock in sdhci-s3c is a bit messy. The parent clock names are passed in platform data, which in turn need to be defined separately for each SoC type.
IMHO it should be possible to create a some kind of virtual clock, lets name it 'ext_hsmmc' (or 'external_hsmmc'). All it's parents should be properly defined in the clock api. In this case methods like set_rate should check all possible sources and select the one that gives the closest result, enabling and disabling relevant parents. This virtual clock might need to have some additional rate related ops (like get_min and get_max) and some function for the driver to get the calculated divider and/or parent index (the driver needs to write these to its registers, which clock api cannot access directly).
This way we would get 2 clocks:
- a general 'hsmmc', which can be used for enabling and disabling the IP (dsys sourced)
- special 'external_hsmmc', which is used for controlling the clk signal outgoing
from SDHCI block.
The driver will need to get and enable these 2 clocks, but I don't think it is a big overhead. Most of the code that is related to 'external' clock should be there
already and will be simplified with this approach.
What do you think about such solution?
Best regards
--
Marek Szyprowski
Samsung Poland R&D Center
PATCH 1/1 ARM: S5PV210: allow clk to use clksrc as parents
In reply to Kukjin KimHello,
Quoted 63 lines · Kukjin Kim
Quoted 1 line · MyungJoo Ham
?(snip)
Quoted 49 lines
Quoted 30 lines
Quoted 1 line
Please refer to below diagram.
------------------------------------
? ? ? ? ? dsys bus
----------------+-------------------? ? ? ? ? ? ? ?| ? ? ? ? ? ? ? ?|1.clk 'lcd' ? ? ? ? ? ? ? ?| ? ? ? ? ? ?+---+-----------+ ? ? ? ? ? ?| ? | FIMD block| ? ? ? ? ? ?| +-+ ? ? ? ? ? | ? ? ? ? ? ?| | ? ? ? ? ? ? | ? ? ? ? ? ?| | |\ ? ? ? ? ?| ? ? ? ? ? ?| +-|m| ? ? ? ? | 2.SCLK_FIMD | ? |u|----+ ? ?| ------------+---|x| ? ?| ? ?| ? ? ? ? ? ?| ? |/ ? ? | ? ?| ? ? ? ? ? ?| ? ? ? ? ?| ? ?|? ? ? ? ? ?+----------+----+
? ? ? ? ? ? ? ? ? ? ? |
inside of SoC ? ? ? ? ?|
-----------------------+--------------------------
outside of SoC ? ? ? ? |
? ? ? ? ? ? ? ? ? ? ? | 3.clk?
? ? ? ? ? ? ? ? ? ? ? |
? ? ? ? ? ? ? +--------------+
? ? ? ? ? ? ? | LCD module ? |
? ? ? ? ? ? ? +--------------+Quoted 3 lines
In
clock.cthe clk 'lcd' means #1 in above diagram. So you mean clk 'lcd' is #3', maybe confused :-(Uh.. yes, I meant #3 by the clk LCD.
This
CLK_GATE_IP- FIMD gates bothACLK_FIMDandSCLK_FIMDat the same time; thus, this should be over the MUX ofSCLK_FIMD, I thought. (User
Manual: Clock Controler - Clock Gating Control Register (CLK_GATE_IP1)) If we should have a struct clk that representsACLK_FIMD, the clock representingACLK_FIMDshould not directly controlCLK_GATE_IP1asCLK_GATE_IP1gatesSCLK_FIMDas well if the User Manual is correct.If we really need to represent
ACLK_FIMDseparately, why don't we create "aclk_fimd" and let "fimd" or "lcd" become a structclksrc_clkthat chooses betweenaclk_fimdandsclk_fimd? How about this? (and other similar clocks)Quoted 4 lines
Anyway, with this feature, we may now think about calling "
clk_disable(parent)" when a clk is doing "clk_set_parent(another_parent)" if the clk is "clk_enable"d. I'd be happy to hear any suggestions and comments on this.Basically, each clock which is used in each device driver such as lcd module clock, camemra module clock should be controlled in its device driver.
Quoted 9 lines
Let's say, in the case of camera module clock, FIMC IP has a specific divider which is used clock dividing for camera module clock. And the divider which is in FIMC IP has own rate for camera module at that time. So cannot/no need to define it in the common clock part such as
clock.c.In other words, can't implement
set_rate()orget_rate()for camera module clock in the common clock part, because FIMC IP block has own specific divider for it.
Ah... I've got it. Although "lcd" of CLK_GATE_IP contols SCLK_FIMD and part of HCLK that goes to FIMD at the same time, as long as the MUX of SCLK_FIMD and that part of HCLK is in VIDCON (VIDCON0:CLKSEL_F), you don't want the control to be reside in arch/arm/mach side.
I also think let LCD(anyone who's got w/ VIDCON0) driver controls and owns the MUX info about SCLK_FIMD/HCLK. What I thought once is that CLK_GATE_IP:CLK_FIMD controls the lcd clock after the MUX as it controls both ACLK_FIMD(from HCLK) and SCLK_FIMD at the same time.
Thank you.
Quoted 33 lines
Quoted 11 lines
E.g.,
struct clk *a =
clk_get("A");clk_set_parent(a, clk_A);clk_enable(a);
some ops with a;
/* we need another clock speed with different source */clk_set_parent(a, clk_B);
some ops with b;Quoted 5 lines
In this example, we, at least, need to enable
clk_Bwhenclk_set_parentis called and we'd betterclk_disable(clk_A)ifclk_enable(clk_A)is called atclk_enable(a);Thanks.
Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.(snip)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Signature · 5 lines
MyungJoo Ham (???), Ph.D. Mobile Software Platform Lab, Digital Media and Communications (DMC) Business Samsung Electronics cell: 82-10-6714-2858