[PATCH v3 00/23] CONFIG_IS_ENABLED vs IS_ENABLED

This patch set gets ready to checks the usage of CONFIG_IS_ENABLED/IS_ENABLED.
After the set has been applied, you can delete test/usage_of_is_enabled_todo.txt and run test/usage_of_is_enabled_commit.sh
The script test/usage_of_is_enabled_check.sh checks for new questionable uses of CONFIG_IS_ENABLED/IS_ENABLED and is added to .azure-pipelines.yml, and .gitlab-ci.yml
version 3 changes: Dropped changes to puma-rk3399 and ringneck-px30 in favor of Quentin Schulz's patch https://patchwork.ozlabs.org/project/uboot/patch/20230301-tsd-env-nowhere-kc... https://patchwork.ozlabs.org/project/uboot/patch/20230301-tsd-env-nowhere-kc... Please apply them before this series.
Drop gateworks: venice: Always define setup_fec and setup_eqos Drop arm: cpu: armv7: ls102xa: fdt: remove eth_device support Simon's patches took care of these
Add to todo list, these new questionable uses that snuck into the code. PARTITION_TYPE_GUID PHY_ATHEROS RTC_SANDBOX
Changes in v3: remove error entirely and prevent with Kconfig new patch to address Tom's concerns - Rebase on Simon's s/CMD_SATA/SATA/ change - commit message updated
Changes in v2: - new patch - delay include of linux/kconfig.h to do from Makefile - as suggested by Simon - delay include of linux/kconfig.h to do from Makefile - as suggested by Simon - delay include of linux/kconfig.h to do from Makefile - as suggested by Simon - delay include of linux/kconfig.h to do from Makefile - as suggested by Simon - delay include of linux/kconfig.h to do from Makefile - as suggested by Simon - include linux/kconfig.h from tools/Makefile - as suggested by Simon - changed condition of when to include field bdf - added protection to another instance of bdf in uart.c - Thanks to Simon for getting this corrected - use normal if, not preprocessor - new in series - use an accessor function gd_set_pci_ram_top
Troy Kisky (23): kconfig: add IS_ENABLED_NOCHECK to bypass usage_of_is_enabled_check cmd: nvedit: remove error check, handle with Kconfig lib: crc32: prepare for CONFIG_IS_ENABLED changes lib: md5: prepare for CONFIG_IS_ENABLED changes lib: sha1: prepare for CONFIG_IS_ENABLED changes lib: sha256: prepare for CONFIG_IS_ENABLED changes lib: sha512: prepare for CONFIG_IS_ENABLED changes watchdog: add and use Kconfig HAS_WATCHDOG_RUNNING tools: prevent CONFIG_IS_ENABLED errors by including linux/kconfig.h tools: Makefile: prepare for CONFIG_IS_ENABLED changes by adding CONFIG_TOOLS_xxx x86: cpu: qemu: qemu: remove SPL use with CONFIG_IS_ENABLED config_distro_bootcmd: remove booting environment variables from SPL environment ofnode: fdt_support definitions needed if OF_CONTROL is enabled fdt_support: always define fdt_fixup_mtdparts m53menlo: define ft_board_setup only if CONFIG_IS_ENABLED(OF_LIBFDT) freescale: common: pfuze: define pfuze_mode_init only if defined(CONFIG_DM_PMIC) ns16550: match when to define bdf with uart code solidrun: mx6cuboxi: use CONFIG_IS_ENABLED(SATA) instead of ifdef CONFIG_SATA wandboard: use CONFIG_IS_ENABLED(SATA) instead of ifdef CONFIG_SATA arm: mach-imx: use CONFIG_$(SPL_)SATA instead of CONFIG_SATA x86: cpu: i386: cpu: only set pci_ram_top if CONFIG_IS_ENABLED(PCI) power: pmic: add dm style definitions if not CONFIG_IS_ENABLED(POWER_LEGACY) CI: add test/usage_of_is_enabled_check.sh
.azure-pipelines.yml | 11 ++ .gitlab-ci.yml | 5 + arch/arm/mach-imx/Makefile | 2 +- arch/arm/mach-omap2/boot-common.c | 5 +- arch/m68k/lib/time.c | 7 +- arch/powerpc/lib/interrupts.c | 5 +- arch/powerpc/lib/ticks.S | 2 +- arch/x86/cpu/apollolake/uart.c | 4 + arch/x86/cpu/i386/cpu.c | 2 +- arch/x86/cpu/qemu/qemu.c | 2 +- board/astro/mcf5373l/fpga.c | 10 +- board/freescale/common/pfuze.c | 2 +- board/menlo/m53menlo/m53menlo.c | 2 + board/solidrun/mx6cuboxi/mx6cuboxi.c | 5 +- board/wandboard/wandboard.c | 5 +- boot/image-board.c | 2 +- cmd/nvedit.c | 32 +--- cmd/ximg.c | 10 +- common/board_f.c | 4 +- common/spl/Kconfig | 4 + drivers/crypto/aspeed/aspeed_hace.c | 2 +- drivers/crypto/hash/hash_sw.c | 2 +- drivers/timer/mpc83xx_timer.c | 5 +- drivers/watchdog/Kconfig | 3 + env/Kconfig | 17 ++- fs/cramfs/uncompress.c | 9 +- include/asm-generic/global_data.h | 6 + include/config_distro_bootcmd.h | 23 +++ include/fdt_support.h | 26 ++-- include/linux/kconfig.h | 5 + include/ns16550.h | 2 +- include/power/pmic.h | 2 +- include/watchdog.h | 7 +- lib/bzip2/bzlib.c | 5 +- lib/bzip2/bzlib_decompress.c | 20 +-- lib/crc32.c | 11 +- lib/md5.c | 7 +- lib/sha1.c | 7 +- lib/sha256.c | 7 +- lib/sha512.c | 11 +- test/usage_of_is_enabled_check.sh | 19 +++ test/usage_of_is_enabled_commit.sh | 12 ++ test/usage_of_is_enabled_correct.sh | 50 +++++++ test/usage_of_is_enabled_exempt.txt | 9 ++ test/usage_of_is_enabled_list.sh | 86 +++++++++++ test/usage_of_is_enabled_splcfg.txt | 21 +++ test/usage_of_is_enabled_todo.txt | 213 +++++++++++++++++++++++++++ tools/Makefile | 3 + 48 files changed, 584 insertions(+), 127 deletions(-) create mode 100755 test/usage_of_is_enabled_check.sh create mode 100755 test/usage_of_is_enabled_commit.sh create mode 100755 test/usage_of_is_enabled_correct.sh create mode 100644 test/usage_of_is_enabled_exempt.txt create mode 100755 test/usage_of_is_enabled_list.sh create mode 100644 test/usage_of_is_enabled_splcfg.txt create mode 100644 test/usage_of_is_enabled_todo.txt

This is for use when a config with an SPL version needs to always check the non-spl verion of the config. It avoids error messages from CI test script usage_of_is_enabled_check.sh
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com ---
(no changes since v2)
Changes in v2: - new patch
include/linux/kconfig.h | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h index 2bc704e1104..19b71723ab3 100644 --- a/include/linux/kconfig.h +++ b/include/linux/kconfig.h @@ -27,6 +27,11 @@ * 0 otherwise. */ #define IS_ENABLED(option) config_enabled(option, 0) +/* + * Using IS_ENABLED_NOCHECK instead of IS_ENABLED prevents + * complaints from test/usage_of_is_enabled_check.sh + */ +#define IS_ENABLED_NOCHECK(option) config_enabled(option, 0)
/* * U-Boot add-on: Helper macros to reference to different macros (prefixed by

Avoid error messages when SPL,TPL,VPL build don't have the environment options of the main build. This is needed when defined(CONFIG_ENV_IS_IN_xxx) is changed to CONFIG_IS_ENABLED(ENV_IS_IN_xxx).
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com ---
Changes in v3: remove error entirely and prevent with Kconfig
cmd/nvedit.c | 32 +++++--------------------------- env/Kconfig | 17 ++++++++++------- 2 files changed, 15 insertions(+), 34 deletions(-)
diff --git a/cmd/nvedit.c b/cmd/nvedit.c index 7cbc3fd573a..96bbf1904b1 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -43,28 +43,6 @@
DECLARE_GLOBAL_DATA_PTR;
-#if defined(CONFIG_ENV_IS_IN_EEPROM) || \ - defined(CONFIG_ENV_IS_IN_FLASH) || \ - defined(CONFIG_ENV_IS_IN_MMC) || \ - defined(CONFIG_ENV_IS_IN_FAT) || \ - defined(CONFIG_ENV_IS_IN_EXT4) || \ - defined(CONFIG_ENV_IS_IN_NAND) || \ - defined(CONFIG_ENV_IS_IN_NVRAM) || \ - defined(CONFIG_ENV_IS_IN_ONENAND) || \ - defined(CONFIG_ENV_IS_IN_SPI_FLASH) || \ - defined(CONFIG_ENV_IS_IN_REMOTE) || \ - defined(CONFIG_ENV_IS_IN_UBI) - -#define ENV_IS_IN_DEVICE - -#endif - -#if !defined(ENV_IS_IN_DEVICE) && \ - !defined(CONFIG_ENV_IS_NOWHERE) -# error Define one of CONFIG_ENV_IS_IN_{EEPROM|FLASH|MMC|FAT|EXT4|\ -NAND|NVRAM|ONENAND|SATA|SPI_FLASH|REMOTE|UBI} or CONFIG_ENV_IS_NOWHERE -#endif - /* * Maximum expected input data size for import command */ @@ -596,7 +574,7 @@ static int do_env_edit(struct cmd_tbl *cmdtp, int flag, int argc, } #endif /* CONFIG_CMD_EDITENV */
-#if defined(CONFIG_CMD_SAVEENV) && defined(ENV_IS_IN_DEVICE) +#if defined(CONFIG_CMD_SAVEENV) && !IS_ENABLED(CONFIG_ENV_IS_DEFAULT) static int do_env_save(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { @@ -1108,7 +1086,7 @@ static int do_env_info(struct cmd_tbl *cmdtp, int flag, int eval_flags = 0; int eval_results = 0; bool quiet = false; -#if defined(CONFIG_CMD_SAVEENV) && defined(ENV_IS_IN_DEVICE) +#if defined(CONFIG_CMD_SAVEENV) && !IS_ENABLED(CONFIG_ENV_IS_DEFAULT) enum env_location loc; #endif
@@ -1151,7 +1129,7 @@ static int do_env_info(struct cmd_tbl *cmdtp, int flag,
/* evaluate whether environment can be persisted */ if (eval_flags & ENV_INFO_IS_PERSISTED) { -#if defined(CONFIG_CMD_SAVEENV) && defined(ENV_IS_IN_DEVICE) +#if defined(CONFIG_CMD_SAVEENV) && !IS_ENABLED(CONFIG_ENV_IS_DEFAULT) loc = env_get_location(ENVOP_SAVE, gd->env_load_prio); if (ENVL_NOWHERE != loc && ENVL_UNKNOWN != loc) { if (!quiet) @@ -1232,7 +1210,7 @@ static struct cmd_tbl cmd_env_sub[] = { #if defined(CONFIG_CMD_RUN) U_BOOT_CMD_MKENT(run, CONFIG_SYS_MAXARGS, 1, do_run, "", ""), #endif -#if defined(CONFIG_CMD_SAVEENV) && defined(ENV_IS_IN_DEVICE) +#if defined(CONFIG_CMD_SAVEENV) && !IS_ENABLED(CONFIG_ENV_IS_DEFAULT) U_BOOT_CMD_MKENT(save, 1, 0, do_env_save, "", ""), #if defined(CONFIG_CMD_ERASEENV) U_BOOT_CMD_MKENT(erase, 1, 0, do_env_erase, "", ""), @@ -1323,7 +1301,7 @@ static char env_help_text[] = #if defined(CONFIG_CMD_RUN) "env run var [...] - run commands in an environment variable\n" #endif -#if defined(CONFIG_CMD_SAVEENV) && defined(ENV_IS_IN_DEVICE) +#if defined(CONFIG_CMD_SAVEENV) && !IS_ENABLED(CONFIG_ENV_IS_DEFAULT) "env save - save environment\n" #if defined(CONFIG_CMD_ERASEENV) "env erase - erase environment\n" diff --git a/env/Kconfig b/env/Kconfig index 2bbe4c466a6..7342397e169 100644 --- a/env/Kconfig +++ b/env/Kconfig @@ -55,20 +55,23 @@ config ENV_MAX_ENTRIES be generous and should work in most cases. This setting can be used to tune behaviour; see lib/hashtable.c for details.
-config ENV_IS_NOWHERE - bool "Environment is not stored" - default y if !ENV_IS_IN_EEPROM && !ENV_IS_IN_EXT4 && \ +config ENV_IS_DEFAULT + def_bool y if !ENV_IS_IN_EEPROM && !ENV_IS_IN_EXT4 && \ !ENV_IS_IN_FAT && !ENV_IS_IN_FLASH && \ !ENV_IS_IN_MMC && !ENV_IS_IN_NAND && \ !ENV_IS_IN_NVRAM && !ENV_IS_IN_ONENAND && \ !ENV_IS_IN_REMOTE && !ENV_IS_IN_SPI_FLASH && \ !ENV_IS_IN_UBI + select ENV_IS_NOWHERE + +config ENV_IS_NOWHERE + bool "Environment is not stored" help - Define this if you don't want to or can't have an environment stored + Define this if you don't care whether or not an environment is stored on a storage medium. In this case the environment will still exist - while U-Boot is running, but once U-Boot exits it will not be - stored. U-Boot will therefore always start up with a default - environment. + while U-Boot is running, but once U-Boot exits it may not be + stored. If no other ENV_IS_IN_ is defined, U-Boot will always start + up with the default environment.
config ENV_IS_IN_EEPROM bool "Environment in EEPROM"

On Mon, Mar 13, 2023 at 02:31:24PM -0700, Troy Kisky wrote:
Avoid error messages when SPL,TPL,VPL build don't have the environment options of the main build. This is needed when defined(CONFIG_ENV_IS_IN_xxx) is changed to CONFIG_IS_ENABLED(ENV_IS_IN_xxx).
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com
Applied to u-boot/next, thanks!

We need to include <linux/kconfig.h> in order to include files that use CONFIG_IS_ENABLED. TO prepare for that be more direct with using defined(USE_HOSTCC).
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com Reviewed-by: Simon Glass sjg@chromium.org ---
(no changes since v2)
Changes in v2: - delay include of linux/kconfig.h to do from Makefile - as suggested by Simon
lib/crc32.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/lib/crc32.c b/lib/crc32.c index aa94d70ef3e..12c104c62a4 100644 --- a/lib/crc32.c +++ b/lib/crc32.c @@ -14,11 +14,14 @@ #else #include <common.h> #include <efi_loader.h> +#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) +#define PET_WDG +#endif #endif #include <compiler.h> #include <u-boot/crc.h>
-#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) +#ifdef PET_WDG #include <watchdog.h> #endif #include "u-boot/zlib.h" @@ -84,7 +87,7 @@ static void __efi_runtime make_crc_table(void) } crc_table_empty = 0; } -#elif !defined(CONFIG_ARM64_CRC32) +#elif !defined(CONFIG_ARM64_CRC32) || defined(USE_HOSTCC) /* ======================================================================== * Table of CRC-32's of all single-byte values (made by make_crc_table) */ @@ -184,7 +187,7 @@ const uint32_t * ZEXPORT get_crc_table() */ uint32_t __efi_runtime crc32_no_comp(uint32_t crc, const Bytef *buf, uInt len) { -#ifdef CONFIG_ARM64_CRC32 +#if defined(CONFIG_ARM64_CRC32) && !defined(USE_HOSTCC) crc = cpu_to_le32(crc); while (len--) crc = __builtin_aarch64_crc32b(crc, *buf++); @@ -243,7 +246,7 @@ uint32_t __efi_runtime crc32(uint32_t crc, const Bytef *p, uInt len) uint32_t crc32_wd(uint32_t crc, const unsigned char *buf, uInt len, uInt chunk_sz) { -#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) +#ifdef PET_WDG const unsigned char *end, *curr; int chunk;

On Mon, Mar 13, 2023 at 02:31:25PM -0700, Troy Kisky wrote:
We need to include <linux/kconfig.h> in order to include files that use CONFIG_IS_ENABLED. TO prepare for that be more direct with using defined(USE_HOSTCC).
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com Reviewed-by: Simon Glass sjg@chromium.org
(no changes since v2)
Changes in v2:
- delay include of linux/kconfig.h to do from Makefile
- as suggested by Simon
lib/crc32.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
We end up with two problems with this (and all of the similar patches) which is first, evb-ast2600 stops petting the watchdog in SPL, and then the CONFIG_IS_ENABLED macros in host tools makes clang very unhappy. I'm also not sure that introducing PET_WDT makes the code clearer than it was before.

We need to include <linux/kconfig.h> in order to include files that use CONFIG_IS_ENABLED. TO prepare for that don't pet the watchdog when USE_HOSTCC is defined.
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com Reviewed-by: Simon Glass sjg@chromium.org ---
(no changes since v2)
Changes in v2: - delay include of linux/kconfig.h to do from Makefile - as suggested by Simon
lib/md5.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/lib/md5.c b/lib/md5.c index 1636ab93661..20d5e87814b 100644 --- a/lib/md5.c +++ b/lib/md5.c @@ -29,7 +29,10 @@
#ifndef USE_HOSTCC #include <common.h> +#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) #include <watchdog.h> +#define PET_WDG +#endif #endif /* USE_HOSTCC */ #include <u-boot/md5.h>
@@ -288,14 +291,14 @@ md5_wd(const unsigned char *input, unsigned int len, unsigned char output[16], unsigned int chunk_sz) { struct MD5Context context; -#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) +#ifdef PET_WDG const unsigned char *end, *curr; int chunk; #endif
MD5Init(&context);
-#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) +#ifdef PET_WDG curr = input; end = input + len; while (curr < end) {

We need to include <linux/kconfig.h> in order to include files that use CONFIG_IS_ENABLED. TO prepare for that don't pet the watchdog when USE_HOSTCC is defined.
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com Reviewed-by: Simon Glass sjg@chromium.org ---
(no changes since v2)
Changes in v2: - delay include of linux/kconfig.h to do from Makefile - as suggested by Simon
lib/sha1.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/lib/sha1.c b/lib/sha1.c index 8d074078934..cd5d7aead8d 100644 --- a/lib/sha1.c +++ b/lib/sha1.c @@ -19,6 +19,9 @@ #ifndef USE_HOSTCC #include <common.h> #include <linux/string.h> +#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) +#define PET_WDG +#endif #else #include <string.h> #endif /* USE_HOSTCC */ @@ -328,14 +331,14 @@ void sha1_csum_wd(const unsigned char *input, unsigned int ilen, unsigned char *output, unsigned int chunk_sz) { sha1_context ctx; -#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) +#ifdef PET_WDG const unsigned char *end, *curr; int chunk; #endif
sha1_starts (&ctx);
-#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) +#ifdef PET_WDG curr = input; end = input + ilen; while (curr < end) {

We need to include <linux/kconfig.h> in order to include files that use CONFIG_IS_ENABLED. TO prepare for that don't pet the watchdog when USE_HOSTCC is defined.
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com Reviewed-by: Simon Glass sjg@chromium.org ---
(no changes since v2)
Changes in v2: - delay include of linux/kconfig.h to do from Makefile - as suggested by Simon
lib/sha256.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/lib/sha256.c b/lib/sha256.c index 4d26aea1c8c..8e1c3992674 100644 --- a/lib/sha256.c +++ b/lib/sha256.c @@ -8,6 +8,9 @@ #ifndef USE_HOSTCC #include <common.h> #include <linux/string.h> +#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) +#define PET_WDG +#endif #else #include <string.h> #endif /* USE_HOSTCC */ @@ -276,7 +279,7 @@ void sha256_csum_wd(const unsigned char *input, unsigned int ilen, unsigned char *output, unsigned int chunk_sz) { sha256_context ctx; -#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) +#ifdef PET_WDG const unsigned char *end; unsigned char *curr; int chunk; @@ -284,7 +287,7 @@ void sha256_csum_wd(const unsigned char *input, unsigned int ilen,
sha256_starts(&ctx);
-#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) +#ifdef PET_WDG curr = (unsigned char *)input; end = input + ilen; while (curr < end) {

We need to include <linux/kconfig.h> in order to include files that use CONFIG_IS_ENABLED. TO prepare for that don't pet the watchdog when USE_HOSTCC is defined.
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com Reviewed-by: Simon Glass sjg@chromium.org ---
(no changes since v2)
Changes in v2: - delay include of linux/kconfig.h to do from Makefile - as suggested by Simon
lib/sha512.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/lib/sha512.c b/lib/sha512.c index fbe8d5f5bfe..a504281bf93 100644 --- a/lib/sha512.c +++ b/lib/sha512.c @@ -13,6 +13,9 @@ #ifndef USE_HOSTCC #include <common.h> #include <linux/string.h> +#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) +#define PET_WDG +#endif #else #include <string.h> #endif /* USE_HOSTCC */ @@ -292,7 +295,7 @@ void sha384_csum_wd(const unsigned char *input, unsigned int ilen, unsigned char *output, unsigned int chunk_sz) { sha512_context ctx; -#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) +#ifdef PET_WDG const unsigned char *end; unsigned char *curr; int chunk; @@ -300,7 +303,7 @@ void sha384_csum_wd(const unsigned char *input, unsigned int ilen,
sha384_starts(&ctx);
-#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) +#ifdef PET_WDG curr = (unsigned char *)input; end = input + ilen; while (curr < end) { @@ -355,7 +358,7 @@ void sha512_csum_wd(const unsigned char *input, unsigned int ilen, unsigned char *output, unsigned int chunk_sz) { sha512_context ctx; -#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) +#ifdef PET_WDG const unsigned char *end; unsigned char *curr; int chunk; @@ -363,7 +366,7 @@ void sha512_csum_wd(const unsigned char *input, unsigned int ilen,
sha512_starts(&ctx);
-#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) +#ifdef PET_WDG curr = (unsigned char *)input; end = input + ilen; while (curr < end) {

defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) is a common pattern. Create an new config symbol HAS_WATCHDOG_RUNNING to express it.
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com ---
Changes in v3: new patch to address Tom's concerns
arch/arm/mach-omap2/boot-common.c | 5 ++--- arch/m68k/lib/time.c | 7 +++---- arch/powerpc/lib/interrupts.c | 5 ++--- arch/powerpc/lib/ticks.S | 2 +- board/astro/mcf5373l/fpga.c | 10 ++++------ boot/image-board.c | 2 +- cmd/ximg.c | 10 ++++------ common/board_f.c | 4 ++-- common/spl/Kconfig | 4 ++++ drivers/crypto/aspeed/aspeed_hace.c | 2 +- drivers/crypto/hash/hash_sw.c | 2 +- drivers/timer/mpc83xx_timer.c | 5 ++--- drivers/watchdog/Kconfig | 3 +++ fs/cramfs/uncompress.c | 9 ++++----- include/watchdog.h | 7 ++----- lib/bzip2/bzlib.c | 5 ++--- lib/bzip2/bzlib_decompress.c | 20 ++++++++------------ lib/crc32.c | 2 +- lib/md5.c | 2 +- lib/sha1.c | 2 +- lib/sha256.c | 2 +- lib/sha512.c | 2 +- 22 files changed, 51 insertions(+), 61 deletions(-)
diff --git a/arch/arm/mach-omap2/boot-common.c b/arch/arm/mach-omap2/boot-common.c index 9a342a1bf95..928a4136781 100644 --- a/arch/arm/mach-omap2/boot-common.c +++ b/arch/arm/mach-omap2/boot-common.c @@ -302,9 +302,8 @@ void spl_board_init(void) #if defined(CONFIG_AM33XX) && defined(CONFIG_SPL_MUSB_NEW) arch_misc_init(); #endif -#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) - hw_watchdog_init(); -#endif + if (CONFIG_IS_ENABLED(HAS_WATCHDOG_RUNNING)) + hw_watchdog_init(); #ifdef CONFIG_AM33XX am33xx_spl_board_init(); #endif diff --git a/arch/m68k/lib/time.c b/arch/m68k/lib/time.c index 2ce69088d94..4a9841667b7 100644 --- a/arch/m68k/lib/time.c +++ b/arch/m68k/lib/time.c @@ -70,11 +70,10 @@ void dtimer_interrupt(void *not_used) timerp->ter = (DTIM_DTER_CAP | DTIM_DTER_REF); timestamp++;
- #if defined(CONFIG_WATCHDOG) || defined (CONFIG_HW_WATCHDOG) - if (CFG_SYS_WATCHDOG_FREQ && (timestamp % (CFG_SYS_WATCHDOG_FREQ)) == 0) { + if (CONFIG_IS_ENABLED(HAS_WATCHDOG_RUNNING) && + CFG_SYS_WATCHDOG_FREQ && + !(timestamp % CFG_SYS_WATCHDOG_FREQ)) schedule(); - } - #endif /* CONFIG_WATCHDOG || CONFIG_HW_WATCHDOG */ return; } } diff --git a/arch/powerpc/lib/interrupts.c b/arch/powerpc/lib/interrupts.c index df312dfa28e..f76b2db9025 100644 --- a/arch/powerpc/lib/interrupts.c +++ b/arch/powerpc/lib/interrupts.c @@ -79,10 +79,9 @@ void timer_interrupt(struct pt_regs *regs)
timestamp++;
-#if defined(CONFIG_WATCHDOG) || defined (CONFIG_HW_WATCHDOG) - if (CFG_SYS_WATCHDOG_FREQ && (timestamp % (CFG_SYS_WATCHDOG_FREQ)) == 0) + if (CONFIG_IS_ENABLED(HAS_WATCHDOG_RUNNING) && + CFG_SYS_WATCHDOG_FREQ && !(timestamp % CFG_SYS_WATCHDOG_FREQ)) schedule(); -#endif /* CONFIG_WATCHDOG || CONFIG_HW_WATCHDOG */
#ifdef CONFIG_LED_STATUS status_led_tick(timestamp); diff --git a/arch/powerpc/lib/ticks.S b/arch/powerpc/lib/ticks.S index 8647d77cc9a..dd9afe693df 100644 --- a/arch/powerpc/lib/ticks.S +++ b/arch/powerpc/lib/ticks.S @@ -41,7 +41,7 @@ wait_ticks: addc r14, r4, r14 /* Compute end time lower */ addze r15, r3 /* and end time upper */
-#if defined(CONFIG_WATCHDOG) || defined(CONFIG_HW_WATCHDOG) +#if CONFIG_IS_ENABLED(HAS_WATCHDOG_RUNNING) bl schedule /* Trigger watchdog, if needed */ #endif 1: bl get_ticks /* Get current time */ diff --git a/board/astro/mcf5373l/fpga.c b/board/astro/mcf5373l/fpga.c index f85737432b3..3e1f507cdfe 100644 --- a/board/astro/mcf5373l/fpga.c +++ b/board/astro/mcf5373l/fpga.c @@ -122,9 +122,8 @@ int altera_write_fn(const void *buf, size_t len, int flush, int cookie) } while (i > 0);
if (bytecount % len_40 == 0) { -#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) - schedule(); -#endif + if (CONFIG_IS_ENABLED(HAS_WATCHDOG_RUNNING)) + schedule(); #ifdef CONFIG_SYS_FPGA_PROG_FEEDBACK putc('.'); /* let them know we are alive */ #endif @@ -342,9 +341,8 @@ int xilinx_fastwr_config_fn(void *buf, size_t len, int flush, int cookie) val <<= 1; } if (bytecount % len_40 == 0) { -#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) - schedule(); -#endif + if (CONFIG_IS_ENABLED(HAS_WATCHDOG_RUNNING)) + schedule(); #ifdef CONFIG_SYS_FPGA_PROG_FEEDBACK putc('.'); /* let them know we are alive */ #endif diff --git a/boot/image-board.c b/boot/image-board.c index 25b60ec30b3..48215bdef78 100644 --- a/boot/image-board.c +++ b/boot/image-board.c @@ -173,7 +173,7 @@ void memmove_wd(void *to, void *from, size_t len, ulong chunksz) if (to == from) return;
- if (IS_ENABLED(CONFIG_HW_WATCHDOG) || IS_ENABLED(CONFIG_WATCHDOG)) { + if (CONFIG_IS_ENABLED(HAS_WATCHDOG_RUNNING)) { if (to > from) { from += len; to += len; diff --git a/cmd/ximg.c b/cmd/ximg.c index 60ed2c9f6f9..a42a944ae19 100644 --- a/cmd/ximg.c +++ b/cmd/ximg.c @@ -186,8 +186,7 @@ do_imgextract(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) if (argc > 3) { switch (comp) { case IH_COMP_NONE: -#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) - { + if (CONFIG_IS_ENABLED(HAS_WATCHDOG_RUNNING)) { size_t l = len; size_t tail; void *to = (void *) dest; @@ -203,11 +202,10 @@ do_imgextract(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) from += tail; l -= tail; } + } else { + printf(" Loading part %d ... ", part); + memmove((char *)dest, (char *)data, len); } -#else /* !(CONFIG_HW_WATCHDOG || CONFIG_WATCHDOG) */ - printf(" Loading part %d ... ", part); - memmove((char *) dest, (char *)data, len); -#endif /* CONFIG_HW_WATCHDOG || CONFIG_WATCHDOG */ break; #ifdef CONFIG_GZIP case IH_COMP_GZIP: diff --git a/common/board_f.c b/common/board_f.c index f3c1ab53b1c..54220bc1d3b 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -84,7 +84,7 @@ __weak void blue_led_off(void) {} * a structure... */
-#if defined(CONFIG_WATCHDOG) || defined(CONFIG_HW_WATCHDOG) +#if CONFIG_IS_ENABLED(HAS_WATCHDOG_RUNNING) static int init_func_watchdog_init(void) { # if defined(CONFIG_HW_WATCHDOG) && \ @@ -106,7 +106,7 @@ int init_func_watchdog_reset(void)
return 0; } -#endif /* CONFIG_WATCHDOG */ +#endif /* HAS_WATCHDOG_RUNNING */
__weak void board_add_ram_info(int use_default) { diff --git a/common/spl/Kconfig b/common/spl/Kconfig index 2c042ad3066..8f0edd66225 100644 --- a/common/spl/Kconfig +++ b/common/spl/Kconfig @@ -1444,6 +1444,10 @@ config SPL_WATCHDOG detects no activity for a while (such as a software crash). This enables the drivers in drivers/watchdog as part of an SPL build.
+config SPL_HAS_WATCHDOG_RUNNING + def_bool y if SPL_WATCHDOG || HW_WATCHDOG + depends on HAS_WATCHDOG_RUNNING + config SPL_YMODEM_SUPPORT bool "Support loading using Ymodem" depends on SPL_SERIAL diff --git a/drivers/crypto/aspeed/aspeed_hace.c b/drivers/crypto/aspeed/aspeed_hace.c index 6b6c8fa6588..28a39e7b599 100644 --- a/drivers/crypto/aspeed/aspeed_hace.c +++ b/drivers/crypto/aspeed/aspeed_hace.c @@ -288,7 +288,7 @@ static int aspeed_hace_digest_wd(struct udevice *dev, enum HASH_ALGO algo, if (rc) return rc;
- if (IS_ENABLED(CONFIG_HW_WATCHDOG) || CONFIG_IS_ENABLED(WATCHDOG)) { + if (CONFIG_IS_ENABLED(HAS_WATCHDOG_RUNNING)) { cur = ibuf; end = ibuf + ilen;
diff --git a/drivers/crypto/hash/hash_sw.c b/drivers/crypto/hash/hash_sw.c index d8065d68ea4..51cead45a4f 100644 --- a/drivers/crypto/hash/hash_sw.c +++ b/drivers/crypto/hash/hash_sw.c @@ -244,7 +244,7 @@ static int sw_hash_digest_wd(struct udevice *dev, enum HASH_ALGO algo, if (rc) return rc;
- if (IS_ENABLED(CONFIG_HW_WATCHDOG) || CONFIG_IS_ENABLED(WATCHDOG)) { + if (CONFIG_IS_ENABLED(HAS_WATCHDOG_RUNNING)) { cur = ibuf; end = ibuf + ilen;
diff --git a/drivers/timer/mpc83xx_timer.c b/drivers/timer/mpc83xx_timer.c index 7814cb6a5d6..a0858cc49b5 100644 --- a/drivers/timer/mpc83xx_timer.c +++ b/drivers/timer/mpc83xx_timer.c @@ -174,10 +174,9 @@ void timer_interrupt(struct pt_regs *regs)
priv->timestamp++;
-#if defined(CONFIG_WATCHDOG) || defined(CONFIG_HW_WATCHDOG) - if (CFG_SYS_WATCHDOG_FREQ && (priv->timestamp % (CFG_SYS_WATCHDOG_FREQ)) == 0) + if (CONFIG_IS_ENABLED(HAS_WATCHDOG_RUNNING) && + CFG_SYS_WATCHDOG_FREQ && !(priv->timestamp % CFG_SYS_WATCHDOG_FREQ)) schedule(); -#endif /* CONFIG_WATCHDOG || CONFIG_HW_WATCHDOG */
#ifdef CONFIG_LED_STATUS status_led_tick(priv->timestamp); diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index b5ac8f7f50d..46c0c1efedf 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -35,6 +35,9 @@ config WATCHDOG_TIMEOUT_MSECS help Watchdog timeout in msec
+config HAS_WATCHDOG_RUNNING + def_bool y if WATCHDOG || HW_WATCHDOG + config HW_WATCHDOG bool
diff --git a/fs/cramfs/uncompress.c b/fs/cramfs/uncompress.c index 0d071b69f4c..80000110d5d 100644 --- a/fs/cramfs/uncompress.c +++ b/fs/cramfs/uncompress.c @@ -62,11 +62,10 @@ int cramfs_uncompress_init (void) stream.next_in = 0; stream.avail_in = 0;
-#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) - stream.outcb = (cb_func)cyclic_run; -#else - stream.outcb = Z_NULL; -#endif /* CONFIG_HW_WATCHDOG */ + if (CONFIG_IS_ENABLED(HAS_WATCHDOG_RUNNING)) + stream.outcb = (cb_func)cyclic_run; + else + stream.outcb = Z_NULL;
err = inflateInit (&stream); if (err != Z_OK) { diff --git a/include/watchdog.h b/include/watchdog.h index ac5f11e376f..60086e1ae30 100644 --- a/include/watchdog.h +++ b/include/watchdog.h @@ -20,7 +20,7 @@ */ int init_func_watchdog_reset(void);
-#if defined(CONFIG_WATCHDOG) || defined(CONFIG_HW_WATCHDOG) +#if CONFIG_IS_ENABLED(HAS_WATCHDOG_RUNNING) #define INIT_FUNC_WATCHDOG_INIT init_func_watchdog_init, #define INIT_FUNC_WATCHDOG_RESET init_func_watchdog_reset, #else @@ -35,10 +35,7 @@ int init_func_watchdog_reset(void); /* * Prototypes from $(CPU)/cpu.c. */ - -#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) - void hw_watchdog_init(void); -#endif +void hw_watchdog_init(void);
#if defined(CONFIG_MPC85xx) void init_85xx_watchdog(void); diff --git a/lib/bzip2/bzlib.c b/lib/bzip2/bzlib.c index bd589aa810c..904ff1332be 100644 --- a/lib/bzip2/bzlib.c +++ b/lib/bzip2/bzlib.c @@ -843,9 +843,8 @@ int BZ_API(BZ2_bzDecompress) ( bz_stream *strm ) if (s->strm != strm) return BZ_PARAM_ERROR;
while (True) { -#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) - schedule(); -#endif + if (CONFIG_IS_ENABLED(HAS_WATCHDOG_RUNNING)) + schedule(); if (s->state == BZ_X_IDLE) return BZ_SEQUENCE_ERROR; if (s->state == BZ_X_OUTPUT) { if (s->smallDecompress) diff --git a/lib/bzip2/bzlib_decompress.c b/lib/bzip2/bzlib_decompress.c index 3b417d57b27..51f21d1e7f6 100644 --- a/lib/bzip2/bzlib_decompress.c +++ b/lib/bzip2/bzlib_decompress.c @@ -417,9 +417,8 @@ Int32 BZ2_decompress ( DState* s )
while (True) {
-#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) - schedule(); -#endif + if (CONFIG_IS_ENABLED(HAS_WATCHDOG_RUNNING)) + schedule(); if (nextSym == EOB) break;
if (nextSym == BZ_RUNA || nextSym == BZ_RUNB) { @@ -502,9 +501,8 @@ Int32 BZ2_decompress ( DState* s ) if (s->mtfbase[0] == 0) { kk = MTFA_SIZE-1; for (ii = 256 / MTFL_SIZE-1; ii >= 0; ii--) { -#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) - schedule(); -#endif + if (CONFIG_IS_ENABLED(HAS_WATCHDOG_RUNNING)) + schedule(); for (jj = MTFL_SIZE-1; jj >= 0; jj--) { s->mtfa[kk] = s->mtfa[s->mtfbase[ii] + jj]; kk--; @@ -567,9 +565,8 @@ Int32 BZ2_decompress ( DState* s ) } while (i != s->origPtr);
-#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) - schedule(); -#endif + if (CONFIG_IS_ENABLED(HAS_WATCHDOG_RUNNING)) + schedule(); s->tPos = s->origPtr; s->nblock_used = 0; if (s->blockRandomised) { @@ -582,9 +579,8 @@ Int32 BZ2_decompress ( DState* s )
} else {
-#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) - schedule(); -#endif + if (CONFIG_IS_ENABLED(HAS_WATCHDOG_RUNNING)) + schedule(); /*-- compute the T^(-1) vector --*/ for (i = 0; i < nblock; i++) { uc = (UChar)(s->tt[i] & 0xff); diff --git a/lib/crc32.c b/lib/crc32.c index 12c104c62a4..124fadb62b0 100644 --- a/lib/crc32.c +++ b/lib/crc32.c @@ -14,7 +14,7 @@ #else #include <common.h> #include <efi_loader.h> -#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) +#if CONFIG_IS_ENABLED(HAS_WATCHDOG_RUNNING) #define PET_WDG #endif #endif diff --git a/lib/md5.c b/lib/md5.c index 20d5e87814b..fb1a2b4ef61 100644 --- a/lib/md5.c +++ b/lib/md5.c @@ -29,7 +29,7 @@
#ifndef USE_HOSTCC #include <common.h> -#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) +#if CONFIG_IS_ENABLED(HAS_WATCHDOG_RUNNING) #include <watchdog.h> #define PET_WDG #endif diff --git a/lib/sha1.c b/lib/sha1.c index cd5d7aead8d..9661fe487a7 100644 --- a/lib/sha1.c +++ b/lib/sha1.c @@ -19,7 +19,7 @@ #ifndef USE_HOSTCC #include <common.h> #include <linux/string.h> -#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) +#if CONFIG_IS_ENABLED(HAS_WATCHDOG_RUNNING) #define PET_WDG #endif #else diff --git a/lib/sha256.c b/lib/sha256.c index 8e1c3992674..5aa7ad7b5c0 100644 --- a/lib/sha256.c +++ b/lib/sha256.c @@ -8,7 +8,7 @@ #ifndef USE_HOSTCC #include <common.h> #include <linux/string.h> -#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) +#if CONFIG_IS_ENABLED(HAS_WATCHDOG_RUNNING) #define PET_WDG #endif #else diff --git a/lib/sha512.c b/lib/sha512.c index a504281bf93..389c792bd22 100644 --- a/lib/sha512.c +++ b/lib/sha512.c @@ -13,7 +13,7 @@ #ifndef USE_HOSTCC #include <common.h> #include <linux/string.h> -#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) +#if CONFIG_IS_ENABLED(HAS_WATCHDOG_RUNNING) #define PET_WDG #endif #else

We need to include <linux/kconfig.h> in order to include files that use CONFIG_IS_ENABLED.
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com Reviewed-by: Simon Glass sjg@chromium.org Reviewed-by: Tom Rini trini@konsulko.com ---
(no changes since v2)
Changes in v2: - include linux/kconfig.h from tools/Makefile - as suggested by Simon
tools/Makefile | 1 + 1 file changed, 1 insertion(+)
diff --git a/tools/Makefile b/tools/Makefile index e13effbb66a..5d6284e6451 100644 --- a/tools/Makefile +++ b/tools/Makefile @@ -298,6 +298,7 @@ endif # !LOGO_BMP # Define _GNU_SOURCE to obtain the getline prototype from stdio.h # HOST_EXTRACFLAGS += -include $(srctree)/include/compiler.h \ + -include $(srctree)/include/linux/kconfig.h \ $(patsubst -I%,-idirafter%, $(filter -I%, $(UBOOTINCLUDE))) \ -I$(srctree)/scripts/dtc/libfdt \ -I$(srctree)/tools \

CONFIG_IS_ENABLED(FIT_SIGNATURE) will check for CONFIG_TOOLS_FIT_SIGNATURE. So define it now in preparation.
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com Reviewed-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/Makefile | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/tools/Makefile b/tools/Makefile index 5d6284e6451..86f1b6b5049 100644 --- a/tools/Makefile +++ b/tools/Makefile @@ -162,8 +162,10 @@ ifdef CONFIG_TOOLS_LIBCRYPTO # This affects include/image.h, but including the board config file # is tricky, so manually define this options here. HOST_EXTRACFLAGS += -DCONFIG_FIT_SIGNATURE +HOST_EXTRACFLAGS += -DCONFIG_TOOLS_FIT_SIGNATURE=1 HOST_EXTRACFLAGS += -DCONFIG_FIT_SIGNATURE_MAX_SIZE=0xffffffff HOST_EXTRACFLAGS += -DCONFIG_FIT_CIPHER +HOST_EXTRACFLAGS += -DCONFIG_TOOLS_FIT_CIPHER=1 endif
# MXSImage needs LibSSL

CONFIG_IS_ENABLED(SPL_X86_32BIT_INIT) would check for CONFIG_SPL_SPL_X86_32BIT_INIT for SPL builds
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com Reviewed-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
arch/x86/cpu/qemu/qemu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/cpu/qemu/qemu.c b/arch/x86/cpu/qemu/qemu.c index e54082df7f9..274978c023b 100644 --- a/arch/x86/cpu/qemu/qemu.c +++ b/arch/x86/cpu/qemu/qemu.c @@ -97,7 +97,7 @@ static void qemu_chipset_init(void) } }
-#if !CONFIG_IS_ENABLED(SPL_X86_32BIT_INIT) +#if CONFIG_IS_ENABLED(X86_32BIT_INIT) int arch_cpu_init(void) { post_code(POST_CPU_INIT);

On Mon, Mar 13, 2023 at 02:31:33PM -0700, Troy Kisky wrote:
CONFIG_IS_ENABLED(SPL_X86_32BIT_INIT) would check for CONFIG_SPL_SPL_X86_32BIT_INIT for SPL builds
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/next, thanks!

SPL environments don't need commands that they can never use. Avoid errors with CONFIG_IS_ENABLED conversions by skipping them now.
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com Reviewed-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
include/config_distro_bootcmd.h | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)
diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h index 9d2a225e7eb..2a136b96a6d 100644 --- a/include/config_distro_bootcmd.h +++ b/include/config_distro_bootcmd.h @@ -35,11 +35,15 @@ #devtypel "_boot=" \ BOOTENV_SHARED_BLKDEV_BODY(devtypel)
+#define BOOTENV_DEV_BLKDEV_NONE(devtypeu, devtypel, instance) + #define BOOTENV_DEV_BLKDEV(devtypeu, devtypel, instance) \ "bootcmd_" #devtypel #instance "=" \ "devnum=" #instance "; " \ "run " #devtypel "_boot\0"
+#define BOOTENV_DEV_NAME_BLKDEV_NONE(devtypeu, devtypel, instance) + #define BOOTENV_DEV_NAME_BLKDEV(devtypeu, devtypel, instance) \ #devtypel #instance " "
@@ -59,6 +63,10 @@ #define BOOTENV_SHARED_MMC BOOTENV_SHARED_BLKDEV(mmc) #define BOOTENV_DEV_MMC BOOTENV_DEV_BLKDEV #define BOOTENV_DEV_NAME_MMC BOOTENV_DEV_NAME_BLKDEV +#elif defined(CONFIG_SPL_BUILD) +#define BOOTENV_SHARED_MMC +#define BOOTENV_DEV_MMC BOOTENV_DEV_BLKDEV_NONE +#define BOOTENV_DEV_NAME_MMC BOOTENV_DEV_NAME_BLKDEV_NONE #else #define BOOTENV_SHARED_MMC #define BOOTENV_DEV_MMC \ @@ -190,6 +198,10 @@ #define BOOTENV_SHARED_SATA BOOTENV_SHARED_BLKDEV(sata) #define BOOTENV_DEV_SATA BOOTENV_DEV_BLKDEV #define BOOTENV_DEV_NAME_SATA BOOTENV_DEV_NAME_BLKDEV +#elif defined(CONFIG_SPL_BUILD) +#define BOOTENV_SHARED_SATA +#define BOOTENV_DEV_SATA BOOTENV_DEV_BLKDEV_NONE +#define BOOTENV_DEV_NAME_SATA BOOTENV_DEV_NAME_BLKDEV_NONE #else #define BOOTENV_SHARED_SATA #define BOOTENV_DEV_SATA \ @@ -293,6 +305,11 @@ BOOTENV_SHARED_BLKDEV_BODY(usb) #define BOOTENV_DEV_USB BOOTENV_DEV_BLKDEV #define BOOTENV_DEV_NAME_USB BOOTENV_DEV_NAME_BLKDEV +#elif defined(CONFIG_SPL_BUILD) +#define BOOTENV_RUN_NET_USB_START +#define BOOTENV_SHARED_USB +#define BOOTENV_DEV_USB BOOTENV_DEV_BLKDEV_NONE +#define BOOTENV_DEV_NAME_USB BOOTENV_DEV_NAME_BLKDEV_NONE #else #define BOOTENV_RUN_NET_USB_START #define BOOTENV_SHARED_USB @@ -395,6 +412,9 @@ "\0" #define BOOTENV_DEV_NAME_DHCP(devtypeu, devtypel, instance) \ "dhcp " +#elif defined(CONFIG_SPL_BUILD) +#define BOOTENV_DEV_DHCP BOOTENV_DEV_BLKDEV_NONE +#define BOOTENV_DEV_NAME_DHCP BOOTENV_DEV_NAME_BLKDEV_NONE #else #define BOOTENV_DEV_DHCP \ BOOT_TARGET_DEVICES_references_DHCP_without_CONFIG_CMD_DHCP @@ -413,6 +433,9 @@ "fi\0" #define BOOTENV_DEV_NAME_PXE(devtypeu, devtypel, instance) \ "pxe " +#elif defined(CONFIG_SPL_BUILD) +#define BOOTENV_DEV_PXE BOOTENV_DEV_BLKDEV_NONE +#define BOOTENV_DEV_NAME_PXE BOOTENV_DEV_NAME_BLKDEV_NONE #else #define BOOTENV_DEV_PXE \ BOOT_TARGET_DEVICES_references_PXE_without_CONFIG_CMD_DHCP_or_PXE

On Mon, Mar 13, 2023 at 02:31:34PM -0700, Troy Kisky wrote:
SPL environments don't need commands that they can never use. Avoid errors with CONFIG_IS_ENABLED conversions by skipping them now.
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/next, thanks!

With the use of CONFIG_IS_ENABLED in code, instead of at the preprocessor level, these defines are still needed if OF_CONTROL is enabled.
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com Reviewed-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
include/fdt_support.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/fdt_support.h b/include/fdt_support.h index 5638bd4f165..eeb83e6251d 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -7,7 +7,8 @@ #ifndef __FDT_SUPPORT_H #define __FDT_SUPPORT_H
-#if defined(CONFIG_OF_LIBFDT) && !defined(USE_HOSTCC) +#if (defined(CONFIG_OF_LIBFDT) || defined(CONFIG_OF_CONTROL)) && \ + !defined(USE_HOSTCC)
#include <asm/u-boot.h> #include <linux/libfdt.h>

On Mon, Mar 13, 2023 at 02:31:35PM -0700, Troy Kisky wrote:
With the use of CONFIG_IS_ENABLED in code, instead of at the preprocessor level, these defines are still needed if OF_CONTROL is enabled.
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/next, thanks!

SPL code wants fdt_fixup_mtdparts defined as a NOP when the function isn't linked in.
Prepare for ifdef CONFIG_OF_LIBFDT being converted to if CONFIG_IS_ENABLED(OF_LIBFDT)
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com Reviewed-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
include/fdt_support.h | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/include/fdt_support.h b/include/fdt_support.h index eeb83e6251d..94497d755a3 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -245,16 +245,6 @@ int fdt_increase_size(void *fdt, int add_len); int fdt_delete_disabled_nodes(void *blob);
struct node_info; -#if defined(CONFIG_FDT_FIXUP_PARTITIONS) -void fdt_fixup_mtdparts(void *fdt, const struct node_info *node_info, - int node_info_size); -#else -static inline void fdt_fixup_mtdparts(void *fdt, - const struct node_info *node_info, - int node_info_size) -{ -} -#endif
void fdt_del_node_and_alias(void *blob, const char *alias);
@@ -412,6 +402,19 @@ int fdt_get_cells_len(const void *blob, char *nr_cells_name);
#endif /* ifdef CONFIG_OF_LIBFDT */
+#if CONFIG_IS_ENABLED(OF_LIBFDT) && defined(CONFIG_FDT_FIXUP_PARTITIONS) +struct node_info; +void fdt_fixup_mtdparts(void *fdt, const struct node_info *node_info, + int node_info_size); +#else +struct node_info; +static inline void fdt_fixup_mtdparts(void *fdt, + const struct node_info *node_info, + int node_info_size) +{ +} +#endif + #ifdef USE_HOSTCC int fdtdec_get_int(const void *blob, int node, const char *prop_name, int default_val);

On Mon, Mar 13, 2023 at 02:31:36PM -0700, Troy Kisky wrote:
SPL code wants fdt_fixup_mtdparts defined as a NOP when the function isn't linked in.
Prepare for ifdef CONFIG_OF_LIBFDT being converted to if CONFIG_IS_ENABLED(OF_LIBFDT)
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com Reviewed-by: Simon Glass sjg@chromium.org
(no changes since v1)
include/fdt_support.h | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-)
As Francesco's thread discusses, this function needs to be removed, by and large, really is the answer.

The function ft_board_setup calls do_fixup_by_path_string which is only available on CONFIG_IS_ENABLED(OF_LIBFDT). This prepares for the conversion.
ft_board_setup is only called from image-fdt which is linked by obj-$(CONFIG_$(SPL_TPL_)OF_LIBFDT) += image-fdt.o
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com Reviewed-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
board/menlo/m53menlo/m53menlo.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/board/menlo/m53menlo/m53menlo.c b/board/menlo/m53menlo/m53menlo.c index 14324c7087d..ca3b81c57ff 100644 --- a/board/menlo/m53menlo/m53menlo.c +++ b/board/menlo/m53menlo/m53menlo.c @@ -264,6 +264,7 @@ void board_preboot_os(void) gpio_direction_output(IMX_GPIO_NR(6, 0), 0); }
+#if CONFIG_IS_ENABLED(OF_LIBFDT) int ft_board_setup(void *blob, struct bd_info *bd) { if (lvds_compat_string) @@ -272,6 +273,7 @@ int ft_board_setup(void *blob, struct bd_info *bd)
return 0; } +#endif
struct display_info_t const displays[] = { {

On Mon, Mar 13, 2023 at 02:31:37PM -0700, Troy Kisky wrote:
The function ft_board_setup calls do_fixup_by_path_string which is only available on CONFIG_IS_ENABLED(OF_LIBFDT). This prepares for the conversion.
ft_board_setup is only called from image-fdt which is linked by obj-$(CONFIG_$(SPL_TPL_)OF_LIBFDT) += image-fdt.o
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/next, thanks!

pfuze_mode_init calls pmic_reg_read which is only available from
obj-$(CONFIG_$(SPL_TPL_)DM_PMIC) += pmic-uclass.o
Prepare for conversion of defined(CONFIG_DM_PMIC) to CONFIG_IS_ENABLED(DM_PMIC).
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com Reviewed-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
board/freescale/common/pfuze.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/board/freescale/common/pfuze.c b/board/freescale/common/pfuze.c index 6dca22960bf..a9288820b2e 100644 --- a/board/freescale/common/pfuze.c +++ b/board/freescale/common/pfuze.c @@ -91,7 +91,7 @@ struct pmic *pfuze_common_init(unsigned char i2cbus)
return p; } -#else +#elif defined(CONFIG_DM_PMIC) int pfuze_mode_init(struct udevice *dev, u32 mode) { unsigned char offset, i, switch_num;

On Mon, Mar 13, 2023 at 02:31:38PM -0700, Troy Kisky wrote:
pfuze_mode_init calls pmic_reg_read which is only available from
obj-$(CONFIG_$(SPL_TPL_)DM_PMIC) += pmic-uclass.o
Prepare for conversion of defined(CONFIG_DM_PMIC) to CONFIG_IS_ENABLED(DM_PMIC).
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/next, thanks!

When switching defined(CONFIG_PCI) to CONFIG_IS_ENABLED(PCI) bdf is no longer accessible. So add preprocessor protection to avoid access.
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com Reviewed-by: Simon Glass sjg@chromium.org ---
(no changes since v2)
Changes in v2: - changed condition of when to include field bdf - added protection to another instance of bdf in uart.c - Thanks to Simon for getting this corrected
arch/x86/cpu/apollolake/uart.c | 4 ++++ include/ns16550.h | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/x86/cpu/apollolake/uart.c b/arch/x86/cpu/apollolake/uart.c index a9362436000..878aa48ed76 100644 --- a/arch/x86/cpu/apollolake/uart.c +++ b/arch/x86/cpu/apollolake/uart.c @@ -79,10 +79,12 @@ void apl_uart_init(pci_dev_t bdf, ulong base)
static int apl_ns16550_probe(struct udevice *dev) { +#if IS_ENABLED_NOCHECK(CONFIG_PCI) && defined(CONFIG_SPL_BUILD) struct apl_ns16550_plat *plat = dev_get_plat(dev);
if (!CONFIG_IS_ENABLED(PCI)) apl_uart_init(plat->ns16550.bdf, plat->ns16550.base); +#endif
return ns16550_serial_probe(dev); } @@ -110,7 +112,9 @@ static int apl_ns16550_of_to_plat(struct udevice *dev) ns.reg_offset = 0; ns.clock = dtplat->clock_frequency; ns.fcr = UART_FCR_DEFVAL; +#if IS_ENABLED_NOCHECK(CONFIG_PCI) && defined(CONFIG_SPL_BUILD) ns.bdf = pci_ofplat_get_devfn(dtplat->reg[0]); +#endif memcpy(plat, &ns, sizeof(ns)); #else int ret; diff --git a/include/ns16550.h b/include/ns16550.h index e7e68663d03..41b977b5b26 100644 --- a/include/ns16550.h +++ b/include/ns16550.h @@ -74,7 +74,7 @@ struct ns16550_plat { int clock; u32 fcr; int flags; -#if defined(CONFIG_PCI) && defined(CONFIG_SPL) +#if IS_ENABLED_NOCHECK(CONFIG_PCI) && defined(CONFIG_SPL_BUILD) int bdf; #endif };

On Mon, Mar 13, 2023 at 02:31:39PM -0700, Troy Kisky wrote:
When switching defined(CONFIG_PCI) to CONFIG_IS_ENABLED(PCI) bdf is no longer accessible. So add preprocessor protection to avoid access.
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com Reviewed-by: Simon Glass sjg@chromium.org
(no changes since v2)
Changes in v2:
- changed condition of when to include field bdf
- added protection to another instance of bdf in uart.c
- Thanks to Simon for getting this corrected
arch/x86/cpu/apollolake/uart.c | 4 ++++ include/ns16550.h | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/x86/cpu/apollolake/uart.c b/arch/x86/cpu/apollolake/uart.c index a9362436000..878aa48ed76 100644 --- a/arch/x86/cpu/apollolake/uart.c +++ b/arch/x86/cpu/apollolake/uart.c @@ -79,10 +79,12 @@ void apl_uart_init(pci_dev_t bdf, ulong base)
static int apl_ns16550_probe(struct udevice *dev) { +#if IS_ENABLED(CONFIG_PCI) && defined(CONFIG_SPL_BUILD) struct apl_ns16550_plat *plat = dev_get_plat(dev);
if (!CONFIG_IS_ENABLED(PCI)) apl_uart_init(plat->ns16550.bdf, plat->ns16550.base); +#endif
return ns16550_serial_probe(dev); }
Looking at this again, this hunk here doesn't make sense. Reading outloud, "if we have CONFIG_PCI enabled and CONFIG_SPL_BUILD, declare plat to be .. and then if we do not have PCI enabled ...". I really don't know what the code is supposed to be doing here.

Hi Tom,
On Wed, 10 May 2023 at 14:41, Tom Rini trini@konsulko.com wrote:
On Mon, Mar 13, 2023 at 02:31:39PM -0700, Troy Kisky wrote:
When switching defined(CONFIG_PCI) to CONFIG_IS_ENABLED(PCI) bdf is no longer accessible. So add preprocessor protection to avoid access.
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com Reviewed-by: Simon Glass sjg@chromium.org
(no changes since v2)
Changes in v2:
- changed condition of when to include field bdf
- added protection to another instance of bdf in uart.c
- Thanks to Simon for getting this corrected
arch/x86/cpu/apollolake/uart.c | 4 ++++ include/ns16550.h | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/x86/cpu/apollolake/uart.c b/arch/x86/cpu/apollolake/uart.c index a9362436000..878aa48ed76 100644 --- a/arch/x86/cpu/apollolake/uart.c +++ b/arch/x86/cpu/apollolake/uart.c @@ -79,10 +79,12 @@ void apl_uart_init(pci_dev_t bdf, ulong base)
static int apl_ns16550_probe(struct udevice *dev) { +#if IS_ENABLED(CONFIG_PCI) && defined(CONFIG_SPL_BUILD) struct apl_ns16550_plat *plat = dev_get_plat(dev);
if (!CONFIG_IS_ENABLED(PCI)) apl_uart_init(plat->ns16550.bdf, plat->ns16550.base);
+#endif
return ns16550_serial_probe(dev);
}
Looking at this again, this hunk here doesn't make sense. Reading outloud, "if we have CONFIG_PCI enabled and CONFIG_SPL_BUILD, declare plat to be .. and then if we do not have PCI enabled ...". I really don't know what the code is supposed to be doing here.
This is because the bdf member is only present in SPL when PCI is enabled.
Perhaps it could be CONFIG_SPL instead of CONFIG_SPL_BUILD ?
You can apply whatever makes it build if you like. I can look at it either tonight or when I get back.
Regards, Simon

On Wed, May 10, 2023 at 02:46:14PM -0600, Simon Glass wrote:
Hi Tom,
On Wed, 10 May 2023 at 14:41, Tom Rini trini@konsulko.com wrote:
On Mon, Mar 13, 2023 at 02:31:39PM -0700, Troy Kisky wrote:
When switching defined(CONFIG_PCI) to CONFIG_IS_ENABLED(PCI) bdf is no longer accessible. So add preprocessor protection to avoid access.
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com Reviewed-by: Simon Glass sjg@chromium.org
(no changes since v2)
Changes in v2:
- changed condition of when to include field bdf
- added protection to another instance of bdf in uart.c
- Thanks to Simon for getting this corrected
arch/x86/cpu/apollolake/uart.c | 4 ++++ include/ns16550.h | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/x86/cpu/apollolake/uart.c b/arch/x86/cpu/apollolake/uart.c index a9362436000..878aa48ed76 100644 --- a/arch/x86/cpu/apollolake/uart.c +++ b/arch/x86/cpu/apollolake/uart.c @@ -79,10 +79,12 @@ void apl_uart_init(pci_dev_t bdf, ulong base)
static int apl_ns16550_probe(struct udevice *dev) { +#if IS_ENABLED(CONFIG_PCI) && defined(CONFIG_SPL_BUILD) struct apl_ns16550_plat *plat = dev_get_plat(dev);
if (!CONFIG_IS_ENABLED(PCI)) apl_uart_init(plat->ns16550.bdf, plat->ns16550.base);
+#endif
return ns16550_serial_probe(dev);
}
Looking at this again, this hunk here doesn't make sense. Reading outloud, "if we have CONFIG_PCI enabled and CONFIG_SPL_BUILD, declare plat to be .. and then if we do not have PCI enabled ...". I really don't know what the code is supposed to be doing here.
This is because the bdf member is only present in SPL when PCI is enabled.
Perhaps it could be CONFIG_SPL instead of CONFIG_SPL_BUILD ?
You can apply whatever makes it build if you like. I can look at it either tonight or when I get back.
Digging at this when you get back is fine since it still makes no sense to say "If PCI is enabled, when PCI is not enabled actually do something". There's no side-effects on "dev" from the call to dev_get_plat which is the only thing that happens, in this case.

Hi Tom,
You are looking at an old patch, here's the new.
commit c969bedb9cb6029360e6fe7e25a331680fabe3ee Author: Troy Kisky troykiskyboundary@gmail.com Date: Thu Feb 23 08:01:46 2023 -0800
ns16550: match when to define bdf with uart code
When switching defined(CONFIG_PCI) to CONFIG_IS_ENABLED(PCI) bdf is no longer accessible. So add preprocessor protection to avoid access.
Series-changes: 2 - changed condition of when to include field bdf - added protection to another instance of bdf in uart.c - Thanks to Simon for getting this corrected
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com Reviewed-by: Simon Glass sjg@chromium.org
diff --git a/arch/x86/cpu/apollolake/uart.c b/arch/x86/cpu/apollolake/uart.c index a9362436000..878aa48ed76 100644 --- a/arch/x86/cpu/apollolake/uart.c +++ b/arch/x86/cpu/apollolake/uart.c @@ -79,10 +79,12 @@ void apl_uart_init(pci_dev_t bdf, ulong base)
static int apl_ns16550_probe(struct udevice *dev) { +#if IS_ENABLED_NOCHECK(CONFIG_PCI) && defined(CONFIG_SPL_BUILD) struct apl_ns16550_plat *plat = dev_get_plat(dev);
if (!CONFIG_IS_ENABLED(PCI)) apl_uart_init(plat->ns16550.bdf, plat->ns16550.base); +#endif
return ns16550_serial_probe(dev); } @@ -110,7 +112,9 @@ static int apl_ns16550_of_to_plat(struct udevice *dev) ns.reg_offset = 0; ns.clock = dtplat->clock_frequency; ns.fcr = UART_FCR_DEFVAL; +#if IS_ENABLED_NOCHECK(CONFIG_PCI) && defined(CONFIG_SPL_BUILD) ns.bdf = pci_ofplat_get_devfn(dtplat->reg[0]); +#endif memcpy(plat, &ns, sizeof(ns)); #else int ret; diff --git a/include/ns16550.h b/include/ns16550.h index e7e68663d03..41b977b5b26 100644 --- a/include/ns16550.h +++ b/include/ns16550.h @@ -74,7 +74,7 @@ struct ns16550_plat { int clock; u32 fcr; int flags; -#if defined(CONFIG_PCI) && defined(CONFIG_SPL) +#if IS_ENABLED_NOCHECK(CONFIG_PCI) && defined(CONFIG_SPL_BUILD) int bdf; #endif }; __________________________
It reads as, If the bdf exists, then if spl var PCI is not enabled, then init uart.
The PCI code will handle it if PCI is enabled.
So, PCI needs to be enabled, and SPL_PCI needs to not be enabled, and currently building for SPL
On Wed, May 10, 2023 at 1:49 PM Tom Rini trini@konsulko.com wrote:
On Wed, May 10, 2023 at 02:46:14PM -0600, Simon Glass wrote:
Hi Tom,
On Wed, 10 May 2023 at 14:41, Tom Rini trini@konsulko.com wrote:
On Mon, Mar 13, 2023 at 02:31:39PM -0700, Troy Kisky wrote:
When switching defined(CONFIG_PCI) to CONFIG_IS_ENABLED(PCI) bdf is no longer accessible. So add preprocessor protection to avoid access.
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com Reviewed-by: Simon Glass sjg@chromium.org
(no changes since v2)
Changes in v2:
- changed condition of when to include field bdf
- added protection to another instance of bdf in uart.c
- Thanks to Simon for getting this corrected
arch/x86/cpu/apollolake/uart.c | 4 ++++ include/ns16550.h | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/x86/cpu/apollolake/uart.c
b/arch/x86/cpu/apollolake/uart.c
index a9362436000..878aa48ed76 100644 --- a/arch/x86/cpu/apollolake/uart.c +++ b/arch/x86/cpu/apollolake/uart.c @@ -79,10 +79,12 @@ void apl_uart_init(pci_dev_t bdf, ulong base)
static int apl_ns16550_probe(struct udevice *dev) { +#if IS_ENABLED(CONFIG_PCI) && defined(CONFIG_SPL_BUILD) struct apl_ns16550_plat *plat = dev_get_plat(dev);
if (!CONFIG_IS_ENABLED(PCI)) apl_uart_init(plat->ns16550.bdf, plat->ns16550.base);
+#endif
return ns16550_serial_probe(dev);
}
Looking at this again, this hunk here doesn't make sense. Reading outloud, "if we have CONFIG_PCI enabled and CONFIG_SPL_BUILD, declare plat to be .. and then if we do not have PCI enabled ...". I really don't know what the code is supposed to be doing here.
This is because the bdf member is only present in SPL when PCI is
enabled.
Perhaps it could be CONFIG_SPL instead of CONFIG_SPL_BUILD ?
You can apply whatever makes it build if you like. I can look at it either tonight or when I get back.
Digging at this when you get back is fine since it still makes no sense to say "If PCI is enabled, when PCI is not enabled actually do something". There's no side-effects on "dev" from the call to dev_get_plat which is the only thing that happens, in this case.
-- Tom

On Wed, May 10, 2023 at 03:05:38PM -0700, Troy Kisky wrote:
Hi Tom,
You are looking at an old patch, here's the new.
It was the new one, sorry, I just edited out NOCHECK at least for now.
commit c969bedb9cb6029360e6fe7e25a331680fabe3ee Author: Troy Kisky troykiskyboundary@gmail.com Date: Thu Feb 23 08:01:46 2023 -0800
ns16550: match when to define bdf with uart code When switching defined(CONFIG_PCI) to CONFIG_IS_ENABLED(PCI) bdf is no longer accessible. So add preprocessor protection to avoid access. Series-changes: 2 - changed condition of when to include field bdf - added protection to another instance of bdf in uart.c - Thanks to Simon for getting this corrected Signed-off-by: Troy Kisky <troykiskyboundary@gmail.com> Reviewed-by: Simon Glass <sjg@chromium.org>
diff --git a/arch/x86/cpu/apollolake/uart.c b/arch/x86/cpu/apollolake/uart.c index a9362436000..878aa48ed76 100644 --- a/arch/x86/cpu/apollolake/uart.c +++ b/arch/x86/cpu/apollolake/uart.c @@ -79,10 +79,12 @@ void apl_uart_init(pci_dev_t bdf, ulong base)
static int apl_ns16550_probe(struct udevice *dev) { +#if IS_ENABLED_NOCHECK(CONFIG_PCI) && defined(CONFIG_SPL_BUILD) struct apl_ns16550_plat *plat = dev_get_plat(dev);
if (!CONFIG_IS_ENABLED(PCI)) apl_uart_init(plat->ns16550.bdf, plat->ns16550.base);
+#endif
return ns16550_serial_probe(dev);
} @@ -110,7 +112,9 @@ static int apl_ns16550_of_to_plat(struct udevice *dev) ns.reg_offset = 0; ns.clock = dtplat->clock_frequency; ns.fcr = UART_FCR_DEFVAL; +#if IS_ENABLED_NOCHECK(CONFIG_PCI) && defined(CONFIG_SPL_BUILD) ns.bdf = pci_ofplat_get_devfn(dtplat->reg[0]); +#endif memcpy(plat, &ns, sizeof(ns)); #else int ret; diff --git a/include/ns16550.h b/include/ns16550.h index e7e68663d03..41b977b5b26 100644 --- a/include/ns16550.h +++ b/include/ns16550.h @@ -74,7 +74,7 @@ struct ns16550_plat { int clock; u32 fcr; int flags; -#if defined(CONFIG_PCI) && defined(CONFIG_SPL) +#if IS_ENABLED_NOCHECK(CONFIG_PCI) && defined(CONFIG_SPL_BUILD) int bdf; #endif }; __________________________
It reads as, If the bdf exists, then if spl var PCI is not enabled, then init uart.
The PCI code will handle it if PCI is enabled.
So, PCI needs to be enabled, and SPL_PCI needs to not be enabled, and currently building for SPL
So the case is CONFIG_PCI=y, CONFIG_SPL_PCI=n and CONFIG_SPL_BUILD=y? That's what the code needs to test and express clearly and likely with a comment.

Prepare for linking setup_sata only when CONFIG_SATA/CONFIG_SPL_SATA is defined.
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com Reviewed-by: Simon Glass sjg@chromium.org ---
Changes in v3: - Rebase on Simon's s/CMD_SATA/SATA/ change - commit message updated
Changes in v2: - use normal if, not preprocessor
board/solidrun/mx6cuboxi/mx6cuboxi.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/board/solidrun/mx6cuboxi/mx6cuboxi.c b/board/solidrun/mx6cuboxi/mx6cuboxi.c index cb14c2f30c9..6fa5cf4d27d 100644 --- a/board/solidrun/mx6cuboxi/mx6cuboxi.c +++ b/board/solidrun/mx6cuboxi/mx6cuboxi.c @@ -275,9 +275,8 @@ int board_early_init_f(void) { setup_iomux_uart();
-#ifdef CONFIG_SATA - setup_sata(); -#endif + if (CONFIG_IS_ENABLED(SATA)) + setup_sata(); setup_fec();
return 0;

On Mon, Mar 13, 2023 at 02:31:40PM -0700, Troy Kisky wrote:
Prepare for linking setup_sata only when CONFIG_SATA/CONFIG_SPL_SATA is defined.
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/next, thanks!

Prepare for linking setup_sata only when CONFIG_SATA/CONFIG_SPL_SATA is defined.
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com ---
(no changes since v2)
Changes in v2: - new in series
board/wandboard/wandboard.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/board/wandboard/wandboard.c b/board/wandboard/wandboard.c index da995dd0f58..48914450a29 100644 --- a/board/wandboard/wandboard.c +++ b/board/wandboard/wandboard.c @@ -352,9 +352,8 @@ static void setup_display(void) int board_early_init_f(void) { setup_iomux_uart(); -#ifdef CONFIG_SATA - setup_sata(); -#endif + if (CONFIG_IS_ENABLED(SATA)) + setup_sata();
return 0; }

On Mon, Mar 13, 2023 at 02:31:41PM -0700, Troy Kisky wrote:
Prepare for linking setup_sata only when CONFIG_SATA/CONFIG_SPL_SATA is defined.
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com
Applied to u-boot/next, thanks!

This avoid an error with enable_sata_clock when defined(CONFIG_SATA) is changed to CONFIG_IS_ENABLED(SATA).
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com Reviewed-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
arch/arm/mach-imx/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile index 4dfc60eedc4..50f26975eac 100644 --- a/arch/arm/mach-imx/Makefile +++ b/arch/arm/mach-imx/Makefile @@ -54,7 +54,7 @@ obj-$(CONFIG_IMX_RDC) += rdc-sema.o ifneq ($(CONFIG_SPL_BUILD),y) obj-$(CONFIG_IMX_BOOTAUX) += imx_bootaux.o endif -obj-$(CONFIG_SATA) += sata.o +obj-$(CONFIG_$(SPL_)SATA) += sata.o obj-$(CONFIG_IMX_HAB) += hab.o obj-$(CONFIG_SYSCOUNTER_TIMER) += syscounter.o endif

On Mon, Mar 13, 2023 at 02:31:42PM -0700, Troy Kisky wrote:
This avoid an error with enable_sata_clock when defined(CONFIG_SATA) is changed to CONFIG_IS_ENABLED(SATA).
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/next, thanks!

This avoids an error when ifdef CONFIG_PCI is changed to if CONFIG_IS_ENABLED(PCI)
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com ---
(no changes since v2)
Changes in v2: - use an accessor function gd_set_pci_ram_top
arch/x86/cpu/i386/cpu.c | 2 +- include/asm-generic/global_data.h | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/x86/cpu/i386/cpu.c b/arch/x86/cpu/i386/cpu.c index c7f6c5a013e..068670dfc41 100644 --- a/arch/x86/cpu/i386/cpu.c +++ b/arch/x86/cpu/i386/cpu.c @@ -415,7 +415,7 @@ int cpu_phys_address_size(void) /* Don't allow PCI region 3 to use memory in the 2-4GB memory hole */ static void setup_pci_ram_top(void) { - gd->pci_ram_top = 0x80000000U; + gd_set_pci_ram_top(0x80000000U); }
static void setup_mtrr(void) diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h index 987fb66c17a..952e17b2c13 100644 --- a/include/asm-generic/global_data.h +++ b/include/asm-generic/global_data.h @@ -560,6 +560,12 @@ static_assert(sizeof(struct global_data) == GD_SIZE); #define gd_event_state() NULL #endif
+#if CONFIG_IS_ENABLED(PCI) +#define gd_set_pci_ram_top(val) gd->pci_ram_top = val +#else +#define gd_set_pci_ram_top(val) +#endif + /** * enum gd_flags - global data flags *

On Mon, Mar 13, 2023 at 02:31:43PM -0700, Troy Kisky wrote:
This avoids an error when ifdef CONFIG_PCI is changed to if CONFIG_IS_ENABLED(PCI)
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com
(no changes since v2)
Changes in v2:
- use an accessor function gd_set_pci_ram_top
arch/x86/cpu/i386/cpu.c | 2 +- include/asm-generic/global_data.h | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/x86/cpu/i386/cpu.c b/arch/x86/cpu/i386/cpu.c index c7f6c5a013e..068670dfc41 100644 --- a/arch/x86/cpu/i386/cpu.c +++ b/arch/x86/cpu/i386/cpu.c @@ -415,7 +415,7 @@ int cpu_phys_address_size(void) /* Don't allow PCI region 3 to use memory in the 2-4GB memory hole */ static void setup_pci_ram_top(void) {
- gd->pci_ram_top = 0x80000000U;
- gd_set_pci_ram_top(0x80000000U);
}
static void setup_mtrr(void) diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h index 987fb66c17a..952e17b2c13 100644 --- a/include/asm-generic/global_data.h +++ b/include/asm-generic/global_data.h @@ -560,6 +560,12 @@ static_assert(sizeof(struct global_data) == GD_SIZE); #define gd_event_state() NULL #endif
+#if CONFIG_IS_ENABLED(PCI) +#define gd_set_pci_ram_top(val) gd->pci_ram_top = val +#else +#define gd_set_pci_ram_top(val) +#endif
/**
- enum gd_flags - global data flags
This ends up being a size change on I think it was coreboot64 and as such I'd like to see it come via the x86 tree, and run time tested first.

On Tue, Mar 14, 2023 at 5:32 AM Troy Kisky troykiskyboundary@gmail.com wrote:
This avoids an error when ifdef CONFIG_PCI is changed to if CONFIG_IS_ENABLED(PCI)
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com
(no changes since v2)
Changes in v2:
- use an accessor function gd_set_pci_ram_top
arch/x86/cpu/i386/cpu.c | 2 +- include/asm-generic/global_data.h | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-)
rebased on u-boot-x86/master, and applied to u-boot-x86, thanks!

This avoids an error in converting to CONFIG_IS_ENABLED(DM_PMIC). Many boards SPL code needs these definitions to compile, even if the functions are not linked.
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com Reviewed-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
include/power/pmic.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/power/pmic.h b/include/power/pmic.h index 70f2709bd0b..636221692d0 100644 --- a/include/power/pmic.h +++ b/include/power/pmic.h @@ -86,7 +86,7 @@ struct pmic { #endif /* CONFIG_IS_ENABLED(POWER_LEGACY) */
/* TODO: Change to CONFIG_IS_ENABLED(DM_PMIC) when SPL_DM_PMIC exists */ -#ifdef CONFIG_DM_PMIC +#if defined(CONFIG_DM_PMIC) || !CONFIG_IS_ENABLED(POWER_LEGACY) /** * U-Boot PMIC Framework * =====================

On Mon, Mar 13, 2023 at 02:31:44PM -0700, Troy Kisky wrote:
This avoids an error in converting to CONFIG_IS_ENABLED(DM_PMIC). Many boards SPL code needs these definitions to compile, even if the functions are not linked.
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/next, thanks!

Add script usage_of_is_enabled_check to print any configs that use CONFIG_IS_ENABLED instead of IS_ENABLED and vice versa.
Add usage_of_is_enabled_commit.sh to generate commits to fix the above issues.
You can remove entries from test/usage_of_is_enabled_todo.txt or the entire file and then run test/usage_of_is_enabled_commit.sh to convert to suggested usage of CONFIG_IS_ENABLED/IS_ENABLED
or run test/usage_of_is_enabled_check.sh to see which configs are still todo.
Signed-off-by: Troy Kisky troykiskyboundary@gmail.com Reviewed-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
.azure-pipelines.yml | 11 ++ .gitlab-ci.yml | 5 + test/usage_of_is_enabled_check.sh | 19 +++ test/usage_of_is_enabled_commit.sh | 12 ++ test/usage_of_is_enabled_correct.sh | 50 +++++++ test/usage_of_is_enabled_exempt.txt | 9 ++ test/usage_of_is_enabled_list.sh | 86 +++++++++++ test/usage_of_is_enabled_splcfg.txt | 21 +++ test/usage_of_is_enabled_todo.txt | 213 ++++++++++++++++++++++++++++ 9 files changed, 426 insertions(+) create mode 100755 test/usage_of_is_enabled_check.sh create mode 100755 test/usage_of_is_enabled_commit.sh create mode 100755 test/usage_of_is_enabled_correct.sh create mode 100644 test/usage_of_is_enabled_exempt.txt create mode 100755 test/usage_of_is_enabled_list.sh create mode 100644 test/usage_of_is_enabled_splcfg.txt create mode 100644 test/usage_of_is_enabled_todo.txt
diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml index 61ada4d681f..a9764006bc3 100644 --- a/.azure-pipelines.yml +++ b/.azure-pipelines.yml @@ -67,6 +67,17 @@ stages: :^doc/ :^arch/arm/dts/ :^scripts/kconfig/lkc.h :^include/linux/kconfig.h :^tools/ && exit 1 || exit 0
+ - job: check_usage_of_is_enabled + displayName: 'Check usage of CONFIG_IS_ENABLED vs IS_ENABLED' + pool: + vmImage: $(ubuntu_vm) + container: + image: $(ci_runner_image) + options: $(container_option) + steps: + # generate list of SPL configs + - script: test/usage_of_is_enabled_check.sh + - job: cppcheck displayName: 'Static code analysis with cppcheck' pool: diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index a89138701dc..b56446e6174 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -137,6 +137,11 @@ check for new CONFIG symbols outside Kconfig: :^doc/ :^arch/arm/dts/ :^scripts/kconfig/lkc.h :^include/linux/kconfig.h :^tools/ && exit 1 || exit 0
+check usage of CONFIG_IS_ENABLED vs IS_ENABLED: + stage: testsuites + script: + - ./test/usage_of_is_enabled_check.sh + # QA jobs for code analytics # static code analysis with cppcheck (we can add --enable=all later) cppcheck: diff --git a/test/usage_of_is_enabled_check.sh b/test/usage_of_is_enabled_check.sh new file mode 100755 index 00000000000..6bd5d9c1ac7 --- /dev/null +++ b/test/usage_of_is_enabled_check.sh @@ -0,0 +1,19 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0+ +# +# Written by Troy Kisky troykiskyboundary@gmail.com + +scriptdir=`dirname "$0"`; +${scriptdir}/usage_of_is_enabled_list.sh | grep -vw FOO; +if [ $? -eq 0 ] ; then + echo "The above may have incorrect usage of IS_ENABLED/"\ +"CONFIG_IS_ENABLED" + echo "Run test/usage_of_is_enabled_commit.sh and "\ +"squash with appropriate commit" + ret=1; +else + ret=0; +fi + +rm ${scriptdir}/splcfg.tmp ${scriptdir}/exclude.tmp +exit ${ret} diff --git a/test/usage_of_is_enabled_commit.sh b/test/usage_of_is_enabled_commit.sh new file mode 100755 index 00000000000..593dbd1428c --- /dev/null +++ b/test/usage_of_is_enabled_commit.sh @@ -0,0 +1,12 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0+ +# +# Written by Troy Kisky troykiskyboundary@gmail.com + +scriptdir=`dirname "$0"`; +${scriptdir}/usage_of_is_enabled_list.sh | \ +xargs -I {} sh -c "${scriptdir}/usage_of_is_enabled_correct.sh {}; \ +git commit -a -m"CONFIG_{}: correct usage of CONFIG_IS_ENABLED/IS_ENABLED";" + + +rm ${scriptdir}/splcfg.tmp ${scriptdir}/exclude.tmp diff --git a/test/usage_of_is_enabled_correct.sh b/test/usage_of_is_enabled_correct.sh new file mode 100755 index 00000000000..8724747beed --- /dev/null +++ b/test/usage_of_is_enabled_correct.sh @@ -0,0 +1,50 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0+ +# +# Written by Troy Kisky troykiskyboundary@gmail.com + +scriptdir=`dirname "$0"`; + +if [ -z "$1" ] ; then + echo missing config + exit 1; +fi +if [ ! -f "${scriptdir}/splcfg.tmp" ] ; then + echo missing splcfg.tmp + exit 1; +fi + + +grep -qw $1 ${scriptdir}/splcfg.tmp +if [ $? -ne 0 ] ; then + # not splcfg + # change CONFIG_IS_ENABLED to IS_ENABLED + git grep -l \ + -e "CONFIG_IS_ENABLED($1)" \ + | \ + xargs -IFile sh -c \ + " \ + sed -i -E "\ +s/CONFIG_IS_ENABLED($1)/IS_ENABLED(CONFIG_$1)/g; \ +" File"; +else + # splcfg + # change IS_ENABLED to CONFIG_IS_ENABLED + # change ifdef to CONFIG_IS_ENABLED + # change ifndef to !CONFIG_IS_ENABLED + # change defined to CONFIG_IS_ENABLED + git grep -l \ + -e "IS_ENABLED(CONFIG_$1)" \ + -e "^#ifdef[ \t]+CONFIG_$1>" \ + -e "^#ifndef[ \t]+CONFIG_$1>" \ + -e "defined(CONFIG_$1)" \ + | \ + xargs -IFile sh -c \ + " \ + sed -i -E "\ +s/([^_])IS_ENABLED(CONFIG_$1)/\1CONFIG_IS_ENABLED($1)/g; \ +s/^#ifdef[ \t]+CONFIG_$1>/#if CONFIG_IS_ENABLED($1)/; \ +s/^#ifndef[ \t]+CONFIG_$1>/#if !CONFIG_IS_ENABLED($1)/; \ +s/defined(CONFIG_$1)/CONFIG_IS_ENABLED($1)/; \ +" File"; +fi diff --git a/test/usage_of_is_enabled_exempt.txt b/test/usage_of_is_enabled_exempt.txt new file mode 100644 index 00000000000..d9fefd6cb6c --- /dev/null +++ b/test/usage_of_is_enabled_exempt.txt @@ -0,0 +1,9 @@ +BLOBLIST +BLOBLIST_FIXED +DM_PMIC_PFUZE100 +FOO +NAND_BOOT +OF_CONTROL +SYS_L2_PL310 +WATCHDOG +X86_64 diff --git a/test/usage_of_is_enabled_list.sh b/test/usage_of_is_enabled_list.sh new file mode 100755 index 00000000000..0f51d3602ca --- /dev/null +++ b/test/usage_of_is_enabled_list.sh @@ -0,0 +1,86 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0+ +# +# Written by Troy Kisky troykiskyboundary@gmail.com + +scriptdir=`dirname "$0"`; +# generate list of excluded configs +{ +# 1. ignore configs that have a number or string for a value +git grep -h -A2 -E "^config " '*Kconfig*' | \ +sed -En '/depends on/!p' | \ +sed -En '/^config/{h;$!d} ;H;x; s/config[ \t]+(.*)\n[ \t]*/config \1 #/p' | \ +sed -E "/#bool/d; /#def_bool/d; /#tristate/d; \ +/#default y/d; /#select/d; /#prompt/d; /#imply/d" | +sed -n -r "s/^config[[:space:]]+([0-9a-zA-Z_]+)/\n{\1}\n/p" | \ +sed -n -r 's/^{([0-9a-zA-Z_]+)}/\1/p' | sort -u; +# 2. configs that are exempt for other reasons +cat ${scriptdir}/usage_of_is_enabled_exempt.txt; +# 3. configs that need converted later +[ -f ${scriptdir}/usage_of_is_enabled_todo.txt ] && \ +cat ${scriptdir}/usage_of_is_enabled_todo.txt +} | sort -u > ${scriptdir}/exclude.tmp + +# generate list of CONFIGs that should use CONFIG_IS_ENABLED +{ +# 1. all obj-$(CONFIG_$(SPL_)xxx in Makefiles +git grep -h 'obj-$(CONFIG_$(SPL_' '*Makefile' | sed -e "s/SPL_TPL_/SPL_/"| \ +sed -n -r 's/obj-$(CONFIG_$(SPL_)([0-9a-zA-Z_]+))/\n{\1}\n/gp'| \ +sed -n -r 's/{([0-9a-zA-Z_]+)}/\1/p'; + +# 2. all SPL_xxx in Kconfig files +git grep -h -E 'config [ST]PL_' '*Kconfig*' | \ +sed -n -r "s/config [ST]PL_([0-9a-zA-Z_]+)/\n{\1}\n/p" | \ +sed -n -r 's/{([0-9a-zA-Z_]+)}/\1/p'; + +# 3. all CONFIG_CMD_xxx which already use CONFIG_IS_ENABLED +# The Makefile for most if these use ifndef CONFIG_SPL_BUILD +# instead of obj-$(CONFIG_$(SPL_)xxx +git grep -h -E 'CONFIG_IS_ENABLED(CMD_' | \ +sed -n -e "s/(CONFIG_IS_ENABLED(CMD_[0-9a-zA-Z_]*))/\n\1\n/gp"| \ +sed -n -r "s/CONFIG_IS_ENABLED((CMD_[0-9a-zA-Z_]+))/\1/p"; + +# 4. A list of other configs that should use CONFIG_IS_ENABLED +# This list could be reduced if obj-$(CONFIG_$(SPL_)xxx was used instead of +# ifndef CONFIG_SPL_BUILD in Makefiles +# usage_of_is_enabled_splcfg.txt mostly contains configs that should always +# be undefined in SPL/TPL +# Note: CONFIG_CLK was included to prevent a change in test_checkpatch.py +# which is checking for an error. +cat ${scriptdir}/usage_of_is_enabled_splcfg.txt; +} | sort -u | \ +comm -23 - ${scriptdir}/exclude.tmp >${scriptdir}/splcfg.tmp + +{ +# generate list of CONFIGs that incorrectly use CONFIG_IS_ENABLED +git grep -h CONFIG_IS_ENABLED | \ +sed -n -e "s/(CONFIG_IS_ENABLED([0-9a-zA-Z_]*))/\n\1\n/gp"| \ +sed -n -r "s/CONFIG_IS_ENABLED(([0-9a-zA-Z_]+))/\1/p" |sort -u| \ +comm -23 - ${scriptdir}/exclude.tmp | \ +comm -23 - ${scriptdir}/splcfg.tmp ; + +# generate list of CONFIGs that incorrectly use IS_ENABLED +git grep -h -w IS_ENABLED | \ +sed -n -e "s/(IS_ENABLED(CONFIG_[0-9a-zA-Z_]*))/\n\1\n/gp"| \ +sed -n -r "s/IS_ENABLED(CONFIG_([0-9a-zA-Z_]+))/\1/p" |sort -u| \ +join - ${scriptdir}/splcfg.tmp; + +# generate list of CONFIGs that incorrectly use ifdef +git grep -h -E "^#ifdef[ \t]+CONFIG_" | \ +sed -n -E "s/(ifdef[ \t]+CONFIG_[0-9a-zA-Z_]+)/\n\1\n/p"| \ +sed -n -E "s/ifdef[ \t]+CONFIG_([0-9a-zA-Z_]+)/\1/p" |sort -u| \ +join - ${scriptdir}/splcfg.tmp ; + +# generate list of CONFIGs that incorrectly use ifndef +git grep -h -E "^#ifndef[ \t]+CONFIG_" | \ +sed -n -E "s/(ifndef[ \t]+CONFIG_[0-9a-zA-Z_]+)/\n\1\n/p"| \ +sed -n -E "s/ifndef[ \t]+CONFIG_([0-9a-zA-Z_]+)/\1/p" |sort -u| \ +join - ${scriptdir}/splcfg.tmp ; + +# generate list of CONFIGs that incorrectly use defined +git grep -h -E "defined(CONFIG_" | \ +sed -n -E "s/(defined(CONFIG_[0-9a-zA-Z_]+))/\n\1\n/gp"| \ +sed -n -E "s/defined(CONFIG_([0-9a-zA-Z_]+))/\1/p" |sort -u| \ +join - ${scriptdir}/splcfg.tmp ; + +} | sort -u; diff --git a/test/usage_of_is_enabled_splcfg.txt b/test/usage_of_is_enabled_splcfg.txt new file mode 100644 index 00000000000..29d6257c5c7 --- /dev/null +++ b/test/usage_of_is_enabled_splcfg.txt @@ -0,0 +1,21 @@ +BZIP2 +CONFIG_CLK +CONSOLE_MUX +DM_EVENT +DM_HWSPINLOCK +DM_RNG +DM_STDIO +EFI_DEVICE_PATH_TO_TEXT +EFI_LOADER +ERRNO_STR +EVENT_DYNAMIC +GENERATE_SMBIOS_TABLE +IOMMU +MMC_HW_PARTITIONING +NAND_CS_INIT +OFNODE_MULTI_TREE +PINCTRL_ARMADA_38X +PRE_CONSOLE_BUFFER +RESET_MEDIATEK +RESET_ROCKCHIP +UT_DM diff --git a/test/usage_of_is_enabled_todo.txt b/test/usage_of_is_enabled_todo.txt new file mode 100644 index 00000000000..652637650dd --- /dev/null +++ b/test/usage_of_is_enabled_todo.txt @@ -0,0 +1,213 @@ +ACPIGEN +ARCH_MVEBU +ARCH_VERSAL_NET +ARM_PSCI_FW +ARMV8_SEC_FIRMWARE_SUPPORT +ATMEL_PIT_TIMER +BLK +BLOCK_CACHE +BOOTCOUNT_LIMIT +BOOTDEV_ETH +BOOTDEV_SPI_FLASH +BOOTSTAGE +BOOTSTD +BZIP2 +CLK +CLK_CCF +CLK_IMX6Q +CMD_DHCP +CMDLINE +CMD_PXE +CONSOLE_MUX +COREBOOT_SYSINFO +CPU +CRC32_VERIFY +CROS_EC_KEYB +DFU_SF_PART +DFU_VIRT +DISPLAY_AER_FULL +DM +DMA +DM_DMA +DM_ETH +DM_GPIO +DM_I2C +DM_KEYBOARD +DM_MMC +DM_PMIC +DM_PMIC_DA9063 +DM_REGULATOR +DM_RNG +DM_RTC +DM_SERIAL +DM_SPI +DM_SPI_FLASH +DM_USB +DM_USB_GADGET +DOS_PARTITION +DWC_AHSATA_AHCI +EFI_DT_FIXUP +EFI_EBBR_2_1_CONFORMANCE +EFI_LOADER +EFI_PARTITION +EFI_SCROLL_ON_CLEAR_SCREEN +EFI_TCG2_PROTOCOL_MEASURE_DTB +EFI_UNICODE_CAPITALIZATION +ENV_APPEND +ENV_IS_IN_EXT4 +ENV_IS_IN_FAT +ENV_IS_IN_FLASH +ENV_IS_IN_MMC +ENV_IS_IN_NAND +ENV_IS_IN_SPI_FLASH +ENV_IS_NOWHERE +ENV_WRITEABLE_LIST +ERRNO_STR +EVENT_DEBUG +EXPO +EXYNOS7870 +EXYNOS7880 +FASTBOOT_UUU_SUPPORT +FAT_WRITE +FIT +FIT_CIPHER +FIT_IMAGE_POST_PROCESS +FIT_SIGNATURE +FIT_VERBOSE +FPGA +FRU_SC +FSL_ISBC_KEY_EXT +FSL_LS_PPA +FS_LOADER +FSP_VERSION2 +GENERATE_ACPI_TABLE +GENERATE_SMBIOS_TABLE +GMAC_ROCKCHIP +GZIP +I2C_EEPROM +I8259_PIC +IMX_RDC +LED +LEGACY_IMAGE_FORMAT +LIB_UUID +LOG +LZ4 +LZMA +LZO +MALTA +MARY +MEMSIZE_IN_BYTES +MIPS_BOOT_CMDLINE_LEGACY +MIPS_BOOT_ENV_LEGACY +MIPS_BOOT_FDT +MMC +MMC_IO_VOLTAGE +MMC_VERBOSE +MULTI_DTB_FIT +MULTIPLEXER +MXC_OCOTP +NAND_DENALI +NET +NO_FB_CLEAR +NXP_FSPI +OF_LIBFDT +OF_LIVE +OFNODE_MULTI_TREE +OF_REAL +OF_TRANSLATE +OPTEE +OPTEE_IMAGE +PARTITIONS +PARTITION_TYPE_GUID +PARTITION_UUIDS +PCI +PCI_PNP +PG_WCOM_UBOOT_UPDATE_SUPPORTED +PHY +PHY_ATHEROS +PHY_CADENCE_SIERRA +PHY_CADENCE_TORRENT +PHY_FIXED +PINCTRL +PKCS7_MESSAGE_PARSER +PLATDATA +POWER_DOMAIN +POWER_I2C +QFW +QFW_PIO +RAM +RANDOM_UUID +RESET_MEDIATEK +RESTORE_EXCEPTION_VECTOR_BASE +RISCV_SMODE +ROCKCHIP_RK8XX_DISABLE_BOOT_ON_POWERON +RSA_PUBLIC_KEY_PARSER +RSA_VERIFY_WITH_PKEY +RTC_SANDBOX +SANDBOX +SATA +SEC_FIRMWARE_ARMV8_PSCI +SEMIHOSTING +SERIAL +SERIAL_PUTS +SERIAL_RX_BUFFER +SHA1 +SHA384 +SHA512 +SHA512_HW_ACCEL +SHA_HW_ACCEL +SHOW_BOOT_PROGRESS +SILENT_CONSOLE +SILENT_CONSOLE_UPDATE_ON_RELOC +SILENT_CONSOLE_UPDATE_ON_SET +SIMPLE_BUS_CORRECT_RANGE +SKIP_LOWLEVEL_INIT +SMC911X_32_BIT +SMP +SPI +SPI_BOOT +SPI_DIRMAP +SPI_FLASH_BAR +SPI_FLASH_MACRONIX +SPI_FLASH_MTD +SPI_FLASH_SFDP_SUPPORT +SPI_NOR_BOOT_SOFT_RESET_EXT_INVERT +STM32_ETZPC +SYS_CONSOLE_IS_IN_ENV +SYS_DCACHE_OFF +SYS_DEVICE_NULLDEV +SYS_ICACHE_OFF +SYSINFO +SYSRESET +SYS_THUMB_BUILD +SYS_WHITE_ON_BLACK +TARGET_DENEB +TARGET_EVB_RK3399 +TARGET_GIEDI +TARGET_KMCOGE5NE +TARGET_KMETER1 +TARGET_ST_STM32MP15x +TEST_KCONFIG +TIMER +_UNDEFINED +UNIT_TEST +USB_CDNS3_GADGET +USB_CDNS3_HOST +USB_ETHER +USB_GADGET +USB_GADGET_OS_DESCRIPTORS +USB_HOST +USB_STORAGE +USE_ARCH_MEMSET +UT_DM +UT_UNICODE +VID +VIRTIO +WDT +X509_CERTIFICATE_PARSER +X86_16BIT_INIT +XILINX_MICROBLAZE0_DELAY_SLOT_EXCEP +XILINX_MICROBLAZE0_USR_EXCEP +ZLIB +ZSTD +ZYNQMP_PSU_INIT_ENABLED

On Mon, Mar 13, 2023 at 02:31:22PM -0700, Troy Kisky wrote:
This patch set gets ready to checks the usage of CONFIG_IS_ENABLED/IS_ENABLED.
After the set has been applied, you can delete test/usage_of_is_enabled_todo.txt and run test/usage_of_is_enabled_commit.sh
The script test/usage_of_is_enabled_check.sh checks for new questionable uses of CONFIG_IS_ENABLED/IS_ENABLED and is added to .azure-pipelines.yml, and .gitlab-ci.yml
version 3 changes: Dropped changes to puma-rk3399 and ringneck-px30 in favor of Quentin Schulz's patch https://patchwork.ozlabs.org/project/uboot/patch/20230301-tsd-env-nowhere-kc... https://patchwork.ozlabs.org/project/uboot/patch/20230301-tsd-env-nowhere-kc... Please apply them before this series.
Drop gateworks: venice: Always define setup_fec and setup_eqos Drop arm: cpu: armv7: ls102xa: fdt: remove eth_device support Simon's patches took care of these
Add to todo list, these new questionable uses that snuck into the code. PARTITION_TYPE_GUID PHY_ATHEROS RTC_SANDBOX
Changes in v3: remove error entirely and prevent with Kconfig new patch to address Tom's concerns
- Rebase on Simon's s/CMD_SATA/SATA/ change
- commit message updated
Changes in v2:
- new patch
- delay include of linux/kconfig.h to do from Makefile
- as suggested by Simon
- delay include of linux/kconfig.h to do from Makefile
- as suggested by Simon
- delay include of linux/kconfig.h to do from Makefile
- as suggested by Simon
- delay include of linux/kconfig.h to do from Makefile
- as suggested by Simon
- delay include of linux/kconfig.h to do from Makefile
- as suggested by Simon
- include linux/kconfig.h from tools/Makefile
- as suggested by Simon
- changed condition of when to include field bdf
- added protection to another instance of bdf in uart.c
- Thanks to Simon for getting this corrected
- use normal if, not preprocessor
- new in series
- use an accessor function gd_set_pci_ram_top
So, I've provided some feedback on a few of these in particular, and applied a number of others. The biggest problems I see are that I'm not sure that overall the changes make the code more readable. When I ran the whole conversion series there were a few places where the resulting code changed because of how things are used today.
Not as a problem for you to jump off and solve, but, looking harder at this makes me think that going for CONFIG_(SPL|TPL|VPL|etc)_FOO makes it harder to use the macros people expect from the kernel, where things are done as a suffix and so we could just use IS_ENABLED(CONFIG_FOO) and have things expand to test for CONFIG_FOO || (CONFIG_FOO_SPL && CONFIG_SPL_BUILD) || ..
participants (4)
-
Bin Meng
-
Simon Glass
-
Tom Rini
-
Troy Kisky