[PATCH] image.h: Change android_image_get_dtb* to use uint and not u32
Eugeniu Rosca
erosca at de.adit-jv.com
Sun Feb 16 17:23:14 CET 2020
Hi Tom,
Cc: Masahiro Yamada
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 at gmail.com>
> Cc: Sam Protsenko <joe.skb7 at gmail.com>
> Signed-off-by: Tom Rini <trini at 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'.
That's because the 'image.h' itself seems to ensure the definition
of u32 and friends, as can be understood from below test:
--------------------- 8< --------------------------
1. Create a dummy C file which includes <image.h>
$ cat cmd/my-test.c
#define FORCE_BREAKAGE
#include <image.h>
2. Force a build-time error on including "int-ll64.h", which defines
u32 and friends for all architectures, as per commit 3747bdbb2bb83a
("arch: types.h: factor out fixed width typedefs to int-ll64.h")
(thanks Yamada-san):
$ git diff -- include/asm-generic/int-ll64.h
diff --git a/include/asm-generic/int-ll64.h b/include/asm-generic/int-ll64.h
index 7451718a61cd..f90dc2826071 100644
--- a/include/asm-generic/int-ll64.h
+++ b/include/asm-generic/int-ll64.h
@@ -15,6 +15,10 @@
* header files exported to user space
*/
+#ifdef FORCE_BREAKAGE
+#error "Included, thus providing (s,u){8,16,32,64}"
+#endif
+
typedef __signed__ char __s8;
typedef unsigned char __u8;
3. Compile the newly created C file for sandbox
$ make cmd/my-test.o
CC cmd/my-test.o
In file included from ./arch/sandbox/include/asm/types.h:9,
from include/linux/types.h:5,
from include/linux/string.h:4,
from include/compiler.h:126,
from include/image.h:18,
from cmd/my-test.c:3:
include/asm-generic/int-ll64.h:19:2: error: #error "Included, thus providing (s,u){8,16,32,64}"
--------------------- 8< --------------------------
So, to summarize, I don't fully understand the motivation behind this
change, unless there is a long-term goal to keep image.h (and/or other
U-Boot headers) free of u32 and Co.
Just to contrast the frequency of 'uint' vs 'u32' in v5.5 Linux release:
$ git log --oneline -S"\bu32\b" --pickaxe-regex v5.4..v5.5 | wc -l
1020
$ git log --oneline -S"\buint\b" --pickaxe-regex v5.4..v5.5 | wc -l
35
Assuming you still want the change, since there is no regression:
Reviewed-by: Eugeniu Rosca <roscaeugeniu at gmail.com>
--
Best Regards
Eugeniu Rosca
More information about the U-Boot
mailing list