Hi Pankaj,
On 07.03.2014 14:56, Pankaj Dubey wrote:
+static void __init exynos5260_clk_top_init(struct device_node *np)
+{
+ struct exynos5260_cmu_info cmu;
+ struct samsung_clk_provider *ctx;
+
+ memset(&cmu, 0, sizeof(cmu));
+
+ cmu.pll_clks = top_pll_clks;
+ cmu.nr_pll_clks = ARRAY_SIZE(top_pll_clks);
+ cmu.mux_clks = top_mux_clks;
+ cmu.nr_mux_clks = ARRAY_SIZE(top_mux_clks);
+ cmu.div_clks = top_div_clks;
+ cmu.nr_div_clks = ARRAY_SIZE(top_div_clks);
+ cmu.gate_clks = top_gate_clks;
+ cmu.nr_gate_clks = ARRAY_SIZE(top_gate_clks);
+ cmu.nr_clk_ids = TOP_NR_CLK;
+ cmu.clk_regs = top_clk_regs;
+ cmu.nr_clk_regs = ARRAY_SIZE(top_clk_regs);
+
+ ctx = exynos5260_cmu_register_one(np, &cmu);
+
+ samsung_clk_of_register_fixed_ext(ctx, fixed_rate_ext_clks,
+ ARRAY_SIZE(fixed_rate_ext_clks),
+ ext_clk_match);
+}
+
+CLK_OF_DECLARE(exynos5260_clk_top, "samsung,exynos5260-clock-top",
+ exynos5260_clk_top_init);
Well with this approach we end up adding 14 such
exynosxxx_clk_xxx_init functions all of which has similar lines of
code. As I know there are many upcoming Exynos SoC which will also
have similar multiple clock controllers (in some of them there are
upto 25 clock domains, and in that case we will end up writing 25 such
init functions) so I have following suggestion where we can have one
more structure which will hold all static data and match_table to
match compatibility string and return CMU_TYPE which can be mapped to
get proper clock_data which can be used in single clock_init function.
Following is some sample code which I have implemented and tested on
one of Exynos SoC. Please let me know your opinion about this.
Yes, this looks better indeed, however there is still a room for
improvement. Please see my comments below.
=============================
static struct exynosxxxx_clock_data exynosxxxx_clk_data[] __initdata = {
Instead of making this an array, particular elements could be separate
structures. This would simplify the code below.
{
.cmu_type = CMU_TYPE_TOP,
.mux_clocks = top_mux_clks,
.div_clocks = top_div_clks,
.pll_clocks = top_pll_clks,
.clk_regs = top_clk_regs,
.nr_mux_clocks = ARRAY_SIZE(top_mux_clks),
.nr_div_clocks = ARRAY_SIZE(top_div_clks),
.nr_pll_clocks = ARRAY_SIZE(top_pll_clks),
.nr_clk_regs = ARRAY_SIZE(top_clk_regs),
.nr_clks = TOP_NR_CLK,
}, {
.cmu_type = CMU_TYPE_EGL,
.mux_clocks = egl_mux_clks,
.div_clocks = egl_div_clks,
.pll_clocks = egl_pll_clks,
.clk_regs = egl_clk_regs,
.nr_mux_clocks = ARRAY_SIZE(egl_mux_clks),
.nr_div_clocks = ARRAY_SIZE(egl_div_clks),
.nr_pll_clocks = ARRAY_SIZE(egl_pll_clks),
.nr_clk_regs = ARRAY_SIZE(egl_clk_regs),
.nr_clks = EGL_NR_CLK,
}, {
/* Add similar elements here */
};
static struct of_device_id cmu_subtype_match_table[] = {
{
.compatible = "exynosxxxx-cmu-top",
.data = (void *)CMU_TYPE_TOP,
Here the data would be just a pointer to respective clock data struct
defined above.
}, {
.compatible = "exynosxxx-cmu-peris",
.data = (void *)CMU_TYPE_PERIS,
}, {
/* Add similar elements here */
};
void __init exynosxxx_clk_init(struct device_node *np)
{
[snip]
match = of_match_node(cmu_subtype_match_table, np);
if (!match)
panic("%s: cmu type (%s) is not supported.\n", __func__,
np->name);
This can't happen, because this function won't be called for any node with
compatible string not declared using CLK_OF_DECLARE().
reg_base = of_iomap(np, 0);
if (!reg_base)
panic("%s: failed to map registers\n", __func__);
cmu_type = (unsigned long) match->data;
for (; i < CMU_TYPE_ALL; i++) {
clk_data = &exynosxxxx_clk_data[i];
if (cmu_type == clk_data->cmu_type)
break;
}
Now clk_data could be taken directly from match->data, without the need to
iterate over an array.
