Re: [PATCH v4 09/16] board: ti: j784s4: Add board support for J784S4 EVM

[Nishanth Menon <nm@ti.com>]

On 22:25-20231001, Apurva Nandan wrote:
> Add board files for J784S4 EVM.
> 
> Signed-off-by: Hari Nagalla <hnagalla@ti.com>
> [ add env and board specific yaml files for binman ]
> Signed-off-by: Neha Malcom Francis <n-francis@ti.com>
> [ cleaned up the env files ]
> Signed-off-by: Manorit Chawdhry <m-chawdhry@ti.com>
> Signed-off-by: Apurva Nandan <a-nandan@ti.com>
> Signed-off-by: Dasnavis Sabiya <sabiya.d@ti.com>
> ---
>  arch/arm/mach-k3/Kconfig         |    1 +
>  board/ti/j784s4/Kconfig          |   63 +
>  board/ti/j784s4/MAINTAINERS      |   22 +
>  board/ti/j784s4/Makefile         |    7 +
>  board/ti/j784s4/board-cfg.yaml   |   37 +
>  board/ti/j784s4/evm.c            |   82 +
>  board/ti/j784s4/j784s4.env       |   35 +
>  board/ti/j784s4/pm-cfg.yaml      |   13 +
>  board/ti/j784s4/rm-cfg.yaml      | 3058 ++++++++++++++++++++++++++++++
>  board/ti/j784s4/sec-cfg.yaml     |  380 ++++
>  board/ti/j784s4/tifs-rm-cfg.yaml | 2656 ++++++++++++++++++++++++++
>  include/configs/j784s4_evm.h     |   20 +

There is all kind of changes mixed in that makes this patch hard to review.

>  12 files changed, 6374 insertions(+)
>  create mode 100644 board/ti/j784s4/Kconfig
>  create mode 100644 board/ti/j784s4/MAINTAINERS
>  create mode 100644 board/ti/j784s4/Makefile
>  create mode 100644 board/ti/j784s4/board-cfg.yaml
>  create mode 100644 board/ti/j784s4/evm.c
>  create mode 100644 board/ti/j784s4/j784s4.env
>  create mode 100644 board/ti/j784s4/pm-cfg.yaml
>  create mode 100644 board/ti/j784s4/rm-cfg.yaml
>  create mode 100644 board/ti/j784s4/sec-cfg.yaml
>  create mode 100644 board/ti/j784s4/tifs-rm-cfg.yaml
>  create mode 100644 include/configs/j784s4_evm.h
[...]

> new file mode 100644
> index 0000000000..025079c6a8
> --- /dev/null
> +++ b/board/ti/j784s4/evm.c
> @@ -0,0 +1,82 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Board specific initialization for J784S4 EVM
> + *
> + * Copyright (C) 2023 Texas Instruments Incorporated - https://www.ti.com/
> + *	Hari Nagalla <hnagalla@ti.com>
> + *
> + */
> +
> +#include <common.h>
> +#include <env.h>
> +#include <fdt_support.h>
> +#include <generic-phy.h>
> +#include <image.h>
> +#include <init.h>
> +#include <log.h>
> +#include <net.h>
> +#include <asm/arch/hardware.h>
> +#include <asm/gpio.h>
> +#include <asm/io.h>
> +#include <spl.h>
> +#include <dm.h>
> +#include <dm/uclass-internal.h>
> +
> +#include "../common/board_detect.h"
> +
> +#define board_is_j784s4_evm()	board_ti_k3_is("J784S4-EVM")
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +int board_init(void)
> +{
> +	return 0;
> +}
> +
> +int dram_init(void)
> +{
> +#ifdef CONFIG_PHYS_64BIT

stop using #ifdef. if (IS_ENABLED())
Checkpatch will warn you on this. you seem to have ignore it.

> +	gd->ram_size = 0x100000000;
> +#else
> +	gd->ram_size = 0x80000000;
> +#endif
> +
> +	return 0;
> +}
> +
> +phys_size_t board_get_usable_ram_top(phys_size_t total_size)
> +{
> +#ifdef CONFIG_PHYS_64BIT
> +	/* Limit RAM used by U-Boot to the DDR low region */
> +	if (gd->ram_top > 0x100000000)
> +		return 0x100000000;
> +#endif
> +
> +	return gd->ram_top;
> +}
> +
> +int dram_init_banksize(void)
> +{
> +	/* Bank 0 declares the memory available in the DDR low region */
> +	gd->bd->bi_dram[0].start = CFG_SYS_SDRAM_BASE;
> +	gd->bd->bi_dram[0].size = 0x7fffffff;
> +	gd->ram_size = 0x80000000;
NAK. Use fdtdec_setup_mem_size_base fdtdec_setup_memory_banksize
And fix up accordingly.

> +
> +#ifdef CONFIG_PHYS_64BIT
> +	/* Bank 1 declares the memory available in the DDR high region */
> +	gd->bd->bi_dram[1].start = CFG_SYS_SDRAM_BASE1;
> +	gd->bd->bi_dram[1].size = 0x77fffffff;
> +	gd->ram_size = 0x800000000;
> +#endif
> +
> +	return 0;
> +}
> +
> +int board_late_init(void)
> +{
> +	return 0;
> +}
> +
> +void spl_board_init(void)
> +{
> +}
> diff --git a/board/ti/j784s4/j784s4.env b/board/ti/j784s4/j784s4.env
> new file mode 100644
> index 0000000000..74f822440e
> --- /dev/null
> +++ b/board/ti/j784s4/j784s4.env
> @@ -0,0 +1,35 @@
> +#include <env/ti/ti_armv7_common.env>
> +#include <env/ti/mmc.env>
> +#include <env/ti/ufs.env>
> +#include <env/ti/k3_dfu.env>
> +
> +#if CONFIG_CMD_REMOTEPROC
> +#include <env/ti/k3_rproc.env>
> +#endif
> +
> +default_device_tree=ti/k3-j784s4-evm.dtb
> +findfdt=
> +	setenv name_fdt ${default_device_tree};
> +	if test CONFIG_DEFAULT_DEVICE_TREE = "k3-j784s4-evm"; then
> +		setenv name_fdt ti/k3-j784s4-evm.dtb; fi;
> +	setenv fdtfile ${name_fdt}

Explained that this is a NAK.
> +name_kern=Image
> +console=ttyS2,115200n8
> +args_all=setenv optargs ${optargs} earlycon=ns16550a,mmio32,0x02880000
> +	${mtdparts}
> +run_kern=booti ${loadaddr} ${rd_spec} ${fdtaddr}
> +
> +boot_targets=ti_mmc mmc0 mmc1 usb pxe dhcp

ti_mmc is a boot_target in stdboot?? no, it wont work.

Also change the boot meth to mmc1 (SD), mmc0(emmc), usb pxe dhcp.

> +boot=mmc
> +mmcdev=1
> +dorprocboot=1
> +bootpart=1:2
> +bootdir=/boot
> +rd_spec=-
> +
> +rproc_fw_binaries= 2 /lib/firmware/j784s4-main-r5f0_0-fw 3 /lib/firmware/j784s4-main-r5f0_1-fw 4 /lib/firmware/j784s4-main-r5f1_0-fw 5 /lib/firmware/j784s4-main-r5f1_1-fw 6 /lib/firmware/j784s4-main-r5f2_0-fw 7 /lib/firmware/j784s4-main-r5f2_1-fw 8 /lib/firmware/j784s4-c71_0-fw 9 /lib/firmware/j784s4-c71_1-fw 10 /lib/firmware/j784s4-c71_2-fw 11 /lib/firmware/j784s4-c71_3-fw

No clue what the above mess is.

> +
> +splashfile=ti.gz
> +splashimage=0x82000000
> +splashpos=m,m
> +splashsource=mmc

You have splash screen support?


-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D