Re: [UPDATED v3][PATCH 1/7] regulator: consumer interface

[David Brownell <david-b@pacbell.net>]

On Sunday 09 March 2008, Liam Girdwood wrote:
> 
> > > +struct regulator *__must_check regulator_get(struct device *dev,
> > > +                                            const char *id);
> > 
> > The semantics of "id" and "dev" are unspecified in this patch,
> > so this isn't a good definition of the consumer interface!
> > 
> 
> 'id' is really the regulator name and will be renamed in the next patch.

Still not helping.  How would a driver know what names to use?
Are those names globally scoped, or local to the device?

Again, "id" and "dev" are unspecified.  I can maybe guess that
you're trying to make this look like <linux/clk.h> ... except
the clock API includes kernel doc in that header.

I *strongly* think new interfaces should not be provided without
documentation... but that's what this patch does.


> > Plus, that works more like a "lookup" than a "get" ... the
> > usual convention is that "get" and "put" update refcounts.
> > But I think I see an assumption here that a regulator may
> > have only one user...
> 
> A regulator only has one user as it's used to store some device specific
> power data. However, a regulator_dev has many users. I'll add a refcount
> on get/put.

I'm still not following.  If there's only one user, why would
you need refcounting?  If your model here is the clock API,
then you should support multiple users ... and then refcounting
is very appropriate.

- Dave