All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Praveen Talari" <praveen.talari@oss.qualcomm.com>
Cc: linux-serial@vger.kernel.org
Subject: Re: [PATCH v2 1/9] pmdomain: arm: Fix -EINVAL from scmi_pd_set_perf_state() on state 0
Date: Thu, 27 Aug 2026 18:16:34 +0000	[thread overview]
Message-ID: <20260827181634.891381F00A3D@smtp.kernel.org> (raw)
In-Reply-To: <20260827-derive_clk_perf_tbl_from_perf_domain_opp_table-v2-1-091697dbeb02@oss.qualcomm.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Returning success for performance state 0 without notifying the SCMI hardware causes a power leak and genpd state desynchronization.
--

commit 2b6c1834f31c5a0d438704d927e8d116c0dd29ad
Author: Praveen Talari <praveen.talari@oss.qualcomm.com>

pmdomain: arm: Fix -EINVAL from scmi_pd_set_perf_state() on state 0

The commit modifies scmi_pd_set_perf_state() to return 0 (success) instead
of -EINVAL when the requested performance state is 0. This change aims to
accommodate the OPP framework which uses state 0 when a device is runtime
suspended.

> diff --git a/drivers/pmdomain/arm/scmi_perf_domain.c b/drivers/pmdomain/arm/scmi_perf_domain.c
> index 3693423459c9c..e390f902a4442 100644
> --- a/drivers/pmdomain/arm/scmi_perf_domain.c
> +++ b/drivers/pmdomain/arm/scmi_perf_domain.c
> @@ -33,7 +33,7 @@ scmi_pd_set_perf_state(struct generic_pm_domain *genpd, unsigned int state)
>  		return 0;
>  
>  	if (!state)
> -		return -EINVAL;
> +		return 0;

[Severity: Medium]
Will this return 0 without notifying the SCMI hardware of the state change?

Since SCMI performance domains are marked with GENPD_FLAG_ALWAYS_ON, they are
never actually powered off. When all devices in the domain suspend and genpd
aggregates the state to 0, it invokes scmi_pd_set_perf_state() with state 0.

If we immediately return 0 here without calling pd->perf_ops->level_set(),
no request is sent to the firmware.

Does this cause the hardware to get stuck at its last requested
high-performance level instead of scaling down, leading to a silent power
leak and a desynchronization between genpd software state and the hardware?

>  
>  	ret = pd->perf_ops->level_set(pd->ph, pd->domain_id, state, false);
>  	if (ret)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827-derive_clk_perf_tbl_from_perf_domain_opp_table-v2-0-091697dbeb02@oss.qualcomm.com?part=1

  reply	other threads:[~2026-08-27 18:16 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-27 17:59 [PATCH v2 0/9] soc: qcom: geni: Derive SE clock configuration from OPP table on SA8255P Praveen Talari
2026-08-27 17:59 ` [PATCH v2 1/9] pmdomain: arm: Fix -EINVAL from scmi_pd_set_perf_state() on state 0 Praveen Talari
2026-08-27 18:16   ` sashiko-bot [this message]
2026-09-01 14:31   ` Ulf Hansson
2026-08-27 17:59 ` [PATCH v2 2/9] soc: qcom: geni-se: Populate clk_perf_tbl with SE source clock frequencies from perf OPP table Praveen Talari
2026-08-27 18:16   ` sashiko-bot
2026-10-07  7:24   ` Mukesh Savaliya
2026-08-27 17:59 ` [PATCH v2 3/9] soc: qcom: geni-se: Add helper to set SE clock rate via OPP Praveen Talari
2026-08-27 18:14   ` sashiko-bot
2026-09-04  9:01   ` Konrad Dybcio
2026-08-27 17:59 ` [PATCH v2 4/9] soc: qcom: geni-se: Remove OPP rate reset from resource deactivation Praveen Talari
2026-08-27 18:14   ` sashiko-bot
2026-09-04  8:49   ` Konrad Dybcio
2026-09-11 17:13     ` Praveen Talari
2026-10-07  8:51   ` Mukesh Savaliya
2026-08-27 17:59 ` [PATCH v2 5/9] serial: qcom-geni: Use geni_se_set_rate() for source clock configuration Praveen Talari
2026-08-27 18:12   ` sashiko-bot
2026-10-08  7:35   ` Mukesh Savaliya
2026-08-27 17:59 ` [PATCH v2 6/9] spi: qcom-geni: Use geni_se_set_rate() for setting source clock frequency Praveen Talari
2026-08-27 18:12   ` sashiko-bot
2026-10-08 19:45   ` Mukesh Savaliya
2026-08-27 17:59 ` [PATCH v2 7/9] i2c: qcom-geni: Vote for SE clock rate using OPP Praveen Talari
2026-08-27 18:11   ` sashiko-bot
2026-09-04  8:52   ` Konrad Dybcio
2026-09-11 17:42     ` Praveen Talari
2026-08-27 17:59 ` [PATCH v2 8/9] i2c: qcom-geni: Use common GENI resource initialization helper Praveen Talari
2026-08-27 18:16   ` sashiko-bot
2026-08-27 17:59 ` [PATCH v2 9/9] i2c: qcom-geni: Use geni_se_set_rate() for setting source clock frequency Praveen Talari
2026-08-27 18:25   ` sashiko-bot
2026-09-01 14:52 ` [PATCH v2 0/9] soc: qcom: geni: Derive SE clock configuration from OPP table on SA8255P Ulf Hansson
2026-09-01 16:48   ` Praveen Talari
2026-09-04  7:56     ` Ulf Hansson
2026-09-04  9:00       ` Konrad Dybcio
2026-09-11 17:33         ` Praveen Talari
2026-09-22 13:06           ` Konrad Dybcio
2026-09-28  8:59             ` Praveen Talari
2026-09-09  6:32       ` Praveen Talari

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=20260827181634.891381F00A3D@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=praveen.talari@oss.qualcomm.com \
    --cc=sashiko-reviews@lists.linux.dev \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.