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

[Pavel Machek <pavel@ucw.cz>]

Hi!

> > >   If the latter, your patchset will presumably
> > >   need a higher standard of review, since once it is integrated, any
> > >   changes will affect several architectures, rather than simply one.
> > [...]
> > I've reviewed the code for most linux/clk.h implementations in the
> > kernel. The OMAP code was... a bit scary for me. I don't have any deep
> > knowledge of this platform, and there were lots of structures, lots of
> > structs embedding struct clk, etc.
> > Tell me please, what do you need, that can't be done with this framework?
> 
> 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.
> 
> But that is not really the main issue.  Ultimately, as long as your 
> implementation remains completely optional, and any public interfaces 
> (like debugfs/sysfs interfaces) are coordinated with other clock interface 
> implementors, I personally have no issues with your code going into the 
> tree somewhere.  With the possible exception of the clk_functions code, 
> clocklib looks pretty clean to me, and a good exemplar of a clock 
> interface implementation.
> 
> However: if the ultimate goal is to make your implementation normative, 
> then I concur with Russell, and I would recommend against merging.  
> 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.

WTF? There are currently around 10 copies of clock code in the tree,
every one slightly different. If this can help us get rid of all that
crap, that's a GOOD THING, normative or not.

> I am concerned that having any implementation as an implicit standard in 
> the tree would increase the risk that, over time, internal implementation 
> assumptions will be considered normative.  This could easily cause more 
> pain in the future for maintainers than it would be worth. 

_Of course_  new implementations should try to use existing
framework. And _of course_, if you have some special requirements, you
are still free to create your own version...
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html