public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: bjorn.andersson@sonymobile•com (Bjorn Andersson)
To: linux-arm-kernel@lists•infradead.org
Subject: [RFC 0/2] Qualcomm RPM sleep states
Date: Wed, 26 Nov 2014 14:49:52 -0800	[thread overview]
Message-ID: <20141126224952.GC2872@sonymobile.com> (raw)
In-Reply-To: <5473C70A.7040705@codeaurora.org>

On Mon 24 Nov 16:02 PST 2014, Stephen Boyd wrote:

> On 11/24/2014 01:59 PM, Bjorn Andersson wrote:
> > On Mon 24 Nov 13:19 PST 2014, Stephen Boyd wrote:
> >
> > [..]
> >> What exactly are we circumventing? I can only guess that we're talking
> >> about the aggregation logic?
> >>
> > We're circumventing the fact that the regulator core doesn't have knowledge
> > about our multiple presented views of the same resource.
> 
> Sorry I don't follow here. Why would the regulator framework care about
> the different views of a resource? Each regulator we make for the
> different views will reflect the request made by Linux for a particular
> set of that RPM resource. So all consumers who are using the S3 active +
> sleep set regulator will aggregate into one request. Likewise, all the
> consumers for the S3 active set regulator will aggregate into another
> request. The only thing that isn't visible is the aggregation between
> the active only and active + sleep regulators, but that can be
> determined by doing a max() of the regulators. Even then, if we consider

My concern was the outcome of this snippet:

regulator_disable(ldo1_active);
regulator_enable(ldo1_both);
regulator_is_enable(ldo1_active);

But after reviewing it again, if one treat it in any other way then 'both' and
'active' being separate on a regulator driver level the end result will not be
sane.

So you're right, if we're to expose X number of regulators per resource they
have to be separate devices.

> that there are other masters also requesting voltages or enable/disable
> for these resources we quickly discover there are other things the
> regulator core doesn't have knowledge about, like what the actual
> voltage is or if the regulator is really off vs. Linux requesting for it
> to be off.
> 

As you say, we're quite a bit down the rabbit hole already by not even being
able to query the hardware for its current state.

> >
> > As the downstream driver shows, if we just implement the right pieces it should
> > work, i.e. give us the correct end result, but it will not be future proof nor
> > pretty. That's why I think we need to discuss how to solve it either in the
> > regulator driver or in the framework.
> >
> > And based on your feedback, it looks like we would have to do something about
> > this in the framework.
> 
> I don't see any problems with making multiple regulators for one RPM
> resource that represent the active set or active + sleep set. Everything
> could be handled in the RPM regulator driver by looking for the other
> regulators that are acting on the same RPM resource and aggregating.
> Maybe you can elaborate on what you think isn't future proof nor pretty
> about this design?
> 

If the regulators are considered completely separate, then the regulator
framework would not notice. I was, incorrectly, assuming that some state was
shared between them.

The "not pretty" part comes from the regulator driver (or a common parent
entity) needing to know what other regulators share a resource and thereby
aggregating the requests. An aggregation that does look a lot like the one
already done in the regulator core.

> As a thought experiment, what if there really was two physical
> independent controllable regulators and a pin from the CPU to the PMIC
> toggled a mux to select between the two. Such a pin would only be
> asserted when the CPU turned off. Would you still want to expose this as
> one regulator to the kernel given that only one supply goes to the CPU
> at any given time?
> 

I think we would have to expose this as two different regulators to the non-cpu
consumers, as that looks like the only way we can affect the "both state".

A possible way around that would be to have the individual regulators exposed
and then provide a regulator with both specified as supply. The regulator core
would aggregate "both" and individual consumer requests to the individual
regulators - without the need of the regulator devices needing to know about
each other.


Something like:
s3a: pm8921_s3_active {
	compatible = "pm8921-smps";
	...
};

s3s: pm8921_s3_sleep {
	compatible = "pm8921-smps";
	sleep; /* <- make requests affect the sleep state only */
};

pm8921_s3 {
	compatible = "dual-supply-regulator";

	active-supply = <&s3a>;
	sleep-supply = <&s3s>;
};

But maybe that's just too crazy...

Regards,
Bjorn

      reply	other threads:[~2014-11-26 22:49 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-10 22:52 [RFC 0/2] Qualcomm RPM sleep states Bjorn Andersson
2014-11-10 22:52 ` [RFC 1/2] mfd: qcom-rpm: Expose sleep state resources to clients Bjorn Andersson
2014-11-11 12:04   ` Lee Jones
2014-11-11 18:33     ` Bjorn Andersson
2014-11-12  9:52       ` Lee Jones
2014-11-12 14:45         ` Lina Iyer
2014-11-12 19:23           ` Bjorn Andersson
2014-11-19 18:06             ` Lina Iyer
2014-11-12 19:55         ` Bjorn Andersson
2014-11-10 22:52 ` [RFC 2/2] regulator: qcom-rpm: Implement RPM assisted disable Bjorn Andersson
2014-11-11  9:11   ` Andreas Färber
2014-11-11 18:34     ` Bjorn Andersson
2014-11-11 11:59   ` Lee Jones
2014-11-11 18:39     ` Bjorn Andersson
2014-11-11 14:21   ` Javier Martinez Canillas
2014-11-11 19:23     ` Bjorn Andersson
2014-11-21 23:10 ` [RFC 0/2] Qualcomm RPM sleep states Stephen Boyd
2014-11-21 23:27   ` Mark Brown
2014-11-21 23:43     ` Stephen Boyd
2014-11-21 23:54       ` Mark Brown
2014-11-22  0:03         ` Stephen Boyd
2014-11-22  0:16         ` Bjorn Andersson
2014-11-24 18:16       ` Mark Brown
2014-11-24 21:19         ` Stephen Boyd
2014-11-25 20:44           ` Mark Brown
2014-11-26  1:02             ` Stephen Boyd
2014-11-26 13:40               ` Mark Brown
2014-11-27  1:51                 ` Stephen Boyd
2014-11-27 18:56                   ` Mark Brown
2014-11-26 23:34             ` Bjorn Andersson
2014-11-27 19:02               ` Mark Brown
2014-11-27 19:42                 ` Bjorn Andersson
2014-11-28 20:16                   ` Mark Brown
2014-12-04 21:15                     ` Stephen Boyd
2014-12-08 18:06                       ` Bjorn Andersson
2014-12-08 19:39                         ` Mark Brown
2014-12-08 20:55                           ` Bjorn Andersson
2014-12-09 18:16                             ` Mark Brown
2014-12-09 19:25                               ` Bjorn Andersson
2014-12-09 20:28                                 ` Mark Brown
2014-12-11 22:36                                   ` Bjorn Andersson
2014-12-15 18:04                                     ` Mark Brown
2014-12-16  6:05                                       ` Bjorn Andersson
2014-12-26 17:09                                         ` Mark Brown
2014-12-29 21:54                                           ` Bjorn Andersson
2014-12-30 16:43                                             ` Mark Brown
2014-11-24 17:02   ` Bjorn Andersson
2014-11-24 21:19     ` Stephen Boyd
2014-11-24 21:59       ` Bjorn Andersson
2014-11-25  0:02         ` Stephen Boyd
2014-11-26 22:49           ` Bjorn Andersson [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141126224952.GC2872@sonymobile.com \
    --to=bjorn.andersson@sonymobile$(echo .)com \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox