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

[Dmitry]

Hi,


2008/5/2, Paul Walmsley <paul@xxxxxxxxx>:
> 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?

OK. I'll introduce the clk_set_rate_propagate that should be used as a
->set_rate callback. The get_rate logic (to default to the parent
clock) seems to be OK from my POV. Will that suite you?

>
>
>  > 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.

Yes and no. Nearly all platforms have some type of clk_register. But
to use it you have to burry deeply in the platform code. There is no
easy way for a driver to provide clocks on both the PXA and SA-1100
arms, not telling about other platforms.

>
>
>  > 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.

Nearly all platform-dependant code is still left in hands of platform
maintainers. There are only few assumptions built into the clocklib.
And I've just agreed to remove one of them.

>
>
>  > >  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.

Tell me more please about "flexible usecounting". Or please provide
pointers to relevant threads I can read.

>
>  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.

I don't think that's the way to go. It's pretty stupid to force all
maintainers to agree to worse code only to make it better.

I still didn't receive any strong objections regarding the PXA and
SA-1100. Few other ARM architectures that have simple clk.h would gain
from clocklib by being able to drop non-platform-specific code. The
AVR32 maintainer seems to be looking positievly to adapting clocklib
once it's merged.

Maybe we should merge the core part only for now, to let all platform
maintainers consider it, think about it, provide patches to clocklib
that will be carefully though out and later (when dust settles) merge
the remaining bits of platform support?

-- 
With best wishes
Dmitry
--
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/