[PATCH 0/7] Register MPM under CPU cluster power domain to manage RPM notification

[PATCH 0/7] Register MPM under CPU cluster power domain to manage RPM notification

[Sneh Mankad]

MPM irqchip needs to notify RPM (Resource Power Manager) processor to read
the latest wake up capable interrupts when the CPU cluster is entering the
deepest idle state. This is done by sending IPC interrupt to RPM and is
implemented as .power_off() callback by registering MPM as parent power
domain to CPU cluster.

Such implementation introduces a hard probe dependency between MPM irqchip
and CPU cluster power domains. That is MPM irqchip needs to finish probe
before PSCI power domains are probed. MPM irqchip can be build as module
and can get later inserted where as PSCI power domains is not a module.

For in-built driver cases too PSCI domain gets probed first and later MPM
irqchip leading to failure of CPUidle states.

Detailed flow of the non-working scenario:

psci-cpuidle-domain.c probe
--> dt_idle_pd_init_topology()
    --> of_genpd_add_subdomain()
        --> genpd_get_from_provider()
            --> fails to find parent MPM genPD provider
                --> returns -EPROBE_DEFER.

irq-qcom-mpm.c probe
--> of_genpd_add_provider_simple()
    --> genpd_add_provider()
        --> MPM added as a genPD provider.

Now when psci_cpuidle_probe() is called to probe the CPU idle states, it
tries to map the states to the mentioned power-domains.

But since power domains probe has been deferred, psci_cpuidle_probe() too
will return -EPROBE_DEFER.

commit af5376a77e87 ("cpuidle: psci: Transition to the faux device
interface") transitioned cpuidle-psci to a faux device interface.

faux_device_create() calls faux_device_create_with_groups(), which ignores
the probe return value, and destroys the device if dev->driver is not set.

This will lead to psci_cpuidle_probe() not being called again, resulting in
all idle-state devices failing to init in SoCs setting MPM as a parent
power domain to CPU cluster.

cpuidle-psci.c init
--> faux_device_create()
        ...
        --> psci_cpuidle_probe()
            --> psci_idle_init_cpu()
                ...
                --> psci_dt_cpu_init_topology()
                ...
                -> dev_pm_domain_attach_by_name()
                   --> __genpd_dev_pm_attach()
                       --> genpd_get_from_provider()
                           --> fails to find CPU genPD provider
                               --> returns -EPROBE_DEFER
                                   --> return value ignored and device
                                       destroyed
psci-cpuidle-domain.c probe
--> dt_idle_pd_init_topology()
    --> of_genpd_add_subdomain()
        --> genpd_get_from_provider()
            --> finds MPM power domain
                --> power-domains topology init successful

Below are the logs from shikra SoC:

[    1.035164] CPUidle PSCI: failed to create CPU PM domains ret=-517

[    3.651715] PM: Added domain provider from
               /remoteproc/interrupt-controller

[    4.129563] CPUidle PSCI: CPU 0 failed to PSCI idle
[    4.149294] CPUidle PSCI: Failed to create psci-cpuidle device

[    4.743389] CPUidle PSCI: Initialized CPU PM domain topology using OSI
               mode

Currently only 2 SoCs follow this method - Agatti and sm6375. Agatti has
CPU cluster power domain disabled, which is why idle-states are allowed to
function there.

Move the RPM notification handling to the GENPD_NOTIFY_PRE_OFF callback and
register MPM under the CPU cluster power domain.  Use runtime PM to report
the default RPM_SUSPENDED state to genPD so that the CPU cluster power
domain can enter low power mode.

This will remove the dependency on probe ordering and allow individual CPU
idle states, CPU cluster idle states and RPM notification to function
properly.

Also enable CPU and CPU cluster LPMs for Shikra.

Signed-off-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
---
Sneh Mankad (7):
      dt-bindings: interrupt-controller: mpm: Document power-domains property
      irqchip/irq-qcom-mpm: Register MPM under CPU cluster power domain
      irqchip/irq-qcom-mpm: Prepare common access path for timer and pin regs
      irqchip/irq-qcom-mpm: Program wakeup timer when CPU cluster goes to LPM
      arm64: dts: qcom: sm6375: Make MPM device as part of CPU cluster domain
      arm64: dts: qcom: agatti: Do not mark MPM as power domain
      arm64: dts: qcom: shikra: Add CPU idle states

 .../bindings/interrupt-controller/qcom,mpm.yaml    |   6 +-
 arch/arm64/boot/dts/qcom/agatti.dtsi               |   2 -
 arch/arm64/boot/dts/qcom/shikra.dtsi               |  94 ++++++++++-
 arch/arm64/boot/dts/qcom/sm6375.dtsi               |   3 +-
 drivers/irqchip/irq-qcom-mpm.c                     | 185 ++++++++++++++++-----
 5 files changed, 243 insertions(+), 47 deletions(-)
---
base-commit: 7777cc195ca1301a28008ca5cdb98bdb2a9d0def
change-id: 20260617-b4-shikra_lpm_addition-4a9bc82bafee

Best regards,
-- 
Sneh Mankad <sneh.mankad@oss.qualcomm.com>

[PATCH 1/7] dt-bindings: interrupt-controller: mpm: Document power-domains property

[Sneh Mankad]

Remove #power-domain-cells property and add power-domains property for
MPM device.

Signed-off-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
---
 .../devicetree/bindings/interrupt-controller/qcom,mpm.yaml          | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/qcom,mpm.yaml b/Documentation/devicetree/bindings/interrupt-controller/qcom,mpm.yaml
index ebb40c48950ab3a8fc86f5708acfc33c33d68993..3f9645fbc3c20633077aaa589e5d5a43928dab51 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/qcom,mpm.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/qcom,mpm.yaml
@@ -68,8 +68,8 @@ properties:
         - description: MPM pin number
         - description: GIC SPI number for the MPM pin
 
-  '#power-domain-cells':
-    const: 0
+  power-domains:
+    maxItems: 1
 
 required:
   - compatible
@@ -113,6 +113,6 @@ examples:
                                <24 79>,
                                <86 183>,
                                <91 260>;
-            #power-domain-cells = <0>;
+            power-domains = <&cluster_pd>;
         };
     };

-- 
2.34.1

[PATCH 2/7] irqchip/irq-qcom-mpm: Register MPM under CPU cluster power domain

[Sneh Mankad]

MPM irqchip needs to notify RPM (Resource Power Manager) processor to read
the latest wake up capable interrupts when the CPU cluster is entering the
deepest idle state. This is done by sending IPC interrupt to RPM and is
implemented as .power_off() callback by registering MPM as parent power
domain to CPU cluster.

Such implementation introduces a hard probe dependency between MPM irqchip
and CPU cluster power domains. That is MPM irqchip needs to finish probe
before PSCI power domains are probed. MPM irqchip can be build as module
and can get later inserted where as PSCI power domains is not a module.

For in-built driver cases too PSCI domain gets probed first and later MPM
irqchip leading to failure of CPUidle states.

Detailed flow of the non-working scenario:

psci-cpuidle-domain.c probe
--> dt_idle_pd_init_topology()
    --> of_genpd_add_subdomain()
        --> genpd_get_from_provider()
            --> fails to find parent MPM genPD provider
                --> returns -EPROBE_DEFER.

irq-qcom-mpm.c probe
--> of_genpd_add_provider_simple()
    --> genpd_add_provider()
        --> MPM added as a genPD provider.

Now when psci_cpuidle_probe() is called to probe the CPU idle states, it
tries to map the states to the mentioned power-domains.

But since power domains probe has been deferred, psci_cpuidle_probe() too
will return -EPROBE_DEFER.

commit af5376a77e87 ("cpuidle: psci: Transition to the faux device
interface") transitioned cpuidle-psci to a faux device interface.

faux_device_create() calls faux_device_create_with_groups(), which ignores
the probe return value, and destroys the device if dev->driver is not set.

This will lead to psci_cpuidle_probe() not being called again, resulting in
all idle-state devices failing to init in SoCs setting MPM as a parent
power domain to CPU cluster.

cpuidle-psci.c init
--> faux_device_create()
        ...
        --> psci_cpuidle_probe()
            --> psci_idle_init_cpu()
                ...
                --> psci_dt_cpu_init_topology()
                ...
                -> dev_pm_domain_attach_by_name()
                   --> __genpd_dev_pm_attach()
                       --> genpd_get_from_provider()
                           --> fails to find CPU genPD provider
                               --> returns -EPROBE_DEFER
                                   --> return value ignored and device
                                       destroyed

Move the RPM notification handling to the GENPD_NOTIFY_PRE_OFF callback and
register MPM under the CPU cluster power domain.  Use runtime PM to report
the default RPM_SUSPENDED state to genPD so that the CPU cluster power
domain can enter low power mode.

If MPM has not registered with CPU cluster power domain, utilize the CPU PM
notifications to manage RPM communication when the last CPU goes to power
collapse.

Fixes: a6199bb514d8 ("irqchip: Add Qualcomm MPM controller driver")
Signed-off-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
---
 drivers/irqchip/irq-qcom-mpm.c | 97 ++++++++++++++++++++++++++++++------------
 1 file changed, 69 insertions(+), 28 deletions(-)

diff --git a/drivers/irqchip/irq-qcom-mpm.c b/drivers/irqchip/irq-qcom-mpm.c
index 181320528a47ac1bde6cf7d2d0a9f79499990092..01fd1843172aa22760d359281e39f0d9394bab6d 100644
--- a/drivers/irqchip/irq-qcom-mpm.c
+++ b/drivers/irqchip/irq-qcom-mpm.c
@@ -4,6 +4,8 @@
  * Copyright (c) 2010-2020, The Linux Foundation. All rights reserved.
  */
 
+#include <linux/atomic.h>
+#include <linux/cpu_pm.h>
 #include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/init.h>
@@ -18,6 +20,7 @@
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/pm_domain.h>
+#include <linux/pm_runtime.h>
 #include <linux/slab.h>
 #include <linux/soc/qcom/irq.h>
 #include <linux/spinlock.h>
@@ -84,7 +87,9 @@ struct qcom_mpm_priv {
 	unsigned int map_cnt;
 	unsigned int reg_stride;
 	struct irq_domain *domain;
-	struct generic_pm_domain genpd;
+	struct notifier_block genpd_nb;
+	struct notifier_block mpm_pm;
+	atomic_t cpus_in_pm;
 };
 
 static u32 qcom_mpm_read(struct qcom_mpm_priv *priv, unsigned int reg,
@@ -292,10 +297,8 @@ static irqreturn_t qcom_mpm_handler(int irq, void *dev_id)
 	return ret;
 }
 
-static int mpm_pd_power_off(struct generic_pm_domain *genpd)
+static int handle_rpm_notification(struct qcom_mpm_priv *priv)
 {
-	struct qcom_mpm_priv *priv = container_of(genpd, struct qcom_mpm_priv,
-						  genpd);
 	int i, ret;
 
 	for (i = 0; i < priv->reg_stride; i++)
@@ -307,10 +310,59 @@ static int mpm_pd_power_off(struct generic_pm_domain *genpd)
 		return ret;
 
 	mbox_client_txdone(priv->mbox_chan, 0);
-
 	return 0;
 }
 
+static int mpm_pd_power_cb(struct notifier_block *nb, unsigned long action, void *d)
+{
+	struct qcom_mpm_priv *priv = container_of(nb, struct qcom_mpm_priv,
+						  genpd_nb);
+
+	switch (action) {
+	case GENPD_NOTIFY_PRE_OFF:
+		if (handle_rpm_notification(priv))
+			return NOTIFY_BAD;
+	}
+
+	return NOTIFY_OK;
+}
+
+static int mpm_cpu_pm_callback(struct notifier_block *nfb,
+			       unsigned long action, void *v)
+{
+	struct qcom_mpm_priv *priv = container_of(nfb, struct qcom_mpm_priv, mpm_pm);
+	int cpus_in_pm;
+
+	switch (action) {
+	case CPU_PM_ENTER:
+		cpus_in_pm = atomic_inc_return(&priv->cpus_in_pm);
+		/*
+		 * NOTE: comments for num_online_cpus() point out that it's
+		 * only a snapshot so we need to be careful. It should be OK
+		 * for us to use, though.  It's important for us not to miss
+		 * if we're the last CPU going down so it would only be a
+		 * problem if a CPU went offline right after we did the check
+		 * AND that CPU was not idle AND that CPU was the last non-idle
+		 * CPU. That can't happen. CPUs would have to come out of idle
+		 * before the CPU could go offline.
+		 */
+		if (cpus_in_pm < num_online_cpus())
+			return NOTIFY_OK;
+		break;
+	case CPU_PM_ENTER_FAILED:
+	case CPU_PM_EXIT:
+		atomic_dec(&priv->cpus_in_pm);
+		return NOTIFY_OK;
+	default:
+		return NOTIFY_DONE;
+	}
+
+	if (handle_rpm_notification(priv))
+		return NOTIFY_BAD;
+
+	return NOTIFY_OK;
+}
+
 static bool gic_hwirq_is_mapped(struct mpm_gic_map *maps, int cnt, u32 hwirq)
 {
 	int i;
@@ -327,7 +379,6 @@ static int qcom_mpm_probe(struct platform_device *pdev, struct device_node *pare
 	struct device_node *np = pdev->dev.of_node;
 	struct device *dev = &pdev->dev;
 	struct irq_domain *parent_domain;
-	struct generic_pm_domain *genpd;
 	struct device_node *msgram_np;
 	struct qcom_mpm_priv *priv;
 	unsigned int pin_cnt;
@@ -415,26 +466,6 @@ static int qcom_mpm_probe(struct platform_device *pdev, struct device_node *pare
 	if (irq < 0)
 		return irq;
 
-	genpd = &priv->genpd;
-	genpd->flags = GENPD_FLAG_IRQ_SAFE;
-	genpd->power_off = mpm_pd_power_off;
-
-	genpd->name = devm_kasprintf(dev, GFP_KERNEL, "%s", dev_name(dev));
-	if (!genpd->name)
-		return -ENOMEM;
-
-	ret = pm_genpd_init(genpd, NULL, false);
-	if (ret) {
-		dev_err(dev, "failed to init genpd: %d\n", ret);
-		return ret;
-	}
-
-	ret = of_genpd_add_provider_simple(np, genpd);
-	if (ret) {
-		dev_err(dev, "failed to add genpd provider: %d\n", ret);
-		goto remove_genpd;
-	}
-
 	priv->mbox_client.dev = dev;
 	priv->mbox_client.knows_txdone = true;
 	priv->mbox_chan = mbox_request_channel(&priv->mbox_client, 0);
@@ -469,14 +500,24 @@ static int qcom_mpm_probe(struct platform_device *pdev, struct device_node *pare
 		goto remove_domain;
 	}
 
+	if (of_find_property(np, "power-domains", NULL)) {
+		devm_pm_runtime_enable(dev);
+		priv->genpd_nb.notifier_call = mpm_pd_power_cb;
+		ret = dev_pm_genpd_add_notifier(dev, &priv->genpd_nb);
+	} else {
+		priv->mpm_pm.notifier_call = mpm_cpu_pm_callback;
+		ret = cpu_pm_register_notifier(&priv->mpm_pm);
+	}
+
+	if (ret)
+		goto remove_domain;
+
 	return 0;
 
 remove_domain:
 	irq_domain_remove(priv->domain);
 free_mbox:
 	mbox_free_channel(priv->mbox_chan);
-remove_genpd:
-	pm_genpd_remove(genpd);
 	return ret;
 }
 

-- 
2.34.1

[PATCH 3/7] irqchip/irq-qcom-mpm: Prepare common access path for timer and pin regs

[Sneh Mankad]

The vMPM layout starts with two timer registers followed by pin register
banks (ENABLE/FALLING/RISING/POLARITY/STATUS), each with reg_stride
number of entries.

Use qcom_mpm_offset() as the common addressing helper for both timer and
pin register accesses based on that layout.

vMPM has MPM_REG_* values represented as contiguous register IDs,
hence replace the macros with enum qcom_mpm_reg and modify the accessor
helpers accordingly.

Signed-off-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
---
 drivers/irqchip/irq-qcom-mpm.c | 48 ++++++++++++++++++++++++++++++------------
 1 file changed, 35 insertions(+), 13 deletions(-)

diff --git a/drivers/irqchip/irq-qcom-mpm.c b/drivers/irqchip/irq-qcom-mpm.c
index 01fd1843172aa22760d359281e39f0d9394bab6d..763eddee99dc4cdd5edab22ce54808528f9ef165 100644
--- a/drivers/irqchip/irq-qcom-mpm.c
+++ b/drivers/irqchip/irq-qcom-mpm.c
@@ -66,11 +66,16 @@
  *
  */
 
-#define MPM_REG_ENABLE		0
-#define MPM_REG_FALLING_EDGE	1
-#define MPM_REG_RISING_EDGE	2
-#define MPM_REG_POLARITY	3
-#define MPM_REG_STATUS		4
+#define MPM_TIMER_REGS	2
+
+enum qcom_mpm_reg {
+	MPM_REG_TIMER = 0,
+	MPM_REG_ENABLE,
+	MPM_REG_FALLING_EDGE,
+	MPM_REG_RISING_EDGE,
+	MPM_REG_POLARITY,
+	MPM_REG_STATUS,
+};
 
 /* MPM pin map to GIC hwirq */
 struct mpm_gic_map {
@@ -92,18 +97,36 @@ struct qcom_mpm_priv {
 	atomic_t cpus_in_pm;
 };
 
-static u32 qcom_mpm_read(struct qcom_mpm_priv *priv, unsigned int reg,
-			 unsigned int index)
+static unsigned int qcom_mpm_offset(struct qcom_mpm_priv *priv, enum qcom_mpm_reg reg,
+				    unsigned int index)
+{
+	unsigned int reg_offset;
+
+	/*
+	 * Per the vMPM register map, TIMER[0..1] starts at register index 0 and all pin-specific
+	 * registers start after the two TIMER regs. Pin-specific register IDs start at
+	 * MPM_REG_ENABLE, so subtract it to convert to a zero-based pin-register group index.
+	 */
+	if (reg == MPM_REG_TIMER)
+		reg_offset = index;
+	else
+		reg_offset = MPM_TIMER_REGS +
+			 (reg - MPM_REG_ENABLE) * priv->reg_stride + index;
+
+	return reg_offset * sizeof(u32);
+}
+
+static u32 qcom_mpm_read(struct qcom_mpm_priv *priv, enum qcom_mpm_reg reg, unsigned int index)
 {
-	unsigned int offset = (reg * priv->reg_stride + index + 2) * 4;
+	unsigned int offset = qcom_mpm_offset(priv, reg, index);
 
 	return readl_relaxed(priv->base + offset);
 }
 
-static void qcom_mpm_write(struct qcom_mpm_priv *priv, unsigned int reg,
+static void qcom_mpm_write(struct qcom_mpm_priv *priv, enum qcom_mpm_reg reg,
 			   unsigned int index, u32 val)
 {
-	unsigned int offset = (reg * priv->reg_stride + index + 2) * 4;
+	unsigned int offset = qcom_mpm_offset(priv, reg, index);
 
 	writel_relaxed(val, priv->base + offset);
 
@@ -144,7 +167,7 @@ static void qcom_mpm_unmask(struct irq_data *d)
 		irq_chip_unmask_parent(d);
 }
 
-static void mpm_set_type(struct qcom_mpm_priv *priv, bool set, unsigned int reg,
+static void mpm_set_type(struct qcom_mpm_priv *priv, bool set, enum qcom_mpm_reg reg,
 			 unsigned int index, unsigned int shift)
 {
 	unsigned long flags, val;
@@ -327,8 +350,7 @@ static int mpm_pd_power_cb(struct notifier_block *nb, unsigned long action, void
 	return NOTIFY_OK;
 }
 
-static int mpm_cpu_pm_callback(struct notifier_block *nfb,
-			       unsigned long action, void *v)
+static int mpm_cpu_pm_callback(struct notifier_block *nfb, unsigned long action, void *v)
 {
 	struct qcom_mpm_priv *priv = container_of(nfb, struct qcom_mpm_priv, mpm_pm);
 	int cpus_in_pm;

-- 
2.34.1

[PATCH 4/7] irqchip/irq-qcom-mpm: Program wakeup timer when CPU cluster goes to LPM

[Sneh Mankad]

The next wakeup timer value needs to be set in MPM timer as the arch timer
interrupt can not wakeup the SoC if after the deepest CPUidle states the
SoC also enters deepest low power state.

To wakeup the SoC in such scenarios the earliest wakeup time is set in MPM
timer and the Resource Power Manager (RPM processor) takes care of setting
the timer in HW.

Add MPM timer programming when CPU cluster enters power collapse.

Signed-off-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
---
 drivers/irqchip/irq-qcom-mpm.c | 44 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/drivers/irqchip/irq-qcom-mpm.c b/drivers/irqchip/irq-qcom-mpm.c
index 763eddee99dc4cdd5edab22ce54808528f9ef165..f43c4a1c35f78b6cdae194dc7ae88c5c307ada94 100644
--- a/drivers/irqchip/irq-qcom-mpm.c
+++ b/drivers/irqchip/irq-qcom-mpm.c
@@ -13,6 +13,7 @@
 #include <linux/io.h>
 #include <linux/irqchip.h>
 #include <linux/irqdomain.h>
+#include <linux/ktime.h>
 #include <linux/mailbox_client.h>
 #include <linux/module.h>
 #include <linux/of.h>
@@ -25,6 +26,8 @@
 #include <linux/soc/qcom/irq.h>
 #include <linux/spinlock.h>
 
+#include <clocksource/arm_arch_timer.h>
+
 /*
  * This is the driver for Qualcomm MPM (MSM Power Manager) interrupt controller,
  * which is commonly found on Qualcomm SoCs built on the RPM architecture.
@@ -77,6 +80,13 @@ enum qcom_mpm_reg {
 	MPM_REG_STATUS,
 };
 
+#define USECS_TO_CYCLES(time_usecs)	xloops_to_cycles((time_usecs) * 0x10C7UL)
+
+static inline unsigned long xloops_to_cycles(u64 xloops)
+{
+	return (xloops * loops_per_jiffy * HZ) >> 32;
+}
+
 /* MPM pin map to GIC hwirq */
 struct mpm_gic_map {
 	int pin;
@@ -84,6 +94,7 @@ struct mpm_gic_map {
 };
 
 struct qcom_mpm_priv {
+	struct device *dev;
 	void __iomem *base;
 	raw_spinlock_t lock;
 	struct mbox_client mbox_client;
@@ -320,6 +331,36 @@ static irqreturn_t qcom_mpm_handler(int irq, void *dev_id)
 	return ret;
 }
 
+static void mpm_write_next_wakeup(struct qcom_mpm_priv *priv)
+{
+	ktime_t now, wakeup = KTIME_MAX;
+	u64 wakeup_us, wakeup_cycles = ~0;
+	u32 lo, hi;
+
+	/* Set highest time when system (timekeeping) is suspended */
+	if (system_state == SYSTEM_SUSPEND)
+		goto exit;
+
+	/* Find the relative wakeup in kernel time scale */
+	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);
+	wakeup_cycles += arch_timer_read_counter();
+
+exit:
+	lo = wakeup_cycles;
+	hi = wakeup_cycles >> 32;
+
+	qcom_mpm_write(priv, MPM_REG_TIMER, 0, lo);
+	qcom_mpm_write(priv, MPM_REG_TIMER, 1, hi);
+}
+
 static int handle_rpm_notification(struct qcom_mpm_priv *priv)
 {
 	int i, ret;
@@ -332,6 +373,7 @@ static int handle_rpm_notification(struct qcom_mpm_priv *priv)
 	if (ret < 0)
 		return ret;
 
+	mpm_write_next_wakeup(priv);
 	mbox_client_txdone(priv->mbox_chan, 0);
 	return 0;
 }
@@ -412,6 +454,8 @@ static int qcom_mpm_probe(struct platform_device *pdev, struct device_node *pare
 	if (!priv)
 		return -ENOMEM;
 
+	priv->dev = &pdev->dev;
+
 	ret = of_property_read_u32(np, "qcom,mpm-pin-count", &pin_cnt);
 	if (ret) {
 		dev_err(dev, "failed to read qcom,mpm-pin-count: %d\n", ret);

-- 
2.34.1

[PATCH 5/7] arm64: dts: qcom: sm6375: Make MPM device as part of CPU cluster domain

[Sneh Mankad]

Do not mark MPM device as power domain since it leads to idle-states init
failure because of probe dependencies.

Instead make it as part of CPU cluster power domain to allow RPM
notification when CPU cluster goes to power collapse.

Signed-off-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/sm6375.dtsi | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm6375.dtsi b/arch/arm64/boot/dts/qcom/sm6375.dtsi
index ccf572bb1549bf6b86b5232be216c9dfd51bb22d..5f729634e1d9b21deac42f9d55d966e528473202 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>;
 		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>;
 		};
 	};

-- 
2.34.1

[PATCH 6/7] arm64: dts: qcom: agatti: Do not mark MPM as power domain

[Sneh Mankad]

Do not mark MPM device as power domain since it leads to idle-states init
failure because of probe dependencies.

CPU cluster power domain node is kept disabled and hence CPU cluster will
never power collapse. Do not register MPM under it in this case.

Signed-off-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/agatti.dtsi | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/agatti.dtsi b/arch/arm64/boot/dts/qcom/agatti.dtsi
index 8a7337239b1ebe2c40d64ed670752a680085b871..05505dc3b400e923a574bc89ec9a8e7382ab803a 100644
--- a/arch/arm64/boot/dts/qcom/agatti.dtsi
+++ b/arch/arm64/boot/dts/qcom/agatti.dtsi
@@ -205,7 +205,6 @@ cpu_pd3: power-domain-cpu3 {
 
 		cluster_pd: power-domain-cpu-cluster {
 			#power-domain-cells = <0>;
-			power-domains = <&mpm>;
 			domain-idle-states = <&cluster_sleep>;
 		};
 	};
@@ -281,7 +280,6 @@ mpm: interrupt-controller {
 			mboxes = <&apcs_glb 1>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
-			#power-domain-cells = <0>;
 			interrupt-parent = <&intc>;
 			qcom,mpm-pin-count = <96>;
 			qcom,mpm-pin-map = <2 275>,  /* TSENS0 uplow */

-- 
2.34.1

[PATCH 7/7] arm64: dts: qcom: shikra: Add CPU idle states

[Sneh Mankad]

Add idle states for the  CPUs as well as the whole cluster. This enables
deeper-than-WFI cpuidle.

Add MPM under cluster_pd power domain.

Signed-off-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/shikra.dtsi | 94 +++++++++++++++++++++++++++++++++++-
 1 file changed, 93 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/shikra.dtsi b/arch/arm64/boot/dts/qcom/shikra.dtsi
index 4e5bc9e17c8ed8914733ac9fdc9b69128b40216e..bbe198d4a476e9d3037439307a0d12ab58049de2 100644
--- a/arch/arm64/boot/dts/qcom/shikra.dtsi
+++ b/arch/arm64/boot/dts/qcom/shikra.dtsi
@@ -40,6 +40,8 @@ cpu0: cpu@0 {
 			compatible = "arm,cortex-a55";
 			reg = <0x0 0x0>;
 			enable-method = "psci";
+			power-domains = <&cpu_pd0>;
+			power-domain-names = "psci";
 			next-level-cache = <&l3>;
 			capacity-dmips-mhz = <1024>;
 			dynamic-power-coefficient = <100>;
@@ -50,6 +52,8 @@ cpu1: cpu@100 {
 			compatible = "arm,cortex-a55";
 			reg = <0x0 0x100>;
 			enable-method = "psci";
+			power-domains = <&cpu_pd1>;
+			power-domain-names = "psci";
 			next-level-cache = <&l3>;
 			capacity-dmips-mhz = <1024>;
 			dynamic-power-coefficient = <100>;
@@ -60,6 +64,8 @@ cpu2: cpu@200 {
 			compatible = "arm,cortex-a55";
 			reg = <0x0 0x200>;
 			enable-method = "psci";
+			power-domains = <&cpu_pd2>;
+			power-domain-names = "psci";
 			next-level-cache = <&l3>;
 			capacity-dmips-mhz = <1024>;
 			dynamic-power-coefficient = <100>;
@@ -70,6 +76,8 @@ cpu3: cpu@300 {
 			compatible = "arm,cortex-a78c";
 			reg = <0x0 0x300>;
 			enable-method = "psci";
+			power-domains = <&cpu_pd3>;
+			power-domain-names = "psci";
 			next-level-cache = <&l2_3>;
 			capacity-dmips-mhz = <1946>;
 			dynamic-power-coefficient = <489>;
@@ -105,6 +113,61 @@ core0 {
 			};
 		};
 
+		idle-states {
+			entry-method = "psci";
+
+			little_cpu_sleep_0: cpu-sleep-0-0 {
+				compatible = "arm,idle-state";
+				idle-state-name = "little-power-down";
+				arm,psci-suspend-param = <0x40000003>;
+				entry-latency-us = <549>;
+				exit-latency-us = <901>;
+				min-residency-us = <1774>;
+				local-timer-stop;
+			};
+
+			little_cpu_sleep_1: cpu-sleep-0-1 {
+				compatible = "arm,idle-state";
+				idle-state-name = "little-pll-power-down";
+				arm,psci-suspend-param = <0x40000004>;
+				entry-latency-us = <702>;
+				exit-latency-us = <915>;
+				min-residency-us = <4001>;
+				local-timer-stop;
+			};
+
+			big_cpu_sleep_0: cpu-sleep-1-0 {
+				compatible = "arm,idle-state";
+				idle-state-name = "big-power-down";
+				arm,psci-suspend-param = <0x40000003>;
+				entry-latency-us = <523>;
+				exit-latency-us = <1244>;
+				min-residency-us = <2207>;
+				local-timer-stop;
+			};
+
+			big_cpu_sleep_1: cpu-sleep-1-1 {
+				compatible = "arm,idle-state";
+				idle-state-name = "big-pll-power-down";
+				arm,psci-suspend-param = <0x40000004>;
+				entry-latency-us = <526>;
+				exit-latency-us = <1854>;
+				min-residency-us = <5555>;
+				local-timer-stop;
+			};
+		};
+
+		domain_idle_states: domain-idle-states {
+			cluster_sleep_apss_off: cluster-sleep-0 {
+				compatible = "domain-idle-state";
+				idle-state-name = "cluster-power-down";
+				arm,psci-suspend-param = <0x41000044>;
+				entry-latency-us = <2752>;
+				exit-latency-us = <3038>;
+				min-residency-us = <6118>;
+			};
+		};
+
 		l3: l3-cache {
 			compatible = "cache";
 			cache-level = <3>;
@@ -144,6 +207,35 @@ pmu-a78c {
 	psci {
 		compatible = "arm,psci-1.0";
 		method = "smc";
+
+		cpu_pd0: power-domain-cpu0 {
+			#power-domain-cells = <0>;
+			power-domains = <&cluster_pd>;
+			domain-idle-states = <&little_cpu_sleep_0 &little_cpu_sleep_1>;
+		};
+
+		cpu_pd1: power-domain-cpu1 {
+			#power-domain-cells = <0>;
+			power-domains = <&cluster_pd>;
+			domain-idle-states = <&little_cpu_sleep_0 &little_cpu_sleep_1>;
+		};
+
+		cpu_pd2: power-domain-cpu2 {
+			#power-domain-cells = <0>;
+			power-domains = <&cluster_pd>;
+			domain-idle-states = <&little_cpu_sleep_0 &little_cpu_sleep_1>;
+		};
+
+		cpu_pd3: power-domain-cpu3 {
+			#power-domain-cells = <0>;
+			power-domains = <&cluster_pd>;
+			domain-idle-states = <&big_cpu_sleep_0 &big_cpu_sleep_1>;
+		};
+
+		cluster_pd: power-domain-cluster {
+			#power-domain-cells = <0>;
+			domain-idle-states = <&cluster_sleep_apss_off>;
+		};
 	};
 
 	rpm: remoteproc {
@@ -217,8 +309,8 @@ mpm: interrupt-controller {
 			mboxes = <&apcs_glb 1>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
-			#power-domain-cells = <0>;
 			interrupt-parent = <&intc>;
+			power-domains = <&cluster_pd>;
 			qcom,mpm-pin-count = <96>;
 			qcom,mpm-pin-map = <2 275>,  /* TSENS0 uplow */
 					   <12 422>, /* DWC3 ss_phy_irq */

-- 
2.34.1

Re: [PATCH 1/7] dt-bindings: interrupt-controller: mpm: Document power-domains property

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The commit incorrectly removes the `#power-domain-cells` provider property and replaces it with a consumer property (`power-domains`), breaking schema validation for existing device trees and misrepresenting the hardware architecture.
- [Low] The modified example code introduces an unresolved phandle (`&cluster_pd`), which causes `make dt_binding_check` to fail.
--

--- Patch [1]: [PATCH 1/7] dt-bindings: interrupt-controller: mpm: Document power-domains property ---
commit eb530c220bf421a5d64eb53cfa4eaaad646e6a1d
Author: Sneh Mankad <sneh.mankad@oss.qualcomm.com>

dt-bindings: interrupt-controller: mpm: Document power-domains property

This commit removes the #power-domain-cells property and replaces it with a
power-domains property for the MPM device binding schema.

> diff --git a/Documentation/devicetree/bindings/interrupt-controller/qcom,mpm.yaml b/Documentation/devicetree/bindings/interrupt-controller/qcom,mpm.yaml
> index ebb40c48950ab..3f9645fbc3c20 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/qcom,mpm.yaml
> +++ b/Documentation/devicetree/bindings/interrupt-controller/qcom,mpm.yaml
[ ... ]
> @@ -68,8 +68,8 @@ properties:
>          - description: MPM pin number
>          - description: GIC SPI number for the MPM pin
>  
> -  '#power-domain-cells':
> -    const: 0
> +  power-domains:
> +    maxItems: 1

[Severity: Medium]
Does this change accurately reflect the hardware architecture?

Looking at the driver implementation in drivers/irqchip/irq-qcom-mpm.c,
qcom_mpm_probe() registers a PM domain provider:

    ret = of_genpd_add_provider_simple(np, genpd);

This driver natively implements a PM domain provider, which relies on the
#power-domain-cells property being present in the device tree.

Additionally, existing in-tree device trees (such as agatti.dtsi) currently
declare #power-domain-cells = <0>; in their mpm nodes. Removing this
provider property and replacing it with a consumer property will likely cause
dtbs_check to fail for those existing boards.

>  
>  required:
>    - compatible
[ ... ]
> @@ -113,6 +113,6 @@ examples:
>                                 <24 79>,
>                                 <86 183>,
>                                 <91 260>;
> -            #power-domain-cells = <0>;
> +            power-domains = <&cluster_pd>;

[Severity: Low]
Is the cluster_pd phandle defined in this example block?

Adding an unresolved phandle reference to the examples section typically
causes the dtc compiler to fail with a 'Reference to non-existent node
or label' error when running make dt_binding_check.

>          };
>      };

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

Re: [PATCH 4/7] irqchip/irq-qcom-mpm: Program wakeup timer when CPU cluster goes to LPM

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Data race between the AP and RPM processor due to incorrect ordering of mailbox notification and timer programming.
- [High] Unhandled KTIME_MAX from unscheduled timers causes massive integer overflows and random hardware wakeups.
- [High] Expired timers produce negative relative times that underflow when cast to unsigned u64.
- [High] Invalid USECS_TO_CYCLES conversion macro uses CPU delay loop calibration instead of arch timer frequency, and truncates to 32 bits.
--

--- Patch [4]: [PATCH 4/7] irqchip/irq-qcom-mpm: Program wakeup timer when CPU cluster goes to LPM ---
commit c68a3cdf8bf38bac4bbcdaf3ea878c066a1246a0
Author: Sneh Mankad <sneh.mankad@oss.qualcomm.com>

irqchip/irq-qcom-mpm: Program wakeup timer when CPU cluster goes to LPM

This commit adds support for programming the next wakeup time into the MPM
timer when the CPU cluster enters power collapse. This allows the RPM processor
to set the hardware timer and wake up the SoC when the architectural timer
interrupt cannot do so during deep low power states.

> diff --git a/drivers/irqchip/irq-qcom-mpm.c b/drivers/irqchip/irq-qcom-mpm.c
> index 763eddee99dc4..f43c4a1c35f78 100644
> --- a/drivers/irqchip/irq-qcom-mpm.c
> +++ b/drivers/irqchip/irq-qcom-mpm.c

[ ... ]

> @@ -77,6 +80,13 @@ enum qcom_mpm_reg {
>  	MPM_REG_STATUS,
>  };
>  
> +#define USECS_TO_CYCLES(time_usecs)	xloops_to_cycles((time_usecs) * 0x10C7UL)
> +
> +static inline unsigned long xloops_to_cycles(u64 xloops)
> +{
> +	return (xloops * loops_per_jiffy * HZ) >> 32;
> +}

[Severity: High]
Does this macro use the correct frequency for the conversion, and is there a
risk of overflow?

In xloops_to_cycles(), loops_per_jiffy represents the CPU-specific delay
loop iterations rather than the fixed frequency of the architectural timer.

Also, the calculation (xloops * loops_per_jiffy * HZ) can overflow a 64-bit
integer for sleep times exceeding 25 seconds. Finally, returning an unsigned
long truncates the result to 32 bits on 32-bit ARM systems, which would
further cap the maximum sleep duration.

[ ... ]

> @@ -320,6 +331,36 @@ static irqreturn_t qcom_mpm_handler(int irq, void *dev_id)
>  	return ret;
>  }
>  
> +static void mpm_write_next_wakeup(struct qcom_mpm_priv *priv)
> +{
> +	ktime_t now, wakeup = KTIME_MAX;
> +	u64 wakeup_us, wakeup_cycles = ~0;
> +	u32 lo, hi;
> +
> +	/* Set highest time when system (timekeeping) is suspended */
> +	if (system_state == SYSTEM_SUSPEND)
> +		goto exit;
> +
> +	/* Find the relative wakeup in kernel time scale */
> +	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);

[Severity: High]
Could this overflow if there are no pending timers?

If dev_pm_genpd_get_next_hrtimer() returns KTIME_MAX, subtracting now from
KTIME_MAX in mpm_write_next_wakeup() yields a massive delta.

When this large delta is converted to microseconds and passed to
USECS_TO_CYCLES(), it seems it would overflow the 64-bit bounds during the
multiplication, leading to a randomized or incorrect wakeup time being
programmed into the hardware.

> +	wakeup_us = ktime_to_us(wakeup);

[Severity: High]
How does this handle expired timers where the scheduled wakeup is in the past?

If the scheduled wakeup is earlier than now, ktime_sub() will return a negative
time delta. When ktime_to_us() returns this negative value, assigning it to
the unsigned u64 wakeup_us variable will cause it to wrap around to a massive
positive number, which then gets passed to the conversion macros.

> +
> +	/* Convert the wakeup to arch timer scale */
> +	wakeup_cycles = USECS_TO_CYCLES(wakeup_us);
> +	wakeup_cycles += arch_timer_read_counter();
> +
> +exit:
> +	lo = wakeup_cycles;
> +	hi = wakeup_cycles >> 32;
> +
> +	qcom_mpm_write(priv, MPM_REG_TIMER, 0, lo);
> +	qcom_mpm_write(priv, MPM_REG_TIMER, 1, hi);
> +}
> +
>  static int handle_rpm_notification(struct qcom_mpm_priv *priv)
>  {
>  	int i, ret;
> @@ -332,6 +373,7 @@ static int handle_rpm_notification(struct qcom_mpm_priv *priv)
>  	if (ret < 0)
>  		return ret;
>  
> +	mpm_write_next_wakeup(priv);

[Severity: High]
Is there a data race here between notifying the RPM and writing the timer?

In handle_rpm_notification(), the mailbox message is sent to the RPM
via mbox_send_message() before mpm_write_next_wakeup() is called to program
the new timer values into the shared registers. 

Because the notification is sent before the write occurs, the RPM could read
the shared registers before the AP has actually written the updated wakeup
time, causing it to use stale or uninitialized values.

>  	mbox_client_txdone(priv->mbox_chan, 0);
>  	return 0;
>  }

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

Re: [PATCH 3/7] irqchip/irq-qcom-mpm: Prepare common access path for timer and pin regs

[sashiko-bot]

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

Pre-existing issues:
- [High] The initialization loop in the probe function writes zeroes to registers based on the device tree property `qcom,mpm-pin-count` without verifying if the calculated `reg_stride` fits within the mapped I/O memory region.
- [High] Lack of bounds checking on `d->hwirq` combined with signed integer conversion allows arbitrary out-of-bounds memory accesses when reading/writing IRQ registers.
--

--- Patch [3]: [PATCH 3/7] irqchip/irq-qcom-mpm: Prepare common access path for timer and pin regs ---
commit 481c7b7ccc27a48778456542a65ce5c589dcaa00
Author: Sneh Mankad <sneh.mankad@oss.qualcomm.com>

irqchip/irq-qcom-mpm: Prepare common access path for timer and pin regs

This commit introduces a common addressing helper, qcom_mpm_offset(),
to calculate memory offsets for both timer and pin registers. It also 
replaces the macro definitions for vMPM registers with an enum and 
updates the read/write accessor functions to use it.

> diff --git a/drivers/irqchip/irq-qcom-mpm.c b/drivers/irqchip/irq-qcom-mpm.c
> index 01fd1843172aa..763eddee99dc4 100644
> --- a/drivers/irqchip/irq-qcom-mpm.c
> +++ b/drivers/irqchip/irq-qcom-mpm.c
[ ... ]
> @@ -92,18 +97,36 @@ struct qcom_mpm_priv {
>  	atomic_t cpus_in_pm;
>  };
>  
> -static u32 qcom_mpm_read(struct qcom_mpm_priv *priv, unsigned int reg,
> -			 unsigned int index)
> +static unsigned int qcom_mpm_offset(struct qcom_mpm_priv *priv, enum qcom_mpm_reg reg,
> +				    unsigned int index)
> +{
> +	unsigned int reg_offset;
> +
> +	/*
> +	 * Per the vMPM register map, TIMER[0..1] starts at register index 0 and all pin-specific
> +	 * registers start after the two TIMER regs. Pin-specific register IDs start at
> +	 * MPM_REG_ENABLE, so subtract it to convert to a zero-based pin-register group index.
> +	 */
> +	if (reg == MPM_REG_TIMER)
> +		reg_offset = index;
> +	else
> +		reg_offset = MPM_TIMER_REGS +
> +			 (reg - MPM_REG_ENABLE) * priv->reg_stride + index;
> +
> +	return reg_offset * sizeof(u32);
> +}

[Severity: High]
This isn't a bug introduced by this patch, but I'm wondering if
priv->reg_stride is validated against the size of the mapped I/O memory region?

Looking at qcom_mpm_probe():

	priv->reg_stride = DIV_ROUND_UP(pin_cnt, 32);

If a very large qcom,mpm-pin-count is provided in the device tree, could the
initialization loop later in qcom_mpm_probe():

	for (i = 0; i < priv->reg_stride; i++) {
		qcom_mpm_write(priv, MPM_REG_ENABLE, i, 0);
		...

write zeroes out of bounds into memory following priv->base?

[Severity: High]
This is a pre-existing issue, but does this function safely handle the index
parameter against malformed device tree inputs?

In qcom_mpm_alloc(), pin is assigned as d->hwirq from the device tree
interrupt specifier without bounds validation against domain->hwirq_max.

Later in qcom_mpm_enable_irq():

	int pin = d->hwirq;
	unsigned int index = pin / 32;

If a sufficiently large d->hwirq (e.g., 0x80000000) is provided, would the
signed integer conversion cause it to wrap into a negative number, resulting
in a massive unsigned index being passed into qcom_mpm_offset() and allowing
arbitrary out-of-bounds memory accesses?

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

Re: [PATCH 6/7] arm64: dts: qcom: agatti: Do not mark MPM as power domain

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing check for KTIME_MAX and expired timers causes 64-bit integer overflow, resulting in an unpredictable hardware wakeup time.
- [High] Mailbox notification is sent to the RPM processor before the timer registers are updated, causing a race where RPM reads stale timer values.
--

--- Patch [6]: [PATCH 6/7] arm64: dts: qcom: agatti: Do not mark MPM as power domain ---
commit 7c3965f0765f0a48723f4b17bac82d687c8614cc
Author: Sneh Mankad <sneh.mankad@oss.qualcomm.com>

arm64: dts: qcom: agatti: Do not mark MPM as power domain
    
Do not mark MPM device as power domain since it leads to idle-states init
failure because of probe dependencies. CPU cluster power domain node is kept
disabled and hence CPU cluster will never power collapse. Do not register
MPM under it in this case.

> diff --git a/arch/arm64/boot/dts/qcom/agatti.dtsi b/arch/arm64/boot/dts/qcom/agatti.dtsi
> index f0b6ae9b81528..ddb038b3bbbc4 100644
> --- a/arch/arm64/boot/dts/qcom/agatti.dtsi
> +++ b/arch/arm64/boot/dts/qcom/agatti.dtsi
> @@ -205,7 +205,6 @@ cpu_pd3: power-domain-cpu3 {
>  
>  		cluster_pd: power-domain-cpu-cluster {
>  			#power-domain-cells = <0>;
> -			power-domains = <&mpm>;
>  			domain-idle-states = <&cluster_sleep>;
>  		};
>  	};

[Severity: High]
Does removing the MPM power domain mapping cause issues with the timer
values?

When no hrtimers are pending, dev_pm_genpd_get_next_hrtimer() returns
KTIME_MAX. Subtracting 'now' leaves a huge positive value, which when
converted to microseconds and multiplied by 0x10C7UL overflows the 64-bit
boundary.

Similarly, if a timer has already expired, the negative difference
becomes a huge positive value when implicitly cast to u64 wakeup_us, leading
to overflow.

Could this happen here in mpm_write_next_wakeup() within
drivers/irqchip/irq-qcom-mpm.c:

	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);

and cause random timer values to be programmed into the MPM hardware?

[ ... ]

> @@ -281,7 +280,6 @@ mpm: interrupt-controller {
>  			mboxes = <&apcs_glb 1>;
>  			interrupt-controller;
>  			#interrupt-cells = <2>;
> -			#power-domain-cells = <0>;
>  			interrupt-parent = <&intc>;
>  			qcom,mpm-pin-count = <96>;
>  			qcom,mpm-pin-map = <2 275>,  /* TSENS0 uplow */

[Severity: High]
Does this change expose a race condition with the RPM processor?

The AP signals the RPM processor to configure hardware state from the shared
vMPM SRAM before actually writing the new timer wakeup values to the SRAM.
Since the RPM is an independent processor, it is highly likely to read the
old, stale timer values.

Could this happen here in handle_rpm_notification() within
drivers/irqchip/irq-qcom-mpm.c:

	/* Notify RPM to write vMPM into HW */
	ret = mbox_send_message(priv->mbox_chan, NULL);
	if (ret < 0)
		return ret;

	mpm_write_next_wakeup(priv);

and cause the hardware to be programmed with the incorrect wakeup time?

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

Re: [PATCH 5/7] arm64: dts: qcom: sm6375: Make MPM device as part of CPU cluster domain

[sashiko-bot]

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

Re: [PATCH 2/7] irqchip/irq-qcom-mpm: Register MPM under CPU cluster power domain

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The `cpus_in_pm` counter is permanently leaked if `handle_rpm_notification()` fails during CPU PM entry.
- [High] The return value of `devm_pm_runtime_enable(dev)` is ignored, potentially breaking cluster power down.
--

--- Patch [2]: [PATCH 2/7] irqchip/irq-qcom-mpm: Register MPM under CPU cluster power domain ---
commit 1e6fda1a51a8d1d0370b919f0b149226ff3d825a
Author: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
Subject: irqchip/irq-qcom-mpm: Register MPM under CPU cluster power domain

This commit resolves a hard probe dependency by moving RPM notification
handling to the GENPD_NOTIFY_PRE_OFF callback and registering MPM under the
CPU cluster power domain. It also provides a CPU PM notification fallback if
the MPM hasn't registered with the CPU cluster power domain.

Fixes: a6199bb514d8 ("irqchip: Add Qualcomm MPM controller driver")

(Note: The standard inline-quoted diff format has been omitted due to automated
recitation restrictions. The regressions found are described using free-form
text below.)

[Severity: High]
In drivers/irqchip/irq-qcom-mpm.c:mpm_cpu_pm_callback(), during the
CPU_PM_ENTER case, atomic_inc_return(&priv->cpus_in_pm) is executed at the
beginning of the transition.

If handle_rpm_notification(priv) subsequently fails, the function returns
NOTIFY_BAD. Because notifier_call_chain_robust() rolls back successfully
executed notifiers but does not send CPU_PM_ENTER_FAILED to the notifier
that failed, the atomic increment is never reverted.

Does this leave cpus_in_pm permanently incremented, causing all subsequent
CPU PM transitions to incorrectly evaluate cpus_in_pm < num_online_cpus()
and permanently preventing the cluster from entering low power mode?

[Severity: High]
In drivers/irqchip/irq-qcom-mpm.c:qcom_mpm_probe(), the return value of
devm_pm_runtime_enable(dev) is not checked.

If devm_pm_runtime_enable() fails, for instance due to a memory allocation
failure in devm_add_action_or_reset(), runtime PM will not be enabled for
the device.

Can this unhandled failure prevent the genpd from managing the device's
power state properly, preventing the CPU cluster power domain from
entering low power mode? Should the return value be checked and handled?

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

Re: [PATCH 1/7] dt-bindings: interrupt-controller: mpm: Document power-domains property

[Konrad Dybcio]

On 7/13/26 12:25 PM, Sneh Mankad wrote:
> Remove #power-domain-cells property and add power-domains property for
> MPM device.

"why"?

Konrad

Re: [PATCH 1/7] dt-bindings: interrupt-controller: mpm: Document power-domains property

[Marc Zyngier]

On Mon, 13 Jul 2026 11:25:41 +0100,
Sneh Mankad <sneh.mankad@oss.qualcomm.com> wrote:
> 
> Remove #power-domain-cells property and add power-domains property for
> MPM device.
> 
> Signed-off-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
> ---
>  .../devicetree/bindings/interrupt-controller/qcom,mpm.yaml          | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/qcom,mpm.yaml b/Documentation/devicetree/bindings/interrupt-controller/qcom,mpm.yaml
> index ebb40c48950ab3a8fc86f5708acfc33c33d68993..3f9645fbc3c20633077aaa589e5d5a43928dab51 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/qcom,mpm.yaml
> +++ b/Documentation/devicetree/bindings/interrupt-controller/qcom,mpm.yaml
> @@ -68,8 +68,8 @@ properties:
>          - description: MPM pin number
>          - description: GIC SPI number for the MPM pin
>  
> -  '#power-domain-cells':
> -    const: 0
> +  power-domains:
> +    maxItems: 1

What makes you think it is OK to change an existing binding in an
incompatible way?

	M.

-- 
Jazz isn't dead. It just smells funny.

Re: [PATCH 4/7] irqchip/irq-qcom-mpm: Program wakeup timer when CPU cluster goes to LPM

[Marc Zyngier]

On Mon, 13 Jul 2026 11:25:44 +0100,
Sneh Mankad <sneh.mankad@oss.qualcomm.com> wrote:
> 
> The next wakeup timer value needs to be set in MPM timer as the arch timer
> interrupt can not wakeup the SoC if after the deepest CPUidle states the
> SoC also enters deepest low power state.
> 
> To wakeup the SoC in such scenarios the earliest wakeup time is set in MPM
> timer and the Resource Power Manager (RPM processor) takes care of setting
> the timer in HW.
> 
> Add MPM timer programming when CPU cluster enters power collapse.
> 
> Signed-off-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
> ---
>  drivers/irqchip/irq-qcom-mpm.c | 44 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/drivers/irqchip/irq-qcom-mpm.c b/drivers/irqchip/irq-qcom-mpm.c
> index 763eddee99dc4cdd5edab22ce54808528f9ef165..f43c4a1c35f78b6cdae194dc7ae88c5c307ada94 100644
> --- a/drivers/irqchip/irq-qcom-mpm.c
> +++ b/drivers/irqchip/irq-qcom-mpm.c
> @@ -13,6 +13,7 @@
>  #include <linux/io.h>
>  #include <linux/irqchip.h>
>  #include <linux/irqdomain.h>
> +#include <linux/ktime.h>
>  #include <linux/mailbox_client.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> @@ -25,6 +26,8 @@
>  #include <linux/soc/qcom/irq.h>
>  #include <linux/spinlock.h>
>  
> +#include <clocksource/arm_arch_timer.h>
> +
>  /*
>   * This is the driver for Qualcomm MPM (MSM Power Manager) interrupt controller,
>   * which is commonly found on Qualcomm SoCs built on the RPM architecture.
> @@ -77,6 +80,13 @@ enum qcom_mpm_reg {
>  	MPM_REG_STATUS,
>  };
>  
> +#define USECS_TO_CYCLES(time_usecs)	xloops_to_cycles((time_usecs) * 0x10C7UL)
> +
> +static inline unsigned long xloops_to_cycles(u64 xloops)
> +{
> +	return (xloops * loops_per_jiffy * HZ) >> 32;
> +}
> +

Do we really need arch-specific code to be literally copied from
arm64's delay.c, without any comment or attempt at making it generic?

Specially after having added the same stuff to rpmh-rsc.c 4 years ago?

See a pattern here?

	M.

-- 
Jazz isn't dead. It just smells funny.