Re: [PATCH 04/10] board: ti: am64x: Set fdtfile from C code instead of findfdt script

[Roger Quadros <rogerq@kernel.org>]

On 08/01/2024 19:32, Nishanth Menon wrote:
> We now can provide a map and have the standard fdtfile variable set from
> code itself. This allows for bootstd to "just work".
> 
> While at this, replace findfdt in environment with a warning as it is no
> longer needed.
> 
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
>  board/ti/am64x/am64x.env | 9 ---------
>  board/ti/am64x/evm.c     | 8 ++++++++
>  2 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/board/ti/am64x/am64x.env b/board/ti/am64x/am64x.env
> index efd736b99be4..9a8812d4ee54 100644
> --- a/board/ti/am64x/am64x.env
> +++ b/board/ti/am64x/am64x.env
> @@ -2,14 +2,6 @@
>  #include <env/ti/mmc.env>
>  #include <env/ti/k3_dfu.env>
>  
> -findfdt=
> -	if test $board_name = am64x_gpevm; then
> -		setenv name_fdt ti/k3-am642-evm.dtb; fi;
> -	if test $board_name = am64x_skevm; then
> -		setenv name_fdt ti/k3-am642-sk.dtb; fi;
> -	if test $name_fdt = undefined; then
> -		echo WARNING: Could not determine device tree to use; fi;
> -	setenv fdtfile ${name_fdt}
>  name_kern=Image
>  console=ttyS2,115200n8
>  args_all=setenv optargs earlycon=ns16550a,mmio32,0x02800000 ${mtdparts}
> @@ -43,7 +35,6 @@ get_fit_usb=load usb ${bootpart} ${addr_fit}
>  usbboot=setenv boot usb;
>  	setenv bootpart 0:2;
>  	usb start;
> -	run findfdt;
>  	run init_usb;
>  	run get_kern_usb;
>  	run get_fdt_usb;
> diff --git a/board/ti/am64x/evm.c b/board/ti/am64x/evm.c
> index a6dcff2eb434..e2f506d2c6ea 100644
> --- a/board/ti/am64x/evm.c
> +++ b/board/ti/am64x/evm.c
> @@ -16,6 +16,7 @@
>  #include <env.h>
>  
>  #include "../common/board_detect.h"
> +#include "../common/fdt_ops.h"
>  
>  #define board_is_am64x_gpevm() (board_ti_k3_is("AM64-GPEVM") || \
>  				board_ti_k3_is("AM64-HSEVM"))
> @@ -180,6 +181,12 @@ int checkboard(void)
>  }
>  
>  #ifdef CONFIG_BOARD_LATE_INIT
> +static struct ti_fdt_map ti_am64_evm_fdt_map[] = {
> +	{"am64x_gpevm", "k3-am642-evm.dtb"},
> +	{"am64x_skevm", "k3-am642-sk.dtb"},

"am64x_gpevm" and "am64x_skevm" strings are used multiple times in this file.
see setup_board_eeprom_env()

Please use a MACRO for them.

What is the logic of choosing this name and can it be updated at this point?
e.g."gp" is misleading in the board name as the boards are now shipped with
HS-FS chip and are no longer GP.


> +	{ /* Sentinel. */ }
> +};
> +
>  static void setup_board_eeprom_env(void)
>  {
>  	char *name = "am64x_gpevm";
> @@ -197,6 +204,7 @@ static void setup_board_eeprom_env(void)
>  
>  invalid_eeprom:
>  	set_board_info_env_am6(name);
> +	ti_set_fdt_env(name, ti_am64_evm_fdt_map);
>  }
>  
>  static void setup_serial(void)

-- 
cheers,
-roger