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    [W:4.566 / U:0.076 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site