[PATCHv2 2/5] image: cleanup pre-processor usage

Simon Glass sjg at chromium.org
Sat Dec 19 03:28:25 CET 2020


Hi Sebastian,

On Mon, 14 Dec 2020 at 16:42, Sebastian Reichel
<sebastian.reichel at collabora.com> wrote:
>
> Replace most #ifdef checks for USE_HOSTCC and CONFIG_*
> with normal if instructions.
>
> Signed-off-by: Sebastian Reichel <sebastian.reichel at collabora.com>
> ---
>  common/image-fit.c | 193 +++++++++++++++++++++------------------------
>  include/image.h    |   4 +
>  2 files changed, 96 insertions(+), 101 deletions(-)

Reviewed-by: Simon Glass <sjg at chromium.org>

>
> diff --git a/common/image-fit.c b/common/image-fit.c
> index c82d4d8015f0..1f382d87e207 100644
> --- a/common/image-fit.c
> +++ b/common/image-fit.c
> @@ -15,7 +15,6 @@
>  #include <u-boot/crc.h>
>  #else
>  #include <linux/compiler.h>
> -#include <linux/kconfig.h>
>  #include <common.h>
>  #include <errno.h>
>  #include <log.h>
> @@ -28,6 +27,7 @@ DECLARE_GLOBAL_DATA_PTR;
>  #include <bootm.h>
>  #include <image.h>
>  #include <bootstage.h>
> +#include <linux/kconfig.h>
>  #include <u-boot/crc.h>
>  #include <u-boot/md5.h>
>  #include <u-boot/sha1.h>
> @@ -486,16 +486,16 @@ void fit_image_print(const void *fit, int image_noffset, const char *p)
>
>         ret = fit_image_get_data_and_size(fit, image_noffset, &data, &size);
>
> -#ifndef USE_HOSTCC
> -       printf("%s  Data Start:   ", p);
> -       if (ret) {
> -               printf("unavailable\n");
> -       } else {
> -               void *vdata = (void *)data;
> +       if (!host_build()) {
> +               printf("%s  Data Start:   ", p);
> +               if (ret) {
> +                       printf("unavailable\n");
> +               } else {
> +                       void *vdata = (void *)data;
>
> -               printf("0x%08lx\n", (ulong)map_to_sysmem(vdata));
> +                       printf("0x%08lx\n", (ulong)map_to_sysmem(vdata));
> +               }
>         }
> -#endif
>
>         printf("%s  Data Size:    ", p);
>         if (ret)
> @@ -1420,7 +1420,6 @@ int fit_all_image_verify(const void *fit)
>         return 1;
>  }
>
> -#ifdef CONFIG_FIT_CIPHER
>  static int fit_image_uncipher(const void *fit, int image_noffset,
>                               void **data, size_t *size)
>  {
> @@ -1444,7 +1443,6 @@ static int fit_image_uncipher(const void *fit, int image_noffset,
>   out:
>         return ret;
>  }
> -#endif /* CONFIG_FIT_CIPHER */
>
>  /**
>   * fit_image_check_os - check whether image node is of a given os type
> @@ -1486,9 +1484,8 @@ int fit_image_check_arch(const void *fit, int noffset, uint8_t arch)
>         uint8_t image_arch;
>         int aarch32_support = 0;
>
> -#ifdef CONFIG_ARM64_SUPPORT_AARCH32
> -       aarch32_support = 1;
> -#endif
> +       if (IS_ENABLED(CONFIG_ARM64_SUPPORT_AARCH32))
> +               aarch32_support = 1;
>
>         if (fit_image_get_arch(fit, noffset, &image_arch))
>                 return 0;
> @@ -1977,13 +1974,13 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
>         }
>
>         bootstage_mark(bootstage_id + BOOTSTAGE_SUB_CHECK_ARCH);
> -#if !defined(USE_HOSTCC) && !defined(CONFIG_SANDBOX)
> -       if (!fit_image_check_target_arch(fit, noffset)) {
> -               puts("Unsupported Architecture\n");
> -               bootstage_error(bootstage_id + BOOTSTAGE_SUB_CHECK_ARCH);
> -               return -ENOEXEC;
> +       if (!host_build() && IS_ENABLED(CONFIG_SANDBOX)) {
> +               if (!fit_image_check_target_arch(fit, noffset)) {
> +                       puts("Unsupported Architecture\n");
> +                       bootstage_error(bootstage_id + BOOTSTAGE_SUB_CHECK_ARCH);
> +                       return -ENOEXEC;
> +               }
>         }
> -#endif
>
>  #ifndef USE_HOSTCC
>         fit_image_get_arch(fit, noffset, &os_arch);
> @@ -2029,9 +2026,8 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
>                 return -ENOENT;
>         }
>
> -#ifdef CONFIG_FIT_CIPHER
>         /* Decrypt data before uncompress/move */
> -       if (IMAGE_ENABLE_DECRYPT) {
> +       if (IS_ENABLED(CONFIG_FIT_CIPHER) && IMAGE_ENABLE_DECRYPT) {
>                 puts("   Decrypting Data ... ");
>                 if (fit_image_uncipher(fit, noffset, &buf, &size)) {
>                         puts("Error\n");
> @@ -2039,12 +2035,10 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
>                 }
>                 puts("OK\n");
>         }
> -#endif
>
> -#if !defined(USE_HOSTCC) && defined(CONFIG_FIT_IMAGE_POST_PROCESS)
>         /* perform any post-processing on the image data */
> -       board_fit_image_post_process(&buf, &size);
> -#endif
> +       if (!host_build() && IS_ENABLED(CONFIG_FIT_IMAGE_POST_PROCESS))
> +               board_fit_image_post_process(&buf, &size);
>
>         len = (ulong)size;
>
> @@ -2166,14 +2160,12 @@ int boot_get_fdt_fit(bootm_headers_t *images, ulong addr,
>         char *fit_uname_config_copy = NULL;
>         char *next_config = NULL;
>         ulong load, len;
> -#ifdef CONFIG_OF_LIBFDT_OVERLAY
>         ulong image_start, image_end;
>         ulong ovload, ovlen;
>         const char *uconfig;
>         const char *uname;
>         void *base, *ov;
>         int i, err, noffset, ov_noffset;
> -#endif
>
>         fit_uname = fit_unamep ? *fit_unamep : NULL;
>
> @@ -2212,84 +2204,83 @@ int boot_get_fdt_fit(bootm_headers_t *images, ulong addr,
>                 goto out;
>
>         /* we need to apply overlays */
> -
> -#ifdef CONFIG_OF_LIBFDT_OVERLAY
> -       image_start = addr;
> -       image_end = addr + fit_get_size(fit);
> -       /* verify that relocation took place by load address not being in fit */
> -       if (load >= image_start && load < image_end) {
> -               /* check is simplified; fit load checks for overlaps */
> -               printf("Overlayed FDT requires relocation\n");
> -               fdt_noffset = -EBADF;
> -               goto out;
> -       }
> -
> -       base = map_sysmem(load, len);
> -
> -       /* apply extra configs in FIT first, followed by args */
> -       for (i = 1; ; i++) {
> -               if (i < count) {
> -                       noffset = fit_conf_get_prop_node_index(fit, cfg_noffset,
> -                                                              FIT_FDT_PROP, i);
> -                       uname = fit_get_name(fit, noffset, NULL);
> -                       uconfig = NULL;
> -               } else {
> -                       if (!next_config)
> -                               break;
> -                       uconfig = next_config;
> -                       next_config = strchr(next_config, '#');
> -                       if (next_config)
> -                               *next_config++ = '\0';
> -                       uname = NULL;
> -
> -                       /*
> -                        * fit_image_load() would load the first FDT from the
> -                        * extra config only when uconfig is specified.
> -                        * Check if the extra config contains multiple FDTs and
> -                        * if so, load them.
> -                        */
> -                       cfg_noffset = fit_conf_get_node(fit, uconfig);
> -
> -                       i = 0;
> -                       count = fit_conf_get_prop_node_count(fit, cfg_noffset,
> -                                                            FIT_FDT_PROP);
> +       if (IS_ENABLED(CONFIG_OF_LIBFDT_OVERLAY)) {

The odd thing about the host build is that it normally does not have
access to the CONFIG options. Presumably
IS_ENABLED(CONFIG_OF_LIBFDT_OVERLAY) is therefore 0

However, in general on the host we want to enable all features
relevant to the host tools.

if (host_build() || IS_ENABLED(CONFIG_OF_LIBFDT_OVERLAY))

(BTW this is a general comment and I don't think it affects this
patch...if you do adjust things it should be done after this series,
since from what I can tell your series does not change behaviour)

Regards,
Simon


More information about the U-Boot mailing list