[U-Boot] [PATCH 02/16] include: image.h: export hash algorithm helper functions

[AKASHI Takahiro]

Heinrich,

On Sat, Nov 16, 2019 at 06:59:31PM +0100, Heinrich Schuchardt wrote:
> On 11/13/19 1:52 AM, AKASHI Takahiro wrote:
> >This commit allows us to use common/image-sig.c even if CONFIG_FIT
> >is disabled but CONFIG_EFI_LOADER is enabled.
> >
> >Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> >---
> >  include/image.h | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> >diff --git a/include/image.h b/include/image.h
> >index bff87f51f01b..b79424a39c22 100644
> >--- a/include/image.h
> >+++ b/include/image.h
> >@@ -53,7 +53,7 @@ struct fdt_region;
> >
> >  #endif /* USE_HOSTCC */
> >
> >-#if IMAGE_ENABLE_FIT
> >+#if IMAGE_ENABLE_FIT || defined(CONFIG_EFI_SECURE_BOOT)
> 
> Is this #if needed at all?
> 
> Did you run Travis to check that defining CONFIG_EFI_SECURE_BOOT does
> not lead to build failures if IMAGE_ENABLE_FIT is not defined?

Before posting, I have confirmed that the following combination of
config options did not cause any build error, at least, on qemu_arm64:
1. CONFIG_EFI_SECURE_BOOT only
2. CONFIG_FIT (with/without CONFIG_FIT_SIGNATURE) only
3. CONFIG_EFI_SECURE_BOOT and CONFIG_FIT (with/without CONFIG_FIT_SIGNATURE)

I also successfully ran Travis although CONFI_EFI_SECURE_BOOT is not
enabled by default.

> >  #include <hash.h>
> >  #include <linux/libfdt.h>
> >  #include <fdt_support.h>
> >@@ -86,13 +86,14 @@ struct fdt_region;
> >  #endif
> >
> >  #if defined(CONFIG_FIT_ENABLE_SHA256_SUPPORT) || \
> >-	defined(CONFIG_SPL_SHA256_SUPPORT)
> >+	defined(CONFIG_SPL_SHA256_SUPPORT) || \
> >+	defined(CONFIG_EFI_SECURE_BOOT)
> 
> IMAGE_ENABLE_SHA256 is only used in common/image-fit.c. So why would you
> change anything here?

Okay, I have forgot to remove it.
At some point, I used to use both calculate_hash() and hash_calculate(),
then dropped the former.

> >  #define IMAGE_ENABLE_SHA256	1
> >  #else
> >  #define IMAGE_ENABLE_SHA256	0
> >  #endif
> >
> >-#endif /* IMAGE_ENABLE_FIT */
> >+#endif /* IMAGE_ENABLE_FIT || defined(CONFIG_EFI_SECURE_BOOT) */
> >
> >  #ifdef CONFIG_SYS_BOOT_GET_CMDLINE
> >  # define IMAGE_BOOT_GET_CMDLINE		1
> >@@ -1261,7 +1262,6 @@ struct crypto_algo *image_get_crypto_algo(const char *full_name);
> >  struct padding_algo *image_get_padding_algo(const char *name);
> >
> >  #if IMAGE_ENABLE_FIT
> >-
> 
> This change is unrelated. It should be in a separate patch. Or at least
> mention in the commit message that you cleaned up other parts of the code.

I don't want to get bothered with minor clean-up patches,
and will drop this hunk.

-Takahiro Akashi

> >  /**
> >   * fit_image_verify_required_sigs() - Verify signatures marked as 'required'
> >   *
> >@@ -1337,7 +1337,7 @@ static inline int fit_image_check_target_arch(const void *fdt, int node)
> >  #define fit_unsupported(msg)
> >  #define fit_unsupported_reset(msg)
> >  #endif /* CONFIG_FIT_VERBOSE */
> >-#endif /* CONFIG_FIT */
> >+#endif /* IMAGE_ENABLE_FIT */
> 
> Same here.
> 
> Best regards
> 
> Heinrich
> 
> >
> >  #if defined(CONFIG_ANDROID_BOOT_IMAGE)
> >  struct andr_img_hdr;
> >