[PATCH] ARM: EXYNOS: move exynos_pm_init into pm.c and remove init_late hook

[pankaj.dubey]

Hi Krzysztof,

On Saturday 10 December 2016 04:25 PM, Krzysztof Kozlowski wrote:
> On Fri, Dec 09, 2016 at 04:01:17PM +0530, Pankaj Dubey wrote:
>> We can safely move exynos_pm_init into pm.c as late_initcall and remove
>> init_late hook from exynos.c. This will remove extern declarations from
>> common.h and move PM specific operations in pm.c rather being scattered
>> across many files.
>>
>> Signed-off-by: Pankaj Dubey <pankaj.dubey at samsung.com>
>> ---
>>  arch/arm/mach-exynos/common.h  |  6 ------
>>  arch/arm/mach-exynos/exynos.c  | 10 ----------
>>  arch/arm/mach-exynos/suspend.c | 13 ++++++++++---
>>  3 files changed, 10 insertions(+), 19 deletions(-)
>>
>> diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
>> index fb12d11..cfd55ba 100644
>> --- a/arch/arm/mach-exynos/common.h
>> +++ b/arch/arm/mach-exynos/common.h
>> @@ -134,12 +134,6 @@ void exynos_clear_boot_flag(unsigned int cpu, unsigned int mode);
>>  
>>  extern u32 exynos_get_eint_wake_mask(void);
>>  
>> -#ifdef CONFIG_PM_SLEEP
>> -extern void __init exynos_pm_init(void);
>> -#else
>> -static inline void exynos_pm_init(void) {}
>> -#endif
>> -
>>  extern void exynos_cpu_resume(void);
>>  extern void exynos_cpu_resume_ns(void);
>>  
>> diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
>> index fa08ef9..040ea66 100644
>> --- a/arch/arm/mach-exynos/exynos.c
>> +++ b/arch/arm/mach-exynos/exynos.c
>> @@ -58,15 +58,6 @@ void __init exynos_sysram_init(void)
>>  	}
>>  }
>>  
>> -static void __init exynos_init_late(void)
>> -{
>> -	if (of_machine_is_compatible("samsung,exynos5440"))
>> -		/* to be supported later */
>> -		return;
>> -
>> -	exynos_pm_init();
>> -}
>> -
>>  static int __init exynos_fdt_map_chipid(unsigned long node, const char *uname,
>>  					int depth, void *data)
>>  {
>> @@ -216,7 +207,6 @@ DT_MACHINE_START(EXYNOS_DT, "SAMSUNG EXYNOS (Flattened Device Tree)")
>>  	.init_early	= exynos_firmware_init,
>>  	.init_irq	= exynos_init_irq,
>>  	.init_machine	= exynos_dt_machine_init,
>> -	.init_late	= exynos_init_late,
>>  	.dt_compat	= exynos_dt_compat,
>>  	.dt_fixup	= exynos_dt_fixup,
>>  MACHINE_END
>> diff --git a/arch/arm/mach-exynos/suspend.c b/arch/arm/mach-exynos/suspend.c
>> index 73df9f3..f318b08 100644
>> --- a/arch/arm/mach-exynos/suspend.c
>> +++ b/arch/arm/mach-exynos/suspend.c
>> @@ -698,21 +698,25 @@ static const struct of_device_id exynos_pmu_of_device_ids[] __initconst = {
>>  
>>  static struct syscore_ops exynos_pm_syscore_ops;
>>  
>> -void __init exynos_pm_init(void)
>> +static int __init exynos_pm_init(void)
>>  {
>>  	const struct of_device_id *match;
>>  	struct device_node *np;
>>  	u32 tmp;
>>  
>> +	if (of_machine_is_compatible("samsung,exynos5440"))
>> +		/* to be supported later */
>> +		return 0;
>> +
>>  	np = of_find_matching_node_and_match(NULL, exynos_pmu_of_device_ids, &match);
>>  	if (!np) {
>>  		pr_err("Failed to find PMU node\n");
>> -		return;
>> +		return -ENODEV;
>>  	}
>>  
>>  	if (WARN_ON(!of_find_property(np, "interrupt-controller", NULL))) {
>>  		pr_warn("Outdated DT detected, suspend/resume will NOT work\n");
>> -		return;
>> +		return -ENODEV;
>>  	}
>>  
>>  	pm_data = (const struct exynos_pm_data *) match->data;
>> @@ -727,4 +731,7 @@ void __init exynos_pm_init(void)
>>  
>>  	register_syscore_ops(&exynos_pm_syscore_ops);
>>  	suspend_set_ops(&exynos_suspend_ops);
>> +
>> +	return 0;
>>  }
>> +late_initcall(exynos_pm_init);
> 
> No. This does not look like multiplatform friendly. Also, basically you are
> reverting 559ba237999d7 without clear explanation of revert itself.
> 

Thanks for review.

I could not understand why this change is not multi-platform friendly,
would you please elaborate more on this.

Well I missed to check history of this file before sending this patch,
as I could not sense any issue as such, we are calling exynos_pm_init
from exynos_init_late which is infact gets called as part of
late_initcall itself. I have tested this with multi_v7_defconfig.

When Thomasz submitted this patch 559ba237999d7 basically there were two
arch_initcalls as "exynos_pm_drvinit" and "exynos_pm_drvinit", the
second one he renamed to exynos_pm_init. At the same time he removed
arch_initcall and made provision so that it can be called from
exynos_init_late. Probably he did because there were two arch_initcalls.
Still I am not sure why he did not opt to convert one of them from
arch_initcall to late_initcall.. how this change affects multiplatform?

As far as intention of this patch, slowly I wanted to reduce dependency
of common.h from pm.c and suspend.c so that one day all these
functionalities which are tightly coupled with machine files can be
loosen and these files can reside along with pmu driver in
"drivers/soc/samsung/".

Thanks,
Pankaj Dubey
> Best regards,
> Krzysztof
> 
> 
>