[PATCH] image.h: isolate android_image_* functions from tooling
Tom Rini
trini at konsulko.com
Thu Mar 12 17:48:01 CET 2020
On Sun, Mar 08, 2020 at 01:11:18AM +0100, Eugeniu Rosca wrote:
> On Feb. 16, 2020, Tom reported [1] build failure of U-Boot in-tree
> tooling after applying https://patchwork.ozlabs.org/cover/1229663/
> ("[v6,0/7] rsa: extend rsa_verify() for UEFI secure boot").
>
> Later on, Heinrich stressed the urgency of the issue in
> https://patchwork.ozlabs.org/patch/1250858/#2379069:
>
> >>>>>>>>>
> We should finalize the topic as it stops EFI patches from being merged
> >>>>>>>>>
>
> On the surface, the problem is caused by U-Boot commits [2-3], which
> employed 'u32' in 'include/image.h', while historically U-Boot tooling
> stayed agnostic on the {u,s}{8,16,32} types.
>
> Thanks to Tom, Yamada-san and Heinrich, the following solutions have
> been put head-to-head ('+' pros, '-' cons):
>
> A. Use an equivalent fixed-size type, i.e. s/u32/uint32_t/ in both
> android function prototypes (image.h) and definitions (c file):
> + quick and low-line-count
> - creates a 'soup' of fixed-sized types in the Android C file
> - will confuse contributors
> - is going against Linux kernel best practices [4]
>
> B. Guard Android functions by '!defined(USE_HOSTCC)' in image.h:
> + quick and low-line-count
> + reflects the reality (no android function is used by tooling)
> + zero impact on other subsystems
> - ifdeffery may look annoying (pre-existing problem of image.h)
>
> C. Make {u8,u16,u32} available in U-Boot tooling:
> + quick and low-line-count
> + [Yamada-san][5]:
> * forbidding u32 for tools is questionable to me
> * Linux kernel and Barebox use {u8,u16,u32} for the tools space
> - breaks U-Boot tradition?
> - has larger impact than [A] and [B]
> - adds type complexity/inconsistency in the tooling space
>
> D. [Yamada-san] Refactor the headers to minimize the code shared
> between U-Boot space and tooling space:
> + probably the long-term solution
> - high effort
> - can be seen/done as an incremental update on top of [B]
>
> Looking at the above, [B] looks like the natural way to go forward.
>
> [1] https://patchwork.ozlabs.org/patch/1238245/#2363052
> [2] commit 7f2531502c74c0 ("image: android: Add routine to get dtbo params")
> [3] commit c3bfad825a71ea ("image: android: Add functions for handling dtb field")
> [4] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e6176fa4728fb6d
> ("checkpatch: add --strict warning for c99 fixed size typedefs : int<size>_t")
> [5] https://patchwork.ozlabs.org/patch/1238245/#2363340
>
> Cc: Masahiro Yamada <masahiroy at kernel.org>
> Cc: Heinrich Schuchardt <xypron.glpk at gmx.de>
> Cc: Sam Protsenko <joe.skb7 at gmail.com>
> Cc: Lokesh Vutla <lokeshvutla at ti.com>
> Cc: Simon Glass <sjg at chromium.org>
> Cc: AKASHI Takahiro <takahiro.akashi at linaro.org>
> Reported-by: Tom Rini <trini at konsulko.com>
> Signed-off-by: Eugeniu Rosca <erosca at de.adit-jv.com>
> Tested-by: Heinrich Schuchardt <xpyron.glpk at gmx.de>
Applied to u-boot/next, thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200312/b778ad9d/attachment.sig>
More information about the U-Boot
mailing list