Re: [PATCH v5 1/3] ARM: EXYNOS: Move code from hotplug.c to platsmp.c

From: Tomasz Figa
Date: Tue Aug 05 2014 - 15:53:12 EST


On 05.08.2014 17:03, Krzysztof Kozlowski wrote:
> On wto, 2014-08-05 at 16:46 +0200, Daniel Lezcano wrote:
>> On 08/05/2014 12:43 PM, Krzysztof Kozlowski wrote:
>>> Cleanup a little the SMP/hotplug code for Exynos by:
>>> 1. Moving completely all functions from hotplug.c into the platsmp.c;
>>> 2. Deleting the hotplug.c file.
>>>
>>> After recent cleanups (e.g. 75ad2ab28f0f "ARM: EXYNOS: use
>>> v7_exit_coherency_flush macro for cache disabling") there was only CPU
>>> power down related code in hotplug.c file. Keeping this file does not
>>> give any benefits.
>>>
>>> The commit only moves code around with one additional observable change:
>>> the hotplug.c was compiled with custom CFLAGS (-march=armv7-a). These
>>> CFLAGS are not necessary any more.
>>
>> What is the benefit of moving this code around ?
>>
>> Usually the cpu hotplug code is located in hotplug.c for most of the
>> platforms, so why moving pm code inside platsmp ?
>
> The benefits are:
>
> 1. Less dependencies, cleaner code because platsmp.c is the only user of
> code located in hotplug.c. So the exynos_cpu_die() had to be declared in
> common.h.
>
> 2. Less dependencies between modules after adding patch 3/3 [1] which
> introduces exynos_set_delayed_reset_assertion() function. The
> exynos_set_delayed_reset_assertion() is called by:
> - cpu_leave_power (hotplug.c)
> - platform_do_lowpower (hotplug.c)
> - exynos_boot_secondary (platsmp.c)
>
> so merging everything into one file seemed simpler and cleaner.

mach-exynos/hotplug.c does not have too much code right now and as
Krzysztof noticed the only user of functions from it is platsmp.c
anyway. We don't need to follow other machs in something that makes no
sense, so

Acked-by: Tomasz Figa <t.figa@xxxxxxxxxxx>

Best regards,
Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/