On 23.01.16 17:39:20, Hanjun Guo wrote:
> @@ -385,10 +386,8 @@ void __init arm64_numa_init(void)
> {
> int ret = -ENODEV;
>
> -#ifdef CONFIG_OF_NUMA
> if (!numa_off)
> - ret = numa_init(arm64_of_numa_init);
> -#endif
> + ret = numa_init(acpi_disabled ? arm64_of_numa_init : arm64_acpi_numa_init);
>
> if (ret)
> numa_init(dummy_numa_init);
Ok, this style is mostly flavor, some people want #ifdefs (my
preference), some not. In any case it must build with or without the
config option set. But first some words why I like #ifdefs:
* Code is easier to understand as you don't need to look at any other
location whether it is enabled or not.
* You can't break the build if the options are not set. Thus, you
also don't need to check if the function is implemented for the
unset case (valid for the coder and also the reviewer). This makes
things a lot easier.
* Total number of lines of code that needs to be implement is
smaller.
However, if we don't ifdef the code, we need empty functions stubs in
the header file for them.
Also, the conditional assignment does not reduce the complexity of the
paths. It just concentrates everything in a single line.
How about the following (similar to x86)?
----
if (!numa_off) {
#ifdef CONFIG_ACPI_NUMA
if (!numa_init(acpi_numa_init))
return 0;
#endif
#ifdef CONFIG_OF_NUMA
if (!numa_init(of_numa_init))
return 0;
#endif
}
return numa_init(dummy_numa_init);
----
Pretty straight and nice.
Note: The !acpi_disabled check needs to be moved to the beginning of
acpi_numa_init(). Variable ret can be removed.
-Robert
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
