Subject: Re: Regulator: drivers that need to know their supply
To: Mark Brown <broonie@kernel.org>
References: <57065FB5.6040707@parkeon.com>
 <20160407170249.GV1924@sirena.org.uk>
Cc: Javier Martinez Canillas <javier@osg.samsung.com>,
        linux-kernel@vger.kernel.org
From: Martin Fuzzey <mfuzzey@parkeon.com>
Message-ID: <5706A435.4050304@parkeon.com>
Date: Thu, 7 Apr 2016 20:17:25 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101
 Thunderbird/38.3.0
MIME-Version: 1.0
In-Reply-To: <20160407170249.GV1924@sirena.org.uk>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2372
Lines: 61

Thanks for your reply.

On 07/04/16 19:02, Mark Brown wrote:
> On Thu, Apr 07, 2016 at 03:25:09PM +0200, Martin Fuzzey wrote:
>
>> To implement the .enable_time() method I need the voltage (which is
>> the supply's voltage).
> No, this is not sensible.  You should be telling the framework about the
> slew rate and letting the framework work out how long it's going to take
> to transition.  Your driver shouldn't be peering around inside other
> regulators, it should be telling the framework what it does itself and
> any handling of interrelationships should be in the framework.

Ok, but I fail to see any way to do that (at least with the framework as 
is).

The framework can tell the driver what slew rate to use (via 
.set_ramp_delay)
And it can ask the driver what the turn on time is (via .enable_time)
It doesn't actullay know the slew rate (as a driver may not have 
configurable
slew rates or the driver may have rounded the slew rate to something it 
supports)

Or are you saying I should extend the framework to add a .get_ramp_delay
driver callback and make the framework do the calculation itself?

Happy to do that if it's the best way but your reply makes it sound
like i'm not using an existing framework functionality and I don't think
that is the case unless I'm overlooking something.

I'm just trying to do what rc5t583 already does in 
rc5t583_regulator_enable_time()
except that in my case I don't know the voltage (since it comes from my 
supply).

While moving it to the framework will avoid the driver knowing about other
regulators it won't fix the underlying issues I mentionned.

For a regulator configured as always-on the framework doesn't lookup and
enable the parent  regulators until regulator_register_resolve_supply()
is caused right at the end of regulator_register().

This means that, when the always-on register is enabled it has no supply
assigned, even at the  framework level.

It also means that currently there is a difference in the switch on order
depending if the regulator is configured as always-on:

Suppose A is the supply for B.

If B is NOT always-on, calling regulator_enable(B) will first enable A 
then B
However if B is always-on, regulator_register(B) will first enable B then A

I'm not sure this actually matters in practice but A then B seems better
from an inrush current point of view.

Regards,

Martin