Re: [PATCH u-boot 1/2] Add a new board for the gigabyte msx4

[Andrew Jeffery <andrew@codeconstruct.com.au>]

On Fri, 2025-11-21 at 16:02 -0800, Marc Olberding wrote:
> Adds a new board for the gigabyte msx4. The primary
> difference is just that we disable the fmc_wdt2 in
> early board init
> 
> Signed-off-by: Marc Olberding <molberding@nvidia.com>
> ---
>  arch/arm/mach-aspeed/ast2600/Kconfig |  9 +++++
>  board/aspeed/ast2600_msx4/Kconfig    | 13 ++++++
>  board/aspeed/ast2600_msx4/Makefile   |  1 +
>  board/aspeed/ast2600_msx4/msx4.c     | 77 ++++++++++++++++++++++++++++++++++++
>  4 files changed, 100 insertions(+)
> 
> diff --git a/arch/arm/mach-aspeed/ast2600/Kconfig b/arch/arm/mach-aspeed/ast2600/Kconfig
> index decf263627fa6ed83123a25268d278fc0f7add2a..6779b000a6ba23dcc4b59515196ecc67d4f429d5 100644
> --- a/arch/arm/mach-aspeed/ast2600/Kconfig
> +++ b/arch/arm/mach-aspeed/ast2600/Kconfig
> @@ -37,6 +37,14 @@ config TARGET_AST2600_IBM
>  	help
>  	  AST2600-IBM is IBM boards for AST2600 BMC based P0WER10+ servers
>  
> +config TARGET_AST2600_MSX4
> +	bool "AST2600-MSX4"
> +	depends on ASPEED_AST2600
> +	help
> +	  AST2600-MSX4 is an Nvidia board for an AST2600 BMC based Intel server.
> +	  It is nominally similar to the EVB, but it turns off support for
> +	  FMC_WDT2
> +
>  config TARGET_AST2600_INTEL
>  	bool "AST2600-INTEL"
>  	depends on ASPEED_AST2600
> @@ -66,6 +74,7 @@ source "board/aspeed/slt_ast2600/Kconfig"
>  source "board/aspeed/ast2600_ibm/Kconfig"
>  source "board/aspeed/ast2600_intel/Kconfig"
>  source "board/aspeed/ast2600_dcscm/Kconfig"
> +source "board/aspeed/ast2600_msx4/Kconfig"
>  source "board/qualcomm/dc-scm-v1/Kconfig"
>  
>  endif
> diff --git a/board/aspeed/ast2600_msx4/Kconfig b/board/aspeed/ast2600_msx4/Kconfig
> new file mode 100644
> index 0000000000000000000000000000000000000000..9096e60aaad31db89998e2d8af8b0adff1f1a62e
> --- /dev/null
> +++ b/board/aspeed/ast2600_msx4/Kconfig
> @@ -0,0 +1,13 @@
> +if TARGET_AST2600_MSX4
> +
> +config SYS_BOARD
> +    default "ast2600_msx4"
> +
> +config SYS_VENDOR
> +    default "aspeed"
> +
> +config SYS_CONFIG_NAME
> +    string "board configuration name"
> +    default ast2600_msx4
> +
> +endif
> diff --git a/board/aspeed/ast2600_msx4/Makefile b/board/aspeed/ast2600_msx4/Makefile
> new file mode 100644
> index 0000000000000000000000000000000000000000..7a01cbec85dafa25a59bbe13534c7f5f36abdc81
> --- /dev/null
> +++ b/board/aspeed/ast2600_msx4/Makefile
> @@ -0,0 +1 @@
> +obj-y += msx4.o
> diff --git a/board/aspeed/ast2600_msx4/msx4.c b/board/aspeed/ast2600_msx4/msx4.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..ce249886ca008e01f9b3b7d81d35eef12ec9eca4
> --- /dev/null
> +++ b/board/aspeed/ast2600_msx4/msx4.c
> @@ -0,0 +1,77 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) Nvidia
> + */
> +#include <common.h>
> +#include <asm/io.h>
> +
> +#define ESPI_BASE	0x1e6ee000
> +#define SCU_BASE	0x1e6e2000
> +
> +static void __maybe_unused disable_fmc_wdt(void)
> +{
> +	u32 reg;
> +
> +	reg = readl(ASPEED_FMC_WDT2);
> +	reg &= ~(0x1);
> +	writel(reg, ASPEED_FMC_WDT2);
> +}

Can we rather add support to the SPI driver, and disable it via a
devicetree property?

That way the option is available to other platforms and minimises the
spread of board file code.

> +
> +static void __maybe_unused espi_init(void)
> +{
> +	u32 reg;
> +
> +	/* skip eSPI init if LPC mode is selected */
> +	reg = readl(SCU_BASE + 0x510);
> +	if (reg & BIT(6))
> +		return;
> +
> +	/*
> +	 * Aspeed STRONGLY NOT recommend to use eSPI early init.
> +	 *
> +	 * This eSPI early init sequence merely set OOB_FREE. It
> +	 * is NOT able to actually handle OOB requests from PCH.
> +	 *
> +	 * During the power on stage, PCH keep waiting OOB_FREE
> +	 * to continue its booting. In general, OOB_FREE is set
> +	 * when BMC firmware is ready. That is, the eSPI kernel
> +	 * driver is mounted and ready to serve eSPI. However,
> +	 * it means that PCH must wait until BMC kernel ready.
> +	 *
> +	 * For customers that request PCH booting as soon as
> +	 * possible. You may use this early init to set OOB_FREE
> +	 * to prevent PCH from blocking by OOB_FREE before BMC
> +	 * kernel ready.
> +	 *
> +	 * If you are not sure what you are doing, DO NOT use it.
> +	 */
> +	reg = readl(ESPI_BASE + 0x000);
> +	reg |= 0xef;
> +	writel(reg, ESPI_BASE + 0x000);
> +
> +	writel(0x0, ESPI_BASE + 0x110);
> +	writel(0x0, ESPI_BASE + 0x114);
> +
> +	reg = readl(ESPI_BASE + 0x00c);
> +	reg |= 0x80000000;
> +	writel(reg, ESPI_BASE + 0x00c);
> +
> +	writel(0xffffffff, ESPI_BASE + 0x094);
> +	writel(0x1, ESPI_BASE + 0x100);
> +	writel(0x1, ESPI_BASE + 0x120);
> +
> +	reg = readl(ESPI_BASE + 0x080);
> +	reg |= 0x50;
> +	writel(reg, ESPI_BASE + 0x080);
> +
> +	reg = readl(ESPI_BASE + 0x000);
> +	reg |= 0x10;
> +	writel(reg, ESPI_BASE + 0x000);

What is the behavioural difference to what's in
board/aspeed/ast2600_intel/intel.c? It's a little annoying to tell
because intel.c uses macro symbols for the register offsets where
you've open-coded the values here.

Can we try to make the implementation common?

Andrew