Re: [patch 2.6.29-rc7 regulator-next] regulator: refcount fixes

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

On Thursday 12 March 2009, Mark Brown wrote:
> On Thu, Mar 12, 2009 at 03:02:55PM -0800, David Brownell wrote:
>
> > One could as easily have "handle" and "regulator" be the
> > same ... so the get/put idioms could work like they do
> > elsewhere in the kernel.
> 
> I really don't see that there is any meaningful difference here; from
> the point of view of the consumer the fact that the thing it gets back
> is a handle to a structure the core uses to keep track of the consumer
> rather than the underlying hardware object is an implementation detail
> that shouldn't make any difference to them.  In terms of the programming
> model it seems like a layering violation to know the difference between
> one opaque structure and another.

You're not stepping back from the current interface, which is
a prerequisite to understanding the points I was making:

 * Almost everywhere else in the kernel, there's only one
   handle (no per-client facet idiom), for which get/put
   works.

   Having the handle alloc/free methods be called get/put
   is a kind of problem.  We want models and idioms to
   converge, not diverge, in almost all cases ... using
   the same names to mean different things isn't good.

 * The thing that *is* per-client is basically a constraint
   set ... but it's called a "regulator", which again is
   confusing.

In the regulator-next tree you've now moved regulator_dev
into the public interface ... that's the handle to the
real hardware.  Sort of a hint that it can't really be
hidden in the way you originally thought.


> > See above.  Currently constraints are hidden for "consumers",
> > behind functional accessors like regulator_set_voltage().
> > There are no explicit constraint objects, as there are for
> > the machines.
> 
> The current interface has been driven by the needs of the users: the
> majority of consumers want to do one operation on a regular basis -
> normally that's enable/disable, most devices are just powering
> themselves up and down, though for some things voltage changes are much
> more common (DVFS being the prime example).  Overall it's been fairly
> similar to the clock API in terms of usage pattern.

Except that the clock interface uses put/get in the normal way;
they are not alloc/free calls, just lookup/refcount calls.


> In terms of looking at redesigning the API

You were the one suggesting the need for a new call, formalizing
a model that didn't previously exist ... not me!  :)

Which is why I suggested taht if you were going to add calls,
it'd be worth thinking a bit more about some existing glitches.