Re: [PATCH/RFC v4 2/4] regulator: fixed: add regulator_ops members for suspend/resume

From: Mark Brown <broonie@kernel.org>
To: Sudeep Holla <sudeep.holla@arm.com>
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>
Subject: Re: [PATCH/RFC v4 2/4] regulator: fixed: add regulator_ops members for suspend/resume
Date: Mon, 29 Jun 2020 17:14:50 +0100	[thread overview]
Message-ID: <20200629161450.GE5499@sirena.org.uk> (raw)
In-Reply-To: <20200629150728.GA27911@bogus>

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

On Mon, Jun 29, 2020 at 04:07:28PM +0100, Sudeep Holla wrote:
> On Mon, Jun 29, 2020 at 04:15:39PM +0200, Geert Uytterhoeven wrote:

> > This is all about how to know what exactly PSCI is powering down during
> > SYSTEM_SUSPEND.  In this specific case, it is about knowing if the eMMC
> > is powered down or not, as Linux should follow a specific procedure to
> > prepare the eMMC for that, and Linux should not if that isn't the case.

> OK, unless you are optimising, you shouldn't care then what PSCI does.
> If you don't need eMMC, just suspend/power it off before you enter system/
> psci suspend.

That only works if the power off procedure doesn't require that power be
removed as part of the procedure.  There's a reasonable argument that
specs that have such requirements are unhelpful but that doesn't mean
that nobody will make hardware with such requrements which creates
problems for generic code that needs to control that hardware if it
can't discover the power state over suspend.

> > I had a quick look at the latest revision of the PSCI specification, and
> > it doesn't look like anything has changed in that area since my old patch
> > series from 2017.  So it still boils down to: we don't know what a
> > specific PSCI implementation will do, as basically anything is
> > compliant, so the only safe thing is to assume the worst.

> 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.

> 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.

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