[PATCH u-boot v2 0/2] aspeed: Add support for MSX4

[PATCH u-boot v2 0/2] aspeed: Add support for MSX4

[Marc Olberding]

Add a device tree flag for the FMC_WDT2 to be disabled.
Also add a device tree for MSX4 that uses the aforementioned flag.
The MSX4 is a granite rapids based reference hardware platform module
for the CX8 SuperNIC Switchboard. It uses the AST2600 BMC SoC for
out of band management.

Patch 1 adds support for the aspeed_spi.c driver to disable the
FMC_WDT2 via a device tree flag, aspeed,watchdog-disable.

Example usage is as such:
```
&fmc {
        pinctrl-names = "default";
        pinctrl-0 = <&pinctrl_fmcquad_default>;
        aspeed,watchdog-disable;
        status = "okay";

};
``` 

Patch 2 adds the devicetree for this board.

Reference architecture for the msx4:
https://developer.nvidia.com/blog/nvidia-connectx-8-supernics-advance-ai-platform-architecture-with-pcie-gen6-connectivity/

There was also some discussion of breaking
out espi_init into a seperate function so it can be reused between
different board files. That will be split out into a seperate,
follow-on patch series, since we're no longer creating a new
board file.

Signed-off-by: Marc Olberding <molberding@nvidia.com>
---
Changes in v2:
- Switched from using a board file to adding support for disabling FMC_WDT2 to aspeed driver
- Added the new device-tree flag to the MSX4 dts
- Dropped board file for the MSX4
- Link to v1: https://lore.kernel.org/r/20251121-msx4-v1-0-fc0118b666c1@nvidia.com

---
Marc Olberding (2):
      drivers: spi: Add support for disabling FMC_WDT2 for aspeed
      arch: arm: dts: Add dts for the nvidia msx4 board

 arch/arm/dts/Makefile                    |   1 +
 arch/arm/dts/ast2600-msx4-bmc-nvidia.dts | 112 +++++++++++++++++++++++++++++++
 drivers/spi/aspeed_spi.c                 |  19 ++++--
 3 files changed, 128 insertions(+), 4 deletions(-)
---
base-commit: 8e15f5c0b1e7b11296ae6c88b686e65d509237d0
change-id: 20251107-msx4-cad1e2e4fa39

Best regards,
-- 
Marc Olberding <molberding@nvidia.com>

[PATCH u-boot v2 1/2] drivers: spi: Add support for disabling FMC_WDT2 for aspeed

[Marc Olberding]

Adds support for disabling the ast2600 FMC_WDT2 through
a device tree entry in the fmc node.
Set `aspeed,watchdog-disable` in your device tree to have
the driver disable it.

Signed-off-by: Marc Olberding <molberding@nvidia.com>
---
 drivers/spi/aspeed_spi.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/aspeed_spi.c b/drivers/spi/aspeed_spi.c
index 54520122f1c48c8b2052b4b1e47445a9b990d25e..de954e477aa15e6d1be042a2aee47f5a501178da 100644
--- a/drivers/spi/aspeed_spi.c
+++ b/drivers/spi/aspeed_spi.c
@@ -30,14 +30,16 @@ struct aspeed_spi_regs {
 					/* 0x30 .. 0x38 Segment Address */
 	u32 _reserved1[5];		/* .. */
 	u32 soft_rst_cmd_ctrl;	/* 0x50 Auto Soft-Reset Command Control */
-	u32 _reserved2[11];		/* .. */
+	u32 _reserved2[4];		/* .. */
+	u32 wdt2_ctrl;			/* 0x64 FMC_WDT2 control */
+	u32 _reserved3[6];		/* .. */
 	u32 dma_ctrl;			/* 0x80 DMA Control/Status */
 	u32 dma_flash_addr;		/* 0x84 DMA Flash Side Address */
 	u32 dma_dram_addr;		/* 0x88 DMA DRAM Side Address */
 	u32 dma_len;			/* 0x8c DMA Length Register */
 	u32 dma_checksum;		/* 0x90 Checksum Calculation Result */
 	u32 timings;			/* 0x94 Read Timing Compensation */
-	u32 _reserved3[1];
+	u32 _reserved4[1];
 	/* not used */
 	u32 soft_strap_status;		/* 0x9c Software Strap Status */
 	u32 write_cmd_filter_ctrl;	/* 0xa0 Write Command Filter Control */
@@ -45,7 +47,7 @@ struct aspeed_spi_regs {
 	u32 lock_ctrl_reset;		/* 0xa8 Lock Control (SRST#) */
 	u32 lock_ctrl_wdt;		/* 0xac Lock Control (Watchdog) */
 	u32 write_addr_filter[8];	/* 0xb0 Write Address Filter */
-	u32 _reserved4[12];
+	u32 _reserved5[12];
 	u32 fully_qualified_cmd[20];	/* 0x100 Fully Qualified Command */
 	u32 addr_qualified_cmd[12];	/* 0x150 Address Qualified Command */
 };
@@ -163,7 +165,8 @@ struct aspeed_spi_regs {
 #define SPI_3B_AUTO_CLR_REG   0x1e6e2510
 #define SPI_3B_AUTO_CLR       BIT(9)
 
-
+/* FMC_WDT2 control register */
+#define WDT2_ENABLE		BIT(0)
 /*
  * flash related info
  */
@@ -267,6 +270,7 @@ struct aspeed_spi_priv {
 	ulong hclk_rate; /* AHB clock rate */
 	u8 num_cs;
 	bool is_fmc;
+	bool disable_wdt;
 
 	struct aspeed_spi_flash flashes[ASPEED_SPI_MAX_CS];
 	u32 flash_count;
@@ -683,6 +687,9 @@ static int aspeed_spi_controller_init(struct aspeed_spi_priv *priv)
 	setbits_le32(&priv->regs->conf,
 		     CONF_ENABLE_W2 | CONF_ENABLE_W1 | CONF_ENABLE_W0);
 
+	if (priv->is_fmc && priv->disable_wdt)
+		clrbits_le32(&priv->regs->wdt2_ctrl, WDT2_ENABLE);
+
 	/*
 	 * Set safe default settings for each device. These will be
 	 * tuned after the SPI flash devices are probed.
@@ -1907,6 +1914,10 @@ static int aspeed_spi_probe(struct udevice *bus)
 	 * SPI controllers
 	 */
 	priv->is_fmc = dev_get_driver_data(bus);
+	if (device_is_compatible(bus, "aspeed,ast2600-fmc") &&
+	    dev_read_bool(bus, "aspeed,watchdog-disable"))
+		priv->disable_wdt = true;
+
 
 	ret = aspeed_spi_controller_init(priv);
 	if (ret)

-- 
2.34.1

[PATCH u-boot v2 2/2] arch: arm: dts: Add dts for the nvidia msx4 board

[Marc Olberding]

Adds a dts for the nvidia msx4 board, a granite rapids based motherboard

Signed-off-by: Marc Olberding <molberding@nvidia.com>
---
 arch/arm/dts/Makefile                    |   1 +
 arch/arm/dts/ast2600-msx4-bmc-nvidia.dts | 112 +++++++++++++++++++++++++++++++
 2 files changed, 113 insertions(+)

diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index dbb2fafc4f4c13302a02875cd678477ab95334cb..aebc5b60abc6b267669650be098315f5d5715074 100755
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -689,6 +689,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
 	ast2600-facebook.dtb \
 	ast2600-fpga.dtb \
 	ast2600-gb200nvl-bmc-nvidia.dtb \
+        ast2600-msx4-bmc-nvidia.dtb \
 	ast2600-greatlakes.dtb \
 	ast2600-intel.dtb \
 	ast2600-intel.dtb \
diff --git a/arch/arm/dts/ast2600-msx4-bmc-nvidia.dts b/arch/arm/dts/ast2600-msx4-bmc-nvidia.dts
new file mode 100644
index 0000000000000000000000000000000000000000..3f5146c65dec3ec569cd46745a13be5466f7218c
--- /dev/null
+++ b/arch/arm/dts/ast2600-msx4-bmc-nvidia.dts
@@ -0,0 +1,112 @@
+// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+
+#include "ast2600-u-boot.dtsi"
+
+/ {
+	model = "AST2600 MSX4 BMC";
+	compatible = "aspeed,ast2600-msx4", "aspeed,ast2600";
+
+	memory {
+		device_type = "memory";
+		reg = <0x80000000 0x40000000>;
+	};
+
+	chosen {
+		stdout-path = &uart5;
+	};
+
+	aliases {
+		spi0 = &fmc;
+		ethernet0 = &mac0;
+	};
+
+	cpus {
+		cpu@0 {
+			clock-frequency = <800000000>;
+		};
+		cpu@1 {
+			clock-frequency = <800000000>;
+		};
+	};
+};
+
+&display_port {
+	status = "okay";
+};
+
+&fmc {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_fmcquad_default>;
+	aspeed,watchdog-disable;
+	status = "okay";
+
+	flash@0 {
+		status = "okay";
+		spi-max-frequency = <10000000>;
+		spi-tx-bus-width = <2>;
+		spi-rx-bus-width = <2>;
+	};
+
+	flash@1 {
+		status = "okay";
+		spi-max-frequency = <10000000>;
+		spi-tx-bus-width = <1>;
+		spi-rx-bus-width = <1>;
+	};
+};
+
+&hace {
+	status = "okay";
+};
+
+&mac0 {
+	phy-mode = "rgmii";
+	phy-handle = <&ethphy3>;
+
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_rgmii1_default>;
+	status = "okay";
+};
+
+&mdio {
+	status = "okay";
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_mdio4_default>;
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	ethphy3: ethernet-phy@0 {
+		compatible = "ethernet-phy-ieee802.3-c22";
+		reg = <2>;
+	};
+};
+
+&scu {
+	mac0-clk-delay = <0x1d 0x0a
+			  0x1d 0x0a
+			  0x1d 0x0a>;
+};
+
+&sdrammc {
+	clock-frequency = <400000000>;
+	aspeed,ecc-enabled;
+	aspeed,ecc-size-mb = <0>;
+};
+
+&uart5 {
+	u-boot,dm-pre-reloc;
+	status = "okay";
+};
+
+&wdt1 {
+	status = "okay";
+};
+
+&wdt2 {
+	status = "okay";
+};
+
+&wdt3 {
+	status = "okay";
+};

-- 
2.34.1

Re: [PATCH u-boot v2 1/2] drivers: spi: Add support for disabling FMC_WDT2 for aspeed

[Andrew Jeffery]

On Tue, 2025-12-02 at 15:52 -0800, Marc Olberding wrote:
> Adds support for disabling the ast2600 FMC_WDT2 through
> a device tree entry in the fmc node.
> Set `aspeed,watchdog-disable` in your device tree to have
> the driver disable it.
> 
> Signed-off-by: Marc Olberding <molberding@nvidia.com>
> ---
>  drivers/spi/aspeed_spi.c | 19 +++++++++++++++----
>  1 file changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/spi/aspeed_spi.c b/drivers/spi/aspeed_spi.c
> index 54520122f1c48c8b2052b4b1e47445a9b990d25e..de954e477aa15e6d1be042a2aee47f5a501178da 100644
> --- a/drivers/spi/aspeed_spi.c
> +++ b/drivers/spi/aspeed_spi.c
> @@ -30,14 +30,16 @@ struct aspeed_spi_regs {
>  					/* 0x30 .. 0x38 Segment Address */
>  	u32 _reserved1[5];		/* .. */
>  	u32 soft_rst_cmd_ctrl;	/* 0x50 Auto Soft-Reset Command Control */
> -	u32 _reserved2[11];		/* .. */
> +	u32 _reserved2[4];		/* .. */
> +	u32 wdt2_ctrl;			/* 0x64 FMC_WDT2 control */
> +	u32 _reserved3[6];		/* .. */

Ugh (passing commentary, not your fault).

>  	u32 dma_ctrl;			/* 0x80 DMA Control/Status */
>  	u32 dma_flash_addr;		/* 0x84 DMA Flash Side Address */
>  	u32 dma_dram_addr;		/* 0x88 DMA DRAM Side Address */
>  	u32 dma_len;			/* 0x8c DMA Length Register */
>  	u32 dma_checksum;		/* 0x90 Checksum Calculation Result */
>  	u32 timings;			/* 0x94 Read Timing Compensation */
> -	u32 _reserved3[1];
> +	u32 _reserved4[1];
>  	/* not used */
>  	u32 soft_strap_status;		/* 0x9c Software Strap Status */
>  	u32 write_cmd_filter_ctrl;	/* 0xa0 Write Command Filter Control */
> @@ -45,7 +47,7 @@ struct aspeed_spi_regs {
>  	u32 lock_ctrl_reset;		/* 0xa8 Lock Control (SRST#) */
>  	u32 lock_ctrl_wdt;		/* 0xac Lock Control (Watchdog) */
>  	u32 write_addr_filter[8];	/* 0xb0 Write Address Filter */
> -	u32 _reserved4[12];
> +	u32 _reserved5[12];
>  	u32 fully_qualified_cmd[20];	/* 0x100 Fully Qualified Command */
>  	u32 addr_qualified_cmd[12];	/* 0x150 Address Qualified Command */
>  };
> @@ -163,7 +165,8 @@ struct aspeed_spi_regs {
>  #define SPI_3B_AUTO_CLR_REG   0x1e6e2510
>  #define SPI_3B_AUTO_CLR       BIT(9)
>  
> -
> +/* FMC_WDT2 control register */
> +#define WDT2_ENABLE		BIT(0)
>  /*
>   * flash related info
>   */
> @@ -267,6 +270,7 @@ struct aspeed_spi_priv {
>  	ulong hclk_rate; /* AHB clock rate */
>  	u8 num_cs;
>  	bool is_fmc;
> +	bool disable_wdt;
>  
>  	struct aspeed_spi_flash flashes[ASPEED_SPI_MAX_CS];
>  	u32 flash_count;
> @@ -683,6 +687,9 @@ static int aspeed_spi_controller_init(struct aspeed_spi_priv *priv)
>  	setbits_le32(&priv->regs->conf,
>  		     CONF_ENABLE_W2 | CONF_ENABLE_W1 | CONF_ENABLE_W0);
>  
> +	if (priv->is_fmc && priv->disable_wdt)
> +		clrbits_le32(&priv->regs->wdt2_ctrl, WDT2_ENABLE);
> +
>  	/*
>  	 * Set safe default settings for each device. These will be
>  	 * tuned after the SPI flash devices are probed.
> @@ -1907,6 +1914,10 @@ static int aspeed_spi_probe(struct udevice *bus)
>  	 * SPI controllers
>  	 */
>  	priv->is_fmc = dev_get_driver_data(bus);
> +	if (device_is_compatible(bus, "aspeed,ast2600-fmc") &&
> +	    dev_read_bool(bus, "aspeed,watchdog-disable"))
> +		priv->disable_wdt = true;

We're not setting it to false, just declaring it above, which means if
this branch isn't taken then its value is undefined.

Perhaps initialise it to false by default.

> +

Unnecessary new-line?

>  
>  	ret = aspeed_spi_controller_init(priv);
>  	if (ret)

Re: [PATCH u-boot v2 2/2] arch: arm: dts: Add dts for the nvidia msx4 board

[Andrew Jeffery]

On Tue, 2025-12-02 at 15:52 -0800, Marc Olberding wrote:
> Adds a dts for the nvidia msx4 board, a granite rapids based
> motherboard
> 
> Signed-off-by: Marc Olberding <molberding@nvidia.com>
> ---
>  arch/arm/dts/Makefile                    |   1 +
>  arch/arm/dts/ast2600-msx4-bmc-nvidia.dts | 112
> +++++++++++++++++++++++++++++++
>  2 files changed, 113 insertions(+)
> 
> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> index
> dbb2fafc4f4c13302a02875cd678477ab95334cb..aebc5b60abc6b267669650be098
> 315f5d5715074 100755
> --- a/arch/arm/dts/Makefile
> +++ b/arch/arm/dts/Makefile
> @@ -689,6 +689,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
>  	ast2600-facebook.dtb \
>  	ast2600-fpga.dtb \
>  	ast2600-gb200nvl-bmc-nvidia.dtb \
> +        ast2600-msx4-bmc-nvidia.dtb \

Whitespace seems funky here?

>  	ast2600-greatlakes.dtb \
>  	ast2600-intel.dtb \
>  	ast2600-intel.dtb \
> diff --git a/arch/arm/dts/ast2600-msx4-bmc-nvidia.dts
> b/arch/arm/dts/ast2600-msx4-bmc-nvidia.dts
> new file mode 100644
> index
> 0000000000000000000000000000000000000000..3f5146c65dec3ec569cd46745a1
> 3be5466f7218c
> --- /dev/null
> +++ b/arch/arm/dts/ast2600-msx4-bmc-nvidia.dts
> @@ -0,0 +1,112 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/dts-v1/;
> +
> +#include "ast2600-u-boot.dtsi"
> +
> +/ {
> +	model = "AST2600 MSX4 BMC";
> +	compatible = "aspeed,ast2600-msx4", "aspeed,ast2600";
> +
> +	memory {
> +		device_type = "memory";
> +		reg = <0x80000000 0x40000000>;
> +	};
> +
> +	chosen {
> +		stdout-path = &uart5;
> +	};
> +
> +	aliases {
> +		spi0 = &fmc;
> +		ethernet0 = &mac0;
> +	};
> +
> +	cpus {
> +		cpu@0 {
> +			clock-frequency = <800000000>;
> +		};
> +		cpu@1 {
> +			clock-frequency = <800000000>;
> +		};
> +	};
> +};
> +
> +&display_port {
> +	status = "okay";
> +};
> +
> +&fmc {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_fmcquad_default>;
> +	aspeed,watchdog-disable;
> +	status = "okay";
> +
> +	flash@0 {
> +		status = "okay";
> +		spi-max-frequency = <10000000>;
> +		spi-tx-bus-width = <2>;
> +		spi-rx-bus-width = <2>;
> +	};
> +
> +	flash@1 {
> +		status = "okay";
> +		spi-max-frequency = <10000000>;
> +		spi-tx-bus-width = <1>;
> +		spi-rx-bus-width = <1>;
> +	};
> +};
> +
> +&hace {
> +	status = "okay";
> +};
> +
> +&mac0 {
> +	phy-mode = "rgmii";
> +	phy-handle = <&ethphy3>;
> +
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_rgmii1_default>;
> +	status = "okay";
> +};
> +
> +&mdio {
> +	status = "okay";

Nit: It'd be nice if the status property were in a consistent location
(wrt ordering in e.g. the fmc node above).

I like it at the top as you have here, makes it clear that the node has
relevant changes.

> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_mdio4_default>;
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	ethphy3: ethernet-phy@0 {
> +		compatible = "ethernet-phy-ieee802.3-c22";
> +		reg = <2>;
> +	};
> +};
> +
> +&scu {
> +	mac0-clk-delay = <0x1d 0x0a
> +			  0x1d 0x0a
> +			  0x1d 0x0a>;
> +};
> +
> +&sdrammc {
> +	clock-frequency = <400000000>;
> +	aspeed,ecc-enabled;
> +	aspeed,ecc-size-mb = <0>;
> +};
> +
> +&uart5 {
> +	u-boot,dm-pre-reloc;
> +	status = "okay";
> +};
> +
> +&wdt1 {
> +	status = "okay";
> +};
> +
> +&wdt2 {
> +	status = "okay";
> +};
> +
> +&wdt3 {
> +	status = "okay";
> +};

Re: [PATCH u-boot v2 1/2] drivers: spi: Add support for disabling FMC_WDT2 for aspeed

[Marc Olberding]

On Wed, Dec 17, 2025 at 09:53:33AM +1030, Andrew Jeffery wrote:
> On Tue, 2025-12-02 at 15:52 -0800, Marc Olberding wrote:
> > Adds support for disabling the ast2600 FMC_WDT2 through
> > a device tree entry in the fmc node.
> > Set `aspeed,watchdog-disable` in your device tree to have
> > the driver disable it.
> > 
> > Signed-off-by: Marc Olberding <molberding@nvidia.com>
> > ---
> >  drivers/spi/aspeed_spi.c | 19 +++++++++++++++----
> >  1 file changed, 15 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/spi/aspeed_spi.c b/drivers/spi/aspeed_spi.c
> > index 54520122f1c48c8b2052b4b1e47445a9b990d25e..de954e477aa15e6d1be042a2aee47f5a501178da 100644
> > --- a/drivers/spi/aspeed_spi.c
> > +++ b/drivers/spi/aspeed_spi.c
> > @@ -30,14 +30,16 @@ struct aspeed_spi_regs {
> >  					/* 0x30 .. 0x38 Segment Address */
> >  	u32 _reserved1[5];		/* .. */
> >  	u32 soft_rst_cmd_ctrl;	/* 0x50 Auto Soft-Reset Command Control */
> > -	u32 _reserved2[11];		/* .. */
> > +	u32 _reserved2[4];		/* .. */
> > +	u32 wdt2_ctrl;			/* 0x64 FMC_WDT2 control */
> > +	u32 _reserved3[6];		/* .. */
> 
> Ugh (passing commentary, not your fault).
> 
Yeah this sucks, if you have a better idea, let me know.

> >  	/*
> >  	 * Set safe default settings for each device. These will be
> >  	 * tuned after the SPI flash devices are probed.
> > @@ -1907,6 +1914,10 @@ static int aspeed_spi_probe(struct udevice *bus)
> >  	 * SPI controllers
> >  	 */
> >  	priv->is_fmc = dev_get_driver_data(bus);
> > +	if (device_is_compatible(bus, "aspeed,ast2600-fmc") &&
> > +	    dev_read_bool(bus, "aspeed,watchdog-disable"))
> > +		priv->disable_wdt = true;
> 
> We're not setting it to false, just declaring it above, which means if
> this branch isn't taken then its value is undefined.
> 
> Perhaps initialise it to false by default.

Ah, I figured that priv would've been callocd by the driver framework
before being passed to probe.
I'll change this to 
priv->disable_wdt = <boolean expression>

in the next rev. Thanks.

> > +
> 
> Unnecessary new-line?
> 
> >  
> >  	ret = aspeed_spi_controller_init(priv);
> >  	if (ret)
ack, will fix.

Thanks for the review.

Re: [PATCH u-boot v2 1/2] drivers: spi: Add support for disabling FMC_WDT2 for aspeed

[Andrew Jeffery]

On Tue, 2025-12-16 at 17:15 -0800, Marc Olberding wrote:
> On Wed, Dec 17, 2025 at 09:53:33AM +1030, Andrew Jeffery wrote:
> > On Tue, 2025-12-02 at 15:52 -0800, Marc Olberding wrote:
> > > Adds support for disabling the ast2600 FMC_WDT2 through
> > > a device tree entry in the fmc node.
> > > Set `aspeed,watchdog-disable` in your device tree to have
> > > the driver disable it.
> > > 
> > > Signed-off-by: Marc Olberding <molberding@nvidia.com>
> > > ---
> > >  drivers/spi/aspeed_spi.c | 19 +++++++++++++++----
> > >  1 file changed, 15 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/spi/aspeed_spi.c b/drivers/spi/aspeed_spi.c
> > > index 54520122f1c48c8b2052b4b1e47445a9b990d25e..de954e477aa15e6d1be042a2aee47f5a501178da 100644
> > > --- a/drivers/spi/aspeed_spi.c
> > > +++ b/drivers/spi/aspeed_spi.c
> > > @@ -30,14 +30,16 @@ struct aspeed_spi_regs {
> > >  					/* 0x30 .. 0x38 Segment Address */
> > >  	u32 _reserved1[5];		/* .. */
> > >  	u32 soft_rst_cmd_ctrl;	/* 0x50 Auto Soft-Reset Command Control */
> > > -	u32 _reserved2[11];		/* .. */
> > > +	u32 _reserved2[4];		/* .. */
> > > +	u32 wdt2_ctrl;			/* 0x64 FMC_WDT2 control */
> > > +	u32 _reserved3[6];		/* .. */
> > 
> > Ugh (passing commentary, not your fault).
> > 
> Yeah this sucks, if you have a better idea, let me know.
> 
> > >  	/*
> > >  	 * Set safe default settings for each device. These will be
> > >  	 * tuned after the SPI flash devices are probed.
> > > @@ -1907,6 +1914,10 @@ static int aspeed_spi_probe(struct udevice *bus)
> > >  	 * SPI controllers
> > >  	 */
> > >  	priv->is_fmc = dev_get_driver_data(bus);
> > > +	if (device_is_compatible(bus, "aspeed,ast2600-fmc") &&
> > > +	    dev_read_bool(bus, "aspeed,watchdog-disable"))
> > > +		priv->disable_wdt = true;
> > 
> > We're not setting it to false, just declaring it above, which means if
> > this branch isn't taken then its value is undefined.
> > 
> > Perhaps initialise it to false by default.
> 
> Ah, I figured that priv would've been callocd by the driver framework
> before being passed to probe.
> I'll change this to 
> priv->disable_wdt = <boolean expression>
> 
> in the next rev. Thanks.

Oh, wait, the stack variable is just redundant? My eyes glazed over
that.

Andrew

Re: [PATCH u-boot v2 1/2] drivers: spi: Add support for disabling FMC_WDT2 for aspeed

[Andrew Jeffery]

On Wed, 2025-12-17 at 11:48 +1030, Andrew Jeffery wrote:
> On Tue, 2025-12-16 at 17:15 -0800, Marc Olberding wrote:
> > On Wed, Dec 17, 2025 at 09:53:33AM +1030, Andrew Jeffery wrote:
> > > On Tue, 2025-12-02 at 15:52 -0800, Marc Olberding wrote:
> > > > Adds support for disabling the ast2600 FMC_WDT2 through
> > > > a device tree entry in the fmc node.
> > > > Set `aspeed,watchdog-disable` in your device tree to have
> > > > the driver disable it.
> > > > 
> > > > Signed-off-by: Marc Olberding <molberding@nvidia.com>
> > > > ---
> > > >  drivers/spi/aspeed_spi.c | 19 +++++++++++++++----
> > > >  1 file changed, 15 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/drivers/spi/aspeed_spi.c
> > > > b/drivers/spi/aspeed_spi.c
> > > > index
> > > > 54520122f1c48c8b2052b4b1e47445a9b990d25e..de954e477aa15e6d1be04
> > > > 2a2aee47f5a501178da 100644
> > > > --- a/drivers/spi/aspeed_spi.c
> > > > +++ b/drivers/spi/aspeed_spi.c
> > > > @@ -30,14 +30,16 @@ struct aspeed_spi_regs {
> > > >  					/* 0x30 .. 0x38
> > > > Segment Address */
> > > >  	u32 _reserved1[5];		/* .. */
> > > >  	u32 soft_rst_cmd_ctrl;	/* 0x50 Auto Soft-Reset
> > > > Command Control */
> > > > -	u32 _reserved2[11];		/* .. */
> > > > +	u32 _reserved2[4];		/* .. */
> > > > +	u32 wdt2_ctrl;			/* 0x64 FMC_WDT2
> > > > control */
> > > > +	u32 _reserved3[6];		/* .. */
> > > 
> > > Ugh (passing commentary, not your fault).
> > > 
> > Yeah this sucks, if you have a better idea, let me know.
> > 
> > > >  	/*
> > > >  	 * Set safe default settings for each device. These
> > > > will be
> > > >  	 * tuned after the SPI flash devices are probed.
> > > > @@ -1907,6 +1914,10 @@ static int aspeed_spi_probe(struct
> > > > udevice *bus)
> > > >  	 * SPI controllers
> > > >  	 */
> > > >  	priv->is_fmc = dev_get_driver_data(bus);
> > > > +	if (device_is_compatible(bus, "aspeed,ast2600-fmc") &&
> > > > +	    dev_read_bool(bus, "aspeed,watchdog-disable"))
> > > > +		priv->disable_wdt = true;
> > > 
> > > We're not setting it to false, just declaring it above, which
> > > means if
> > > this branch isn't taken then its value is undefined.
> > > 
> > > Perhaps initialise it to false by default.
> > 
> > Ah, I figured that priv would've been callocd by the driver
> > framework
> > before being passed to probe.
> > I'll change this to 
> > priv->disable_wdt = <boolean expression>
> > 
> > in the next rev. Thanks.
> 
> Oh, wait, the stack variable is just redundant? My eyes glazed over
> that.

Nope, ignore that too, I misinterpreted the diff.

Ugh, sorry for the noise.

Re: [PATCH u-boot v2 1/2] drivers: spi: Add support for disabling FMC_WDT2 for aspeed

[Marc Olberding]

On Wed, Dec 17, 2025 at 11:51:23AM +1030, Andrew Jeffery wrote:
> On Wed, 2025-12-17 at 11:48 +1030, Andrew Jeffery wrote:
> > On Tue, 2025-12-16 at 17:15 -0800, Marc Olberding wrote:
> > > On Wed, Dec 17, 2025 at 09:53:33AM +1030, Andrew Jeffery wrote:
> > > > On Tue, 2025-12-02 at 15:52 -0800, Marc Olberding wrote:
> > > > > Adds support for disabling the ast2600 FMC_WDT2 through
> > > > > a device tree entry in the fmc node.
> > > > > Set `aspeed,watchdog-disable` in your device tree to have
> > > > > the driver disable it.
> > > > > 
> > > > > Signed-off-by: Marc Olberding <molberding@nvidia.com>
> > > > > ---
> > > > >  drivers/spi/aspeed_spi.c | 19 +++++++++++++++----
> > > > >  1 file changed, 15 insertions(+), 4 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/spi/aspeed_spi.c
> > > > > b/drivers/spi/aspeed_spi.c
> > > > > index
> > > > > 54520122f1c48c8b2052b4b1e47445a9b990d25e..de954e477aa15e6d1be04
> > > > > 2a2aee47f5a501178da 100644
> > > > > --- a/drivers/spi/aspeed_spi.c
> > > > > +++ b/drivers/spi/aspeed_spi.c
> > > > > @@ -30,14 +30,16 @@ struct aspeed_spi_regs {
> > > > >  					/* 0x30 .. 0x38
> > > > > Segment Address */
> > > > >  	u32 _reserved1[5];		/* .. */
> > > > >  	u32 soft_rst_cmd_ctrl;	/* 0x50 Auto Soft-Reset
> > > > > Command Control */
> > > > > -	u32 _reserved2[11];		/* .. */
> > > > > +	u32 _reserved2[4];		/* .. */
> > > > > +	u32 wdt2_ctrl;			/* 0x64 FMC_WDT2
> > > > > control */
> > > > > +	u32 _reserved3[6];		/* .. */
> > > > 
> > > > Ugh (passing commentary, not your fault).
> > > > 
> > > Yeah this sucks, if you have a better idea, let me know.
> > > 
> > > > >  	/*
> > > > >  	 * Set safe default settings for each device. These
> > > > > will be
> > > > >  	 * tuned after the SPI flash devices are probed.
> > > > > @@ -1907,6 +1914,10 @@ static int aspeed_spi_probe(struct
> > > > > udevice *bus)
> > > > >  	 * SPI controllers
> > > > >  	 */
> > > > >  	priv->is_fmc = dev_get_driver_data(bus);
> > > > > +	if (device_is_compatible(bus, "aspeed,ast2600-fmc") &&
> > > > > +	    dev_read_bool(bus, "aspeed,watchdog-disable"))
> > > > > +		priv->disable_wdt = true;
> > > > 
> > > > We're not setting it to false, just declaring it above, which
> > > > means if
> > > > this branch isn't taken then its value is undefined.
> > > > 
> > > > Perhaps initialise it to false by default.
> > > 
> > > Ah, I figured that priv would've been callocd by the driver
> > > framework
> > > before being passed to probe.
> > > I'll change this to 
> > > priv->disable_wdt = <boolean expression>
> > > 
> > > in the next rev. Thanks.
> > 
> > Oh, wait, the stack variable is just redundant? My eyes glazed over
> > that.
> 
> Nope, ignore that too, I misinterpreted the diff.
> 
> Ugh, sorry for the noise.

No worries, I think the unconditional assignment is cleaner
and I've already made the change. I had to go cleanup your device tree
comments anyways, I'll send out v3 once I flash to hardware.

Thanks,
Marc

Re: [PATCH u-boot v2 1/2] drivers: spi: Add support for disabling FMC_WDT2 for aspeed

[Иван Михайлов]

On Wed, Dec 3, 2025 at 2:53 AM Marc Olberding <molberding@nvidia.com> wrote:
>
> Adds support for disabling the ast2600 FMC_WDT2 through
> a device tree entry in the fmc node.
> Set `aspeed,watchdog-disable` in your device tree to have
> the driver disable it.

Marc, FMC_WDT2 doesn't disable watchdog, it controls ABR mode.
Watchdog with or without ABR still in operational mode.
So, maybe aspeed,abr-disable?

Below namings probably should be corrected.

>
> Signed-off-by: Marc Olberding <molberding@nvidia.com>
> ---
>  drivers/spi/aspeed_spi.c | 19 +++++++++++++++----
>  1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/spi/aspeed_spi.c b/drivers/spi/aspeed_spi.c
> index 54520122f1c48c8b2052b4b1e47445a9b990d25e..de954e477aa15e6d1be042a2aee47f5a501178da 100644
> --- a/drivers/spi/aspeed_spi.c
> +++ b/drivers/spi/aspeed_spi.c
> @@ -30,14 +30,16 @@ struct aspeed_spi_regs {
>                                         /* 0x30 .. 0x38 Segment Address */
>         u32 _reserved1[5];              /* .. */
>         u32 soft_rst_cmd_ctrl;  /* 0x50 Auto Soft-Reset Command Control */
> -       u32 _reserved2[11];             /* .. */
> +       u32 _reserved2[4];              /* .. */
> +       u32 wdt2_ctrl;                  /* 0x64 FMC_WDT2 control */
> +       u32 _reserved3[6];              /* .. */
>         u32 dma_ctrl;                   /* 0x80 DMA Control/Status */
>         u32 dma_flash_addr;             /* 0x84 DMA Flash Side Address */
>         u32 dma_dram_addr;              /* 0x88 DMA DRAM Side Address */
>         u32 dma_len;                    /* 0x8c DMA Length Register */
>         u32 dma_checksum;               /* 0x90 Checksum Calculation Result */
>         u32 timings;                    /* 0x94 Read Timing Compensation */
> -       u32 _reserved3[1];
> +       u32 _reserved4[1];
>         /* not used */
>         u32 soft_strap_status;          /* 0x9c Software Strap Status */
>         u32 write_cmd_filter_ctrl;      /* 0xa0 Write Command Filter Control */
> @@ -45,7 +47,7 @@ struct aspeed_spi_regs {
>         u32 lock_ctrl_reset;            /* 0xa8 Lock Control (SRST#) */
>         u32 lock_ctrl_wdt;              /* 0xac Lock Control (Watchdog) */
>         u32 write_addr_filter[8];       /* 0xb0 Write Address Filter */
> -       u32 _reserved4[12];
> +       u32 _reserved5[12];
>         u32 fully_qualified_cmd[20];    /* 0x100 Fully Qualified Command */
>         u32 addr_qualified_cmd[12];     /* 0x150 Address Qualified Command */
>  };
> @@ -163,7 +165,8 @@ struct aspeed_spi_regs {
>  #define SPI_3B_AUTO_CLR_REG   0x1e6e2510
>  #define SPI_3B_AUTO_CLR       BIT(9)
>
> -
> +/* FMC_WDT2 control register */
> +#define WDT2_ENABLE            BIT(0)
>  /*
>   * flash related info
>   */
> @@ -267,6 +270,7 @@ struct aspeed_spi_priv {
>         ulong hclk_rate; /* AHB clock rate */
>         u8 num_cs;
>         bool is_fmc;
> +       bool disable_wdt;
>
>         struct aspeed_spi_flash flashes[ASPEED_SPI_MAX_CS];
>         u32 flash_count;
> @@ -683,6 +687,9 @@ static int aspeed_spi_controller_init(struct aspeed_spi_priv *priv)
>         setbits_le32(&priv->regs->conf,
>                      CONF_ENABLE_W2 | CONF_ENABLE_W1 | CONF_ENABLE_W0);
>
> +       if (priv->is_fmc && priv->disable_wdt)
> +               clrbits_le32(&priv->regs->wdt2_ctrl, WDT2_ENABLE);
> +
>         /*
>          * Set safe default settings for each device. These will be
>          * tuned after the SPI flash devices are probed.
> @@ -1907,6 +1914,10 @@ static int aspeed_spi_probe(struct udevice *bus)
>          * SPI controllers
>          */
>         priv->is_fmc = dev_get_driver_data(bus);
> +       if (device_is_compatible(bus, "aspeed,ast2600-fmc") &&
> +           dev_read_bool(bus, "aspeed,watchdog-disable"))
> +               priv->disable_wdt = true;
> +
>
>         ret = aspeed_spi_controller_init(priv);
>         if (ret)
>
> --
> 2.34.1
>

This patch go through the standard upstream process or just openbmc u-boot?
Do you have plans to do any changes around the linux kernel with
fmc_wdt2 + spi part?

Thanks.

Re: [PATCH u-boot v2 1/2] drivers: spi: Add support for disabling FMC_WDT2 for aspeed

[Marc Olberding]

On Sat, Jan 03, 2026 at 12:24:07AM +0300, Иван Михайлов wrote:
> On Wed, Dec 3, 2025 at 2:53 AM Marc Olberding <molberding@nvidia.com> wrote:
> >
> > Adds support for disabling the ast2600 FMC_WDT2 through
> > a device tree entry in the fmc node.
> > Set `aspeed,watchdog-disable` in your device tree to have
> > the driver disable it.
> 
> Marc, FMC_WDT2 doesn't disable watchdog, it controls ABR mode.
> Watchdog with or without ABR still in operational mode.
> So, maybe aspeed,abr-disable?
> 
> Below namings probably should be corrected.
We aren't disabling ABR mode with this change, right? That's only
done through hardware straps or OTP changes. All this is doing is clearing bit 0
of FMC64, which per the datasheet disables the watchdog. The idea here is
to just allow boot to progress normally, without the watchdog. For ping pong update,
userspace can flash the alternative SPI and re-enable the watchdog timer on complete,
and the BMC will boot from the new image upon reset. Let me know if I'm misunderstanding
your comment.

As far as the name itself, that came from Andrew the maintainer. I'll defer to Andrew
on what he wants. Andrew, any thoughts here?

> > 2.34.1
> >
> 
> This patch go through the standard upstream process or just openbmc u-boot?
> Do you have plans to do any changes around the linux kernel with
> fmc_wdt2 + spi part?
> 
> Thanks.

Looking at the state of downstream versus upstream, it will require (relatively minimal)
porting between the two as there are a number of differences in the upstream u-boot compared to downstream.

I'll defer to andrew on how he wants me to do this, but it will effectively be two different
patches.

As far as the linux-kernel changes, for the platform I'm targetting this for, I don't
plan on needing the functionality in userspace + kernel.

Thanks
Marc

Re: [PATCH u-boot v2 1/2] drivers: spi: Add support for disabling FMC_WDT2 for aspeed

[Ivan Mikhaylov]

On Tue, Jan 6, 2026 at 3:05 AM Marc Olberding <molberding@nvidia.com> wrote:
>
> On Sat, Jan 03, 2026 at 12:24:07AM +0300, Иван Михайлов wrote:
> > On Wed, Dec 3, 2025 at 2:53 AM Marc Olberding <molberding@nvidia.com> wrote:
> > >
> > > Adds support for disabling the ast2600 FMC_WDT2 through
> > > a device tree entry in the fmc node.
> > > Set `aspeed,watchdog-disable` in your device tree to have
> > > the driver disable it.
> >
> > Marc, FMC_WDT2 doesn't disable watchdog, it controls ABR mode.
> > Watchdog with or without ABR still in operational mode.
> > So, maybe aspeed,abr-disable?
> >
> > Below namings probably should be corrected.
> We aren't disabling ABR mode with this change, right? That's only
> done through hardware straps or OTP changes. All this is doing is clearing bit 0
> of FMC64, which per the datasheet disables the watchdog. The idea here is
> to just allow boot to progress normally, without the watchdog. For ping pong update,
> userspace can flash the alternative SPI and re-enable the watchdog timer on complete,
> and the BMC will boot from the new image upon reset. Let me know if I'm misunderstanding
> your comment.
>

Marc, when you clrbits_le on FMC64/FMC_WDT2, then you disable ABR mode, I
assume you can check it with evb board or ast2600-a3 to prove. On my board
ast2600-a3 it works in that way I described with enabled OTP strap for ABR.

Also description of it in 14.2.2 Alternative Boot Recovery Function.

FMC_WDT2 & WDT2 as far as I know are different, you're not disabling
WDT2 with disabling FMC_WDT2.

According to the spec, it's right it disables/enables the watchdog with 0 bit
but which one. Probably FMC_WDT2, not WDT2, and it still works as should and
disabling WDT2/WDTX in different sections - WDT0C 0 bit and WDT30.

Thanks.

Re: [PATCH u-boot v2 1/2] drivers: spi: Add support for disabling FMC_WDT2 for aspeed

[Marc Olberding]

On Tue, Jan 06, 2026 at 05:52:29PM +0300, Ivan Mikhaylov wrote:
> On Tue, Jan 6, 2026 at 3:05 AM Marc Olberding <molberding@nvidia.com> wrote:
> >
> > On Sat, Jan 03, 2026 at 12:24:07AM +0300, Иван Михайлов wrote:
> > > On Wed, Dec 3, 2025 at 2:53 AM Marc Olberding <molberding@nvidia.com> wrote:
> > > >
> > > > Adds support for disabling the ast2600 FMC_WDT2 through
> > > > a device tree entry in the fmc node.
> > > > Set `aspeed,watchdog-disable` in your device tree to have
> > > > the driver disable it.
> > >
> > > Marc, FMC_WDT2 doesn't disable watchdog, it controls ABR mode.
> > > Watchdog with or without ABR still in operational mode.
> > > So, maybe aspeed,abr-disable?
> > >
> > > Below namings probably should be corrected.
> > We aren't disabling ABR mode with this change, right? That's only
> > done through hardware straps or OTP changes. All this is doing is clearing bit 0
> > of FMC64, which per the datasheet disables the watchdog. The idea here is
> > to just allow boot to progress normally, without the watchdog. For ping pong update,
> > userspace can flash the alternative SPI and re-enable the watchdog timer on complete,
> > and the BMC will boot from the new image upon reset. Let me know if I'm misunderstanding
> > your comment.
> >
> 
> Marc, when you clrbits_le on FMC64/FMC_WDT2, then you disable ABR mode, I
> assume you can check it with evb board or ast2600-a3 to prove. On my board
> ast2600-a3 it works in that way I described with enabled OTP strap for ABR.
> 
> Also description of it in 14.2.2 Alternative Boot Recovery Function.
Ack, I see your point. I'll be honest, I don't have a strong opinion on the naming here.
I'll wait for Andrew to chime in since he had provided naming preferences previously and
I'd prefer to avoid churn until people agree on naming. It was aspeed,disable-fmc-wdt2 before,
if I remember correctly, and he had suggested this binding instead. As he is the maintainer and
I didn't have a strong opinion, I changed it to aspeed,disable-watchdog. I see your point,
but at the end of the day Andrew needs to sign off on whatever change I make and we landed on this
binding because of his feedback.

> FMC_WDT2 & WDT2 as far as I know are different, you're not disabling
> WDT2 with disabling FMC_WDT2.

> According to the spec, it's right it disables/enables the watchdog with 0 bit
> but which one. Probably FMC_WDT2, not WDT2, and it still works as should and
> disabling WDT2/WDTX in different sections - WDT0C 0 bit and WDT30.
> 
> Thanks.
The intention here is to disable fmc_wdt2, and to your point ABR,
which the datasheet and testing supports that we're doing with the change.
I have no intention of touching the other watchdogs with this,
support for the existing watchdogs seems to be well plumbed already throughout u-boot and the kernel.

Thanks for the review,
Marc

Re: [PATCH u-boot v2 1/2] drivers: spi: Add support for disabling FMC_WDT2 for aspeed

[Andrew Jeffery]

On Tue, 2026-01-06 at 17:52 +0300, Ivan Mikhaylov wrote:
> On Tue, Jan 6, 2026 at 3:05 AM Marc Olberding <molberding@nvidia.com> wrote:
> > 
> > On Sat, Jan 03, 2026 at 12:24:07AM +0300, Иван Михайлов wrote:
> > > On Wed, Dec 3, 2025 at 2:53 AM Marc Olberding <molberding@nvidia.com> wrote:
> > > > 
> > > > Adds support for disabling the ast2600 FMC_WDT2 through
> > > > a device tree entry in the fmc node.
> > > > Set `aspeed,watchdog-disable` in your device tree to have
> > > > the driver disable it.
> > > 
> > > Marc, FMC_WDT2 doesn't disable watchdog, it controls ABR mode.
> > > Watchdog with or without ABR still in operational mode.
> > > So, maybe aspeed,abr-disable?
> > > 
> > > Below namings probably should be corrected.
> > We aren't disabling ABR mode with this change, right? That's only
> > done through hardware straps or OTP changes. All this is doing is clearing bit 0
> > of FMC64, which per the datasheet disables the watchdog. The idea here is
> > to just allow boot to progress normally, without the watchdog. For ping pong update,
> > userspace can flash the alternative SPI and re-enable the watchdog timer on complete,
> > and the BMC will boot from the new image upon reset. Let me know if I'm misunderstanding
> > your comment.
> > 
> 
> Marc, when you clrbits_le on FMC64/FMC_WDT2, then you disable ABR mode, I
> assume you can check it with evb board or ast2600-a3 to prove. On my board
> ast2600-a3 it works in that way I described with enabled OTP strap for ABR.

Can you elaborate on the sequences involved?

The reset definitions for the FMC suggest it's affected by an ARM
reset, which should in-turn reinitialise FMC_WDT2 and follow the boot
flow to enable the ABR again. In my understanding disabling it in
firmware shouldn't prevent ABR operations subsequent to future resets?

Further, the aspeed,watchdog-disable property is to be taken in the
context of the node with the FMC compatible string, referring to the
FMC's watchdog and not the (separate) SoC watchdogs. However the FMC
does have multiple watchdogs, one for address mode detection and the
other for ABR. So maybe the name of the property could be improved in
that regard. I think it's still reasonable to have watchdog in the
name. My immediate reaction is that "aspeed,abr-disable" overstates its
behaviour. How about "aspeed,abr-watchdog-disable"? Previous
suggestions were "fmc-wdt2-disable", and the current "aspeed,watchdog-
disable"

> 
> Also description of it in 14.2.2 Alternative Boot Recovery Function.

The second bullet in that section says:

   When the firmware booting successfully, it should disable FMC_WDT2
   to stop booting switch

This is what Marc is trying to achieve by providing the property. Other
platforms may want to make the choice elsewhere.

Andrew

Re: [PATCH u-boot v2 1/2] drivers: spi: Add support for disabling FMC_WDT2 for aspeed

[Ivan Mikhaylov]

On Mon, 2026-01-12 at 11:42 +1030, Andrew Jeffery wrote:
> On Tue, 2026-01-06 at 17:52 +0300, Ivan Mikhaylov wrote:
> > On Tue, Jan 6, 2026 at 3:05 AM Marc Olberding
> > <molberding@nvidia.com> wrote:
> > > 
> > > On Sat, Jan 03, 2026 at 12:24:07AM +0300, Иван Михайлов wrote:
> > > > On Wed, Dec 3, 2025 at 2:53 AM Marc Olberding
> > > > <molberding@nvidia.com> wrote:
> > > > > 
> > > > > Adds support for disabling the ast2600 FMC_WDT2 through
> > > > > a device tree entry in the fmc node.
> > > > > Set `aspeed,watchdog-disable` in your device tree to have
> > > > > the driver disable it.
> > > > 
> > > > Marc, FMC_WDT2 doesn't disable watchdog, it controls ABR mode.
> > > > Watchdog with or without ABR still in operational mode.
> > > > So, maybe aspeed,abr-disable?
> > > > 
> > > > Below namings probably should be corrected.
> > > We aren't disabling ABR mode with this change, right? That's only
> > > done through hardware straps or OTP changes. All this is doing is
> > > clearing bit 0
> > > of FMC64, which per the datasheet disables the watchdog. The idea
> > > here is
> > > to just allow boot to progress normally, without the watchdog.
> > > For ping pong update,
> > > userspace can flash the alternative SPI and re-enable the
> > > watchdog timer on complete,
> > > and the BMC will boot from the new image upon reset. Let me know
> > > if I'm misunderstanding
> > > your comment.
> > > 
> > 
> > Marc, when you clrbits_le on FMC64/FMC_WDT2, then you disable ABR
> > mode, I
> > assume you can check it with evb board or ast2600-a3 to prove. On
> > my board
> > ast2600-a3 it works in that way I described with enabled OTP strap
> > for ABR.
> 
> Can you elaborate on the sequences involved?

Andrew, same as in this patch but with mw in u-boot run script with
enabled OTP strap for ABR.

> The reset definitions for the FMC suggest it's affected by an ARM
> reset, which should in-turn reinitialise FMC_WDT2 and follow the boot
> flow to enable the ABR again. In my understanding disabling it in
> firmware shouldn't prevent ABR operations subsequent to future
> resets?

As I know, yes.

> 
> Further, the aspeed,watchdog-disable property is to be taken in the
> context of the node with the FMC compatible string, referring to the
> FMC's watchdog and not the (separate) SoC watchdogs. However the FMC
> does have multiple watchdogs, one for address mode detection and the
> other for ABR. So maybe the name of the property could be improved in
> that regard. I think it's still reasonable to have watchdog in the
> name. My immediate reaction is that "aspeed,abr-disable" overstates
> its
> behaviour. How about "aspeed,abr-watchdog-disable"? Previous
> suggestions were "fmc-wdt2-disable", and the current
> "aspeed,watchdog-
> disable"
> 

I like both of these: "aspeed,abr-watchdog-disable", "aspeed,fmc-wdt2-
disable".

> > 
> > Also description of it in 14.2.2 Alternative Boot Recovery
> > Function.
> 
> The second bullet in that section says:
> 
>    When the firmware booting successfully, it should disable FMC_WDT2
>    to stop booting switch
> 
> This is what Marc is trying to achieve by providing the property.
> Other
> platforms may want to make the choice elsewhere.
> 
> Andrew

I'm not saying that is Marc is doing something wrong about it, I just
want to say that need to distinguish WDT2 and FMC_WDT2 because it
puzzles when you reading the code in which you see something like:

/* FMC_WDT2 ... */
#define WDT2_ENABLE ...

I'd be use exactly what it should be:
/* FMC_WDT2 ... */
#define FMC_WDT2_ENABLE ...

and everything else which relates to FMC_WDT2.

Thanks.

Re: [PATCH u-boot v2 1/2] drivers: spi: Add support for disabling FMC_WDT2 for aspeed

[Andrew Jeffery]

Apologies for the delayed reply.

On Tue, 2026-01-13 at 23:55 +0300, Ivan Mikhaylov wrote:
> On Mon, 2026-01-12 at 11:42 +1030, Andrew Jeffery wrote:
> > On Tue, 2026-01-06 at 17:52 +0300, Ivan Mikhaylov wrote:
> > > On Tue, Jan 6, 2026 at 3:05 AM Marc Olberding
> > > <molberding@nvidia.com> wrote:
> > > > 
> > > > On Sat, Jan 03, 2026 at 12:24:07AM +0300, Иван Михайлов wrote:
> > > > > On Wed, Dec 3, 2025 at 2:53 AM Marc Olberding
> > > > > <molberding@nvidia.com> wrote:
> > > > > > 
> > > > > > Adds support for disabling the ast2600 FMC_WDT2 through
> > > > > > a device tree entry in the fmc node.
> > > > > > Set `aspeed,watchdog-disable` in your device tree to have
> > > > > > the driver disable it.
> > > > > 
> > > > > Marc, FMC_WDT2 doesn't disable watchdog, it controls ABR mode.
> > > > > Watchdog with or without ABR still in operational mode.
> > > > > So, maybe aspeed,abr-disable?
> > > > > 
> > > > > Below namings probably should be corrected.
> > > > We aren't disabling ABR mode with this change, right? That's only
> > > > done through hardware straps or OTP changes. All this is doing is
> > > > clearing bit 0
> > > > of FMC64, which per the datasheet disables the watchdog. The idea
> > > > here is
> > > > to just allow boot to progress normally, without the watchdog.
> > > > For ping pong update,
> > > > userspace can flash the alternative SPI and re-enable the
> > > > watchdog timer on complete,
> > > > and the BMC will boot from the new image upon reset. Let me know
> > > > if I'm misunderstanding
> > > > your comment.
> > > > 
> > > 
> > > Marc, when you clrbits_le on FMC64/FMC_WDT2, then you disable ABR
> > > mode, I
> > > assume you can check it with evb board or ast2600-a3 to prove. On
> > > my board
> > > ast2600-a3 it works in that way I described with enabled OTP strap
> > > for ABR.
> > 
> > Can you elaborate on the sequences involved?
> 
> Andrew, same as in this patch but with mw in u-boot run script with
> enabled OTP strap for ABR.
> 
> > The reset definitions for the FMC suggest it's affected by an ARM
> > reset, which should in-turn reinitialise FMC_WDT2 and follow the boot
> > flow to enable the ABR again. In my understanding disabling it in
> > firmware shouldn't prevent ABR operations subsequent to future
> > resets?
> 
> As I know, yes.
> 
> > 
> > Further, the aspeed,watchdog-disable property is to be taken in the
> > context of the node with the FMC compatible string, referring to the
> > FMC's watchdog and not the (separate) SoC watchdogs. However the FMC
> > does have multiple watchdogs, one for address mode detection and the
> > other for ABR. So maybe the name of the property could be improved in
> > that regard. I think it's still reasonable to have watchdog in the
> > name. My immediate reaction is that "aspeed,abr-disable" overstates
> > its
> > behaviour. How about "aspeed,abr-watchdog-disable"? Previous
> > suggestions were "fmc-wdt2-disable", and the current
> > "aspeed,watchdog-
> > disable"
> > 
> 
> I like both of these: "aspeed,abr-watchdog-disable", "aspeed,fmc-wdt2-
> disable".

Alright, let's go with the "aspeed,abr-watchdog-disable".

> 
> > > 
> > > Also description of it in 14.2.2 Alternative Boot Recovery
> > > Function.
> > 
> > The second bullet in that section says:
> > 
> >    When the firmware booting successfully, it should disable FMC_WDT2
> >    to stop booting switch
> > 
> > This is what Marc is trying to achieve by providing the property.
> > Other
> > platforms may want to make the choice elsewhere.
> > 
> > Andrew
> 
> I'm not saying that is Marc is doing something wrong about it, I just
> want to say that need to distinguish WDT2 and FMC_WDT2 because it
> puzzles when you reading the code in which you see something like:
> 
> /* FMC_WDT2 ... */
> #define WDT2_ENABLE ...
> 
> I'd be use exactly what it should be:
> /* FMC_WDT2 ... */
> #define FMC_WDT2_ENABLE ...
> 
> and everything else which relates to FMC_WDT2.

Understood.

Andrew