Alex,
On Wed, Nov 04, 2009 at 04:58:40 +0530, Sripathy, Vishwanath wrote:
>> @@ -134,13 +135,13 @@ static struct omap_clk omap34xx_clks[] = {
>> CLK(NULL, "omap_12m_fck", &omap_12m_fck, CK_343X),
>> CLK(NULL, "dpll4_m2_ck", &dpll4_m2_ck, CK_343X),
>> CLK(NULL, "dpll4_m2x2_ck", &dpll4_m2x2_ck, CK_343X),
>> - CLK(NULL, "dpll4_m3_ck", &dpll4_m3_ck, CK_343X),
>> + CLK(NULL, "dpll4_m3_ck", &dpll4_m3_ck, CK_343X | CK_363X),
>> CLK(NULL, "dpll4_m3x2_ck", &dpll4_m3x2_ck, CK_343X),
>> - CLK(NULL, "dpll4_m4_ck", &dpll4_m4_ck, CK_343X),
>> + CLK(NULL, "dpll4_m4_ck", &dpll4_m4_ck, CK_343X | CK_363X),
>> CLK(NULL, "dpll4_m4x2_ck", &dpll4_m4x2_ck, CK_343X),
>> - CLK(NULL, "dpll4_m5_ck", &dpll4_m5_ck, CK_343X),
>> + CLK(NULL, "dpll4_m5_ck", &dpll4_m5_ck, CK_343X | CK_363X),
>> CLK(NULL, "dpll4_m5x2_ck", &dpll4_m5x2_ck, CK_343X),
>> - CLK(NULL, "dpll4_m6_ck", &dpll4_m6_ck, CK_343X),
>> + CLK(NULL, "dpll4_m6_ck", &dpll4_m6_ck, CK_343X | CK_363X),
>> CLK(NULL, "dpll4_m6x2_ck", &dpll4_m6x2_ck, CK_343X),
>> CLK(NULL, "emu_per_alwon_ck", &emu_per_alwon_ck, CK_343X),
>> CLK(NULL, "dpll5_ck", &dpll5_ck, CK_3430ES2),
>
>Doesn't it make more sense to have separate dpll4_*_ck's for 363X so as to avoid the >clksel_mask_3630?
>
Then you will have to duplicate all the nodes in dpll4 clock tree which is redundant. There is no much difference in clock tree (between 3630 and 3430) except that clksel width is changed for 3630 which I feel does not justify adding a new clock tree. Updating the clksel mask is the simplest way to achieve it.
>> @@ -1216,6 +1217,10 @@ int __init omap2_clk_init(void)
>> cpu_mask |= RATE_IN_3430ES2;
>> cpu_clkflg |= CK_3430ES2;
>> }
>> + if (cpu_is_omap36xx()) {
>> + dpll4_dd.mult_mask = OMAP3630_PERIPH_DPLL_MULT_MASK;
>> + cpu_mask |= RATE_IN_363X;
>
>Extra space before '|'.
>
>> + }
>> }
>
>I think there's an indentation problem.
>
>>
>> clk_init(&omap2_clk_functions);
>> @@ -1225,6 +1230,11 @@ int __init omap2_clk_init(void)
>>
>> for (c = omap34xx_clks; c < omap34xx_clks + ARRAY_SIZE(omap34xx_clks); c++)
>> if (c->cpu & cpu_clkflg) {
>> + /* for 3630, change the mask value for clocks which are
>> + marked as CK_363X*/
>> + if (cpu_is_omap36xx())
>> + if (c->cpu & CK_363X)
>
>Extra space before '&'.
>
>> + c->lk.clk->clksel_mask = c->lk.clk->clksel_mask_3630;
>
>This looks longer than normally allowed.
What is longer? I did not get the comment.
>
>Regards,
>--
>Alex
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
