Hi Thomas,
> Add a new clock provider for ARM clock domain. This clock provider
> is composed of multiple components which include mux_core, div_core,
> div_core2, div_corem0, div_corem1, div_periph, div_atb, div_pclk_dbg,
> div_copy and div_hpm. This composition of mutiple components into
> a single clock provider helps with faster completion of CPU clock
> speed switching during DVFS operations.
>
> Signed-off-by: Thomas Abraham <thomas.ab@xxxxxxxxxxx>
> ---
> drivers/clk/samsung/clk-exynos4.c | 96
> ++++++++++++++++++++++++++++++++++++- 1 files changed, 95
> insertions(+), 1 deletions(-)
>
> diff --git a/drivers/clk/samsung/clk-exynos4.c
> b/drivers/clk/samsung/clk-exynos4.c index d967571..4bf2f93 100644
> --- a/drivers/clk/samsung/clk-exynos4.c
> +++ b/drivers/clk/samsung/clk-exynos4.c
> @@ -108,8 +108,11 @@
> #define APLL_CON0 0x14100
> #define E4210_MPLL_CON0 0x14108
> #define SRC_CPU 0x14200
> +#define STAT_CPU 0x14400
> #define DIV_CPU0 0x14500
> #define DIV_CPU1 0x14504
> +#define DIV_STAT_CPU0 0x14600
> +#define DIV_STAT_CPU1 0x14604
> #define GATE_SCLK_CPU 0x14800
> #define GATE_IP_CPU 0x14900
> #define E4X12_DIV_ISP0 0x18300
> @@ -289,7 +292,7 @@ static unsigned long exynos4_clk_regs[]
> __initdata = { };
>
> /* list of all parent clock list */
> -PNAME(mout_apll_p) = { "fin_pll", "fout_apll", };
> +PNAME(mout_apll_p) = { "fin_pll", "fout_apll1", };
> PNAME(mout_mpll_p) = { "fin_pll", "fout_mpll", };
> PNAME(mout_epll_p) = { "fin_pll", "fout_epll", };
> PNAME(mout_vpllsrc_p) = { "fin_pll", "sclk_hdmi24m", };
> @@ -306,6 +309,7 @@ PNAME(mout_onenand_p) = {"aclk133", "aclk160", };
> PNAME(mout_onenand1_p) = {"mout_onenand", "sclk_vpll", };
>
> /* Exynos 4210-specific parent groups */
> +PNAME(armclk_p) = { "fout_apll", };
Here you only give no parent clock, but at samsung_coreclk_register()
it is expected to provide list of parents.
> PNAME(sclk_vpll_p4210) = { "mout_vpllsrc", "fout_vpll", };
> PNAME(mout_core_p4210) = { "mout_apll", "sclk_mpll", };
> PNAME(sclk_ampll_p4210) = { "sclk_mpll", "sclk_apll", };
> @@ -1089,6 +1093,92 @@ static struct samsung_pll_clock
> exynos4x12_plls[nr_plls] __initdata = { VPLL_LOCK, VPLL_CON0, NULL),
> };
>
> +#define EXYNOS4210_DIV_CPU0(apll, pclk_dbg, atb, periph, corem1,
> corem0) \
> + ((apll << 24) | (pclk_dbg << 20) | (atb << 16) | \
> + (periph << 12) | (corem1 << 8) | (corem0 << 4))
> +#define EXYNOS4210_DIV_CPU1(hpm, copy) \
> + ((hpm << 4) | (copy << 0))
> +static const unsigned long exynos4210_armclk_data[][2] = {
> + { EXYNOS4210_DIV_CPU0(7, 1, 4, 3, 7, 3),
> EXYNOS4210_DIV_CPU1(0, 5), },
> + { EXYNOS4210_DIV_CPU0(7, 1, 4, 3, 7, 3),
> EXYNOS4210_DIV_CPU1(0, 4), },
> + { EXYNOS4210_DIV_CPU0(7, 1, 3, 3, 7, 3),
> EXYNOS4210_DIV_CPU1(0, 3), },
> + { EXYNOS4210_DIV_CPU0(7, 1, 3, 3, 7, 3),
> EXYNOS4210_DIV_CPU1(0, 3), },
> + { EXYNOS4210_DIV_CPU0(7, 1, 3, 3, 7, 3),
> EXYNOS4210_DIV_CPU1(0, 3), },
> + { EXYNOS4210_DIV_CPU0(0, 1, 3, 1, 3, 1),
> EXYNOS4210_DIV_CPU1(0, 3), }, +};
> +
What do you think about adding those parameters (like CPU dividers) as
an attribute to /cpus/cpu@0 node?
> +static const unsigned long exynos4210_armclk_freqs[] = {
> + 1200000 , 1000000, 800000, 500000, 400000, 200000,
> +};
Those freq's are going to be defined at /cpus/cpu@0 at operating-points
attribute (or if possible took from cpufreq_frequency table).
> +
> +static const struct samsung_core_clock_freq_table
> exyno4210_armclk_table = {
> + .freq = exynos4210_armclk_freqs,
> + .freq_count = ARRAY_SIZE(exynos4210_armclk_freqs),
> + .data = (void *)exynos4210_armclk_data,
> +};
> +
> +static int exynos4210_armclk_set_rate(struct clk_hw *hw, unsigned
> long drate,
> + unsigned long prate)
> +{
> + struct samsung_core_clock *armclk;
> + const struct samsung_core_clock_freq_table *freq_tbl;
> + unsigned long *freq_data;
> + unsigned long mux_reg, idx;
> + void __iomem *base;
> +
> + if (drate == prate)
> + return 0;
> +
> + armclk = container_of(hw, struct samsung_core_clock, hw);
> + freq_tbl = armclk->freq_table;
> + freq_data = (unsigned long *)freq_tbl->data;
> + base = armclk->ctrl_base;
> +
> + for (idx = 0; idx < freq_tbl->freq_count; idx++, freq_data
> += 2)
> + if ((freq_tbl->freq[idx] * 1000) == drate)
> + break;
> +
> + if (!armclk->fout_pll)
> + armclk->fout_pll = __clk_lookup("fout_apll");\
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^[*]
I'm a bit confused here for two reasons. Please correct me if I'm wrong.
1. You go into this ->set_rate() because of calling clk_set_rate at
"arm_clk" clock (numbered as 12 at clk-exynos4.c) at cpufreq-cpu0.c
In a Exynos4210 we have:
XXTI-> APLL -> fout_apll -> mout_apll -> mout_core -> div_core
-> div_core2 -> arm_clk
In the code you call directly the fout_apll which changes
frequency. Then the change shall be propagated to all registered
clocks.
I think, that DIV and DIV1 shall be reduced before PLL change [*],
to reflect the changes at CCF.
> +
> + if (drate < prate) {
> + mux_reg = readl(base + SRC_CPU);
> + writel(mux_reg | (1 << 16), base + SRC_CPU);
> + while (((readl(base + STAT_CPU) >> 16) & 0x7) != 2)
> + ;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ [**]
2. I think, the above shall be done in a following way:
clk_set_parent(mout_core, mout_mpll);
clk_set_rate(armclk->fout_pll, drate);
clk_set_parent(mout_core, mout_apll);
The direct write to registers [**] doesn't look compliant to CCF.
I'd rather thought about using "mout_core" instead of "arm_clk". Then we
would get access to the parent directly:
struct clk *parent = clk_get_parent(hw->clk);
so we set the parents explicitly (at clk registration) and call
->recalc_rate for clocks which are lower in the tree (like "div_core",
"div_core2").
> + clk_set_rate(armclk->fout_pll, drate);
> + }
> +
> + writel(freq_data[0], base + DIV_CPU0);
> + while (readl(base + DIV_STAT_CPU0) != 0)
> + ;
> + writel(freq_data[1], base + DIV_CPU1);
> + while (readl(base + DIV_STAT_CPU1) != 0)
> + ;
> +
> + if (drate > prate) {
> + mux_reg = readl(base + SRC_CPU);
> + writel(mux_reg | (1 << 16), base + SRC_CPU);
> + while (((readl(base + STAT_CPU) >> 16) & 0x7) != 2)
> + ;
> +
> + clk_set_rate(armclk->fout_pll, drate);
> + }
> +
> + mux_reg = readl(base + SRC_CPU);
> + writel(mux_reg & ~(1 << 16), base + SRC_CPU);
> + while (((readl(base + STAT_CPU) >> 16) & 0x7) != 1)
> + ;
> + return 0;
> +}
> +
> +static const struct clk_ops exynos4210_armclk_clk_ops = {
> + .recalc_rate = samsung_core_clock_recalc_rate,
> + .round_rate = samsung_core_clk_round_rate,
> + .set_rate = exynos4210_armclk_set_rate,
> +};
> +
> /* register exynos4 clocks */
> static void __init exynos4_clk_init(struct device_node *np,
> enum exynos4_soc exynos4_soc,
> @@ -1164,6 +1254,10 @@ static void __init exynos4_clk_init(struct
> device_node *np, ARRAY_SIZE(exynos4210_gate_clks));
> samsung_clk_register_alias(exynos4210_aliases,
> ARRAY_SIZE(exynos4210_aliases));
> + samsung_coreclk_register("armclk", armclk_p,
> + ARRAY_SIZE(armclk_p), "fout_apll",
> + &exynos4210_armclk_clk_ops, arm_clk,
> + &exyno4210_armclk_table);
> } else {
> samsung_clk_register_mux(exynos4x12_mux_clks,
> ARRAY_SIZE(exynos4x12_mux_clks));
--
Best regards,
Lukasz Majewski
Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
