Re: [PATCH 0/5] Clocklib: generic clocks framework

From: Paul Walmsley
Date: Fri May 02 2008 - 01:23:57 EST


Hello Dmitry,

On Sat, 26 Apr 2008, Dmitry wrote:

> 2008/4/25, Paul Walmsley <paul@xxxxxxxxx>:
> >
> > I wouldn't pretend to have a comprehensive list at this point. But from a
> > brief look, your clk_round_rate() and clk_set_rate() implementations will
> > not work for the present OMAP clock tree. In OMAP, many parent clocks do
> > not have the same rate as their children.
>
> You can easily override any calculations in your clk->set_rate/clk->round_rate/
> clk->get_rate functions.

The problem is that parent clocks shouldn't be automatically set to the
same rate as child clocks. We could probably hack around it in
clk->set_rate to ignore those spurious rate changes, but really they
shouldn't be done in the first place, at least on OMAP. clk->set_rate
should handle parent rate changes if necessary. Removing this from
clocklib looks trivial - involves deleting about ten lines of clocklib
code?

> My first goals are:
> 1) to have an infrastructure to plug in my clocks in a platform-independant way

Doesn't the existing clock framework do this already? Or are you really
referring to your clk_functions/multiple consumer additions? If the
latter, then we all would be better served by discussing why the existing
clock interface doesn't meet your needs, and figuring out what to do about
it, rather than by merging code that initially appears to be common, but
with you having the ultimate intention of extending the common clock
interface to implement the features that you want.

> 2) to drop lots of copies of nearly the same code.

I don't really see much of a problem with the current situation, as long
as all of the implementations use the same interface. Right now,
architectures are free to change their underlying code without having to
worry about breaking other architectures or dealing with new gatekeepers,
and that's a big plus for the status quo. Russell is the person who
shoulders the burden here of multiple implementations, and when this
becomes a pressing problem, I would expect him to be doing most of the
advocacy for common code.

> > Assumptions that you make in clocklib may not work well for future chips.
> > Changing clocklib will affect many architectures. For example, perhaps
> > someone may wish to implement clocks as an actual in-memory tree rather
> > than a list. Or perhaps someone may need to handle clock usecounting
> > differently, for situations when multiple clocks might share the same
> > enable/disable code, but with different parents.
>
> Sorry, but WTF? Do you prefer to keep a lot of code and disallow
> merging a generification just because of some-that-may-even-not-exist
> cases? That sounds
> pretty... strange.
> And your examples are really strange.

Those examples are real; they have been discussed in the past or proposed
for the future for OMAP clock framework. Regarding the second example,
that's a very real issue for us right now with our interface clocks, and
flexible usecounting is one of the ways that we've considered solving it.
As far as the tree example goes, we've got about 200 struct clks in the
latest OMAP clock framework, and this will probably increase by 50% over
the next year. We have clock tree operations that have to move both up
and down the tree. Right now we're doing sequential scans on a clock
list, but chances are quite high that we will be doing this very
differently a few months.

I cited these examples not because I was interested in having you tell me
that my examples were unrealistic or "strange" or should be done
differently, but simply to reinforce the message that a top-down approach
to a generic clocklib is not likely to be appreciated much by maintainers.
I know you keep saying that your code is intended to be be strictly
optional. If you want to prove it, why not consider a bottom-up approach
instead? Right now you've got patches that convert two architectures to
use your clocklib. Instead of using a common library, just duplicate your
clock code for each architecture. There's not much to it. Propose
patches to individual architecture maintainers, and persuade them to
replace their existing code with your own.

If you can get a large fraction to use your identical code, and agree that
it's at least as good as what they're using right now, then you'd have a
pretty strong case for creating some kind of common library. If not,
well, those maintainers probably would not switch to your current common
clocklib either, so there's really no downside.


- Paul
--
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/