Re: [PATCH V1] board: xilinx: zynq: Add a new cmd to run postconfig

On 12/13/24 09:52, xueweiwujxw wrote:

This patch adds a new command named "post_config" to execute the "ps7_post_config" function.

Signed-off-by: xueweiwujxw xueweiwujxw@gmail.com

first of all please spell your name properly here.

---

board/xilinx/zynq/Kconfig | 17 +++++++++++++++++ board/xilinx/zynq/Makefile | 2 +- board/xilinx/zynq/cmds.c | 20 ++++++++++++++++++++ test/py/tests/test_zynq_post_config.py | 16 ++++++++++++++++ 4 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 test/py/tests/test_zynq_post_config.py

When the patch is applied. make xilinx_zynq_virt_defconfig make

CC arch/arm/cpu/armv7/syslib.o board/xilinx/zynq/cmds.c:21:10: fatal error: ps7_init_gpl.h: No such file or directory 21 | #include "ps7_init_gpl.h" | ^~~~~~~~~~~~~~~~ compilation terminated.

Please look below.

diff --git a/board/xilinx/zynq/Kconfig b/board/xilinx/zynq/Kconfig index d6f40631cc..a1624c71c0 100644 --- a/board/xilinx/zynq/Kconfig +++ b/board/xilinx/zynq/Kconfig @@ -30,4 +30,21 @@ config CMD_ZYNQ_RSA and authentication feature enabled while generating BOOT.BIN using Xilinx bootgen tool.

+config CMD_POST_CONFIG

• bool "Enable post_config command"
• default y
• help

•  This option enables the 'post_config' command in U-Boot.
  

•  The 'post_config' command is used to execute the PS7 post-configuration
  

•  process after loading an FPGA bitstream file on Xilinx Zynq devices.
  

•  This command ensures that the programmable logic (PL) and
  

•  processing system (PS) are properly synchronized after
  

•  FPGA configuration. It is specifically useful in designs where
  

•  additional initialization steps are required for the PS to
  

•  correctly interact with the configured PL.
  

ps7_init() is called only from SPL in current configuration. And ps7_post_config() is called from spl_board_prepare_for_boot() already. It means cases where SPL loads bitstream should be working just fine.

Then we can switch to case where bitstream is loaded via u-boot command. The biggest question is if your ps7_init_gpl is aligned with the design you are actually loading. Likely yes but in general it doesn't need to be.

Then we are getting to another question if calling ps7_post_config in this context make sense or if it is better to do it in a different way.

I think you should work on better description why you think that this should be called/done. And it is not clear what else you want to do after bitstream is loaded. If this is about enabling ps-pl interface and getting pl out of reset we can talk about it.

•  This option is only applicable to Xilinx Zynq platforms and
  

That's what it is said already because this file is sourced only when Zynq is selected.

•  has no effect on other architectures. If you are not using the
  

•  PS7 subsystem or do not require post-configuration steps,
  

•  this option can be disabled to reduce the size of the U-Boot binary.
  

• endif

diff --git a/board/xilinx/zynq/Makefile b/board/xilinx/zynq/Makefile index f40fe382f5..366a4d931a 100644 --- a/board/xilinx/zynq/Makefile +++ b/board/xilinx/zynq/Makefile @@ -32,7 +32,7 @@ endif endif

ifndef CONFIG_XPL_BUILD -obj-$(CONFIG_CMD_ZYNQ) += cmds.o +obj-$(CONFIG_CMD_ZYNQ) += $(init-objs) cmds.o

No idea what you are trying to do here.

obj-$(CONFIG_CMD_ZYNQ_RSA) += bootimg.o endif

diff --git a/board/xilinx/zynq/cmds.c b/board/xilinx/zynq/cmds.c index 05ecb75406..60278fcdf6 100644 --- a/board/xilinx/zynq/cmds.c +++ b/board/xilinx/zynq/cmds.c @@ -18,6 +18,7 @@ #include <zynqpl.h> #include <fpga.h> #include <zynq_bootimg.h> +#include "ps7_init_gpl.h"

DECLARE_GLOBAL_DATA_PTR;

@@ -548,3 +549,22 @@ U_BOOT_LONGHELP(zynq, U_BOOT_CMD(zynq, 6, 0, do_zynq, "Zynq specific commands", zynq_help_text );

+static int do_ps7_fpga_post_config(struct cmd_tbl *cmdtp, int flag, int argc,

• 		   char *const argv[])
  

+{

• int ret = PS7_INIT_SUCCESS;
• if (IS_ENABLED(CONFIG_CMD_POST_CONFIG))

Clearly wrong. You should enable this command only when Kconfig is enabled.

• ret = ps7_post_config();
  

• if (ret != PS7_INIT_SUCCESS)

• puts("ERROR:run board ps7 post config failed.\n");
  

• else

• puts("INFO: run board ps7 post config succeed.\n");
  

you have return value from command. No need to be verbose. But you can but under DEBUG only.

• return ret;

use cmd_process_error()

+}

+U_BOOT_CMD(post_config, 1, 0, do_ps7_fpga_post_config,

•   "run ps7 post config after fpga load bin file",	""
  

+);

Obviously own command make no sense. You have zynq command already and this would be it's subcommand.

diff --git a/test/py/tests/test_zynq_post_config.py b/test/py/tests/test_zynq_post_config.py new file mode 100644 index 0000000000..4493f8f190 --- /dev/null +++ b/test/py/tests/test_zynq_post_config.py @@ -0,0 +1,16 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# Test for the "post_config" command +# +# xueweiwujxw

This should be full copyright with name, year, etc

+import pytest

+@pytest.mark.buildconfigspec('cmd_post_config') +def test_post_config(u_boot_console):

• """Test the 'post_config' command on Zynq platform"""
• output = u_boot_console.run_command("post_config")
• assert "INFO: run board ps7 post config succeed." in output

Thanks, Michal