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

[Ivan Mikhaylov <fr0st61te@gmail.com>]

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.