Hi Raffaele,
On Mon, Jul 26, 2010 at 22:09:20, Raffaele Recalcati wrote:
> 2010/7/22 Nori, Sekhar <[email protected]>
>
[...]
> > diff --git a/arch/arm/mach-davinci/clock.c
> b/arch/arm/mach-davinci/clock.c
> > index f29a526..6e45808 100644
> > --- a/arch/arm/mach-davinci/clock.c
> > +++ b/arch/arm/mach-davinci/clock.c
> > @@ -254,7 +254,15 @@ static unsigned long
> clk_sysclk_recalc(struct clk *clk)
> > u32 v, plldiv;
> > struct pll_data *pll;
> > unsigned long rate = clk->rate;
> > + struct clk *parent = clk;
> >
> > + if (clk == NULL || IS_ERR(clk))
> > + return -EINVAL;
> > + while (parent->parent->parent)
> > + parent = parent->parent;
> > +
> > + if (parent == clk)
> > + return -EPERM;
>
>
> It is not clear to me why this change in needed. It is not
> described in the patch description as well. Most likely this
> needs to be carved into a separate patch as well describing
> what is wrong with the existing clk_sysclk_recalc() routine.
>
>
>
>
> now, whith the last check, we don't need that modifications, but only
>
> /* Otherwise, the parent must be a PLL */
> - if (WARN_ON(!parent->pll_data))
> + if (!clk->parent->pll_data)
>
> clkout2 is a sub-divider and so its parent is not a pll.
This function is meant to recalculate the rate for a sysclk.
For clkout2, a new recalculate function should be written.
[...]
> > +int dm365_clkout2_set_rate(unsigned long rate)
>
>
> Is clockout2 specific to DM365? DM355/DM6446 manuals mention
> clkout signal as well. If this routine can cater to more SoCs
> with simple modifications, you can attempt to generalize it.
>
>
>
> we check in dm355 and clkout2 is really a different clock.
> it seems difficult to integrate.
> we'd prefer not to do it.
Okay.
[...]
>
> > +
> > + /* check all possibilities to get best fitting for the
> required freq */
>
> > + i_min_err = min_err = INT_MAX;
> > + for (i = 0x0F; i > 0; i--) {
> > + if (clk->parent->set_rate) {
> > + ret = clk_set_rate(clk->parent, rate *
> i) ;
> > + err = clk_get_rate(clk->parent) - rate *
> i;
> > + if (min_err > abs(err)) {
> > + min_err = abs(err);
> > + i_min_err = i;
> > + }
> > + }
> > + }
>
>
> Why should the child touch the parent's clock output? Users of
> the
> clock framework should be able to set these rates independently.
>
>
>
> right.
> we tried.
> the problem is that the clkout2 is used for uda1345 system clock.
> without chenig the parent we can't get close.
> the sound is really too fast.
>
You should be able to change both clocks independently.
Example, in your board code:
xxx uda135_set_clk_rate(xxx)
{
clk_set_rate(sysclkN, desired_rate);
clk_set_rate(clkout2, desired_rate);
}
That should work?
>
>
>
> Tomorrow, if you agree, I'll send you 2 patches:
> -patch1: clkout2
This is fine..
> -patch2: removing warn from sysclk recalc
... but as I wrote above, still don't see a need for this.
Thanks,
Sekhar
_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
