Messages in this thread | | | Subject | Re: [UPDATED v3][PATCH 1/7] regulator: consumer interface | From | Liam Girdwood <> | Date | Tue, 11 Mar 2008 10:20:41 +0000 |
| |
On Mon, 2008-03-10 at 16:39 -0800, David Brownell wrote: > 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?
Platform data i.e. "WM8350-DCDC1". The links I gave in a previous mail have examples for this wrt LED. Backlight drivers.
> Are those names globally scoped, or local to the device? >
They are global.
> 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. >
Fair point, I'll add docs for v4.
> > > > 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. >
Maybe the naming is confusing here. We should probably rename regulator to regulator_client or similar.
Liam
| |