lkml.org 
[lkml]   [2009]   [Feb]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH E 11/14] OMAP clock: track child clocks
    Hello Russell,

    On Sat, 14 Feb 2009, Russell King - ARM Linux wrote:

    > However, looking a little deeper, there's more issues in the reparenting
    > area. I don't think this code has been tested at all... In
    > _omap2_clksel_get_src_field, there is this:
    >
    > for (clkr = clks->rates; clkr->div; clkr++) {
    > if (clkr->flags & (cpu_mask | DEFAULT_RATE))
    > break; /* Found the default rate for this platform */
    > }
    >
    > which is bogus - it will find the first entry which is _either_ marked
    > as a default rate _or_ is supported by the SoC. This means (for
    > instance) that:
    >
    > static const struct clksel_rate core_l3_core_rates[] = {
    > { .div = 1, .val = 1, .flags = RATE_IN_24XX },
    > { .div = 2, .val = 2, .flags = RATE_IN_242X },
    > { .div = 4, .val = 4, .flags = RATE_IN_24XX | DEFAULT_RATE },
    >
    > will give us divisor 1 rather than presumably the one we want, that being
    > divisor 4. I think the test above should be:
    >
    > for (clkr = clks->rates; clkr->div; clkr++) {
    > if (clkr->flags & cpu_mask &&
    > clkr->flags & DEFAULT_RATE)
    > break; /* Found the default rate for this platform */
    > }
    >
    > so we find an entry which is supported _and_ is the default for the SoC.

    Agreed.

    > There's also a second issue - the comments before omap2_divisor_to_clksel()
    > indicate that this function returns 0xffffffff on error. Unfortunately,
    > this is not so, it actually returns zero on error. Moreover, we test
    > the result of the function against ~0, so we'll never deal with the error
    > case. This really should be fixed so that we return the right value for
    > the error case. (Further comments on this in a follow up.)

    Agreed here also.

    > So, below is a patch which fixes both of these issues.

    Looks good, thanks Russell.

    Acked-by: Paul Walmsley <paul@pwsan.com>


    - Paul


    \
     
     \ /
      Last update: 2009-02-23 00:39    [W:2.343 / U:0.108 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site