lkml.org 
[lkml]   [2009]   [Jan]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH A 01/10] OMAP2/3: Add non-CORE DPLL rate set code and M, N programming
Hello Russell,

On Wed, 28 Jan 2009, Russell King - ARM Linux wrote:

> Since it's been posted to lists, comments are going to be made...

Yes, that was the point.

> On Tue, Jan 27, 2009 at 07:12:47PM -0700, Paul Walmsley wrote:
> > + /*
> > + * According to the 12-5 CDP code from TI, "Limitation 2.5"
> > + * on 3430ES1 prevents us from changing DPLL multipliers or dividers
> > + * on DPLL4.
> > + */
> > + if (omap_rev() == OMAP3430_REV_ES1_0 &&
> > + !strcmp("dpll4_ck", clk->name)) {
> > + printk(KERN_ERR "clock: DPLL4 cannot change rate due to "
> > + "silicon 'Limitation 2.5' on 3430ES1.\n");
> > + return -EINVAL;
> > + }
>
> Yuck. That's revolting and extremely fragile. Don't play these games.
> You've got plenty of free flag bits in clk->flags which could be used to
> prevent the DPLL from being changed. You've also got other ways to
> prevent it - eg, setting dpll_data to NULL.

dpll_data is used for other DPLL register settings (such as autoidle and
mode setting), so we should probably leave dpll_data as-is. Your proposed
fix in your subsequent message sounds good (viz., adding a separate static
omap3_dpll4_set_rate() function).

> However, what's worse is that, below...
>
> > +static int omap3_noncore_dpll_set_rate(struct clk *clk, unsigned long rate)
> > +{

...

> > + omap3_noncore_dpll_program(clk, dd->last_rounded_m, dd->last_rounded_n,
> > + freqsel);
>
> The return value from the above test isn't checked, so this function
> will succeed even for dpll4_ck.

Indeed, the return value should be passed along to the caller. The
previous check does, however, prevent the DPLL4 registers from being
written on 3430ES1.

From your subsequent message, it sounds like you've merged a version of
this patch with your proposed fixes. Please let me know if you'd
like me to send an updated version of this patch anyway.


- Paul


\
 
 \ /
  Last update: 2009-01-29 08:23    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site