Re: [PATCH 1/3] Clocklib: add generic framework for managing clocks.

[Paul Mundt <lethal@linux-sh.org>]

On Wed, Mar 26, 2008 at 05:04:41PM +0100, Haavard Skinnemoen wrote:
> On Wed, 26 Mar 2008 18:52:03 +0300
> Dmitry Baryshkov <dbaryshkov@gmail.com> wrote:
> 
> > +struct clk {
> > +	struct list_head node;
> > +	struct clk	*parent;
> > +
> > +	const char	*name;
> > +	struct module	*owner;
> > +
> > +	int		users;
> > +	unsigned long	rate;
> > +	int		delay;
> > +
> > +	int (*can_get)	(struct clk *, struct device *);
> > +	int (*set_parent) (struct clk *, struct clk *);
> > +	int (*enable)	(struct clk *);
> > +	void (*disable)	(struct clk *);
> > +	unsigned long (*getrate) (struct clk*);
> > +	int (*setrate)	(struct clk *, unsigned long);
> > +	long (*roundrate) (struct clk *, unsigned long);
> > +
> > +	void		*priv;
> > +};
> 
> Hmm...this is exactly twice as big as the struct I'm currently using,
> it doesn't contain all the fields I need, and it's undocumented.
> 
Conversely it also has fields that I don't need. If struct clk could have
been done generically without growing to insane sizes, it would have been
done so in linux/clk.h a long time ago. The main thing there is API
consistency for drivers, leaving the details up to the architecture.

It's true that there is significant overlap between the different users
of the clock framework, but it's also not clear that there's any clean
way to share a common implementation (especially since struct clk means
totally different things on different architectures). I suspect everyone
in the CC list has been through this before, also.