linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sudeep Holla <sudeep.holla@arm.com>
To: Ranjani Vaidyanathan <ranjani.vaidyanathan@nxp.com>
Cc: Peng Fan <peng.fan@nxp.com>,
	"Peng Fan (OSS)" <peng.fan@oss.nxp.com>,
	"cristian.marussi@arm.com" <cristian.marussi@arm.com>,
	Sudeep Holla <sudeep.holla@arm.com>,
	"ulf.hansson@linaro.org" <ulf.hansson@linaro.org>,
	"arm-scmi@vger.kernel.org" <arm-scmi@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [EXT] Re: [PATCH] pmdomain: arm: scmi_pm_domain: Initialize state as off
Date: Tue, 14 Jan 2025 15:24:01 +0000	[thread overview]
Message-ID: <Z4aBkezSWOPCXcUh@bogus> (raw)
In-Reply-To: <PA4PR04MB9485E9C126E48A088D7E399B921F2@PA4PR04MB9485.eurprd04.prod.outlook.com>

Hi Ranjani,

On Mon, Jan 13, 2025 at 07:54:06PM +0000, Ranjani Vaidyanathan wrote:
> Hello Sudeep,
>
> Will try to explain the situation we are facing.
> 1. We have multiple agents running, Agent-A is booted up first before Linux
> is booted and powers up a shared power domain PD-X.
> 2. Linux boots and gets the power state of PD-X. And its already ON. And
> then PD -X is initialized with a default ON state.
> 3. When the driver that needs PD-X  is probed, Linux sees that the power
> domain status is ON and never makes an SCMI call to power up the PD-X for
> Linux Agent.
> 4. Agent-A now is shutdown/suspends. Linux will crash because the platform
> disables PD-X because it has no other requests for PD-X.
>

Thanks for the detailed explanation. I understand the issue now.

I would like to discuss if the below alternative approach works for you.
We can debate the pros and cons. I see with the approach in this patch
proposed by Peng we would avoid querying and setting genpd all together
during the genpd initialisation which is good. But if there are any genpd
left on by the platform or bootloader(same agent), it will not get turned
off when Linux tries to turn off the unused genpds(IIRC this could be the
reason for the current state of code). While your platform may find sending
those commands unnecessary, there was some usecase where SCMI platform kept
all resources ON by default for faster boot and expects OSPM to turn off
unused resources. So we need to support both the cases. I hope my below
patch should suffice.

Regards,
Sudeep

---->8

From dc755fa02d351e71c727da1c396e2d346b496096 Mon Sep 17 00:00:00 2001
From: Sudeep Holla <sudeep.holla@arm.com>
Date: Tue, 14 Jan 2025 15:08:44 +0000
Subject: [PATCH] pmdomain: arm: scmi_pm_domain: Send an explicit request to
 set the current state

On a system with multiple active SCMI agents, one agent(other than OSPM/
Linux or bootloader) would request to turn on a shared power domain
before the Linux boots/initialise the genpds. So when the Linux boots
and gets the power state as already ON, it just registers the genpd with
a default ON state.

However, when the driver that needs this shared power domain is probed
genpd sees that the power domain status is ON and never makes any SCMI
call to power it up which is correct. But, since Linux didn't make an
explicit request to turn on the shared power domain, the SCMI platform
firmware will not know if the OSPM agent is actively using it.

Suppose the other agent that requested the shared power domain to be
powered ON requests to power it OFF as it no longer needs it, the SCMI
platform firmware needs to turn it off if there are no active users of
it which in the above scenaro is the case.

As a result of SCMI platform firmware turning off the resource, OSPM/
Linux will crash the moment as it expects the shared power domain to be
powered ON.

Send an explicit request to set the current state when setting up the
genpd power domains. The other option is to not read the state and set
the genpds as default OFF, but it can't handle the scenario on certain
platforms where SCMI platform keeps all the power domains turned ON by
default for faster boot and expect the OSPM to turn off the unused
domains if power saving is required.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/pmdomain/arm/scmi_pm_domain.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/pmdomain/arm/scmi_pm_domain.c b/drivers/pmdomain/arm/scmi_pm_domain.c
index a7784a8bb5db..255c8c36a15c 100644
--- a/drivers/pmdomain/arm/scmi_pm_domain.c
+++ b/drivers/pmdomain/arm/scmi_pm_domain.c
@@ -96,6 +96,8 @@ static int scmi_pm_domain_probe(struct scmi_device *sdev)
 			continue;
 		}

+		power_ops->state_set(ph, i, state);
+
 		scmi_pd->domain = i;
 		scmi_pd->ph = ph;
 		scmi_pd->name = power_ops->name_get(ph, i);
--
2.34.1


  reply	other threads:[~2025-01-14 15:24 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-10  6:13 [PATCH] pmdomain: arm: scmi_pm_domain: Initialize state as off Peng Fan (OSS)
2025-01-13 10:31 ` Sudeep Holla
2025-01-13 11:37   ` Peng Fan
2025-01-13 13:49     ` Sudeep Holla
2025-01-13 15:30       ` [EXT] " Ranjani Vaidyanathan
2025-01-13 17:20         ` Sudeep Holla
2025-01-13 17:54           ` Cristian Marussi
2025-01-13 19:40             ` Ranjani Vaidyanathan
2025-01-13 19:54           ` Ranjani Vaidyanathan
2025-01-14 15:24             ` Sudeep Holla [this message]
2025-01-14 16:09               ` Ranjani Vaidyanathan
2025-01-14 18:16                 ` Sudeep Holla
2025-01-15  9:15                 ` Cristian Marussi
2025-01-15 18:42                   ` Ranjani Vaidyanathan
2025-01-16 16:18                     ` Cristian Marussi
2025-01-17  1:22                       ` Peng Fan
2025-01-17  5:13                     ` Dan Carpenter
2025-01-13 19:33       ` Cristian Marussi

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=Z4aBkezSWOPCXcUh@bogus \
    --to=sudeep.holla@arm.com \
    --cc=arm-scmi@vger.kernel.org \
    --cc=cristian.marussi@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=peng.fan@nxp.com \
    --cc=peng.fan@oss.nxp.com \
    --cc=ranjani.vaidyanathan@nxp.com \
    --cc=ulf.hansson@linaro.org \
    /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).