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

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 2723 bytes --]

On Sun, Oct 01, 2023 at 10:25:38PM +0530, Apurva Nandan wrote:

> Add board files for J784S4 EVM.
[snip]
> diff --git a/board/ti/j784s4/evm.c b/board/ti/j784s4/evm.c
> 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>

Here and elsewhere, do not add common.h and instead add what headers you
need.

> +#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>

This is a huge list for such little code, please see what's actually
needed.

> +#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
> +	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;
> +
> +#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)
> +{
> +}

So, lets split this in to two files, to make life easier on future
J784S4-based platforms.  The basically dummy functions (empty/return 0)
should go in "base.c" and everything that's about the EVM itself should
go in evm.c.  This will make it easier to rm evm.c and add
custom-platform.c that just sets the memory size and anything else
that's really board specific.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]