[U-Boot] [PATCH v2 2/7] spl: nand: rename the SYS_NAND_U_BOOT_OFFS Kconfig option

[Scott Wood <oss@buserror.net>]

On Sat, 2016-06-04 at 08:06 +0200, Boris Brezillon wrote:
> On Fri, 03 Jun 2016 20:08:49 -0500
> Scott Wood <oss@buserror.net> wrote:
>
> > 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 do that, then that zero will override CONFIG_SYS_NAND_U_BOOT_OFFS from
the header.

> > 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.

Zero *is* a valid value.  Several boards already have that value for this
symbol.  Even if that weren't the case,  we want a mechanism for migrating
from header value to kconfig value that works for more than just this one
specific symbol.

> 
> > 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?

Then the build fails with a redefined symbol, and the user learns their
lesson. :-)

The "SYS" in CONFIG_SYS means it's not a user-tunable knob.  From README:

> There are two classes of configuration variables:
> 
> * Configuration _OPTIONS_:
>   These are selectable by the user and have names beginning with
>   "CONFIG_".
> 
> * Configuration _SETTINGS_:
>   These depend on the hardware etc. and should not be meddled with if
>   you don't know what you're doing; they have names beginning with
>   "CONFIG_SYS_".

-Scott