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

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

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