Re: [PATCH] regulator: debugfs: Adding debugfs functions into regulator framework

[Mark Brown <broonie@opensource.wolfsonmicro.com>]

On Tue, Dec 07, 2010 at 02:04:02PM -0800, Brandon Leong wrote:

[Please don't top post, and please quote properly - I've added a layer
of quotation to my text.]

> So is it decided that we should use BUG_ON() now?

I'd prefer it, other people's mileage may vary.  But please do also
engage with the big picture stuff I was talking about.

> Also, regarding this issue:

> > > +        if (val)
> > > +                err_info = regulator_enable(data);
> > > +        else
> > > +                err_info = regulator_disable(data);

> > This isn't going to do what people expect - the refcounting really is
> > going to surprise people, especially as you read back the physical
> > enable/disable state through the same file.  Abuse of this file is
> > likely to confuse any actual consumers we have too.

> ----

> Could you clarify the issue with this? All I am doing here is if the user
> enters a "1", then enable, if the user enters a "0" then disable.

Right, but remember that the regulator API does refcounting.  Writing
to the file won't always have an immediate effect, it'll update the
refcount which may or may not do what's expected to the actual hardware.

One other thing: your code checks for operations before it creates files
but the operations may not be permitted by the constraints.