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

From: Alexey Klimov

Date: Tue Sep 29 2026 - 19:54:42 EST


On Mon Sep 7, 2026 at 11:09 AM BST, Krzysztof Kozlowski wrote:
> On 28/08/2026 07:36, Alexey Klimov wrote:
>> void __iomem *pmu_base_addr;
>> @@ -407,6 +409,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);
>> +}
>
> cleanup follows the setup usually, so this function should be after
> setup_cpuhp_and_cpuidle().

Thanks. I sent new version where this is updated.

>> +
>> static int setup_cpuhp_and_cpuidle(struct device *dev)
>> {
>> struct device_node *intr_gen_node __free(device_node) =
>> @@ -458,16 +471,46 @@ 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;
>> +
>> + 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;
>
> You have only one state to clean here, no? Error paths must be specific
> - clean only what's needed, not a catch-all with if-checks.

Heh, I thought it will more readable that way. Yes, one state.
I reworked this error path in new version v3 that I sent.

Thanks,
Alexey