On Fri, 03 Jun 2016 20:08:49 -0500 Scott Wood oss@buserror.net wrote:
On Wed, 2016-06-01 at 13:23 +0200, Boris Brezillon wrote:
The SYS_NAND_U_BOOT_OFFS is quite generic, but the Kconfig entry is forced to explicitly depend on platforms that are not already defining it in their include/configs/<board>.h header.
Rename this Kconfig option into SPL_NAND_U_BOOT_OFFS, remove the dependency on NAND_SUNXI and make it dependent on SPL selection.
common/spl/spl_nand.c now sets CONFIG_SYS_NAND_U_BOOT_OFFS to CONFIG_SPL_NAND_U_BOOT_OFFS only when it's undefined. This way we stay compatible with the existing behavior.
Signed-off-by: Boris Brezillon boris.brezillon@free-electrons.com Acked-by: Hans de Goede hdegoede@redhat.com
common/spl/spl_nand.c | 4 ++++ drivers/mtd/nand/Kconfig | 9 +++++---- drivers/mtd/nand/sunxi_nand_spl.c | 8 ++++---- 3 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/common/spl/spl_nand.c b/common/spl/spl_nand.c index bbd9546..612bd4a 100644 --- a/common/spl/spl_nand.c +++ b/common/spl/spl_nand.c @@ -10,6 +10,10 @@ #include <asm/io.h> #include <nand.h>
+#ifndef CONFIG_SYS_NAND_U_BOOT_OFFS +#define CONFIG_SYS_NAND_U_BOOT_OFFS CONFIG_SPL_NAND_U_BOOT_OFFS +#endif
[snip]
-# Enhance depends when converting drivers to Kconfig which use this config -config SYS_NAND_U_BOOT_OFFS +if SPL
+# This option should be used in replacement of CONFIG_SYS_NAND_U_BOOT_OFFS. +# CONFIG_SYS_NAND_U_BOOT_OFFS is still preferred if defined. +config SPL_NAND_U_BOOT_OFFS hex "Location in NAND to read U-Boot from" default 0x8000 if NAND_SUNXI
- depends on NAND_SUNXI help Set the offset from the start of the nand where u-boot should be loaded from.
This doesn't work. CONFIG_SPL_NAND_U_BOOT_OFFS will always be defined when SPL is defined, and the user will be forced to enter a value before kconfig will continue (or kconfig will error out in an automated build).
Yes, CONFIG_SPL_NAND_U_BOOT_OFFS will always be defined, but won't be used if CONFIG_SYS_NAND_U_BOOT_OFFS is defined in the config header file. And for the "user will forced to enter a value before Kconfig continue" comment, we could just have
config SPL_NAND_U_BOOT_OFFS hex "Location in NAND to read U-Boot from" default 0x8000 if NAND_SUNXI default 0x0 ...
If you want to do this there needs to be a separate bool config that controls whether the hex config exists.
I can add an extra Kconfig option, but is it really necessary: if people are relying on it they will choose a valid value, and leave it to 0 otherwise. It's just a detail, so I'm fine adding this extra option if you think it's really useful.
And there'd be no need to rename hex symbol.
Well, functionally there's no problem keeping the existing SYS_ prefix if we add this extra option to activate the U_OFFS config in Kconfig, but I'm not sure this is a good idea to reuse config header names in Kconfig.
And what happens if the user enabled this option (some like to enable everything :-)) and CONFIG_SYS_NAND_U_BOOT_OFFS is also defined in the board config header? That's what I was trying to avoid. By renaming the Kconfig option we make sure the value defined in include/configs/<board>.h always take precedence over the Kconfig value.