Hi Nori,
> > --- 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.
>
Due to the fact that clkout2 is after the second divider I guess I'll have
to semplify the recalc function.
I'll try.
>
> [...]
>
> > > +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);
> }
>
Doing like that I should put a logic in that driver.
It is not wrong, because I'm writing a cpu_dai, but, anyway, I will change
the sysclk9 that, maybe, is used from someone else.
Isn't there the possibility to "occupy" a sysclk?
>
> 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.
>
Can't I use clk->flags to have a conditional behaviour of clk_sysclk_recalc
if it is called from pll1_sysclk9 or from clkout2 ?
Thx,
Raffaele
_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
