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

["Alexey Klimov" <alexey.klimov@linaro.org>]

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