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

[Dmitry <dbaryshkov@gmail.com>]

Hi,

2008/3/26, Haavard Skinnemoen <haavard.skinnemoen@atmel.com>:
> 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.

I've added a more sofisticated arch convertion patch (the clocklib for
ARM PXA chips).

Basically mode becomes enable/disable (however it may be better to merge back
those pointers into one function). And dev and index go to priv data.

The documentation will come later.

>
>  I have quite a few clocks, so the increased memory consumption is quite
>  significant. What are the advantages of this?

At maximum 55, IIUC. I counted 32 or so additional bytes in the struct
(over avr32-specific one). That would count up to 1.5 K overhead. Is
that really too much for current kernels?


OTOH this would bring unification of platform code, allow
configurations when a non-platform driver would provide it's own
clocks (think about multi-function companion chips when there is a
"core" which manages "clocks" for it's "periferal" devices. Currently
if one tries to implement such driver, he is forced to either bind it
to platform code, or to implement non-standard
my_device_clock_enable()-like functions.

Also you aren't forced to use this API. simply don't select
HAVE_CLOCK_LIB and leave
all things as they are. E.g. gpiolib is now merged, however not all
gpio-providing platforms
are using it.

-- 
With best wishes
Dmitry