[PATCH 0/3] Exynos PMU fixes for cpu hotplug and cpuidle routines

[PATCH 0/3] Exynos PMU fixes for cpu hotplug and cpuidle routines

[Alexey Klimov]

This was reported by Sashiko here:
https://sashiko.dev/#/patchset/20260513-exynos850-cpuhotplug-v4-0-54fec5f65362@linaro.org?part=3
and was mainly introduced by enabling cpu hotplug
support and cpuidle for gs101-based SoCs.

One patch removes strange usage of smp_processor_id() and
other patches deal with a few missing error paths issues
here and there in setup_cpuhp_and_cpuidle() and around.

Tested on gs101-raven device, I don't see any regressions
but testing from others will be appreciated.

Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
---
Alexey Klimov (3):
      soc: samsung: exynos-pmu: use target cpu ID in hotplug callbacks
      soc: samsung: exynos-pmu: fix use-after-free of interrupt generator node
      soc: samsung: exynos-pmu: fix error paths in cpuhotplug/idle states setup

 drivers/soc/samsung/exynos-pmu.c | 75 ++++++++++++++++++++++++++++++++--------
 1 file changed, 60 insertions(+), 15 deletions(-)
---
base-commit: e98d21c170b01ddef366f023bbfcf6b31509fa83
change-id: 20260605-exynos-pmu-cpuhp-idle-fixes-32f5ed7c969f

Best regards,
-- 
Alexey Klimov <alexey.klimov@linaro.org>

[PATCH 1/3] soc: samsung: exynos-pmu: use target cpu ID in hotplug callbacks

[Alexey Klimov]

The CPU hotplug state callbacks __gs101_cpu_pmu_online() and
__gs101_cpu_pmu_offline() currently partially use smp_processor_id() to
determine the target register offset for the CPU inform hints. This may
be fine for cpuidle flow but broken for cpu hotplug where the target
cpu is passed as an argument and could be different from cpu where
that is executing (e.g. CPU 0 offlining CPU 1), meaning that
smp_processor_id() returns the id of local CPU but hotplug flow
deals with another CPU core undergoing the transition.

This causes the pmu driver to write power down and power on configuration
hints to the wrong hardware registers, messing up the power state of active
cores and failing to configure the target core. Fix this by removing the
cpuhint variable entirely and utilizing the target 'cpu' argument passed
to the callbacks by the hotplug core infrastructure.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260513-exynos850-cpuhotplug-v4-0-54fec5f65362@linaro.org?part=3
Fixes: 598995027b91 ("soc: samsung: exynos-pmu: enable CPU hotplug support for gs101")
Cc: stable@vger.kernel.org
Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
---
 drivers/soc/samsung/exynos-pmu.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/soc/samsung/exynos-pmu.c b/drivers/soc/samsung/exynos-pmu.c
index d58376c38179..6e635872247a 100644
--- a/drivers/soc/samsung/exynos-pmu.c
+++ b/drivers/soc/samsung/exynos-pmu.c
@@ -235,11 +235,10 @@ EXPORT_SYMBOL_GPL(exynos_get_pmu_regmap_by_phandle);
 static int __gs101_cpu_pmu_online(unsigned int cpu)
 	__must_hold(&pmu_context->cpupm_lock)
 {
-	unsigned int cpuhint = smp_processor_id();
 	u32 reg, mask;
 
 	/* clear cpu inform hint */
-	regmap_write(pmu_context->pmureg, GS101_CPU_INFORM(cpuhint),
+	regmap_write(pmu_context->pmureg, GS101_CPU_INFORM(cpu),
 		     CPU_INFORM_CLEAR);
 
 	mask = BIT(cpu);
@@ -296,12 +295,10 @@ static int gs101_cpuhp_pmu_online(unsigned int cpu)
 static int __gs101_cpu_pmu_offline(unsigned int cpu)
 	__must_hold(&pmu_context->cpupm_lock)
 {
-	unsigned int cpuhint = smp_processor_id();
 	u32 reg, mask;
 
 	/* set cpu inform hint */
-	regmap_write(pmu_context->pmureg, GS101_CPU_INFORM(cpuhint),
-		     CPU_INFORM_C2);
+	regmap_write(pmu_context->pmureg, GS101_CPU_INFORM(cpu), CPU_INFORM_C2);
 
 	mask = BIT(cpu);
 	regmap_update_bits(pmu_context->pmuintrgen, GS101_GRP2_INTR_BID_ENABLE,

-- 
2.51.0

Re: [PATCH 1/3] soc: samsung: exynos-pmu: use target cpu ID in hotplug callbacks

[Peter Griffin]

Hi Alexey,

Thanks for your patch.

On Fri, 5 Jun 2026 at 21:19, Alexey Klimov <alexey.klimov@linaro.org> wrote:
>
> The CPU hotplug state callbacks __gs101_cpu_pmu_online() and
> __gs101_cpu_pmu_offline() currently partially use smp_processor_id() to
> determine the target register offset for the CPU inform hints. This may
> be fine for cpuidle flow but broken for cpu hotplug where the target
> cpu is passed as an argument and could be different from cpu where
> that is executing (e.g. CPU 0 offlining CPU 1), meaning that
> smp_processor_id() returns the id of local CPU but hotplug flow
> deals with another CPU core undergoing the transition.

This was intentional. The powermode hint is always programmed based on
the currently executing CPU core in the gs101 downstream code (for
both CPU Idle and CPU hotplug paths). See
https://android.googlesource.com/kernel/google-modules/raviole-device/+/refs/heads/android-gs-raviole-mainline/drivers/soc/google/cal-if/pmucal_powermode.c#15
and the pmu_intr_gen is done based on the actual CPU being
enabled/disabled.

It's possible Exynos850 requires something different. I suggest
checking the equivalent function in the e850 downstream kernel.

>
> This causes the pmu driver to write power down and power on configuration
> hints to the wrong hardware registers, messing up the power state of active
> cores and failing to configure the target core. Fix this by removing the
> cpuhint variable entirely and utilizing the target 'cpu' argument passed
> to the callbacks by the hotplug core infrastructure.

Unfortunately I think you're introducing the bug you describe with this patch.

regards,

Peter

>
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260513-exynos850-cpuhotplug-v4-0-54fec5f65362@linaro.org?part=3
> Fixes: 598995027b91 ("soc: samsung: exynos-pmu: enable CPU hotplug support for gs101")
> Cc: stable@vger.kernel.org
> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
> ---
>  drivers/soc/samsung/exynos-pmu.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/soc/samsung/exynos-pmu.c b/drivers/soc/samsung/exynos-pmu.c
> index d58376c38179..6e635872247a 100644
> --- a/drivers/soc/samsung/exynos-pmu.c
> +++ b/drivers/soc/samsung/exynos-pmu.c
> @@ -235,11 +235,10 @@ EXPORT_SYMBOL_GPL(exynos_get_pmu_regmap_by_phandle);
>  static int __gs101_cpu_pmu_online(unsigned int cpu)
>         __must_hold(&pmu_context->cpupm_lock)
>  {
> -       unsigned int cpuhint = smp_processor_id();
>         u32 reg, mask;
>
>         /* clear cpu inform hint */
> -       regmap_write(pmu_context->pmureg, GS101_CPU_INFORM(cpuhint),
> +       regmap_write(pmu_context->pmureg, GS101_CPU_INFORM(cpu),
>                      CPU_INFORM_CLEAR);
>
>         mask = BIT(cpu);
> @@ -296,12 +295,10 @@ static int gs101_cpuhp_pmu_online(unsigned int cpu)
>  static int __gs101_cpu_pmu_offline(unsigned int cpu)
>         __must_hold(&pmu_context->cpupm_lock)
>  {
> -       unsigned int cpuhint = smp_processor_id();
>         u32 reg, mask;
>
>         /* set cpu inform hint */
> -       regmap_write(pmu_context->pmureg, GS101_CPU_INFORM(cpuhint),
> -                    CPU_INFORM_C2);
> +       regmap_write(pmu_context->pmureg, GS101_CPU_INFORM(cpu), CPU_INFORM_C2);
>
>         mask = BIT(cpu);
>         regmap_update_bits(pmu_context->pmuintrgen, GS101_GRP2_INTR_BID_ENABLE,
>
> --
> 2.51.0
>

[PATCH 2/3] soc: samsung: exynos-pmu: fix use-after-free of interrupt generator node

[Alexey Klimov]

The setup_cpuhp_and_cpuidle() parses the device tree node for the
interrupt generation block via of_parse_phandle() and decrements its
reference count using of_node_put() immediately after fetching the resource
address. However, later the intr_gen_node pointer is passed into
of_syscon_register_regmap().

Fix this by moving the of_node_put() invocation to after the
of_syscon_register_regmap() call, and adding it to correct error paths.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260513-exynos850-cpuhotplug-v4-0-54fec5f65362@linaro.org?part=3
Fixes: 78b72897a5c8 ("soc: samsung: exynos-pmu: Enable CPU Idle for gs101")
Cc: stable@vger.kernel.org
Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
---
 drivers/soc/samsung/exynos-pmu.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/samsung/exynos-pmu.c b/drivers/soc/samsung/exynos-pmu.c
index 6e635872247a..9636287f6794 100644
--- a/drivers/soc/samsung/exynos-pmu.c
+++ b/drivers/soc/samsung/exynos-pmu.c
@@ -428,23 +428,30 @@ static int setup_cpuhp_and_cpuidle(struct device *dev)
 	 * syscon provided regmap.
 	 */
 	ret = of_address_to_resource(intr_gen_node, 0, &intrgen_res);
-	of_node_put(intr_gen_node);
+	if (ret) {
+		of_node_put(intr_gen_node);
+		return ret;
+	}
 
 	virt_addr = devm_ioremap(dev, intrgen_res.start,
 				 resource_size(&intrgen_res));
-	if (!virt_addr)
+	if (!virt_addr) {
+		of_node_put(intr_gen_node);
 		return -ENOMEM;
+	}
 
 	pmu_context->pmuintrgen = devm_regmap_init_mmio(dev, virt_addr,
 							&regmap_pmu_intr);
 	if (IS_ERR(pmu_context->pmuintrgen)) {
 		dev_err(dev, "failed to initialize pmu-intr-gen regmap\n");
+		of_node_put(intr_gen_node);
 		return PTR_ERR(pmu_context->pmuintrgen);
 	}
 
 	/* register custom mmio regmap with syscon */
 	ret = of_syscon_register_regmap(intr_gen_node,
 					pmu_context->pmuintrgen);
+	of_node_put(intr_gen_node);
 	if (ret)
 		return ret;
 

-- 
2.51.0

Re: [PATCH 2/3] soc: samsung: exynos-pmu: fix use-after-free of interrupt generator node

[Peter Griffin]

Hi Alexey,

Thanks for your patch.

On Fri, 5 Jun 2026 at 21:19, Alexey Klimov <alexey.klimov@linaro.org> wrote:
>
> The setup_cpuhp_and_cpuidle() parses the device tree node for the
> interrupt generation block via of_parse_phandle() and decrements its
> reference count using of_node_put() immediately after fetching the resource
> address. However, later the intr_gen_node pointer is passed into
> of_syscon_register_regmap().
>
> Fix this by moving the of_node_put() invocation to after the
> of_syscon_register_regmap() call, and adding it to correct error paths.

I think  using
__free(device_node) = of_parse_phandle

would be a cleaner/simpler fix.

Peter




Peter.

>
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260513-exynos850-cpuhotplug-v4-0-54fec5f65362@linaro.org?part=3
> Fixes: 78b72897a5c8 ("soc: samsung: exynos-pmu: Enable CPU Idle for gs101")
> Cc: stable@vger.kernel.org
> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
> ---
>  drivers/soc/samsung/exynos-pmu.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/soc/samsung/exynos-pmu.c b/drivers/soc/samsung/exynos-pmu.c
> index 6e635872247a..9636287f6794 100644
> --- a/drivers/soc/samsung/exynos-pmu.c
> +++ b/drivers/soc/samsung/exynos-pmu.c
> @@ -428,23 +428,30 @@ static int setup_cpuhp_and_cpuidle(struct device *dev)
>          * syscon provided regmap.
>          */
>         ret = of_address_to_resource(intr_gen_node, 0, &intrgen_res);
> -       of_node_put(intr_gen_node);
> +       if (ret) {
> +               of_node_put(intr_gen_node);
> +               return ret;
> +       }
>
>         virt_addr = devm_ioremap(dev, intrgen_res.start,
>                                  resource_size(&intrgen_res));
> -       if (!virt_addr)
> +       if (!virt_addr) {
> +               of_node_put(intr_gen_node);
>                 return -ENOMEM;
> +       }
>
>         pmu_context->pmuintrgen = devm_regmap_init_mmio(dev, virt_addr,
>                                                         &regmap_pmu_intr);
>         if (IS_ERR(pmu_context->pmuintrgen)) {
>                 dev_err(dev, "failed to initialize pmu-intr-gen regmap\n");
> +               of_node_put(intr_gen_node);
>                 return PTR_ERR(pmu_context->pmuintrgen);
>         }
>
>         /* register custom mmio regmap with syscon */
>         ret = of_syscon_register_regmap(intr_gen_node,
>                                         pmu_context->pmuintrgen);
> +       of_node_put(intr_gen_node);
>         if (ret)
>                 return ret;
>
>
> --
> 2.51.0
>

[PATCH 3/3] soc: samsung: exynos-pmu: fix error paths in cpuhotplug/idle states setup

[Alexey Klimov]

The setup_cpuhp_and_cpuidle() initialisation sequence currently ignores
the return values of cpuhp_setup_state(), cpu_pm_register_notifier(), and
register_reboot_notifier(). If any of these registrations fail during
probe() routine, the driver returns 0, leaving the driver partially
configured.

Furthermore, if anything after setup_cpuhp_and_cpuidle() fails in probe()
routine, for instance devm_mfd_add_devices(), the probe() lacks an error
path and leaves notifiers and cpu hotplug states registered.

Introduce variables for the cpu hotplug state IDs in exynos_pmu_context
struct, that should be initialised to CPUHP_INVALID by default. Check all
return codes in setup_cpuhp_and_cpuidle(), and add an error path to remove
registered states on failure. Finally, add destroy_cpuhp_and_cpuidle()
helper to safely tear down notifiers and cpu hotplug states.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260513-exynos850-cpuhotplug-v4-0-54fec5f65362@linaro.org?part=3
Fixes: 78b72897a5c8 ("soc: samsung: exynos-pmu: Enable CPU Idle for gs101")
Cc: stable@vger.kernel.org
Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
---
 drivers/soc/samsung/exynos-pmu.c | 57 ++++++++++++++++++++++++++++++++++------
 1 file changed, 49 insertions(+), 8 deletions(-)

diff --git a/drivers/soc/samsung/exynos-pmu.c b/drivers/soc/samsung/exynos-pmu.c
index 9636287f6794..846313a28e9a 100644
--- a/drivers/soc/samsung/exynos-pmu.c
+++ b/drivers/soc/samsung/exynos-pmu.c
@@ -38,6 +38,8 @@ struct exynos_pmu_context {
 	unsigned long *in_cpuhp;
 	bool sys_insuspend;
 	bool sys_inreboot;
+	int cpuhp_prepare_state;
+	int cpuhp_online_state;
 };
 
 void __iomem *pmu_base_addr;
@@ -404,6 +406,17 @@ static struct notifier_block exynos_cpupm_reboot_nb = {
 	.notifier_call = exynos_cpupm_reboot_notifier,
 };
 
+static void destroy_cpuhp_and_cpuidle(void)
+{
+	cpu_pm_unregister_notifier(&gs101_cpu_pm_notifier);
+	unregister_reboot_notifier(&exynos_cpupm_reboot_nb);
+
+	if (pmu_context->cpuhp_prepare_state != CPUHP_INVALID)
+		cpuhp_remove_state(pmu_context->cpuhp_prepare_state);
+	if (pmu_context->cpuhp_online_state != CPUHP_INVALID)
+		cpuhp_remove_state(pmu_context->cpuhp_online_state);
+}
+
 static int setup_cpuhp_and_cpuidle(struct device *dev)
 {
 	struct device_node *intr_gen_node;
@@ -465,16 +478,42 @@ static int setup_cpuhp_and_cpuidle(struct device *dev)
 		gs101_cpuhp_pmu_online(cpu);
 
 	/* register CPU hotplug callbacks */
-	cpuhp_setup_state(CPUHP_BP_PREPARE_DYN,	"soc/exynos-pmu:prepare",
-			  gs101_cpuhp_pmu_online, NULL);
+	pmu_context->cpuhp_prepare_state = CPUHP_INVALID;
+	pmu_context->cpuhp_online_state = CPUHP_INVALID;
 
-	cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "soc/exynos-pmu:online",
-			  NULL, gs101_cpuhp_pmu_offline);
+	ret = cpuhp_setup_state(CPUHP_BP_PREPARE_DYN, "soc/exynos-pmu:prepare",
+				gs101_cpuhp_pmu_online, NULL);
+	if (ret < 0)
+		return ret;
+
+	pmu_context->cpuhp_prepare_state = ret;
+
+	ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "soc/exynos-pmu:online",
+				NULL, gs101_cpuhp_pmu_offline);
+	if (ret < 0)
+		goto clean_cpuhp_states;
+
+	pmu_context->cpuhp_online_state = ret;
 
 	/* register CPU PM notifiers for cpuidle */
-	cpu_pm_register_notifier(&gs101_cpu_pm_notifier);
-	register_reboot_notifier(&exynos_cpupm_reboot_nb);
-	return 0;
+	ret = cpu_pm_register_notifier(&gs101_cpu_pm_notifier);
+	if (ret)
+		goto clean_cpuhp_states;
+
+	ret = register_reboot_notifier(&exynos_cpupm_reboot_nb);
+	if (!ret)
+		/* Success */
+		return ret;
+
+	cpu_pm_unregister_notifier(&gs101_cpu_pm_notifier);
+
+clean_cpuhp_states:
+	if (pmu_context->cpuhp_prepare_state != CPUHP_INVALID)
+		cpuhp_remove_state(pmu_context->cpuhp_prepare_state);
+	if (pmu_context->cpuhp_online_state != CPUHP_INVALID)
+		cpuhp_remove_state(pmu_context->cpuhp_online_state);
+
+	return ret;
 }
 
 static int exynos_pmu_probe(struct platform_device *pdev)
@@ -548,8 +587,10 @@ static int exynos_pmu_probe(struct platform_device *pdev)
 
 	ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE, exynos_pmu_devs,
 				   ARRAY_SIZE(exynos_pmu_devs), NULL, 0, NULL);
-	if (ret)
+	if (ret) {
+		destroy_cpuhp_and_cpuidle();
 		return ret;
+	}
 
 	if (devm_of_platform_populate(dev))
 		dev_err(dev, "Error populating children, reboot and poweroff might not work properly\n");

-- 
2.51.0

Re: [PATCH 3/3] soc: samsung: exynos-pmu: fix error paths in cpuhotplug/idle states setup

[Peter Griffin]

Hi Alexey,

Thanks for your patch!

On Fri, 5 Jun 2026 at 21:19, Alexey Klimov <alexey.klimov@linaro.org> wrote:
>
> The setup_cpuhp_and_cpuidle() initialisation sequence currently ignores
> the return values of cpuhp_setup_state(), cpu_pm_register_notifier(), and
> register_reboot_notifier(). If any of these registrations fail during
> probe() routine, the driver returns 0, leaving the driver partially
> configured.

I originally made the failure non-fatal because the system still boots
without the notifiers registered (and all other Arm64 Exynos SoCs
upstream don't register notifiers and AFAICT have broken cpu hotplug
and cpu idle).

In hindsight, that seems like a mistake. I think your patch to fully
unwind everything in case of failure makes more sense.  See small
comment below about destroy_cpuhp_and_cpuidle()

>
> Furthermore, if anything after setup_cpuhp_and_cpuidle() fails in probe()
> routine, for instance devm_mfd_add_devices(), the probe() lacks an error
> path and leaves notifiers and cpu hotplug states registered.
>
> Introduce variables for the cpu hotplug state IDs in exynos_pmu_context
> struct, that should be initialised to CPUHP_INVALID by default. Check all
> return codes in setup_cpuhp_and_cpuidle(), and add an error path to remove
> registered states on failure. Finally, add destroy_cpuhp_and_cpuidle()
> helper to safely tear down notifiers and cpu hotplug states.
>
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260513-exynos850-cpuhotplug-v4-0-54fec5f65362@linaro.org?part=3
> Fixes: 78b72897a5c8 ("soc: samsung: exynos-pmu: Enable CPU Idle for gs101")
> Cc: stable@vger.kernel.org
> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
> ---
>  drivers/soc/samsung/exynos-pmu.c | 57 ++++++++++++++++++++++++++++++++++------
>  1 file changed, 49 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/soc/samsung/exynos-pmu.c b/drivers/soc/samsung/exynos-pmu.c
> index 9636287f6794..846313a28e9a 100644
> --- a/drivers/soc/samsung/exynos-pmu.c
> +++ b/drivers/soc/samsung/exynos-pmu.c
> @@ -38,6 +38,8 @@ struct exynos_pmu_context {
>         unsigned long *in_cpuhp;
>         bool sys_insuspend;
>         bool sys_inreboot;
> +       int cpuhp_prepare_state;
> +       int cpuhp_online_state;
>  };
>
>  void __iomem *pmu_base_addr;
> @@ -404,6 +406,17 @@ static struct notifier_block exynos_cpupm_reboot_nb = {
>         .notifier_call = exynos_cpupm_reboot_notifier,
>  };
>
> +static void destroy_cpuhp_and_cpuidle(void)
> +{
> +       cpu_pm_unregister_notifier(&gs101_cpu_pm_notifier);
> +       unregister_reboot_notifier(&exynos_cpupm_reboot_nb);
> +
> +       if (pmu_context->cpuhp_prepare_state != CPUHP_INVALID)
> +               cpuhp_remove_state(pmu_context->cpuhp_prepare_state);
> +       if (pmu_context->cpuhp_online_state != CPUHP_INVALID)
> +               cpuhp_remove_state(pmu_context->cpuhp_online_state);
> +}
> +
>  static int setup_cpuhp_and_cpuidle(struct device *dev)
>  {
>         struct device_node *intr_gen_node;
> @@ -465,16 +478,42 @@ static int setup_cpuhp_and_cpuidle(struct device *dev)
>                 gs101_cpuhp_pmu_online(cpu);
>
>         /* register CPU hotplug callbacks */
> -       cpuhp_setup_state(CPUHP_BP_PREPARE_DYN, "soc/exynos-pmu:prepare",
> -                         gs101_cpuhp_pmu_online, NULL);
> +       pmu_context->cpuhp_prepare_state = CPUHP_INVALID;
> +       pmu_context->cpuhp_online_state = CPUHP_INVALID;
>
> -       cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "soc/exynos-pmu:online",
> -                         NULL, gs101_cpuhp_pmu_offline);
> +       ret = cpuhp_setup_state(CPUHP_BP_PREPARE_DYN, "soc/exynos-pmu:prepare",
> +                               gs101_cpuhp_pmu_online, NULL);
> +       if (ret < 0)
> +               return ret;
> +
> +       pmu_context->cpuhp_prepare_state = ret;
> +
> +       ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "soc/exynos-pmu:online",
> +                               NULL, gs101_cpuhp_pmu_offline);
> +       if (ret < 0)
> +               goto clean_cpuhp_states;
> +
> +       pmu_context->cpuhp_online_state = ret;
>
>         /* register CPU PM notifiers for cpuidle */
> -       cpu_pm_register_notifier(&gs101_cpu_pm_notifier);
> -       register_reboot_notifier(&exynos_cpupm_reboot_nb);
> -       return 0;
> +       ret = cpu_pm_register_notifier(&gs101_cpu_pm_notifier);
> +       if (ret)
> +               goto clean_cpuhp_states;
> +
> +       ret = register_reboot_notifier(&exynos_cpupm_reboot_nb);
> +       if (!ret)
> +               /* Success */
> +               return ret;
> +
> +       cpu_pm_unregister_notifier(&gs101_cpu_pm_notifier);
> +
> +clean_cpuhp_states:
> +       if (pmu_context->cpuhp_prepare_state != CPUHP_INVALID)
> +               cpuhp_remove_state(pmu_context->cpuhp_prepare_state);
> +       if (pmu_context->cpuhp_online_state != CPUHP_INVALID)
> +               cpuhp_remove_state(pmu_context->cpuhp_online_state);
> +
> +       return ret;
>  }
>
>  static int exynos_pmu_probe(struct platform_device *pdev)
> @@ -548,8 +587,10 @@ static int exynos_pmu_probe(struct platform_device *pdev)
>
>         ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE, exynos_pmu_devs,
>                                    ARRAY_SIZE(exynos_pmu_devs), NULL, 0, NULL);
> -       if (ret)
> +       if (ret) {
> +               destroy_cpuhp_and_cpuidle();

You only want to do this if pmu_cpuhp == true, as currently only gs101
registers the notifiers.

Thanks,

Peter

Re: [PATCH 3/3] soc: samsung: exynos-pmu: fix error paths in cpuhotplug/idle states setup

[Alexey Klimov]

On Wed Jun 10, 2026 at 2:34 PM BST, Peter Griffin wrote:
> Hi Alexey,

Hi Peter,

> Thanks for your patch!
>
> On Fri, 5 Jun 2026 at 21:19, Alexey Klimov <alexey.klimov@linaro.org> wrote:
>>
>> The setup_cpuhp_and_cpuidle() initialisation sequence currently ignores
>> the return values of cpuhp_setup_state(), cpu_pm_register_notifier(), and
>> register_reboot_notifier(). If any of these registrations fail during
>> probe() routine, the driver returns 0, leaving the driver partially
>> configured.
>
> I originally made the failure non-fatal because the system still boots
> without the notifiers registered (and all other Arm64 Exynos SoCs
> upstream don't register notifiers and AFAICT have broken cpu hotplug
> and cpu idle).
>
> In hindsight, that seems like a mistake. I think your patch to fully
> unwind everything in case of failure makes more sense.  See small
> comment below about destroy_cpuhp_and_cpuidle()

Wait, setup_cpuhp_and_cpuidle() should be non-fatal and shouldn't
return any errors?
Why do we need to have notifiers (say cpu_pm_register_notifier())
registered if, for instance, cpuhp_setup_state() fails?

The other thing I didn't get is that this doesn't deal with handling
errors/return values of cpuhp_setup_state() in probe() and there
are still a lot of errors returned from setup_cpuhp_and_cpuidle().


>> Furthermore, if anything after setup_cpuhp_and_cpuidle() fails in probe()
>> routine, for instance devm_mfd_add_devices(), the probe() lacks an error
>> path and leaves notifiers and cpu hotplug states registered.
>>
>> Introduce variables for the cpu hotplug state IDs in exynos_pmu_context
>> struct, that should be initialised to CPUHP_INVALID by default. Check all
>> return codes in setup_cpuhp_and_cpuidle(), and add an error path to remove
>> registered states on failure. Finally, add destroy_cpuhp_and_cpuidle()
>> helper to safely tear down notifiers and cpu hotplug states.
>>
>> Reported-by: Sashiko <sashiko-bot@kernel.org>
>> Closes: https://sashiko.dev/#/patchset/20260513-exynos850-cpuhotplug-v4-0-54fec5f65362@linaro.org?part=3
>> Fixes: 78b72897a5c8 ("soc: samsung: exynos-pmu: Enable CPU Idle for gs101")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
>> ---
>>  drivers/soc/samsung/exynos-pmu.c | 57 ++++++++++++++++++++++++++++++++++------
>>  1 file changed, 49 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/soc/samsung/exynos-pmu.c b/drivers/soc/samsung/exynos-pmu.c
>> index 9636287f6794..846313a28e9a 100644
>> --- a/drivers/soc/samsung/exynos-pmu.c
>> +++ b/drivers/soc/samsung/exynos-pmu.c
>> @@ -38,6 +38,8 @@ struct exynos_pmu_context {
>>         unsigned long *in_cpuhp;
>>         bool sys_insuspend;
>>         bool sys_inreboot;
>> +       int cpuhp_prepare_state;
>> +       int cpuhp_online_state;
>>  };
>>
>>  void __iomem *pmu_base_addr;
>> @@ -404,6 +406,17 @@ static struct notifier_block exynos_cpupm_reboot_nb = {
>>         .notifier_call = exynos_cpupm_reboot_notifier,
>>  };
>>
>> +static void destroy_cpuhp_and_cpuidle(void)
>> +{
>> +       cpu_pm_unregister_notifier(&gs101_cpu_pm_notifier);
>> +       unregister_reboot_notifier(&exynos_cpupm_reboot_nb);
>> +
>> +       if (pmu_context->cpuhp_prepare_state != CPUHP_INVALID)
>> +               cpuhp_remove_state(pmu_context->cpuhp_prepare_state);
>> +       if (pmu_context->cpuhp_online_state != CPUHP_INVALID)
>> +               cpuhp_remove_state(pmu_context->cpuhp_online_state);
>> +}
>> +
>>  static int setup_cpuhp_and_cpuidle(struct device *dev)
>>  {
>>         struct device_node *intr_gen_node;
>> @@ -465,16 +478,42 @@ static int setup_cpuhp_and_cpuidle(struct device *dev)
>>                 gs101_cpuhp_pmu_online(cpu);
>>
>>         /* register CPU hotplug callbacks */
>> -       cpuhp_setup_state(CPUHP_BP_PREPARE_DYN, "soc/exynos-pmu:prepare",
>> -                         gs101_cpuhp_pmu_online, NULL);
>> +       pmu_context->cpuhp_prepare_state = CPUHP_INVALID;
>> +       pmu_context->cpuhp_online_state = CPUHP_INVALID;
>>
>> -       cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "soc/exynos-pmu:online",
>> -                         NULL, gs101_cpuhp_pmu_offline);
>> +       ret = cpuhp_setup_state(CPUHP_BP_PREPARE_DYN, "soc/exynos-pmu:prepare",
>> +                               gs101_cpuhp_pmu_online, NULL);
>> +       if (ret < 0)
>> +               return ret;
>> +
>> +       pmu_context->cpuhp_prepare_state = ret;
>> +
>> +       ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "soc/exynos-pmu:online",
>> +                               NULL, gs101_cpuhp_pmu_offline);
>> +       if (ret < 0)
>> +               goto clean_cpuhp_states;
>> +
>> +       pmu_context->cpuhp_online_state = ret;
>>
>>         /* register CPU PM notifiers for cpuidle */
>> -       cpu_pm_register_notifier(&gs101_cpu_pm_notifier);
>> -       register_reboot_notifier(&exynos_cpupm_reboot_nb);
>> -       return 0;
>> +       ret = cpu_pm_register_notifier(&gs101_cpu_pm_notifier);
>> +       if (ret)
>> +               goto clean_cpuhp_states;
>> +
>> +       ret = register_reboot_notifier(&exynos_cpupm_reboot_nb);
>> +       if (!ret)
>> +               /* Success */
>> +               return ret;
>> +
>> +       cpu_pm_unregister_notifier(&gs101_cpu_pm_notifier);
>> +
>> +clean_cpuhp_states:
>> +       if (pmu_context->cpuhp_prepare_state != CPUHP_INVALID)
>> +               cpuhp_remove_state(pmu_context->cpuhp_prepare_state);
>> +       if (pmu_context->cpuhp_online_state != CPUHP_INVALID)
>> +               cpuhp_remove_state(pmu_context->cpuhp_online_state);
>> +
>> +       return ret;
>>  }
>>
>>  static int exynos_pmu_probe(struct platform_device *pdev)
>> @@ -548,8 +587,10 @@ static int exynos_pmu_probe(struct platform_device *pdev)
>>
>>         ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE, exynos_pmu_devs,
>>                                    ARRAY_SIZE(exynos_pmu_devs), NULL, 0, NULL);
>> -       if (ret)
>> +       if (ret) {
>> +               destroy_cpuhp_and_cpuidle();
>
> You only want to do this if pmu_cpuhp == true, as currently only gs101
> registers the notifiers.

Thanks! That's good catch.

Best regards,
Alexey

Re: [PATCH 3/3] soc: samsung: exynos-pmu: fix error paths in cpuhotplug/idle states setup

[Peter Griffin]

Hi Alexey,

On Wed, 10 Jun 2026 at 16:07, Alexey Klimov <alexey.klimov@linaro.org> wrote:
>
> On Wed Jun 10, 2026 at 2:34 PM BST, Peter Griffin wrote:
> > Hi Alexey,
>
> Hi Peter,
>
> > Thanks for your patch!
> >
> > On Fri, 5 Jun 2026 at 21:19, Alexey Klimov <alexey.klimov@linaro.org> wrote:
> >>
> >> The setup_cpuhp_and_cpuidle() initialisation sequence currently ignores
> >> the return values of cpuhp_setup_state(), cpu_pm_register_notifier(), and
> >> register_reboot_notifier(). If any of these registrations fail during
> >> probe() routine, the driver returns 0, leaving the driver partially
> >> configured.
> >
> > I originally made the failure non-fatal because the system still boots
> > without the notifiers registered (and all other Arm64 Exynos SoCs
> > upstream don't register notifiers and AFAICT have broken cpu hotplug
> > and cpu idle).
> >
> > In hindsight, that seems like a mistake. I think your patch to fully
> > unwind everything in case of failure makes more sense.  See small
> > comment below about destroy_cpuhp_and_cpuidle()
>
> Wait, setup_cpuhp_and_cpuidle() should be non-fatal and shouldn't
> return any errors?

I suggest you re-read my above comment above ^^

Peter.

Re: [PATCH 3/3] soc: samsung: exynos-pmu: fix error paths in cpuhotplug/idle states setup

[Alexey Klimov]

Hi Peter,

On Thu Jun 11, 2026 at 8:07 AM BST, Peter Griffin wrote:
> Hi Alexey,
>
> On Wed, 10 Jun 2026 at 16:07, Alexey Klimov <alexey.klimov@linaro.org> wrote:
>>
>> On Wed Jun 10, 2026 at 2:34 PM BST, Peter Griffin wrote:
>> > Hi Alexey,
>>
>> Hi Peter,
>>
>> > Thanks for your patch!
>> >
>> > On Fri, 5 Jun 2026 at 21:19, Alexey Klimov <alexey.klimov@linaro.org> wrote:
>> >>
>> >> The setup_cpuhp_and_cpuidle() initialisation sequence currently ignores
>> >> the return values of cpuhp_setup_state(), cpu_pm_register_notifier(), and
>> >> register_reboot_notifier(). If any of these registrations fail during
>> >> probe() routine, the driver returns 0, leaving the driver partially
>> >> configured.
>> >
>> > I originally made the failure non-fatal because the system still boots
>> > without the notifiers registered (and all other Arm64 Exynos SoCs
>> > upstream don't register notifiers and AFAICT have broken cpu hotplug
>> > and cpu idle).
>> >
>> > In hindsight, that seems like a mistake. I think your patch to fully
>> > unwind everything in case of failure makes more sense.  See small
>> > comment below about destroy_cpuhp_and_cpuidle()
>>
>> Wait, setup_cpuhp_and_cpuidle() should be non-fatal and shouldn't
>> return any errors?
>
> I suggest you re-read my above comment above ^^

Could you please clarify what specifically addresses my question about
notifiers?

Looking further into this, it seems that, for instance, if one of the
hotplug states fails to register then tracking of pmu_context->in_cpuhp
becomes broken.
If reboot notifier silently fails to be registered, then it is unclear how
this from gs101_cpu_pmu_offline() supposed to work:

/* Ignore CPU_PM_ENTER event in reboot or suspend sequence. */
if (pmu_context->sys_insuspend || pmu_context->sys_inreboot) {
	raw_spin_unlock(&pmu_context->cpupm_lock);
	return NOTIFY_OK;
}

If c2 idles are used during reboot/shutdown then they fail or what?

I am not saying that patch is correct and some rework is needed but I don't
get why we should completely ignore errors from hotplug states registration
and should not check registration of notifiers. At least warning should be
shown to user that pm functionality might be unreliable.

Best regards,
Alexey.

Re: [PATCH 3/3] soc: samsung: exynos-pmu: fix error paths in cpuhotplug/idle states setup

[Peter Griffin]

Hi Alexey,

On Mon, 22 Jun 2026 at 19:57, Alexey Klimov <alexey.klimov@linaro.org> wrote:
[..]
> >> >
> >> > I originally made the failure non-fatal because the system still boots
> >> > without the notifiers registered (and all other Arm64 Exynos SoCs
> >> > upstream don't register notifiers and AFAICT have broken cpu hotplug
> >> > and cpu idle).
> >> >
> >> > In hindsight, that seems like a mistake. I think your patch to fully
> >> > unwind everything in case of failure makes more sense.  See small
> >> > comment below about destroy_cpuhp_and_cpuidle()
> >>
> >> Wait, setup_cpuhp_and_cpuidle() should be non-fatal and shouldn't
> >> return any errors?
> >
> > I suggest you re-read my above comment above ^^
>
> Could you please clarify what specifically addresses my question about
> notifiers?

Sure, I was referring to this part of my previous reply:

> >> > In hindsight, that seems like a mistake. I think your patch to fully
> >> > unwind everything in case of failure makes more sense.

[..]
>
> If c2 idles are used during reboot/shutdown then they fail or what?

This followed similar logic to the Samsung downstream kernel drivers.
I have no extra information about it beyond the downstream kernel
source. It seemed reasonable though that CPU's will be hotplugged
during suspend and reboot so you may wish to ignore these.

The proper solution of course is a fully PSCI compliant firmware,
which doesn't require these side channel hints.

>
> I am not saying that patch is correct and some rework is needed but I don't
> get why we should completely ignore errors from hotplug states registration
> and should not check registration of notifiers. At least warning should be
> shown to user that pm functionality might be unreliable.

As mentioned above, and in my previous reply, I think your proposed
patch is a good idea.

regards,

Peter