Re: [PATCH v4 06/11] board: ti: am62lx: add initial board files

Hi Bryan,

On 2026-08-04T21:55:02, Bryan Brattlof <[email protected]> wrote:
> board: ti: am62lx: add initial board files
>
> Add the initial board files to support the AM62L3-SK reference boards
> for basic boot via SD and MMC

The other patches in this series (defconfig target AM62L3_EVM,
MAINTAINERS entry 'AM62Lx BOARD', docs am62lx_evm.rst, yaml files
describing AM62L-EVM) all refer to the EVM, not an SK. Please fix the
commit message so the board name matches - and also add a full stop
and drop the mention of 'SD and MMC' (or spell out 'SD and eMMC' if
that is what you mean).

>
> Tested-by: Anshul Dalal <[email protected]>
> Reviewed-by: Anshul Dalal <[email protected]>
> Signed-off-by: Bryan Brattlof <[email protected]>
>
> board/ti/am62lx/Kconfig        |  19 +++
>  board/ti/am62lx/MAINTAINERS    |   8 +
>  board/ti/am62lx/Makefile       |   7 +
>  board/ti/am62lx/am62lx.env     |   8 +
>  board/ti/am62lx/board-cfg.yaml |  32 ++++
>  board/ti/am62lx/evm.c          |  19 +++
>  board/ti/am62lx/sec-cfg.yaml   | 380 +++++++++++++++++++++++++++++++++++++++++
>  7 files changed, 473 insertions(+)

> diff --git a/board/ti/am62lx/evm.c b/board/ti/am62lx/evm.c
> new file mode 100644
> index 00000000000..cafafb8ede8
> --- /dev/null
> +++ b/board/ti/am62lx/evm.c
> @@ -0,0 +1,19 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Board specific initialization for AM62Lx platforms
> + *
> + * Copyright (C) 2026 Texas Instruments Incorporated - https://www.ti.com/
> + *
> + */
> +
> +#include <asm/arch/hardware.h>
> +#include <asm/io.h>
> +#include <dm/uclass.h>
> +#include <env.h>
> +#include <fdt_support.h>
> +#include <spl.h>
> +
> +int board_init(void)
> +{
> +     return 0;
> +}

None of these includes are used by an empty board_init(). Please drop
them and only add each one when the code that needs it lands. Also,
none of the other K3 boards (am62x, am62px, am62ax, am64x, j721e, ...)
provide a board_init() stub at all, so I suspect the whole function
can go away - leaving evm.c effectively empty. If that is the case,
you might as well not add evm.c and the Makefile entry until there is
something real to put in it.

> diff --git a/board/ti/am62lx/am62lx.env b/board/ti/am62lx/am62lx.env
> new file mode 100644
> index 00000000000..0dd86db400e
> --- /dev/null
> +++ b/board/ti/am62lx/am62lx.env
> @@ -0,0 +1,8 @@
> +kernel_addr_r=0x82000000
> +fdt_addr_r=0x88000000
> +fdtoverlay_addr_r=0x89000000
> +ramdisk_addr_r=0x88080000
> +pxefile_addr_r=0x80100000
> +
> +dfu_alt_info_ram=
> +     u-boot.img ram 0x83000000 0x400000

Just to check - the other am62* boards pull in env/ti/ti_common.env,
env/ti/mmc.env, env/ti/k3_dfu.env and set
console/boot_targets/mmcdev/etc. but it looks like this board is
relying on standard boot defaults? That's fine if so.

> diff --git a/board/ti/am62lx/board-cfg.yaml b/board/ti/am62lx/board-cfg.yaml
> new file mode 100644
> index 00000000000..abb3174d4bc
> --- /dev/null
> +++ b/board/ti/am62lx/board-cfg.yaml
> @@ -0,0 +1,32 @@
> +  bm_writer_cfg:
> +    subhdr:
> +      magic: 0x7D8A
> +      size: 5
> +    allowed_host: 10
> +
> +  reserved: [0, 0, 0, 0, 0, 0, 0]

A one-line comment pointing at where bm_writer_cfg and the reserved
block are documented in the TRM (or the tifs firmware header) would
help here. Same for the magic values in sec-cfg.yaml

Regards,
Simon