public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Eugeniu Rosca <roscaeugeniu@gmail.com>
To: u-boot@lists.denx.de
Subject: [PATCH] image.h: Change android_image_get_dtb* to use uint and not u32
Date: Sun, 16 Feb 2020 23:04:33 +0100	[thread overview]
Message-ID: <20200216220433.GA973@erosca> (raw)
In-Reply-To: <20200216165323.GC18302@bill-the-cat>

Hi Tom,

On Sun, Feb 16, 2020 at 11:53:23AM -0500, Tom Rini wrote:
> On Sun, Feb 16, 2020 at 05:23:14PM +0100, Eugeniu Rosca wrote:
> > On Fri, Feb 14, 2020 at 12:38:19PM -0500, Tom Rini wrote:
> > > The image.h header can be used fairly widely in U-Boot builds.  We
> > > cannot use u32 here as it may be used in cases where we don't have that
> > > typedef available and don't want to expose it either.  Use uint instead
> > > here.
> > > 
> > > Cc: Eugeniu Rosca <roscaeugeniu@gmail.com>
> > > Cc: Sam Protsenko <joe.skb7@gmail.com>
> > > Signed-off-by: Tom Rini <trini@konsulko.com>
> > > ---
> > >  include/image.h | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/include/image.h b/include/image.h
> > > index b316d167d8d7..1dc3b48d8689 100644
> > > --- a/include/image.h
> > > +++ b/include/image.h
> > > @@ -1425,9 +1425,9 @@ int android_image_get_ramdisk(const struct andr_img_hdr *hdr,
> > >  			      ulong *rd_data, ulong *rd_len);
> > >  int android_image_get_second(const struct andr_img_hdr *hdr,
> > >  			      ulong *second_data, ulong *second_len);
> > > -bool android_image_get_dtbo(ulong hdr_addr, ulong *addr, u32 *size);
> > > -bool android_image_get_dtb_by_index(ulong hdr_addr, u32 index, ulong *addr,
> > > -				    u32 *size);
> > > +bool android_image_get_dtbo(ulong hdr_addr, ulong *addr, uint *size);
> > > +bool android_image_get_dtb_by_index(ulong hdr_addr, uint index, ulong *addr,
> > > +				    uint *size);
> > 
> > While I think the change is harmless and brings some consistency and
> > visual comfort when reviewing the types employed in 'include/image.h',
> > I can hardly imagine a real-life breakage introduced by u32 in
> > 'include/image.h'.
> 
> I ran in to this in practice with
> http://patchwork.ozlabs.org/project/uboot/list/?series=155410&state=*
> applied.

Applying this series to u-boot/master, I am running into below build
failure [1], which I believe is something you try to fix in this patch.

It looks to me that U-Boot's 'include/image.h' is used not only by
files which are compiled for the target device, but also by files
located in 'tools/', which are compiled for the host with -DUSE_HOSTCC.
After inspecting the 'tools/' path of U-Boot repository, it looks like
the definition of 'u32' is indeed missing there, so I believe that's
the root cause of the build failure.

W.r.t. 'android_image_*' functions, I really doubt that they were
designed to be compiled with USE_HOSTCC. If so, then IMHO we shouldn't
try to make them compliant with USE_HOSTCC compilation, since this
will impose additional constraints/requirements to the development style
of those functions. IMHO we should just hide the android_image functions
on enabling -DUSE_HOSTCC, as shown in [2]. What's your view on that?

[1] Build error after applying to u-boot/master below series:
   http://patchwork.ozlabs.org/project/uboot/list/?series=155410&state=*

In file included from include/u-boot/rsa-mod-exp.h:10,
                 from ./tools/../lib/rsa/rsa-verify.c:22,
                 from tools/lib/rsa/rsa-verify.c:1:
include/image.h:1440:58: error: unknown type name ?u32?
 1440 | bool android_image_get_dtbo(ulong hdr_addr, ulong *addr, u32 *size);
      |                                                          ^~~
include/image.h:1441:53: error: unknown type name ?u32?
 1441 | bool android_image_get_dtb_by_index(ulong hdr_addr, u32 index, ulong *addr,
      |                                                     ^~~
include/image.h:1442:9: error: unknown type name ?u32?
 1442 |         u32 *size);
      |         ^~~
  HOSTCC  tools/asn1_compiler
make[1]: *** [scripts/Makefile.host:114: tools/lib/rsa/rsa-verify.o] Error 1
make[1]: *** Waiting for unfinished jobs....
  HOSTLD  tools/mkenvimage
make: *** [Makefile:1728: tools] Error 2

[2] Hide the android_image_* functions when USE_HOSTCC is enabled
diff --git a/include/image.h b/include/image.h
index ebec329582eb..0cdb2165fdaf 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1429,7 +1429,7 @@ struct cipher_algo *image_get_cipher_algo(const char *full_name);
 #endif /* CONFIG_FIT_VERBOSE */
 #endif /* CONFIG_FIT */
 
-#if defined(CONFIG_ANDROID_BOOT_IMAGE)
+#if defined(CONFIG_ANDROID_BOOT_IMAGE) && !defined(USE_HOSTCC)
 struct andr_img_hdr;
 int android_image_check_header(const struct andr_img_hdr *hdr);
 int android_image_get_kernel(const struct andr_img_hdr *hdr, int verify,
@@ -1449,7 +1449,7 @@ void android_print_contents(const struct andr_img_hdr *hdr);
 bool android_image_print_dtb_contents(ulong hdr_addr);
 #endif
 
-#endif /* CONFIG_ANDROID_BOOT_IMAGE */
+#endif /* CONFIG_ANDROID_BOOT_IMAGE && !USE_HOSTCC */
 
 /**
  * board_fit_config_name_match() - Check for a matching board name

-- 
Best Regards
Eugeniu Rosca

  reply	other threads:[~2020-02-16 22:04 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-14 17:38 [PATCH] image.h: Change android_image_get_dtb* to use uint and not u32 Tom Rini
2020-02-16 16:23 ` Eugeniu Rosca
2020-02-16 16:53   ` Tom Rini
2020-02-16 22:04     ` Eugeniu Rosca [this message]
2020-02-17  7:45       ` Masahiro Yamada
2020-02-17 10:47         ` Eugeniu Rosca
2020-02-17 17:35         ` Tom Rini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200216220433.GA973@erosca \
    --to=roscaeugeniu@gmail.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox