From: Sudeep Holla <sudeep.holla@arm.com>
To: Mark Brown <broonie@kernel.org>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>,
Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>,
"ulf.hansson@linaro.org" <ulf.hansson@linaro.org>,
"lgirdwood@gmail.com" <lgirdwood@gmail.com>,
"magnus.damm@gmail.com" <magnus.damm@gmail.com>,
"linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-renesas-soc@vger.kernel.org"
<linux-renesas-soc@vger.kernel.org>,
Sudeep Holla <sudeep.holla@arm.com>
Subject: Re: [PATCH/RFC v4 2/4] regulator: fixed: add regulator_ops members for suspend/resume
Date: Mon, 29 Jun 2020 18:42:46 +0100 [thread overview]
Message-ID: <20200629174220.GA4188@bogus> (raw)
In-Reply-To: <20200629172651.GG5499@sirena.org.uk>
On Mon, Jun 29, 2020 at 06:26:51PM +0100, Mark Brown wrote:
> On Mon, Jun 29, 2020 at 05:42:07PM +0100, Sudeep Holla wrote:
> > On Mon, Jun 29, 2020 at 05:14:50PM +0100, Mark Brown wrote:
> > > On Mon, Jun 29, 2020 at 04:07:28PM +0100, Sudeep Holla wrote:
> > >
> > > > The specification states clearly:
> > > > "... all devices in the system must be in a state that is compatible
> > > > with entry into the system state. These preconditions are beyond the scope
> > > > of this specification and are therefore not described here."
> > > > "Prior to the call, the OS must disable all sources of wakeup other than
> > > > those it needs to support for its implementation of suspend to RAM."
>
> > > This gets a bit circular for a generic OS since the OS needs some way to
> > > figure out what it's supposed to do on a given platform - for example
> > > the OS may be happy to use wakeup sources that the firmware is just
> > > going to cut power on.
>
> > While I understand the sentiments here, PSCI is targeted to address CPU
> > power state management mainly and system states like suspend/reset and
> > poweroff which involves last CPU. This is one of the reason it is out of
> > the scope of the specification.
>
> Sure, but as soon as we start talking about the last CPU stuff we're
> inevitably talking about the system as a whole.
>
> > Here is my understanding. DT specifies all the wakeup sources. Linux
> > can configure those and user may choose to enable a subset of them is
> > wakeup. As stated in the spec and also based on what we already do in
> > the kernel, we disable all other wakeup sources.
>
> > The PSCI firmware can then either read those from the interrupt controller
> > or based on static platform understanding, must not disable those wakeup
> > sources.
>
> That bit about static platform understanding isn't super helpful for the
> OS, so long as the firmware might do that the OS is pretty much out of
> luck.
>
I don't disagree. That's one of the reason I wanted to gather requirement
few years back when PSCI system suspend was introduced. I couldn't
convince PSCI spec authors myself.
> > > > I see nothing has been fixed in the firmware too and we are still
> > > > discussing the same after 3 years 😄. Clearly we should start trusting
> > > > firmware and built capability to fix and replace it if there are bugs
> > > > just like kernel and stop hacking around in the kernel to deal with
> > > > just broken platform/psci firmware.
>
> > > This isn't just an issue of buggy firmware as far as I can see, it's
> > > also a lack of ability for the OS and firmware to communicate
> > > information about their intentions to each other. As things stand you'd
> > > need to put static information in the DT.
>
> > It is easy for DT to get out of sync with firmware unless it is generated
> > by the same firmware. That's the reason why I am against such multiple
>
> The ability for things to get out of sync also concerns me as I said
> further back in the thread but I'm not sure we have much alternative,
> realistically we're going to need some facility to work around firmware
> that isn't ideal.
>
I understand and I agree. That's the main reason I want to understand this
better and provide a generic solution. The current pm_suspend_via_firmware
seem to have different intentions(at-least that's what I grasped quickly
reading through the document)
> > sources of information. I understand ACPI has more flexibility and I did
>
> ACPI has a much stronger idea of what the system looks like which helps
> it a lot here.
>
Yes, I wanted something similar initially but didn't get good response
both from community and PSCI spec authors.
> > Each device or platform having its specific property in DT is not scalable.
> > Not sure if it is a generic problem. If it is, I would like to understand
> > more details on such lack of ability for communtication between OS and
> > firmware.
>
> It seems like a generic issue from where I'm sitting.
I can't disagree with that. The description of the issue and the solution
in the thread is. We need make it more generic and provide generic solution,.
I already see 2-3 threads addressing this in isolation as specific issue.
Also we need to check with DT maintainers if there are fine with the idea
of binding to address this issue.
--
Regards,
Sudeep
next prev parent reply other threads:[~2020-06-29 19:00 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-26 9:32 [PATCH/RFC v4 0/4] treewide: add regulator condition on _mmc_suspend() Yoshihiro Shimoda
2020-06-26 9:32 ` [PATCH/RFC v4 1/4] regulator: core: add prepare and resume_early Yoshihiro Shimoda
2020-06-26 14:30 ` Mark Brown
2020-06-29 2:12 ` Yoshihiro Shimoda
2020-06-26 9:32 ` [PATCH/RFC v4 2/4] regulator: fixed: add regulator_ops members for suspend/resume Yoshihiro Shimoda
2020-06-26 14:39 ` Mark Brown
2020-06-29 2:42 ` Yoshihiro Shimoda
2020-06-29 12:57 ` Mark Brown
2020-06-29 13:40 ` Sudeep Holla
2020-06-29 14:15 ` Geert Uytterhoeven
2020-06-29 15:07 ` Sudeep Holla
2020-06-29 16:14 ` Mark Brown
2020-06-29 16:42 ` Sudeep Holla
2020-06-29 17:26 ` Mark Brown
2020-06-29 17:42 ` Sudeep Holla [this message]
2020-06-30 8:29 ` Yoshihiro Shimoda
2020-06-26 9:32 ` [PATCH/RFC v4 3/4] mmc: core: Call mmc_poweroff_nofity() if regulators are disabled Yoshihiro Shimoda
2020-06-26 15:13 ` Mark Brown
2020-06-29 2:49 ` Yoshihiro Shimoda
2020-06-26 15:53 ` Sergei Shtylyov
2020-06-29 5:16 ` Yoshihiro Shimoda
2020-06-26 9:32 ` [PATCH/RFC v4 4/4] arm64: dts: renesas: add regulator-off-in-suspend property for eMMC Yoshihiro Shimoda
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=20200629174220.GA4188@bogus \
--to=sudeep.holla@arm.com \
--cc=broonie@kernel.org \
--cc=geert@linux-m68k.org \
--cc=lgirdwood@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mmc@vger.kernel.org \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=magnus.damm@gmail.com \
--cc=ulf.hansson@linaro.org \
--cc=yoshihiro.shimoda.uh@renesas.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).