[PATCH 0/1] rpi: Fix compilation when CONFIG_VIDEO is disabled
This patch will fix a compilation error if CONFIG_VIDEO is set to disabled.
Martin Stolpe (1): rpi: Fix build error when CONFIG_VIDEO is disabled for Raspberry Pi
board/raspberrypi/rpi/rpi.c | 2 +- boot/fdt_simplefb.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
When the CONFIG_VIDEO option is set to disabled for Raspberry Pi devices the build will fail with the following error message: "undefined reference to `fdt_simplefb_enable_and_mem_rsv'."
Signed-off-by: Martin Stolpe martin.stolpe@gmail.com ---
board/raspberrypi/rpi/rpi.c | 2 +- boot/fdt_simplefb.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c index ab5ea85cf9..bc49708f85 100644 --- a/board/raspberrypi/rpi/rpi.c +++ b/board/raspberrypi/rpi/rpi.c @@ -572,7 +572,7 @@ int ft_board_setup(void *blob, struct bd_info *bd) node = fdt_node_offset_by_compatible(blob, -1, "simple-framebuffer"); if (node < 0) fdt_simplefb_add_node(blob); - else + else if (IS_ENABLED(CONFIG_VIDEO)) fdt_simplefb_enable_and_mem_rsv(blob);
#ifdef CONFIG_EFI_LOADER diff --git a/boot/fdt_simplefb.c b/boot/fdt_simplefb.c index 5341554845..b6e916ff7d 100644 --- a/boot/fdt_simplefb.c +++ b/boot/fdt_simplefb.c @@ -86,6 +86,7 @@ int fdt_simplefb_add_node(void *blob) return fdt_simplefb_configure_node(blob, off); }
+#if IS_ENABLED(CONFIG_VIDEO) /** * fdt_simplefb_enable_existing_node() - enable simple-framebuffer DT node * @@ -103,7 +104,6 @@ static int fdt_simplefb_enable_existing_node(void *blob) return fdt_simplefb_configure_node(blob, off); }
-#if IS_ENABLED(CONFIG_VIDEO) int fdt_simplefb_enable_and_mem_rsv(void *blob) { int ret;
Hi,
On 09-20 08:32, Martin Stolpe wrote:
When the CONFIG_VIDEO option is set to disabled for Raspberry Pi devices the build will fail with the following error message: "undefined reference to `fdt_simplefb_enable_and_mem_rsv'."
Signed-off-by: Martin Stolpe martin.stolpe@gmail.com
board/raspberrypi/rpi/rpi.c | 2 +- boot/fdt_simplefb.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c index ab5ea85cf9..bc49708f85 100644 --- a/board/raspberrypi/rpi/rpi.c +++ b/board/raspberrypi/rpi/rpi.c @@ -572,7 +572,7 @@ int ft_board_setup(void *blob, struct bd_info *bd) node = fdt_node_offset_by_compatible(blob, -1, "simple-framebuffer"); if (node < 0) fdt_simplefb_add_node(blob);
- else
- else if (IS_ENABLED(CONFIG_VIDEO)) fdt_simplefb_enable_and_mem_rsv(blob);
I think there is one more user of this function which end of same situation stm32mp1.
#ifdef CONFIG_EFI_LOADER diff --git a/boot/fdt_simplefb.c b/boot/fdt_simplefb.c index 5341554845..b6e916ff7d 100644 --- a/boot/fdt_simplefb.c +++ b/boot/fdt_simplefb.c @@ -86,6 +86,7 @@ int fdt_simplefb_add_node(void *blob) return fdt_simplefb_configure_node(blob, off); }
+#if IS_ENABLED(CONFIG_VIDEO) /**
- fdt_simplefb_enable_existing_node() - enable simple-framebuffer DT node
@@ -103,7 +104,6 @@ static int fdt_simplefb_enable_existing_node(void *blob) return fdt_simplefb_configure_node(blob, off); }
-#if IS_ENABLED(CONFIG_VIDEO) int fdt_simplefb_enable_and_mem_rsv(void *blob) {
Perhaps it will be better if above check is moved inside this function making it empty in case of CONFIG_VIDEO is not set. This way you will not need to modify it users.
Regards, Ivan
Hi,
Am Fr., 20. Sept. 2024 um 10:10 Uhr schrieb Ivan T. Ivanov <iivanov@suse.de
:
Hi,
On 09-20 08:32, Martin Stolpe wrote:
When the CONFIG_VIDEO option is set to disabled for Raspberry Pi devices the build will fail with the following error message: "undefined reference to `fdt_simplefb_enable_and_mem_rsv'."
Signed-off-by: Martin Stolpe martin.stolpe@gmail.com
board/raspberrypi/rpi/rpi.c | 2 +- boot/fdt_simplefb.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c index ab5ea85cf9..bc49708f85 100644 --- a/board/raspberrypi/rpi/rpi.c +++ b/board/raspberrypi/rpi/rpi.c @@ -572,7 +572,7 @@ int ft_board_setup(void *blob, struct bd_info *bd) node = fdt_node_offset_by_compatible(blob, -1,
"simple-framebuffer");
if (node < 0) fdt_simplefb_add_node(blob);
else
else if (IS_ENABLED(CONFIG_VIDEO)) fdt_simplefb_enable_and_mem_rsv(blob);I think there is one more user of this function which end of same situation stm32mp1.
Would it make sense to replace the config option CONFIG_FDT_SIMPLEFB with CONFIG_VIDEO? CONFIG_FDT_SIMPLEFB is only used in the Raspberry Pi default configurations but in no other board configuration and I don't see it being used in the code besides in stm32mp1.c.
Regards Martin
Hi Martin, Ivan
On 24/09/24 11:56, Martin Stolpe wrote:
Hi,
Am Fr., 20. Sept. 2024 um 10:10 Uhr schrieb Ivan T. Ivanov <iivanov@suse.de
:
Hi,
On 09-20 08:32, Martin Stolpe wrote:
When the CONFIG_VIDEO option is set to disabled for Raspberry Pi devices the build will fail with the following error message: "undefined reference to `fdt_simplefb_enable_and_mem_rsv'."
Thanks for the patch.
Signed-off-by: Martin Stolpe martin.stolpe@gmail.com
board/raspberrypi/rpi/rpi.c | 2 +- boot/fdt_simplefb.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c index ab5ea85cf9..bc49708f85 100644 --- a/board/raspberrypi/rpi/rpi.c +++ b/board/raspberrypi/rpi/rpi.c @@ -572,7 +572,7 @@ int ft_board_setup(void *blob, struct bd_info *bd) node = fdt_node_offset_by_compatible(blob, -1,
"simple-framebuffer");
if (node < 0) fdt_simplefb_add_node(blob);
else
else if (IS_ENABLED(CONFIG_VIDEO)) fdt_simplefb_enable_and_mem_rsv(blob);I think there is one more user of this function which end of same situation stm32mp1.
Would it make sense to replace the config option CONFIG_FDT_SIMPLEFB with CONFIG_VIDEO? CONFIG_FDT_SIMPLEFB is only used in the Raspberry Pi default configurations but in no other board configuration and I don't see it being used in the code besides in stm32mp1.c.
CONFIG_FDT_SIMPLEFB is only used in splash-screen context which in-turn depends on CONFIG_VIDEO. So CONFIG_FDT_SIMPLEFB is in a way dependent on CONFIG_VIDEO. We had fixed similar issue in past in vendor tree and by making CONFIG_FDT_SIMPLEFB dependent on CONFIG_VIDEO using below set of patches [1] which I was planning to post upstream too.
Kindly let me know If these patches look good to you and fix your problem too, I can post same set of patches to upstream too.
[1] : https://git.ti.com/cgit/ti-u-boot/ti-u-boot/commit/?h=ti-u-boot-2024.04&... [2] : https://git.ti.com/cgit/ti-u-boot/ti-u-boot/commit/?h=ti-u-boot-2024.04&...
Regards Devarsh
Regards Martin
Hi,
Am Di., 24. Sept. 2024 um 08:38 Uhr schrieb Devarsh Thakkar <devarsht@ti.com
:
CONFIG_FDT_SIMPLEFB is only used in splash-screen context which in-turn depends on CONFIG_VIDEO. So CONFIG_FDT_SIMPLEFB is in a way dependent on CONFIG_VIDEO. We had fixed similar issue in past in vendor tree and by making CONFIG_FDT_SIMPLEFB dependent on CONFIG_VIDEO using below set of patches [1] which I was planning to post upstream too.
Kindly let me know If these patches look good to you and fix your problem too, I can post same set of patches to upstream too.
[1] :
https://git.ti.com/cgit/ti-u-boot/ti-u-boot/commit/?h=ti-u-boot-2024.04&... [2] :
https://git.ti.com/cgit/ti-u-boot/ti-u-boot/commit/?h=ti-u-boot-2024.04&...
These patches look good to me.
If I understand the code correctly the frame buffer node should only be created if CONFIG_FDT_SIMPLEFB is enabled. Thus I would change the code like this:
Signed-off-by: Martin Stolpe martin.stolpe@gmail.com ---
board/raspberrypi/rpi/rpi.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c index ab5ea85cf9..47db441696 100644 --- a/board/raspberrypi/rpi/rpi.c +++ b/board/raspberrypi/rpi/rpi.c @@ -569,11 +569,13 @@ int ft_board_setup(void *blob, struct bd_info *bd)
update_fdt_from_fw(blob, (void *)fw_dtb_pointer);
- node = fdt_node_offset_by_compatible(blob, -1, "simple-framebuffer"); - if (node < 0) - fdt_simplefb_add_node(blob); - else - fdt_simplefb_enable_and_mem_rsv(blob); + if (CONFIG_IS_ENABLED(FDT_SIMPLEFB)) { + node = fdt_node_offset_by_compatible(blob, -1, "simple-framebuffer"); + if (node < 0) + fdt_simplefb_add_node(blob); + else + fdt_simplefb_enable_and_mem_rsv(blob); + }
#ifdef CONFIG_EFI_LOADER /* Reserve the spin table */
Hi Martin,
On 26/09/24 17:56, Martin Stolpe wrote:
Hi,
Am Di., 24. Sept. 2024 um 08:38 Uhr schrieb Devarsh Thakkar <devarsht@ti.com mailto:devarsht@ti.com>:
CONFIG_FDT_SIMPLEFB is only used in splash-screen context which in-turn depends on CONFIG_VIDEO. So CONFIG_FDT_SIMPLEFB is in a way dependent on CONFIG_VIDEO. We had fixed similar issue in past in vendor tree and by making CONFIG_FDT_SIMPLEFB dependent on CONFIG_VIDEO using below set of patches [1] which I was planning to post upstream too. Kindly let me know If these patches look good to you and fix your problem too, I can post same set of patches to upstream too. [1] : https://git.ti.com/cgit/ti-u-boot/ti-u-boot/commit/?h=ti-u-boot-2024.04&id=1199800505f11f2162030cb641c6d0c9276d5c9c <https://git.ti.com/cgit/ti-u-boot/ti-u-boot/commit/?h=ti-u-boot-2024.04&id=1199800505f11f2162030cb641c6d0c9276d5c9c> [2] : https://git.ti.com/cgit/ti-u-boot/ti-u-boot/commit/?h=ti-u-boot-2024.04&id=5b4b8eac243cbd86286ff2cf57ca0469c4d86345 <https://git.ti.com/cgit/ti-u-boot/ti-u-boot/commit/?h=ti-u-boot-2024.04&id=5b4b8eac243cbd86286ff2cf57ca0469c4d86345>These patches look good to me.
Thanks for taking a look, I have posted them to the list [1], it would be great if you could provide a Reviewed-by or Tested-by too.
If I understand the code correctly the frame buffer node should only be created if CONFIG_FDT_SIMPLEFB is enabled. Thus I would change the code like this:
Yes, that's true, I think you also need to conditionally compile ft_board_setup only when CONFIG_OF_BOARD_SETUP is enabled.
[1]: https://lore.kernel.org/all/20240925151354.480704-4-devarsht@ti.com/
Regards Devarsh
Hi Devarsh,
Am Do., 26. Sept. 2024 um 18:45 Uhr schrieb Devarsh Thakkar <devarsht@ti.com
:
Thanks for taking a look, I have posted them to the list [1], it would be great if you could provide a Reviewed-by or Tested-by too.
I've tried to compile the project for am64x_evm_a53 with the patches applied and I've added the following line to am64x_evm_a53_defconfig: CONFIG_OF_BOARD_SETUP=y When I then try to compile U-Boot using the following command: make am64x_evm_a53_defconfig; make
I get the following error message: aarch64-linux-gnu-ld.bfd: boot/image-fdt.o: in function `image_setup_libfdt': /home/mstolpe/repositories/build_base_station/build/workspace/sources/u-boot-copy/boot/image-fdt.c:633:(.text.image_setup_libfdt+0x150): undefined reference to `ft_board_setup' aarch64-linux-gnu-ld.bfd: cmd/fdt.o: in function `do_fdt': /home/mstolpe/repositories/build_base_station/build/workspace/sources/u-boot-copy/cmd/fdt.c:687:(.text.do_fdt+0xf98): undefined reference to `ft_board_setup' Segmentation fault (core dumped) make: *** [Makefile:1801: u-boot] Error 139 make: *** Deleting file 'u-boot'
I guess this shouldn't happen?
BR Martin
Hi Martin,
On 07/10/24 18:24, Martin Stolpe wrote:
Hi Devarsh,
Am Do., 26. Sept. 2024 um 18:45 Uhr schrieb Devarsh Thakkar <devarsht@ti.com
:
Thanks for taking a look, I have posted them to the list [1], it would be great if you could provide a Reviewed-by or Tested-by too.
I've tried to compile the project for am64x_evm_a53 with the patches applied and I've added the following line to am64x_evm_a53_defconfig: CONFIG_OF_BOARD_SETUP=y When I then try to compile U-Boot using the following command: make am64x_evm_a53_defconfig; make
I get the following error message: aarch64-linux-gnu-ld.bfd: boot/image-fdt.o: in function `image_setup_libfdt': /home/mstolpe/repositories/build_base_station/build/workspace/sources/u-boot-copy/boot/image-fdt.c:633:(.text.image_setup_libfdt+0x150): undefined reference to `ft_board_setup' aarch64-linux-gnu-ld.bfd: cmd/fdt.o: in function `do_fdt': /home/mstolpe/repositories/build_base_station/build/workspace/sources/u-boot-copy/cmd/fdt.c:687:(.text.do_fdt+0xf98): undefined reference to `ft_board_setup' Segmentation fault (core dumped) make: *** [Makefile:1801: u-boot] Error 139 make: *** Deleting file 'u-boot'
I guess this shouldn't happen?
I think this is expected as you are trying to enable CONFIG_OF_BOARD_SETUP but AM64 platform does not implement ft_board_setup.
Platforms need to implement ft_board_setup for platform specific knobs (for e.g. simplefb for splash etc) if using CONFIG_OF_BOARD_SETUP as per below snippet in boot/image-fdt.c :
if (IS_ENABLED(CONFIG_OF_BOARD_SETUP)) {
const char *skip_board_fixup;
skip_board_fixup = env_get("skip_board_fixup");
if (skip_board_fixup && ((int)simple_strtol(skip_board_fixup, NULL, 10) == 1)) { printf("skip board fdt fixup\n");
} else {
fdt_ret = ft_board_setup(blob, gd->bd);
if (fdt_ret) {
printf("ERROR: board-specific fdt fixup failed: %s\n", fdt_strerror(fdt_ret));
goto err;
}
}
}
Regards Devarsh
BR Martin
Hi,
Sorry if this is a stupid question but what do I have to do to add a " Reviewed-by" from myself to this patch series?
BR Martin
On Fri, 11 Oct 2024 at 13:22, Martin Stolpe martinstolpe@gmail.com wrote:
Hi,
Sorry if this is a stupid question but what do I have to do to add a " Reviewed-by" from myself to this patch series?
Just reply all with the reviewed-by and patchwork will automatically apply it when I accept it, I think there needs to be a new v2 of this patch.
participants (4)
-
Devarsh Thakkar -
Ivan T. Ivanov -
Martin Stolpe -
Peter Robinson