Messages in this thread | | | Date | Mon, 2 Jun 2014 16:20:14 +0100 | From | Mark Brown <> | Subject | Re: [PATCH 1/2] regulators: Add definition of regulator_set_voltage_time() for !CONFIG_REGULATOR |
| |
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. [unhandled content-type:application/pgp-signature] | |