lkml.org 
[lkml]   [2012]   [Mar]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v6 2/3] clk: introduce the common clock framework
    On Fri, 9 Mar 2012, Mike Turquette wrote:
    > +inline unsigned long __clk_get_enable_count(struct clk *clk)
    > +{
    > + return !clk ? -EINVAL : clk->enable_count;

    Returning negative error codes in a function with a return value
    unsigned long is a bit strange at least. Shouldn't that be long ?

    > +#ifndef __LINUX_CLK_PRIVATE_H
    > +#define __LINUX_CLK_PRIVATE_H
    > +
    > +#include <linux/clk-provider.h>
    > +#include <linux/list.h>
    > +
    > +/*
    > + * WARNING: Do not include clk-private.h from any file that implements struct
    > + * clk_ops. Doing so is a layering violation!
    > + *
    > + * This header exists only to allow for statically initialized clock data. Any
    > + * static clock data must be defined in a separate file from the logic that
    > + * implements the clock operations for that same data.

    Now the question is whether you should provide a data structure which
    is explicitely used for static initialization and instead of having
    struct clk static you register the static initializer structure, which
    would be initdata. I don't think that anything needs clocks before the
    memory allocators are up and running. The clocks which are necessary
    to get that far have to be enabled in the boot loader anyway.

    The static initialization question should not hold off this set from
    being merged, though settling it before growing users would be nice.

    Otherwise this is a very well done infrastructure implementation!
    Thanks a lot Mike!

    Reviewed-by: Thomas Gleixner <tglx@linutronix.de>


    \
     
     \ /
      Last update: 2012-03-10 11:55    [W:0.021 / U:31.128 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site