[PATCH 0/2] Keep the access to dtb_dt_embedded() within fdtdec

The 1st patch addresses comments from the post-review, available by link [1].
The 2nd patch fixes problems of dtb_dt_embedded() with checkpatch.
Links: [1] https://lore.kernel.org/u-boot/CAFLszTgEKamsa6FTnjzrEWQBLkqAR7EBbZqffx09AKgQ...
To: Tom Rini trini@konsulko.com To: Simon Glass sjg@chromium.org To: Rasmus Villemoes rasmus.villemoes@prevas.dk To: Ilias Apalodimas ilias.apalodimas@linaro.org To: Raymond Mao raymond.mao@linaro.org To: Marek Vasut marek.vasut+renesas@mailbox.org To: Sughosh Ganu sughosh.ganu@linaro.org To: Christian Marangi ansuelsmth@gmail.com To: Lad Prabhakar prabhakar.mahadev-lad.rj@bp.renesas.com To: Evgeny Bachinin EABachinin@salutedevices.com Cc: kernel@salutedevices.com Cc: evgen89bachinin@gmail.com Cc: u-boot@lists.denx.de
Signed-off-by: Evgeny Bachinin EABachinin@salutedevices.com --- Tested: * scripts/kernel-doc -v -none include/fdtdec.h * on production board with Amlogic AXG SoC & OF_EMBED=y * CI: https://github.com/u-boot/u-boot/pull/710
--- Evgeny Bachinin (2): fdtdec: encapsulate dtb_dt_embedded() within fdtdec: dtb_dt_embedded: replace ifdefs by IS_ENABLED()
common/board_r.c | 4 ++-- include/fdtdec.h | 24 +++++++----------------- lib/fdtdec.c | 26 ++++++++++++++++++++++++-- 3 files changed, 33 insertions(+), 21 deletions(-) --- base-commit: b841e559cd26ffcb20f22e8ee75debed9616c002 change-id: 20241207-dtb_dt_embedded_within_fdtdec-751a40ccf787
Best regards,

Patch keeps the access to dtb_dt_embedded() within fdtdec API, by means of new API function introduction. This new function is a common place for updating appropriate global_data fields for OF_EMBED case.
Also, the consequence of the patch is movement of '___dtb_dt_*begin' symbols' declaration from header file, because nobody used symbols outside the lib/fdtdec.c.
Signed-off-by: Evgeny Bachinin EABachinin@salutedevices.com Suggested-by: Simon Glass sjg@chromium.org --- common/board_r.c | 4 ++-- include/fdtdec.h | 24 +++++++----------------- lib/fdtdec.c | 26 ++++++++++++++++++++++++-- 3 files changed, 33 insertions(+), 21 deletions(-)
diff --git a/common/board_r.c b/common/board_r.c index 88dc756b2a5e5a44f55f9b0fd012adc798a8afdb..3c6a4c1f5df80d5f5bf86ed4047fccf981dbc244 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -155,11 +155,11 @@ static int initr_reloc_global_data(void)
/* * For CONFIG_OF_EMBED case the FDT is embedded into ELF, available by - * __dtb_dt_begin. After U-boot ELF self-relocation to RAM top address + * __dtb_dt_begin. After U-Boot ELF self-relocation to RAM top address * it is worth to update fdt_blob in global_data */ if (IS_ENABLED(CONFIG_OF_EMBED)) - gd->fdt_blob = dtb_dt_embedded(); + fdtdec_setup_embed();
#ifdef CONFIG_EFI_LOADER /* diff --git a/include/fdtdec.h b/include/fdtdec.h index 555c952037964e6b1d9419156171f7ef4b35f448..88eeb5129780878a6fa4ac7a357e4e704347ab43 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -136,23 +136,6 @@ struct fdt_pci_addr { u32 phys_lo; };
-extern u8 __dtb_dt_begin[]; /* embedded device tree blob */ -extern u8 __dtb_dt_spl_begin[]; /* embedded device tree blob for SPL/TPL */ - -/* Get a pointer to the embedded devicetree, if there is one, else NULL */ -static inline u8 *dtb_dt_embedded(void) -{ -#ifdef CONFIG_OF_EMBED -# ifdef CONFIG_XPL_BUILD - return __dtb_dt_spl_begin; -# else - return __dtb_dt_begin; -# endif -#else - return NULL; -#endif -} - /** * Compute the size of a resource. * @@ -1155,6 +1138,13 @@ int fdtdec_set_carveout(void *blob, const char *node, const char *prop_name, const char *name, const char **compatibles, unsigned int count, unsigned long flags);
+/** + * fdtdec_setup_embed - pick up embedded DTS + * + * Should be invoked under CONFIG_OF_EMBED guard. + */ +void fdtdec_setup_embed(void); + /** * Set up the device tree ready for use */ diff --git a/lib/fdtdec.c b/lib/fdtdec.c index b06559880296276bebd434e9d1de6d2fa923fa98..9cb94a158441cfd291a0f173c321a722e896bcb2 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -93,6 +93,23 @@ static const char *const fdt_src_name[] = { [FDTSRC_BLOBLIST] = "bloblist", };
+extern u8 __dtb_dt_begin[]; /* embedded device tree blob */ +extern u8 __dtb_dt_spl_begin[]; /* embedded device tree blob for SPL/TPL */ + +/* Get a pointer to the embedded devicetree, if there is one, else NULL */ +static u8 *dtb_dt_embedded(void) +{ +#ifdef CONFIG_OF_EMBED +# ifdef CONFIG_XPL_BUILD + return __dtb_dt_spl_begin; +# else + return __dtb_dt_begin; +# endif +#else + return NULL; +#endif +} + const char *fdtdec_get_srcname(void) { return fdt_src_name[gd->fdt_src]; @@ -1664,6 +1681,12 @@ static void setup_multi_dtb_fit(void) } }
+void fdtdec_setup_embed(void) +{ + gd->fdt_blob = dtb_dt_embedded(); + gd->fdt_src = FDTSRC_EMBED; +} + int fdtdec_setup(void) { int ret = -ENOENT; @@ -1699,8 +1722,7 @@ int fdtdec_setup(void) gd->fdt_blob = fdt_find_separate(); gd->fdt_src = FDTSRC_SEPARATE; } else { /* embed dtb in ELF file for testing / development */ - gd->fdt_blob = dtb_dt_embedded(); - gd->fdt_src = FDTSRC_EMBED; + fdtdec_setup_embed(); } }

On Tue, 10 Dec 2024 at 15:40, Evgeny Bachinin EABachinin@salutedevices.com wrote:
Patch keeps the access to dtb_dt_embedded() within fdtdec API, by means of new API function introduction. This new function is a common place for updating appropriate global_data fields for OF_EMBED case.
Also, the consequence of the patch is movement of '___dtb_dt_*begin' symbols' declaration from header file, because nobody used symbols outside the lib/fdtdec.c.
Signed-off-by: Evgeny Bachinin EABachinin@salutedevices.com Suggested-by: Simon Glass sjg@chromium.org
common/board_r.c | 4 ++-- include/fdtdec.h | 24 +++++++----------------- lib/fdtdec.c | 26 ++++++++++++++++++++++++-- 3 files changed, 33 insertions(+), 21 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Thank you
diff --git a/common/board_r.c b/common/board_r.c index 88dc756b2a5e5a44f55f9b0fd012adc798a8afdb..3c6a4c1f5df80d5f5bf86ed4047fccf981dbc244 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -155,11 +155,11 @@ static int initr_reloc_global_data(void)
/* * For CONFIG_OF_EMBED case the FDT is embedded into ELF, available by
* __dtb_dt_begin. After U-boot ELF self-relocation to RAM top address
* __dtb_dt_begin. After U-Boot ELF self-relocation to RAM top address * it is worth to update fdt_blob in global_data */ if (IS_ENABLED(CONFIG_OF_EMBED))
gd->fdt_blob = dtb_dt_embedded();
fdtdec_setup_embed();
#ifdef CONFIG_EFI_LOADER /* diff --git a/include/fdtdec.h b/include/fdtdec.h index 555c952037964e6b1d9419156171f7ef4b35f448..88eeb5129780878a6fa4ac7a357e4e704347ab43 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -136,23 +136,6 @@ struct fdt_pci_addr { u32 phys_lo; };
-extern u8 __dtb_dt_begin[]; /* embedded device tree blob */ -extern u8 __dtb_dt_spl_begin[]; /* embedded device tree blob for SPL/TPL */
-/* Get a pointer to the embedded devicetree, if there is one, else NULL */ -static inline u8 *dtb_dt_embedded(void) -{ -#ifdef CONFIG_OF_EMBED -# ifdef CONFIG_XPL_BUILD
return __dtb_dt_spl_begin;
-# else
return __dtb_dt_begin;
-# endif -#else
return NULL;
-#endif -}
/**
- Compute the size of a resource.
@@ -1155,6 +1138,13 @@ int fdtdec_set_carveout(void *blob, const char *node, const char *prop_name, const char *name, const char **compatibles, unsigned int count, unsigned long flags);
+/**
- fdtdec_setup_embed - pick up embedded DTS
- Should be invoked under CONFIG_OF_EMBED guard.
- */
+void fdtdec_setup_embed(void);
/**
- Set up the device tree ready for use
*/ diff --git a/lib/fdtdec.c b/lib/fdtdec.c index b06559880296276bebd434e9d1de6d2fa923fa98..9cb94a158441cfd291a0f173c321a722e896bcb2 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -93,6 +93,23 @@ static const char *const fdt_src_name[] = { [FDTSRC_BLOBLIST] = "bloblist", };
+extern u8 __dtb_dt_begin[]; /* embedded device tree blob */ +extern u8 __dtb_dt_spl_begin[]; /* embedded device tree blob for SPL/TPL */
+/* Get a pointer to the embedded devicetree, if there is one, else NULL */ +static u8 *dtb_dt_embedded(void) +{ +#ifdef CONFIG_OF_EMBED +# ifdef CONFIG_XPL_BUILD
return __dtb_dt_spl_begin;
+# else
return __dtb_dt_begin;
+# endif +#else
return NULL;
+#endif +}
const char *fdtdec_get_srcname(void) { return fdt_src_name[gd->fdt_src]; @@ -1664,6 +1681,12 @@ static void setup_multi_dtb_fit(void) } }
+void fdtdec_setup_embed(void) +{
gd->fdt_blob = dtb_dt_embedded();
gd->fdt_src = FDTSRC_EMBED;
+}
int fdtdec_setup(void) { int ret = -ENOENT; @@ -1699,8 +1722,7 @@ int fdtdec_setup(void) gd->fdt_blob = fdt_find_separate(); gd->fdt_src = FDTSRC_SEPARATE; } else { /* embed dtb in ELF file for testing / development */
gd->fdt_blob = dtb_dt_embedded();
gd->fdt_src = FDTSRC_EMBED;
fdtdec_setup_embed(); } }
-- 2.34.1

Patch fixes the checkpatch warnings like: ``` WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' #94: FILE: lib/fdtdec.c:102: +#ifdef CONFIG_OF_EMBED ```
Signed-off-by: Evgeny Bachinin EABachinin@salutedevices.com --- lib/fdtdec.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 9cb94a158441cfd291a0f173c321a722e896bcb2..7f1418890860a759efcf2b84551d24a3697e4a4b 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -99,15 +99,15 @@ extern u8 __dtb_dt_spl_begin[]; /* embedded device tree blob for SPL/TPL */ /* Get a pointer to the embedded devicetree, if there is one, else NULL */ static u8 *dtb_dt_embedded(void) { -#ifdef CONFIG_OF_EMBED -# ifdef CONFIG_XPL_BUILD - return __dtb_dt_spl_begin; -# else - return __dtb_dt_begin; -# endif -#else - return NULL; -#endif + u8 *addr = NULL; + + if (IS_ENABLED(CONFIG_OF_EMBED)) { + addr = __dtb_dt_begin; + + if (IS_ENABLED(CONFIG_XPL_BUILD)) + addr = __dtb_dt_spl_begin; + } + return addr; }
const char *fdtdec_get_srcname(void)

On Tue, 10 Dec 2024 at 15:40, Evgeny Bachinin EABachinin@salutedevices.com wrote:
Patch fixes the checkpatch warnings like:
WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' #94: FILE: lib/fdtdec.c:102: +#ifdef CONFIG_OF_EMBED
Signed-off-by: Evgeny Bachinin EABachinin@salutedevices.com
lib/fdtdec.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Yes that is better and I'm pleased that it works
diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 9cb94a158441cfd291a0f173c321a722e896bcb2..7f1418890860a759efcf2b84551d24a3697e4a4b 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -99,15 +99,15 @@ extern u8 __dtb_dt_spl_begin[]; /* embedded device tree blob for SPL/TPL */ /* Get a pointer to the embedded devicetree, if there is one, else NULL */ static u8 *dtb_dt_embedded(void) { -#ifdef CONFIG_OF_EMBED -# ifdef CONFIG_XPL_BUILD
return __dtb_dt_spl_begin;
-# else
return __dtb_dt_begin;
-# endif -#else
return NULL;
-#endif
u8 *addr = NULL;
if (IS_ENABLED(CONFIG_OF_EMBED)) {
addr = __dtb_dt_begin;
if (IS_ENABLED(CONFIG_XPL_BUILD))
addr = __dtb_dt_spl_begin;
}
return addr;
}
const char *fdtdec_get_srcname(void)
-- 2.34.1

On Wed, 11 Dec 2024 01:39:56 +0300, Evgeny Bachinin wrote:
The 1st patch addresses comments from the post-review, available by link [1].
The 2nd patch fixes problems of dtb_dt_embedded() with checkpatch.
Links: [1] https://lore.kernel.org/u-boot/CAFLszTgEKamsa6FTnjzrEWQBLkqAR7EBbZqffx09AKgQ...
[...]
Applied to u-boot/next, thanks!
participants (3)
-
Evgeny Bachinin
-
Simon Glass
-
Tom Rini