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

[Mark Brown <broonie@sirena.org.uk>]

On Sat, Mar 14, 2009 at 02:29:24PM -0700, David Brownell wrote:

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

I'm pretty sure I understand exactly what you're saying but we just
disagree on this one.  Probably best that we agree to disagree.

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

Looking at things from the point of view of the consumer I just don't
find that it makes any difference since as far as the consumer is
concerned it's all opaque objects manipulated via an API.  I certainly
don't experience any conceptual jar switching between this and things
like the clock API, the patterns in clients the same especially for a
basic consumer doing something along the lines of:

	foo = foo_get(dev, name);
	foo_enable(foo);
	foo_disable(foo);
	foo_put(foo);

Even once you start setting more properties in there I'm struggling to
see the difference being visible.

I feel that you are focusing too much on the implementation here, not
the interface, but like I say I think we're just going to have to agree
to disagree on this one.

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

You could go for regulated_supply or something.  I think that at this
point it's just not worth the trouble to try to change the name, though.

If it helps think of the per client object as representing the
particular physical supply to the physical device.  We could represent
them internally as pass through regulators but since the implementation
should still be opaque to the consumers I don't think that it's worth
doing that unless it buys us something in the implementation.

I'm not overly concerned about the implementation right now since it's
not causing any problems and the opacity we have now for the consumers
supports later refactoring.  Things like the issues with working with
struct device for I2C and SPI devices seem to be causing far more
pressing problems in actual use.

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

It's only exposed to the drivers for the regulator hardware; it's not
visible to consumers unless they go peering into the driver API.  The
drivers for the regulators have always had a direct handle on themselves
for obvious reasons - the only change here is that they now know a bit
more about the implementation.