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

[Dmitry]

Hi,

2008/3/26, Paul Mundt <lethal@xxxxxxxxxxxx>:
> 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@xxxxxxxxx> 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.

In reality, as noted David Brownell on LAKML, there is no API consistency.
The driver has no way to know whether clk API is implemented at all or whether
the "optional" functions do exist.

Moreover clocks aren't limited only to platform-specific code. As an
example of the in-tree driver that will benefit from clocklib you can
check drivers/mfd/sm501.c which  contains  it's own set of functions
to manage clocks.

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

That's one of the initial reasons of this patchserie: I have a device
that can be installed on several platforms. And I want for this device
to provide clocks to some other devices.

Simply saying "Oh, things are different" is behaving like a ostrich:
hiding a head in the sand.

As most generification patches do, this one has it's drawbacks, but
they are not so big, as you describe.

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