Re: [PATCH 1/2] regulators: Add definition of regulator_set_voltage_time() for !CONFIG_REGULATOR

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2165 bytes --]

On Mon, Jun 02, 2014 at 06:44:35PM +0530, Viresh Kumar wrote:
> On 2 June 2014 17:53, Mark Brown <broonie@kernel.org> wrote:

> > If the consumer tried to set a voltage presumably it cares if that
> > voltage was set - for example if your cpufreq driver tries to increase
> > the voltage of a core supply so that it can then raise the frequency the
> > user is going to be upset if the voltage was not actually raised and it
> > goes off and raises the clock rate causing the system to become unstable.

> If the driver continued despite getting regulator as NULL, it means that
> regulator isn't a MUST for it. For example a CPUFreq driver may work
> with or without a regulator.

No, NULL is a perfectly valid regulator - the *only* thing that a caller
should check for is IS_ERR().  You are missing the point of the stubs,
and indeed the fact that real physical supplies can have exactly the
same limitations as the dummy supplies do and therefore the presence of
a physical regulator should not in itself indcate anything about what
you can do with it.

> Now if the dummy calls return *error* for some cases then these driver
> will have to do
> if(xyz)
>     API-call()..

Consumers should be implementing error checking code regardless.  If we
don't need to implement any error checking there's rather a lot of the
kernel we can go and delete...

> And so dummy APIs like clk_set_rate(), clk_get_rate(),
> regulator_set_voltage() must return zero..

Please re-read and think about my CPUfreq example.  How do you expect
that to work sanely if we don't care if any of the operations worked?

> To get rid of this in drivers these dummy routines *must* behave as
> they passed, if the drivers really care about them then they must
> quit as soon as regulator_get() returned NULL.

> This is why we have such implementations in clk framework which are
> very well thought earlier.

> Does this make sense?

No, not at all and I don't think it applies to the clock API either -
it's got similar issues with real physical clocks not always supporting
all operations.  Consider for a moment what happens if we try to set and
then use a clock rate ona fixed clock.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]