From: kgene.kim@samsung.com (Kukjin Kim)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] S5PV210 Correct clock source control register properties
Date: Fri, 18 Jun 2010 14:36:56 +0900 [thread overview]
Message-ID: <003301cb0ea8$474d1640$d5e742c0$%kim@samsung.com> (raw)
In-Reply-To: <1276766116-6839-1-git-send-email-myungjoo.ham@samsung.com>
MyungJoo Ham wrote:
>
> The clock source controls (struct clksrc_clk) have been often accessing
> CLK_GATE_IPx, which are for clock gating and are accessed by each clock
> as well as the clock source. This duplicated clock issue may incur
> lockup problems when there are two modules accessing the same clock with
> different names.
>
Actually, S5V210/S5PC110 CLK_GATE_IPx can disable the clock operation of each
IP if it is not required. And this reduces dynamic power. CLK_SRC_MASKx is
used to control clock mux of each IP's SCLK.
Basically CLK_SRC_MASKx is for mux control (output masking) of each IP
source, and actually that can _not_ control relevant every source clock of
each IP. So CLK_GATE_IPx exists, can do it.
I know, in the several case, i.e, camera and fimc, each sclk_xxx need to
control. And can be controlled each sclk_xxx by using your patch, but in this
case, need to additional control relevant clock gating for dynamic clock
gating of each IP in its device driver.
> Besides, the clock source control is supposed to control (setting values
> of MASK, SRC, DIV), not to turn on/off individual clock.
>
> Another point is that there are registers (CLK_SRC_MASK0/1) specialized
> for masking clock source control in S5PV210/S5PC110 according to the
> user manual (rev 20100201).
>
> Therefore, accessing CLK_SRC_MASK0/1 (rather than accessing
> CLK_GATE_IPx) for the clock source control is safer and fits to the
> semantics of S5PV210/S5PC110 registers. And fortuneatly, each clock
> source defined in clock.c has corresponding bit at CLK_SRC_MASK0/1
> except for MFC, G2D, and G3D.
>
> In this patch,
>
> - DAC, HDMI, AUDIO 0/1/2, UCLK(uart) 0/1/2/3, MIXER, FIMC 0/1/2,
> FIMC, MMC 0/1/2/3, CSIS, SPI 0/1, PWI, and PWM clock sources are
> modified to use CLK_SRC_MASK0/1, which were using CLK_GATE_IPx.
>
> - CAM 0/1 did not have enable/disable control. They now access
> CLK_SRC_MASK0.
>
> - MFC, G3D, G2D were using CLK_GATE_IPx. However, as there are no clocks
> defined to control MFC, G3D, and G2D, we kept them to access
> CLK_GATE_IPx. These may need to be modified (erase .enable, .ctrlbit
> from sclk_mfc, sclk_g2d, sclk_g3d and create clocks: g3d, g2d, mfc)
> later.
>
Hmm..need to check about multimedia usage.
And in my opinion, it would be helpful to understand that should be short and
clear.
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> ---
> arch/arm/mach-s5pv210/clock.c | 101
++++++++++++++++++++++-------------------
> 1 files changed, 55 insertions(+), 46 deletions(-)
>
> diff --git a/arch/arm/mach-s5pv210/clock.c b/arch/arm/mach-s5pv210/clock.c
> index ec5ad8c..08c1063 100644
> --- a/arch/arm/mach-s5pv210/clock.c
> +++ b/arch/arm/mach-s5pv210/clock.c
> @@ -183,6 +183,11 @@ static int s5pv210_clk_mask0_ctrl(struct clk *clk, int
enable)
> return s5p_gatectrl(S5P_CLK_SRC_MASK0, clk, enable);
> }
>
> +static int s5pv210_clk_mask1_ctrl(struct clk *clk, int enable)
> +{
> + return s5p_gatectrl(S5P_CLK_SRC_MASK1, clk, enable);
> +}
> +
> static struct clk clk_sclk_hdmi27m = {
> .name = "sclk_hdmi27m",
> .id = -1,
> @@ -497,8 +502,8 @@ static struct clksrc_clk clk_sclk_dac = {
> .clk = {
> .name = "sclk_dac",
> .id = -1,
> - .ctrlbit = (1 << 10),
> - .enable = s5pv210_clk_ip1_ctrl,
> + .ctrlbit = (1 << 2),
> + .enable = s5pv210_clk_mask0_ctrl,
> },
> .sources = &clkset_sclk_dac,
> .reg_src = { .reg = S5P_CLK_SRC1, .shift = 8, .size = 1 },
> @@ -527,8 +532,8 @@ static struct clksrc_clk clk_sclk_hdmi = {
> .clk = {
> .name = "sclk_hdmi",
> .id = -1,
> - .enable = s5pv210_clk_ip1_ctrl,
> - .ctrlbit = (1 << 11),
> + .enable = s5pv210_clk_mask0_ctrl,
> + .ctrlbit = (1 << 0),
> },
> .sources = &clkset_sclk_hdmi,
> .reg_src = { .reg = S5P_CLK_SRC1, .shift = 0, .size = 1 },
> @@ -565,8 +570,8 @@ static struct clksrc_clk clk_sclk_audio0 = {
> .clk = {
> .name = "sclk_audio",
> .id = 0,
> - .enable = s5pv210_clk_ip3_ctrl,
> - .ctrlbit = (1 << 4),
> + .enable = s5pv210_clk_mask0_ctrl,
> + .ctrlbit = (1 << 24),
> },
> .sources = &clkset_sclk_audio0,
> .reg_src = { .reg = S5P_CLK_SRC6, .shift = 0, .size = 4 },
> @@ -594,8 +599,8 @@ static struct clksrc_clk clk_sclk_audio1 = {
> .clk = {
> .name = "sclk_audio",
> .id = 1,
> - .enable = s5pv210_clk_ip3_ctrl,
> - .ctrlbit = (1 << 5),
> + .enable = s5pv210_clk_mask0_ctrl,
> + .ctrlbit = (1 << 25),
> },
> .sources = &clkset_sclk_audio1,
> .reg_src = { .reg = S5P_CLK_SRC6, .shift = 4, .size = 4 },
> @@ -623,8 +628,8 @@ static struct clksrc_clk clk_sclk_audio2 = {
> .clk = {
> .name = "sclk_audio",
> .id = 2,
> - .enable = s5pv210_clk_ip3_ctrl,
> - .ctrlbit = (1 << 6),
> + .enable = s5pv210_clk_mask0_ctrl,
> + .ctrlbit = (1 << 26),
> },
> .sources = &clkset_sclk_audio2,
> .reg_src = { .reg = S5P_CLK_SRC6, .shift = 8, .size = 4 },
> @@ -680,8 +685,8 @@ static struct clksrc_clk clksrcs[] = {
> .clk = {
> .name = "uclk1",
> .id = 0,
> - .ctrlbit = (1<<17),
> - .enable = s5pv210_clk_ip3_ctrl,
> + .ctrlbit = (1 << 12),
> + .enable = s5pv210_clk_mask0_ctrl,
> },
> .sources = &clkset_uart,
> .reg_src = { .reg = S5P_CLK_SRC4, .shift = 16, .size = 4 },
> @@ -690,8 +695,8 @@ static struct clksrc_clk clksrcs[] = {
> .clk = {
> .name = "uclk1",
> .id = 1,
> - .enable = s5pv210_clk_ip3_ctrl,
> - .ctrlbit = (1 << 18),
> + .enable = s5pv210_clk_mask0_ctrl,
> + .ctrlbit = (1 << 13),
> },
> .sources = &clkset_uart,
> .reg_src = { .reg = S5P_CLK_SRC4, .shift = 20, .size = 4 },
> @@ -700,8 +705,8 @@ static struct clksrc_clk clksrcs[] = {
> .clk = {
> .name = "uclk1",
> .id = 2,
> - .enable = s5pv210_clk_ip3_ctrl,
> - .ctrlbit = (1 << 19),
> + .enable = s5pv210_clk_mask0_ctrl,
> + .ctrlbit = (1 << 14),
> },
> .sources = &clkset_uart,
> .reg_src = { .reg = S5P_CLK_SRC4, .shift = 24, .size = 4 },
> @@ -710,8 +715,8 @@ static struct clksrc_clk clksrcs[] = {
> .clk = {
> .name = "uclk1",
> .id = 3,
> - .enable = s5pv210_clk_ip3_ctrl,
> - .ctrlbit = (1 << 20),
> + .enable = s5pv210_clk_mask0_ctrl,
> + .ctrlbit = (1 << 15),
> },
> .sources = &clkset_uart,
> .reg_src = { .reg = S5P_CLK_SRC4, .shift = 28, .size = 4 },
> @@ -720,8 +725,8 @@ static struct clksrc_clk clksrcs[] = {
> .clk = {
> .name = "sclk_mixer",
> .id = -1,
> - .enable = s5pv210_clk_ip1_ctrl,
> - .ctrlbit = (1 << 9),
> + .enable = s5pv210_clk_mask0_ctrl,
> + .ctrlbit = (1 << 1),
> },
> .sources = &clkset_sclk_mixer,
> .reg_src = { .reg = S5P_CLK_SRC1, .shift = 4, .size = 1 },
> @@ -738,8 +743,8 @@ static struct clksrc_clk clksrcs[] = {
> .clk = {
> .name = "sclk_fimc",
> .id = 0,
> - .enable = s5pv210_clk_ip0_ctrl,
> - .ctrlbit = (1 << 24),
> + .enable = s5pv210_clk_mask1_ctrl,
> + .ctrlbit = (1 << 2),
> },
> .sources = &clkset_group2,
> .reg_src = { .reg = S5P_CLK_SRC3, .shift = 12, .size = 4 },
> @@ -748,8 +753,8 @@ static struct clksrc_clk clksrcs[] = {
> .clk = {
> .name = "sclk_fimc",
> .id = 1,
> - .enable = s5pv210_clk_ip0_ctrl,
> - .ctrlbit = (1 << 25),
> + .enable = s5pv210_clk_mask1_ctrl,
> + .ctrlbit = (1 << 3),
> },
> .sources = &clkset_group2,
> .reg_src = { .reg = S5P_CLK_SRC3, .shift = 16, .size = 4 },
> @@ -758,8 +763,8 @@ static struct clksrc_clk clksrcs[] = {
> .clk = {
> .name = "sclk_fimc",
> .id = 2,
> - .enable = s5pv210_clk_ip0_ctrl,
> - .ctrlbit = (1 << 26),
> + .enable = s5pv210_clk_mask1_ctrl,
> + .ctrlbit = (1 << 4),
> },
> .sources = &clkset_group2,
> .reg_src = { .reg = S5P_CLK_SRC3, .shift = 20, .size = 4 },
> @@ -768,6 +773,8 @@ static struct clksrc_clk clksrcs[] = {
> .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 },
> @@ -776,6 +783,8 @@ static struct clksrc_clk clksrcs[] = {
> .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 },
> @@ -784,8 +793,8 @@ static struct clksrc_clk clksrcs[] = {
> .clk = {
> .name = "sclk_fimd",
> .id = -1,
> - .enable = s5pv210_clk_ip1_ctrl,
> - .ctrlbit = (1 << 0),
> + .enable = s5pv210_clk_mask0_ctrl,
> + .ctrlbit = (1 << 5),
> },
> .sources = &clkset_group2,
> .reg_src = { .reg = S5P_CLK_SRC1, .shift = 20, .size = 4 },
> @@ -794,8 +803,8 @@ static struct clksrc_clk clksrcs[] = {
> .clk = {
> .name = "sclk_mmc",
> .id = 0,
> - .enable = s5pv210_clk_ip2_ctrl,
> - .ctrlbit = (1 << 16),
> + .enable = s5pv210_clk_mask0_ctrl,
> + .ctrlbit = (1 << 8),
> },
> .sources = &clkset_group2,
> .reg_src = { .reg = S5P_CLK_SRC4, .shift = 0, .size = 4 },
> @@ -804,8 +813,8 @@ static struct clksrc_clk clksrcs[] = {
> .clk = {
> .name = "sclk_mmc",
> .id = 1,
> - .enable = s5pv210_clk_ip2_ctrl,
> - .ctrlbit = (1 << 17),
> + .enable = s5pv210_clk_mask0_ctrl,
> + .ctrlbit = (1 << 9),
> },
> .sources = &clkset_group2,
> .reg_src = { .reg = S5P_CLK_SRC4, .shift = 4, .size = 4 },
> @@ -814,8 +823,8 @@ static struct clksrc_clk clksrcs[] = {
> .clk = {
> .name = "sclk_mmc",
> .id = 2,
> - .enable = s5pv210_clk_ip2_ctrl,
> - .ctrlbit = (1 << 18),
> + .enable = s5pv210_clk_mask0_ctrl,
> + .ctrlbit = (1 << 10),
> },
> .sources = &clkset_group2,
> .reg_src = { .reg = S5P_CLK_SRC4, .shift = 8, .size = 4 },
> @@ -824,8 +833,8 @@ static struct clksrc_clk clksrcs[] = {
> .clk = {
> .name = "sclk_mmc",
> .id = 3,
> - .enable = s5pv210_clk_ip2_ctrl,
> - .ctrlbit = (1 << 19),
> + .enable = s5pv210_clk_mask0_ctrl,
> + .ctrlbit = (1 << 11),
> },
> .sources = &clkset_group2,
> .reg_src = { .reg = S5P_CLK_SRC4, .shift = 12, .size = 4 },
> @@ -864,8 +873,8 @@ static struct clksrc_clk clksrcs[] = {
> .clk = {
> .name = "sclk_csis",
> .id = -1,
> - .enable = s5pv210_clk_ip0_ctrl,
> - .ctrlbit = (1 << 31),
> + .enable = s5pv210_clk_mask0_ctrl,
> + .ctrlbit = (1 << 6),
> },
> .sources = &clkset_group2,
> .reg_src = { .reg = S5P_CLK_SRC1, .shift = 24, .size = 4 },
> @@ -874,8 +883,8 @@ static struct clksrc_clk clksrcs[] = {
> .clk = {
> .name = "sclk_spi",
> .id = 0,
> - .enable = s5pv210_clk_ip3_ctrl,
> - .ctrlbit = (1 << 12),
> + .enable = s5pv210_clk_mask0_ctrl,
> + .ctrlbit = (1 << 16),
> },
> .sources = &clkset_group2,
> .reg_src = { .reg = S5P_CLK_SRC5, .shift = 0, .size = 4 },
> @@ -884,8 +893,8 @@ static struct clksrc_clk clksrcs[] = {
> .clk = {
> .name = "sclk_spi",
> .id = 1,
> - .enable = s5pv210_clk_ip3_ctrl,
> - .ctrlbit = (1 << 13),
> + .enable = s5pv210_clk_mask0_ctrl,
> + .ctrlbit = (1 << 17),
> },
> .sources = &clkset_group2,
> .reg_src = { .reg = S5P_CLK_SRC5, .shift = 4, .size = 4 },
> @@ -894,8 +903,8 @@ static struct clksrc_clk clksrcs[] = {
> .clk = {
> .name = "sclk_pwi",
> .id = -1,
> - .enable = &s5pv210_clk_ip4_ctrl,
> - .ctrlbit = (1 << 2),
> + .enable = &s5pv210_clk_mask0_ctrl,
> + .ctrlbit = (1 << 29),
> },
> .sources = &clkset_group2,
> .reg_src = { .reg = S5P_CLK_SRC6, .shift = 20, .size = 4 },
> @@ -904,8 +913,8 @@ static struct clksrc_clk clksrcs[] = {
> .clk = {
> .name = "sclk_pwm",
> .id = -1,
> - .enable = s5pv210_clk_ip3_ctrl,
> - .ctrlbit = (1 << 23),
> + .enable = s5pv210_clk_mask0_ctrl,
> + .ctrlbit = (1 << 19),
> },
> .sources = &clkset_group2,
> .reg_src = { .reg = S5P_CLK_SRC5, .shift = 12, .size = 4 },
> --
I'm not sure that need to control of all sclk_xxx like above method, mux
output masking.
Thanks.
Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.
next prev parent reply other threads:[~2010-06-18 5:36 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-06-17 9:15 [PATCH] S5PV210 Correct clock source control register properties MyungJoo Ham
2010-06-18 5:36 ` Kukjin Kim [this message]
2010-06-18 6:03 ` MyungJoo Ham
2010-06-18 9:58 ` Kukjin Kim
2010-06-18 10:12 ` MyungJoo Ham
2010-06-18 9:50 ` Kukjin Kim
2010-06-18 10:25 ` MyungJoo Ham
2010-06-19 3:55 ` Kukjin Kim
2010-06-19 4:14 ` Kyungmin Park
2010-06-22 7:19 ` MyungJoo Ham
2010-06-23 5:01 ` Kukjin Kim
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='003301cb0ea8$474d1640$d5e742c0$%kim@samsung.com' \
--to=kgene.kim@samsung.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).