All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Sneh Mankad" <sneh.mankad@oss.qualcomm.com>
Cc: devicetree@vger.kernel.org, robh@kernel.org, conor+dt@kernel.org
Subject: Re: [PATCH 5/7] arm64: dts: qcom: sm6375: Make MPM device as part of CPU cluster domain
Date: Mon, 13 Jul 2026 10:40:39 +0000	[thread overview]
Message-ID: <20260713104039.BB3AD1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260713-b4-shikra_lpm_addition-v1-5-3d858df2cbbf@oss.qualcomm.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Severe integer overflow in `mpm_write_next_wakeup` when no hrtimers are pending causes immediate spurious wakeups.

Pre-existing issues:
- [High] The MPM power domain probe dependency and DT validation bug fixed in sm6375.dtsi remains in agatti.dtsi and shikra.dtsi, permanently breaking CPU idle states.
--

--- Patch [5]: [PATCH 5/7] arm64: dts: qcom: sm6375: Make MPM device as part of CPU cluster domain ---
commit b04aa9c0b185843b6783d946292d656585dedf48
Author: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
arm64: dts: qcom: sm6375: Make MPM device as part of CPU cluster domain

This commit removes the power domain provider property from the MPM device
in the sm6375 device tree to prevent idle-state initialization failures.
It then assigns the MPM device to the CPU cluster power domain.

> diff --git a/arch/arm64/boot/dts/qcom/sm6375.dtsi b/arch/arm64/boot/dts/qcom/sm6375.dtsi
> index ccf572bb1549b..5f729634e1d9b 100644
> --- a/arch/arm64/boot/dts/qcom/sm6375.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm6375.dtsi
> @@ -318,7 +318,7 @@ mpm: interrupt-controller {
>  		mboxes = <&ipcc IPCC_CLIENT_AOP IPCC_MPROC_SIGNAL_SMP2P>;
>  		interrupt-controller;
>  		#interrupt-cells = <2>;
> -		#power-domain-cells = <0>;
> +		power-domains = <&cluster_pd>;

[Severity: High]
This is a pre-existing issue, but does the same problem still exist in other
device trees like agatti.dtsi and shikra.dtsi?

Because the qcom_mpm driver no longer registers a power domain, the cluster_pd
node in agatti.dtsi incorrectly relies on &mpm as a power domain provider:

arch/arm64/boot/dts/qcom/agatti.dtsi:cluster_pd {
    ...
	power-domains = <&mpm>;
    ...
}

Will this cause cluster_pd to perpetually fail to probe with -EPROBE_DEFER
and permanently break the CPU power-collapse idle state?

Also, both agatti.dtsi and shikra.dtsi incorrectly retain the deprecated
#power-domain-cells = <0>; property in their MPM nodes:

arch/arm64/boot/dts/qcom/agatti.dtsi:mpm {
    ...
	#power-domain-cells = <0>;
    ...
}

>  		interrupt-parent = <&intc>;
>  		qcom,mpm-pin-count = <96>;
>  		qcom,mpm-pin-map = <5 296>,  /* Soundwire wake_irq */
> @@ -505,7 +505,6 @@ cpu_pd7: power-domain-cpu7 {
>  
>  		cluster_pd: power-domain-cpu-cluster0 {
>  			#power-domain-cells = <0>;
> -			power-domains = <&mpm>;
>  			domain-idle-states = <&cluster_sleep_0>;
>  		};
>  	};

[Severity: High]
Does this change trigger an integer overflow in mpm_write_next_wakeup() when
no hrtimers are pending?

If dev_pm_genpd_get_next_hrtimer() returns KTIME_MAX, it is processed without
bounds checking:

drivers/irqchip/irq-qcom-mpm.c:mpm_write_next_wakeup() {
    ...
	wakeup = dev_pm_genpd_get_next_hrtimer(priv->dev);

	/* Find the relative wakeup in kernel time scale */
	now = ktime_get();
	wakeup = ktime_sub(wakeup, now);
	wakeup_us = ktime_to_us(wakeup);

	/* Convert the wakeup to arch timer scale */
	wakeup_cycles = USECS_TO_CYCLES(wakeup_us);
    ...
}

This evaluates to a massive value which causes xloops_to_cycles() to overflow
the 64-bit integer multiple times over:

drivers/irqchip/irq-qcom-mpm.c:xloops_to_cycles() {
    ...
	return (xloops * loops_per_jiffy * HZ) >> 32;
    ...
}

Could this result in the hardware being programmed with a pseudo-random timer
value, triggering an immediate or severely premature wakeup that destroys deep
sleep power savings?

Should mpm_write_next_wakeup() explicitly check for wakeup == KTIME_MAX to
avoid this overflow?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713-b4-shikra_lpm_addition-v1-0-3d858df2cbbf@oss.qualcomm.com?part=5

  reply	other threads:[~2026-07-13 10:40 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-13 10:25 [PATCH 0/7] Register MPM under CPU cluster power domain to manage RPM notification Sneh Mankad
2026-07-13 10:25 ` [PATCH 1/7] dt-bindings: interrupt-controller: mpm: Document power-domains property Sneh Mankad
2026-07-13 10:36   ` sashiko-bot
2026-07-13 11:26   ` Konrad Dybcio
2026-07-13 15:11   ` Marc Zyngier
2026-07-13 10:25 ` [PATCH 2/7] irqchip/irq-qcom-mpm: Register MPM under CPU cluster power domain Sneh Mankad
2026-07-13 10:41   ` sashiko-bot
2026-07-13 10:25 ` [PATCH 3/7] irqchip/irq-qcom-mpm: Prepare common access path for timer and pin regs Sneh Mankad
2026-07-13 10:39   ` sashiko-bot
2026-07-13 10:25 ` [PATCH 4/7] irqchip/irq-qcom-mpm: Program wakeup timer when CPU cluster goes to LPM Sneh Mankad
2026-07-13 10:38   ` sashiko-bot
2026-07-13 15:18   ` Marc Zyngier
2026-07-13 10:25 ` [PATCH 5/7] arm64: dts: qcom: sm6375: Make MPM device as part of CPU cluster domain Sneh Mankad
2026-07-13 10:40   ` sashiko-bot [this message]
2026-07-13 10:25 ` [PATCH 6/7] arm64: dts: qcom: agatti: Do not mark MPM as power domain Sneh Mankad
2026-07-13 10:39   ` sashiko-bot
2026-07-13 10:25 ` [PATCH 7/7] arm64: dts: qcom: shikra: Add CPU idle states Sneh Mankad

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=20260713104039.BB3AD1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=sneh.mankad@oss.qualcomm.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 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.