Re: [PATCH v5 3/4] clk: introduce the common clock framework

[Turquette, Mike]

On Wed, Mar 7, 2012 at 10:27 PM, Andrew Lunn <andrew@xxxxxxx> wrote:
>> Assuming that some day OMAP code can be refactored to allow for lazy
>> (or at least initcall-based) registration of clocks then perhaps your
>> suggestion can take root.  Which leads me to this question: are there
>> any other platforms out there that require the level of expose to
>> struct clk present in this patchset?  OMAP does, for now, but if that
>> changes then I need to know if others require this as well.
>
> Hi Mike
>
> For kirkwood, i use static clk's for all but my root clock. I cannot
> statically know the rate of the root clock, so i have to determine it
> at boot time using heuristics, PCI ID, etc.
>
> I used statics thinking it would be less code. No idea if it actually
> is, and there is nothing stopping me moving to creating the clocks
> after creating the root clock.
>
> One comment i have about the current static clks. I completely missing
> you need to call __clk_init() on them and so ended up with lots of
> division by zero errors, since they did not have a parent, and so the
> code seemed not be to able to determine the rate.
>
> So
>
> 1) Please add __clk_init() to the documentation in the section about
>   static clocks.

Done.

>
> 2) Should maybe the name change? It seems strange having to call a
>   __X() function. If this is a function which is supposed to be used,
>   drop the __. Maybe clk_static_register()?

That function is used internally by clk_register and is only exposed
by clk-private.h, so I think the naming scheme is sane.  I basically
want to create a sense of worry in anyone using clk-private.h :-)

>
> 3) Maybe, if the parent is missing, clk_get_rate() should return an
>   error?

Non-root, non-fixed-rate, orphan clocks can return an error in this
case.  Will update for V6.  Any idea on best -EERROR?  I'm thinking
ENODEV, but ECHILD and EPIPE are kinda funny in this context.

Regards,
Mike

>
> Andrew
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/