[U-Boot] [PATCH 00/11] zap: Do not use macros that are equivalent to IS_ENABLED(CONFIG_...)
Please stop such coding habit as follows:
#ifdef CONFIG_FOO # define ENABLE_FOO 1 #else # define ENABLE_FOO 0 #endif
Use IS_ENABLED(CONFIG_FOO), instead.
Masahiro Yamada (11): image: zap IMAGE_ENABLE_RAMDISK_HIGH image: zap IMAGE_ENABLE_OF_LIBFDT image: zap IMAGE_BOOT_GET_CMDLINE image: zap IMAGE_OF_BOARD_SETUP image: zap IMAGE_OF_SYSTEM_SETUP ARM: bootm: BOOTM_ENABLE_SERIAL_TAG ARM: bootm: BOOTM_ENABLE_CMDLINE_TAG ARM: bootm: BOOTM_ENABLE_REVISION_TAG ARM: bootm: BOOTM_ENABLE_MEMORY_TAG ARM: bootm: BOOTM_ENABLE_INITRD_TAG ARM: bootm: drop redundant #ifdef conditional
arch/arc/lib/bootm.c | 2 +- arch/arm/include/asm/bootm.h | 22 ---------------------- arch/arm/lib/bootm.c | 16 +++++++--------- common/image-fdt.c | 6 +++--- common/image.c | 10 +++++----- include/image.h | 30 ------------------------------ 6 files changed, 16 insertions(+), 70 deletions(-)
Do not use a macro that just ends up in unreadable code.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com ---
common/image.c | 2 +- include/image.h | 6 ------ 2 files changed, 1 insertion(+), 7 deletions(-)
diff --git a/common/image.c b/common/image.c index c36927f..d980725 100644 --- a/common/image.c +++ b/common/image.c @@ -1354,7 +1354,7 @@ int image_setup_linux(bootm_headers_t *images) return ret; } } - if (IMAGE_ENABLE_RAMDISK_HIGH) { + if (IS_ENABLED(CONFIG_SYS_BOOT_RAMDISK_HIGH)) { rd_len = images->rd_end - images->rd_start; ret = boot_ramdisk_high(lmb, images->rd_start, rd_len, initrd_start, initrd_end); diff --git a/include/image.h b/include/image.h index 299d6d2..9eab991 100644 --- a/include/image.h +++ b/include/image.h @@ -96,12 +96,6 @@ struct lmb;
#endif /* CONFIG_FIT */
-#ifdef CONFIG_SYS_BOOT_RAMDISK_HIGH -# define IMAGE_ENABLE_RAMDISK_HIGH 1 -#else -# define IMAGE_ENABLE_RAMDISK_HIGH 0 -#endif - #ifdef CONFIG_OF_LIBFDT # define IMAGE_ENABLE_OF_LIBFDT 1 #else
Do not use a macro that just ends up in unreadable code.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com ---
arch/arc/lib/bootm.c | 2 +- arch/arm/lib/bootm.c | 4 ++-- common/image.c | 6 +++--- include/image.h | 6 ------ 4 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/arch/arc/lib/bootm.c b/arch/arc/lib/bootm.c index 04d9d9c..c8569a6 100644 --- a/arch/arc/lib/bootm.c +++ b/arch/arc/lib/bootm.c @@ -75,7 +75,7 @@ static void boot_jump_linux(bootm_headers_t *images, int flag)
cleanup_before_linux();
- if (IMAGE_ENABLE_OF_LIBFDT && images->ft_len) { + if (IS_ENABLED(CONFIG_OF_LIBFDT) && images->ft_len) { r0 = 2; r2 = (unsigned int)images->ft_addr; } else { diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c index a477cae..85adad1 100644 --- a/arch/arm/lib/bootm.c +++ b/arch/arm/lib/bootm.c @@ -205,7 +205,7 @@ static void boot_prep_linux(bootm_headers_t *images) { char *commandline = getenv("bootargs");
- if (IMAGE_ENABLE_OF_LIBFDT && images->ft_len) { + if (IS_ENABLED(CONFIG_OF_LIBFDT) && images->ft_len) { #ifdef CONFIG_OF_LIBFDT debug("using: FDT\n"); if (image_setup_linux(images)) { @@ -302,7 +302,7 @@ static void boot_jump_linux(bootm_headers_t *images, int flag) bootstage_mark(BOOTSTAGE_ID_RUN_OS); announce_and_cleanup(fake);
- if (IMAGE_ENABLE_OF_LIBFDT && images->ft_len) + if (IS_ENABLED(CONFIG_OF_LIBFDT) && images->ft_len) r2 = (unsigned long)images->ft_addr; else r2 = gd->bd->bi_boot_params; diff --git a/common/image.c b/common/image.c index d980725..2c5e0b5 100644 --- a/common/image.c +++ b/common/image.c @@ -1343,7 +1343,7 @@ int image_setup_linux(bootm_headers_t *images) ulong rd_len; int ret;
- if (IMAGE_ENABLE_OF_LIBFDT) + if (IS_ENABLED(CONFIG_OF_LIBFDT)) boot_fdt_add_mem_rsv_regions(lmb, *of_flat_tree);
if (IMAGE_BOOT_GET_CMDLINE) { @@ -1362,13 +1362,13 @@ int image_setup_linux(bootm_headers_t *images) return ret; }
- if (IMAGE_ENABLE_OF_LIBFDT) { + if (IS_ENABLED(CONFIG_OF_LIBFDT)) { ret = boot_relocate_fdt(lmb, of_flat_tree, &of_size); if (ret) return ret; }
- if (IMAGE_ENABLE_OF_LIBFDT && of_size) { + if (IS_ENABLED(CONFIG_OF_LIBFDT) && of_size) { ret = image_setup_libfdt(images, *of_flat_tree, of_size, lmb); if (ret) return ret; diff --git a/include/image.h b/include/image.h index 9eab991..d19f45c 100644 --- a/include/image.h +++ b/include/image.h @@ -96,12 +96,6 @@ struct lmb;
#endif /* CONFIG_FIT */
-#ifdef CONFIG_OF_LIBFDT -# define IMAGE_ENABLE_OF_LIBFDT 1 -#else -# define IMAGE_ENABLE_OF_LIBFDT 0 -#endif - #ifdef CONFIG_SYS_BOOT_GET_CMDLINE # define IMAGE_BOOT_GET_CMDLINE 1 #else
Do not use a macro that just ends up in unreadable code.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com ---
common/image.c | 2 +- include/image.h | 6 ------ 2 files changed, 1 insertion(+), 7 deletions(-)
diff --git a/common/image.c b/common/image.c index 2c5e0b5..afbc349 100644 --- a/common/image.c +++ b/common/image.c @@ -1346,7 +1346,7 @@ int image_setup_linux(bootm_headers_t *images) if (IS_ENABLED(CONFIG_OF_LIBFDT)) boot_fdt_add_mem_rsv_regions(lmb, *of_flat_tree);
- if (IMAGE_BOOT_GET_CMDLINE) { + if (IS_ENABLED(CONFIG_SYS_BOOT_GET_CMDLINE)) { ret = boot_get_cmdline(lmb, &images->cmdline_start, &images->cmdline_end); if (ret) { diff --git a/include/image.h b/include/image.h index d19f45c..8da4ef0 100644 --- a/include/image.h +++ b/include/image.h @@ -96,12 +96,6 @@ struct lmb;
#endif /* CONFIG_FIT */
-#ifdef CONFIG_SYS_BOOT_GET_CMDLINE -# define IMAGE_BOOT_GET_CMDLINE 1 -#else -# define IMAGE_BOOT_GET_CMDLINE 0 -#endif - #ifdef CONFIG_OF_BOARD_SETUP # define IMAGE_OF_BOARD_SETUP 1 #else
Do not use a macro that just ends up in unreadable code.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com ---
common/image-fdt.c | 4 ++-- include/image.h | 6 ------ 2 files changed, 2 insertions(+), 8 deletions(-)
diff --git a/common/image-fdt.c b/common/image-fdt.c index 5e4e5bd..465f13f 100644 --- a/common/image-fdt.c +++ b/common/image-fdt.c @@ -483,7 +483,7 @@ int image_setup_libfdt(bootm_headers_t *images, void *blob, printf("ERROR: arch-specific fdt fixup failed\n"); goto err; } - if (IMAGE_OF_BOARD_SETUP) { + if (IS_ENABLED(CONFIG_OF_BOARD_SETUP)) { fdt_ret = ft_board_setup(blob, gd->bd); if (fdt_ret) { printf("ERROR: board-specific fdt fixup failed: %s\n", @@ -522,7 +522,7 @@ int image_setup_libfdt(bootm_headers_t *images, void *blob, goto err;
#if defined(CONFIG_SOC_KEYSTONE) - if (IMAGE_OF_BOARD_SETUP) + if (IS_ENABLED(CONFIG_OF_BOARD_SETUP)) ft_board_setup_ex(blob, gd->bd); #endif
diff --git a/include/image.h b/include/image.h index 8da4ef0..0bf79a8 100644 --- a/include/image.h +++ b/include/image.h @@ -96,12 +96,6 @@ struct lmb;
#endif /* CONFIG_FIT */
-#ifdef CONFIG_OF_BOARD_SETUP -# define IMAGE_OF_BOARD_SETUP 1 -#else -# define IMAGE_OF_BOARD_SETUP 0 -#endif - #ifdef CONFIG_OF_SYSTEM_SETUP # define IMAGE_OF_SYSTEM_SETUP 1 #else
Do not use a macro that just ends up in unreadable code.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com ---
common/image-fdt.c | 2 +- include/image.h | 6 ------ 2 files changed, 1 insertion(+), 7 deletions(-)
diff --git a/common/image-fdt.c b/common/image-fdt.c index 465f13f..966dece 100644 --- a/common/image-fdt.c +++ b/common/image-fdt.c @@ -491,7 +491,7 @@ int image_setup_libfdt(bootm_headers_t *images, void *blob, goto err; } } - if (IMAGE_OF_SYSTEM_SETUP) { + if (IS_ENABLED(CONFIG_OF_SYSTEM_SETUP)) { fdt_ret = ft_system_setup(blob, gd->bd); if (fdt_ret) { printf("ERROR: system-specific fdt fixup failed: %s\n", diff --git a/include/image.h b/include/image.h index 0bf79a8..90c06a1 100644 --- a/include/image.h +++ b/include/image.h @@ -96,12 +96,6 @@ struct lmb;
#endif /* CONFIG_FIT */
-#ifdef CONFIG_OF_SYSTEM_SETUP -# define IMAGE_OF_SYSTEM_SETUP 1 -#else -# define IMAGE_OF_SYSTEM_SETUP 0 -#endif - /* * Operating System Codes */
Do not use a macro that just ends up in unreadable code.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com ---
arch/arm/include/asm/bootm.h | 2 -- arch/arm/lib/bootm.c | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/arch/arm/include/asm/bootm.h b/arch/arm/include/asm/bootm.h index 436c35a..f16a522 100644 --- a/arch/arm/include/asm/bootm.h +++ b/arch/arm/include/asm/bootm.h @@ -42,10 +42,8 @@ extern void udc_disconnect(void); #endif
#ifdef CONFIG_SERIAL_TAG - #define BOOTM_ENABLE_SERIAL_TAG 1 void get_board_serial(struct tag_serialnr *serialnr); #else - #define BOOTM_ENABLE_SERIAL_TAG 0 static inline void get_board_serial(struct tag_serialnr *serialnr) { } diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c index 85adad1..cba0ceb 100644 --- a/arch/arm/lib/bootm.c +++ b/arch/arm/lib/bootm.c @@ -216,7 +216,7 @@ static void boot_prep_linux(bootm_headers_t *images) } else if (BOOTM_ENABLE_TAGS) { debug("using: ATAGS\n"); setup_start_tag(gd->bd); - if (BOOTM_ENABLE_SERIAL_TAG) + if (IS_ENABLED(CONFIG_SERIAL_TAG)) setup_serial_tag(¶ms); if (BOOTM_ENABLE_CMDLINE_TAG) setup_commandline_tag(gd->bd, commandline);
Do not use a macro that just ends up in unreadable code.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com ---
arch/arm/include/asm/bootm.h | 6 ------ arch/arm/lib/bootm.c | 2 +- 2 files changed, 1 insertion(+), 7 deletions(-)
diff --git a/arch/arm/include/asm/bootm.h b/arch/arm/include/asm/bootm.h index f16a522..eda8228 100644 --- a/arch/arm/include/asm/bootm.h +++ b/arch/arm/include/asm/bootm.h @@ -29,12 +29,6 @@ extern void udc_disconnect(void); # define BOOTM_ENABLE_MEMORY_TAGS 0 #endif
-#ifdef CONFIG_CMDLINE_TAG - #define BOOTM_ENABLE_CMDLINE_TAG 1 -#else - #define BOOTM_ENABLE_CMDLINE_TAG 0 -#endif - #ifdef CONFIG_INITRD_TAG #define BOOTM_ENABLE_INITRD_TAG 1 #else diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c index cba0ceb..d621644 100644 --- a/arch/arm/lib/bootm.c +++ b/arch/arm/lib/bootm.c @@ -218,7 +218,7 @@ static void boot_prep_linux(bootm_headers_t *images) setup_start_tag(gd->bd); if (IS_ENABLED(CONFIG_SERIAL_TAG)) setup_serial_tag(¶ms); - if (BOOTM_ENABLE_CMDLINE_TAG) + if (IS_ENABLED(CONFIG_CMDLINE_TAG)) setup_commandline_tag(gd->bd, commandline); if (BOOTM_ENABLE_REVISION_TAG) setup_revision_tag(¶ms);
Do not use a macro that just ends up in unreadable code.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com ---
arch/arm/include/asm/bootm.h | 2 -- arch/arm/lib/bootm.c | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/arch/arm/include/asm/bootm.h b/arch/arm/include/asm/bootm.h index eda8228..c83fb87 100644 --- a/arch/arm/include/asm/bootm.h +++ b/arch/arm/include/asm/bootm.h @@ -44,10 +44,8 @@ static inline void get_board_serial(struct tag_serialnr *serialnr) #endif
#ifdef CONFIG_REVISION_TAG - #define BOOTM_ENABLE_REVISION_TAG 1 u32 get_board_rev(void); #else - #define BOOTM_ENABLE_REVISION_TAG 0 static inline u32 get_board_rev(void) { return 0; diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c index d621644..2818aee 100644 --- a/arch/arm/lib/bootm.c +++ b/arch/arm/lib/bootm.c @@ -220,7 +220,7 @@ static void boot_prep_linux(bootm_headers_t *images) setup_serial_tag(¶ms); if (IS_ENABLED(CONFIG_CMDLINE_TAG)) setup_commandline_tag(gd->bd, commandline); - if (BOOTM_ENABLE_REVISION_TAG) + if (IS_ENABLED(CONFIG_REVISION_TAG)) setup_revision_tag(¶ms); if (BOOTM_ENABLE_MEMORY_TAGS) setup_memory_tags(gd->bd);
Do not use a macro that just ends up in unreadable code.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com ---
arch/arm/include/asm/bootm.h | 6 ------ arch/arm/lib/bootm.c | 2 +- 2 files changed, 1 insertion(+), 7 deletions(-)
diff --git a/arch/arm/include/asm/bootm.h b/arch/arm/include/asm/bootm.h index c83fb87..576b6f1 100644 --- a/arch/arm/include/asm/bootm.h +++ b/arch/arm/include/asm/bootm.h @@ -23,12 +23,6 @@ extern void udc_disconnect(void); # define BOOTM_ENABLE_TAGS 0 #endif
-#ifdef CONFIG_SETUP_MEMORY_TAGS -# define BOOTM_ENABLE_MEMORY_TAGS 1 -#else -# define BOOTM_ENABLE_MEMORY_TAGS 0 -#endif - #ifdef CONFIG_INITRD_TAG #define BOOTM_ENABLE_INITRD_TAG 1 #else diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c index 2818aee..da6434e 100644 --- a/arch/arm/lib/bootm.c +++ b/arch/arm/lib/bootm.c @@ -222,7 +222,7 @@ static void boot_prep_linux(bootm_headers_t *images) setup_commandline_tag(gd->bd, commandline); if (IS_ENABLED(CONFIG_REVISION_TAG)) setup_revision_tag(¶ms); - if (BOOTM_ENABLE_MEMORY_TAGS) + if (IS_ENABLED(CONFIG_SETUP_MEMORY_TAGS)) setup_memory_tags(gd->bd); if (BOOTM_ENABLE_INITRD_TAG) { if (images->rd_start && images->rd_end) {
Do not use a macro that just ends up in unreadable code.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com ---
arch/arm/include/asm/bootm.h | 6 ------ arch/arm/lib/bootm.c | 2 +- 2 files changed, 1 insertion(+), 7 deletions(-)
diff --git a/arch/arm/include/asm/bootm.h b/arch/arm/include/asm/bootm.h index 576b6f1..6cf410c 100644 --- a/arch/arm/include/asm/bootm.h +++ b/arch/arm/include/asm/bootm.h @@ -23,12 +23,6 @@ extern void udc_disconnect(void); # define BOOTM_ENABLE_TAGS 0 #endif
-#ifdef CONFIG_INITRD_TAG - #define BOOTM_ENABLE_INITRD_TAG 1 -#else - #define BOOTM_ENABLE_INITRD_TAG 0 -#endif - #ifdef CONFIG_SERIAL_TAG void get_board_serial(struct tag_serialnr *serialnr); #else diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c index da6434e..d036d3e 100644 --- a/arch/arm/lib/bootm.c +++ b/arch/arm/lib/bootm.c @@ -224,7 +224,7 @@ static void boot_prep_linux(bootm_headers_t *images) setup_revision_tag(¶ms); if (IS_ENABLED(CONFIG_SETUP_MEMORY_TAGS)) setup_memory_tags(gd->bd); - if (BOOTM_ENABLE_INITRD_TAG) { + if (IS_ENABLED(CONFIG_INITRD_TAG)) { if (images->rd_start && images->rd_end) { setup_initrd_tag(gd->bd, images->rd_start, images->rd_end);
This if-block is compiled only when IS_ENABLED(CONFIG_OF_LIBFDT) is true. The #ifdef CONFIG_OF_LIBFDT inside of it is redundant.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com ---
arch/arm/lib/bootm.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c index d036d3e..7e729f7 100644 --- a/arch/arm/lib/bootm.c +++ b/arch/arm/lib/bootm.c @@ -206,13 +206,11 @@ static void boot_prep_linux(bootm_headers_t *images) char *commandline = getenv("bootargs");
if (IS_ENABLED(CONFIG_OF_LIBFDT) && images->ft_len) { -#ifdef CONFIG_OF_LIBFDT debug("using: FDT\n"); if (image_setup_linux(images)) { printf("FDT creation failed! hanging..."); hang(); } -#endif } else if (BOOTM_ENABLE_TAGS) { debug("using: ATAGS\n"); setup_start_tag(gd->bd);
On Friday, December 18, 2015 at 03:04:59 AM, Masahiro Yamada wrote:
Please stop such coding habit as follows:
#ifdef CONFIG_FOO # define ENABLE_FOO 1 #else # define ENABLE_FOO 0 #endif
Oh yes, this is horrible, kill this with fire :)
Use IS_ENABLED(CONFIG_FOO), instead.
Masahiro Yamada (11): image: zap IMAGE_ENABLE_RAMDISK_HIGH image: zap IMAGE_ENABLE_OF_LIBFDT image: zap IMAGE_BOOT_GET_CMDLINE image: zap IMAGE_OF_BOARD_SETUP image: zap IMAGE_OF_SYSTEM_SETUP ARM: bootm: BOOTM_ENABLE_SERIAL_TAG ARM: bootm: BOOTM_ENABLE_CMDLINE_TAG ARM: bootm: BOOTM_ENABLE_REVISION_TAG ARM: bootm: BOOTM_ENABLE_MEMORY_TAG ARM: bootm: BOOTM_ENABLE_INITRD_TAG ARM: bootm: drop redundant #ifdef conditional
arch/arc/lib/bootm.c | 2 +- arch/arm/include/asm/bootm.h | 22 ---------------------- arch/arm/lib/bootm.c | 16 +++++++--------- common/image-fdt.c | 6 +++--- common/image.c | 10 +++++----- include/image.h | 30 ------------------------------ 6 files changed, 16 insertions(+), 70 deletions(-)
Best regards, Marek Vasut
Hi Masahiro,
On 17 December 2015 at 20:23, Marek Vasut marex@denx.de wrote:
On Friday, December 18, 2015 at 03:04:59 AM, Masahiro Yamada wrote:
Please stop such coding habit as follows:
#ifdef CONFIG_FOO # define ENABLE_FOO 1 #else # define ENABLE_FOO 0 #endif
Oh yes, this is horrible, kill this with fire :)
Use IS_ENABLED(CONFIG_FOO), instead.
Masahiro Yamada (11): image: zap IMAGE_ENABLE_RAMDISK_HIGH image: zap IMAGE_ENABLE_OF_LIBFDT image: zap IMAGE_BOOT_GET_CMDLINE image: zap IMAGE_OF_BOARD_SETUP image: zap IMAGE_OF_SYSTEM_SETUP ARM: bootm: BOOTM_ENABLE_SERIAL_TAG ARM: bootm: BOOTM_ENABLE_CMDLINE_TAG ARM: bootm: BOOTM_ENABLE_REVISION_TAG ARM: bootm: BOOTM_ENABLE_MEMORY_TAG ARM: bootm: BOOTM_ENABLE_INITRD_TAG ARM: bootm: drop redundant #ifdef conditional
arch/arc/lib/bootm.c | 2 +- arch/arm/include/asm/bootm.h | 22 ---------------------- arch/arm/lib/bootm.c | 16 +++++++--------- common/image-fdt.c | 6 +++--- common/image.c | 10 +++++----- include/image.h | 30 ------------------------------ 6 files changed, 16 insertions(+), 70 deletions(-)
I definitely agree it would be good to drop this. But does it work? I thought IS_ENABLED() only worked for Kconfig options?
Regards, Simon
Hi Simon,
2015-12-19 3:57 GMT+09:00 Simon Glass sjg@chromium.org:
6 files changed, 16 insertions(+), 70 deletions(-)
I definitely agree it would be good to drop this. But does it work? I thought IS_ENABLED() only worked for Kconfig options?
Ah, I missed that.
IS_ENABLED() only works with macros defined as 1.
Most of include/configs/*.h define macros without values, so this series does not work.
I've marked it as Rejected.
Hi Masahiro,
On 19 December 2015 at 01:39, Masahiro Yamada yamada.masahiro@socionext.com wrote:
Hi Simon,
2015-12-19 3:57 GMT+09:00 Simon Glass sjg@chromium.org:
6 files changed, 16 insertions(+), 70 deletions(-)
I definitely agree it would be good to drop this. But does it work? I thought IS_ENABLED() only worked for Kconfig options?
Ah, I missed that.
IS_ENABLED() only works with macros defined as 1.
Most of include/configs/*.h define macros without values, so this series does not work.
I've marked it as Rejected.
I suppose we need to convert them all to Kconfig.
Regards, Simon
participants (3)
-
Marek Vasut -
Masahiro Yamada -
Simon Glass