Hi Alex,
On Wed, 19 May 2021 at 11:44, Alex G <mr.nuke...@gmail.com> wrote:
>
>
>
> On 5/19/21 11:36 AM, Simon Glass wrote:
> > Hi Alexandru,
> >
> > On Mon, 17 May 2021 at 10:38, Alexandru Gagniuc <mr.nuke...@gmail.com>
> > wrote:
> >>
> >> From: Simon Glass <s...@chromium.org>
> >>
> >> We already have a host Kconfig for SHA1. Use CONFIG_IS_ENABLED(SHA1)
> >> directly in the code shared with the host build, so we can drop the
> >> unnecessary indirection.
> >>
> >> Signed-off-by: Simon Glass <s...@chromium.org>
> >> Reviewed-by: Alexandru Gagniuc <mr.nuke...@gmail.com>
> >> Signed-off-by: Alexandru Gagniuc <mr.nuke...@gmail.com>
> >> ---
> >> common/image-fit.c | 2 +-
> >> include/image.h | 8 --------
> >> 2 files changed, 1 insertion(+), 9 deletions(-)
> >>
> >> diff --git a/common/image-fit.c b/common/image-fit.c
> >> index e614643fe3..24e92a8e92 100644
> >> --- a/common/image-fit.c
> >> +++ b/common/image-fit.c
> >> @@ -1218,7 +1218,7 @@ int calculate_hash(const void *data, int data_len,
> >> const char *algo,
> >> CHUNKSZ_CRC32);
> >> *((uint32_t *)value) = cpu_to_uimage(*((uint32_t
> >> *)value));
> >> *value_len = 4;
> >> - } else if (IMAGE_ENABLE_SHA1 && strcmp(algo, "sha1") == 0) {
> >> + } else if (CONFIG_IS_ENABLED(SHA1) && strcmp(algo, "sha1") == 0) {
> >
> > This can only work if the my host Kconfig patch comes first. Otherwise
> > this code will just be skipped on the host.
>
> I was scratching my head too as to why this works in practice, but not
> in theory. There is a #define CONFIG_SHA1 in image.h.
>
> Although not a perfect fix, we go from two ways to enable SHA1 ("#define
> IMAGE_ENABLE_SHA1", and "#define CONFIG_SHA1"), to just one. That's why
> I think this change is an improvement, and part of this series.
No, we really should not do that...everything needs to be in Kconfig.
[..]
Regards,
Simon
