[RFC] regulator: Shared regulators (configured by bootloader)

[RFC] regulator: Shared regulators (configured by bootloader)

[Viresh Kumar]

Hi Mark/Liam,

I am looking to solve a problem faced by some of the Qualcomm
platforms and want your suggestions on how should we fix it. One of my
ex-colleague tried to solve [1] this problem but that thread never
concluded (and I don't really agree with the solution it offered).


Problem statement

A regulator is shared by multiple devices and their drivers can get
probed in any order. The regulator is configured (with constraints
suitable for one or more devices) by the bootloader (i.e. regulator
was ON at kernel boot). While the drivers get probed, they may choose
to disable or reconfigure the regulator. And that is of course the
right thing to do for those drivers. But this might not be ideal for
other devices which share the regulator and are currently using it
(but haven't been probed yet).

A typical use case in cell phones is to put up a simple splash screen
in the bootloader and replace that screen when userspace is running
(typically with the home screen). If a shared regulator is enabled in
the bootloader to power the screen and left on during kernel
initialization, it's possible that a non-screen driver may disable or
reconfigure the regulator from its probe() and that would generate a
glitch at the screen, which wouldn't be nice.


Solution(s)

There is no way for us to know when the kernel has done booting,
specially with the drivers compiled as modules, as they can be
inserted anytime. For example, if the LCD driver is never inserted to
the kernel, then we should never allow to regulator to get disabled
(Even if that results in loss of energy).

I was wondering if we should add some DT properties to consumer
devices to list down such constraints and create "proxy" regulators
for them as soon as possible during the kernel boot (maybe from
regulator_register()). Of course that would require us to traverse all
DT nodes containing such properties, as the device drivers shouldn't
be up by then.

And finally once the drivers are probed and try to get regulator for
the device, we remove these proxy regulators (only after creating the
real ones). If driver for some proxy regulator doesn't get probed,
then that regulator stays ON for ever.

Suggestions ? Take your time to reply, I know that the merge window
has just started.

-- 
viresh

[1] https://lkml.org/lkml/2016/5/9/64

Re: [RFC] regulator: Shared regulators (configured by bootloader)

[Mark Brown]

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

On Mon, May 08, 2017 at 03:51:02PM +0530, Viresh Kumar wrote:

> I am looking to solve a problem faced by some of the Qualcomm
> platforms and want your suggestions on how should we fix it. One of my
> ex-colleague tried to solve [1] this problem but that thread never
> concluded (and I don't really agree with the solution it offered).

Please engage with the feedback I offered then.  I see no point in
repeating myself here, I'm just going to provide the same feedback as
before and if I'm going to be ignored (which appears to be the case) it
doesn't seem like it's worth the effort.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [RFC] regulator: Shared regulators (configured by bootloader)

[Viresh Kumar]

On 14-05-17, 18:30, Mark Brown wrote:
> On Mon, May 08, 2017 at 03:51:02PM +0530, Viresh Kumar wrote:
> 
> > I am looking to solve a problem faced by some of the Qualcomm
> > platforms and want your suggestions on how should we fix it. One of my
> > ex-colleague tried to solve [1] this problem but that thread never
> > concluded (and I don't really agree with the solution it offered).
> 
> Please engage with the feedback I offered then.  I see no point in
> repeating myself here, I'm just going to provide the same feedback as
> before and if I'm going to be ignored (which appears to be the case) it
> doesn't seem like it's worth the effort.

I didn't ignore any of that. Most of the discussions then happened around the
solution which Pingbo tried to implement (boot-protection flag). And that had
obvious flaws as it depended on the state of the kernel to be known
(booting/booted). And that will never work, as you pointed out then, because of
the way kernel works (specially with modules).

You said this in one of the replies [1]:

> I think you need to be looking at some combination of getting the
> devices you're interested in started up early and more precisely
> describing the end result you're trying to achieve.  The issues with
> probe deferral do seem related here, it's another symptom of not really
> making any decisions about init ordering and so sometimes making bad
> ones.

I don't think we should depend on the order in which the devices get added. This
is going to break for sure.

I have some idea about the end-result we want to achieve (will take LCD as an
example here):
- The regulator should always fulfill the requirement of the LCD device set from
  the bootloader, until a point where the kernel driver has taken over.
- If the LCD kernel driver never turns up, then the regulator shall continue
  satisfying the requirements set from the bootloader. Even if that means that
  some devices may not get what they request for and perform badly. Someone
  needs to fix the LCD driver and make sure it comes up.


I failed to find any other workable solution that anyone may have suggested in
those email threads. Can you please point me to those (if any)?

Thanks.

-- 
viresh

[1] https://marc.info/?l=linux-kernel&m=146540618625247

Re: [RFC] regulator: Shared regulators (configured by bootloader)

[Mark Brown]

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

On Mon, May 15, 2017 at 04:17:13PM +0530, Viresh Kumar wrote:
> On 14-05-17, 18:30, Mark Brown wrote:
> > On Mon, May 08, 2017 at 03:51:02PM +0530, Viresh Kumar wrote:

> > > I am looking to solve a problem faced by some of the Qualcomm
> > > platforms and want your suggestions on how should we fix it. One of my
> > > ex-colleague tried to solve [1] this problem but that thread never
> > > concluded (and I don't really agree with the solution it offered).

> > Please engage with the feedback I offered then.  I see no point in
> > repeating myself here, I'm just going to provide the same feedback as
> > before and if I'm going to be ignored (which appears to be the case) it
> > doesn't seem like it's worth the effort.

> I didn't ignore any of that. Most of the discussions then happened around the
> solution which Pingbo tried to implement (boot-protection flag). And that had
> obvious flaws as it depended on the state of the kernel to be known
> (booting/booted). And that will never work, as you pointed out then, because of
> the way kernel works (specially with modules).

You didn't comment in any way on any of the feedback, and have indeed
taken a step back to proxy regulators which were the first thing
proposed that time as well - the dependency on the boot state came about
as a result of Pingbo trying to solve solve some of the problems with
proxy regulators.  If nothing's said and the discussion just winds
backwards it's hard to tell if that's intentional or not.

> > I think you need to be looking at some combination of getting the
> > devices you're interested in started up early and more precisely
> > describing the end result you're trying to achieve.  The issues with

Please note the above bit about describing the end result you're trying
to achieve, it's a very important part of the problem.

> > probe deferral do seem related here, it's another symptom of not really
> > making any decisions about init ordering and so sometimes making bad
> > ones.

> I don't think we should depend on the order in which the devices get added. This
> is going to break for sure.

We want to do this anyway since if it's critical that things stay alive
it's also likely going to be important that they become responsive as
fast as possible, and it'd have more general applications.  I rather
suspect it's going to work a lot better in practice than you seem to be
imagining and it's less problematic for both systems with multiple use
cases and things like upgraded kernels with improved regulator (or clock
or power domain or whatever) support causing things to explode.

> - If the LCD kernel driver never turns up, then the regulator shall continue
>   satisfying the requirements set from the bootloader. Even if that means that
>   some devices may not get what they request for and perform badly. Someone
>   needs to fix the LCD driver and make sure it comes up.

This is a fundamental problem.  Users shouldn't have to load (or in the
worst case write) a driver for hardware they've no intention of using at
runtime just because whoever wrote the DT thought it was important, and
when people do want such behaviour they should be able to say so in
terms of the objective they want to achieve rather than having to figure
out how the hardware is wired together in order to do so.  It shouldn't
really be tied to the firmware interface at all, we have some pretty
similar goals for the displays on ACPI systems.

> I failed to find any other workable solution that anyone may have suggested in
> those email threads. Can you please point me to those (if any)?

Proxy regulators are just operating at the wrong abstraction level, take
a step back and think about the goal you're trying to achieve and design
an interface for doing that.  The user interfaces should be at the level
of the goal the user is trying to achieve rather than down in the
implementation details.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [RFC] regulator: Shared regulators (configured by bootloader)

[Viresh Kumar]

On 14-06-17, 18:13, Mark Brown wrote:
> On Mon, May 15, 2017 at 04:17:13PM +0530, Viresh Kumar wrote:

> > > I think you need to be looking at some combination of getting the
> > > devices you're interested in started up early and more precisely
> > > describing the end result you're trying to achieve.  The issues with
> 
> Please note the above bit about describing the end result you're trying
> to achieve, it's a very important part of the problem.

Right and this is where we need to start with for sure.

> > > probe deferral do seem related here, it's another symptom of not really
> > > making any decisions about init ordering and so sometimes making bad
> > > ones.
> 
> > I don't think we should depend on the order in which the devices get added. This
> > is going to break for sure.
> 
> We want to do this anyway since if it's critical that things stay alive
> it's also likely going to be important that they become responsive as
> fast as possible, and it'd have more general applications.  I rather
> suspect it's going to work a lot better in practice than you seem to be
> imagining and it's less problematic for both systems with multiple use
> cases and things like upgraded kernels with improved regulator (or clock
> or power domain or whatever) support causing things to explode.

Yes, but there can be weird dependencies between different components that want
to interact. For example a supply shared between LCD and I2C controller (Not
sure if such configurations are there in any of the hardware we have), where the
same i2c controller is used to access the LCD controller's registers. Both are
enabled at boot and the supply is configured to satisfy both. If the voltage
requirements of the I2C controller are below that of LCD, then we can't decide
on which one to probe first. We can't probe LCD first as its bus isn't active
and if we probe I2C first, then it may take the supply down to a level that
isn't acceptable for the LCD (which was on from boot).

> > - If the LCD kernel driver never turns up, then the regulator shall continue
> >   satisfying the requirements set from the bootloader. Even if that means that
> >   some devices may not get what they request for and perform badly. Someone
> >   needs to fix the LCD driver and make sure it comes up.

> This is a fundamental problem.  Users shouldn't have to load (or in the
> worst case write) a driver for hardware they've no intention of using at
> runtime just because whoever wrote the DT thought it was important

I haven't thought about it earlier, but yeah you are absolutely right here. We
just can't force something like this on the user.

> and
> when people do want such behaviour they should be able to say so in
> terms of the objective they want to achieve rather than having to figure
> out how the hardware is wired together in order to do so.

Right. Where do you suggest such user-specific configuration should be present?
I hope userspace as we want different users (that want to use LCD vs doesn't
want to use LCD) to use the same kernel image?

> > I failed to find any other workable solution that anyone may have suggested in
> > those email threads. Can you please point me to those (if any)?
> 
> Proxy regulators are just operating at the wrong abstraction level, take
> a step back and think about the goal you're trying to achieve and design
> an interface for doing that.  The user interfaces should be at the level
> of the goal the user is trying to achieve rather than down in the
> implementation details.

Are we talking about writing a new framework here which can take care of the
constraints (which can support any resource type) set by the bootloader until
the time:

- the device get probed by its driver
- userspace overrides the constraint

Did I misunderstood your comment ?

Thanks for your feedback Mark.

-- 
viresh

Re: [RFC] regulator: Shared regulators (configured by bootloader)

[Mark Brown]

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

On Fri, Jun 16, 2017 at 03:29:17PM +0530, Viresh Kumar wrote:

> Yes, but there can be weird dependencies between different components that want
> to interact. For example a supply shared between LCD and I2C controller (Not
> sure if such configurations are there in any of the hardware we have), where the
> same i2c controller is used to access the LCD controller's registers. Both are
> enabled at boot and the supply is configured to satisfy both. If the voltage
> requirements of the I2C controller are below that of LCD, then we can't decide
> on which one to probe first. We can't probe LCD first as its bus isn't active
> and if we probe I2C first, then it may take the supply down to a level that
> isn't acceptable for the LCD (which was on from boot).

There are good solid reasons why it's extremely rare to see variable
voltage shared supplies.

> > and
> > when people do want such behaviour they should be able to say so in
> > terms of the objective they want to achieve rather than having to figure
> > out how the hardware is wired together in order to do so.

> Right. Where do you suggest such user-specific configuration should be present?
> I hope userspace as we want different users (that want to use LCD vs doesn't
> want to use LCD) to use the same kernel image?

The command line seems to be a good first start and would ensure that we
have something that works for all firmwares, not just DT.  For DT the
chosen node is intended for things like this although it's questionable
how valuable this is with typical FDT systems.

> > Proxy regulators are just operating at the wrong abstraction level, take
> > a step back and think about the goal you're trying to achieve and design
> > an interface for doing that.  The user interfaces should be at the level
> > of the goal the user is trying to achieve rather than down in the
> > implementation details.

> Are we talking about writing a new framework here which can take care of the
> constraints (which can support any resource type) set by the bootloader until
> the time:

> - the device get probed by its driver
> - userspace overrides the constraint

> Did I misunderstood your comment ?

I don't know that it needs to be a framework particularly, seems more
like an extension of the device model to work back through the
dependencies and let things do something useful with them.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]