On Sat, 04 Jun 2016 02:14:09 -0500 Scott Wood <[email protected]> wrote:
> On Sat, 2016-06-04 at 08:06 +0200, Boris Brezillon wrote: > > On Fri, 03 Jun 2016 20:08:49 -0500 > > Scott Wood <[email protected]> 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. Nope, because the condition is #ifndef CONFIG_SYS_NAND_U_BOOT_OFFS #define CONFIG_SYS_NAND_U_BOOT_OFFS CONFIG_SPL_NAND_U_BOOT_OFFS #endif and not #ifdef CONFIG_SPL_NAND_U_BOOT_OFFS #define CONFIG_SYS_NAND_U_BOOT_OFFS CONFIG_SPL_NAND_U_BOOT_OFFS #endif So CONFIG_SYS_NAND_U_BOOT_OFFS is always preferred over CONFIG_SPL_NAND_U_BOOT_OFFS if it's defined. > > > > 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. Sure, 0 is a perfectly valid value. The "default 0" is just here to prevent make from blocking because of a missing definition in the _defconfig. > > > > > > 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. :-) Fair enough. > > 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_". Okay. I'll switch back to SYS_NAND_U_BOOT_OFFS, add an intermediate option to unlock this one in the config menu and rename CONFIG_SPL_NAND_U_BOOT_OFFS_REDUND into CONFIG_SYS_NAND_U_BOOT_OFFS_REDUND. -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot

