> On Sep 6, 2021, at 11:58 PM, Cédric Le Goater <c...@kaod.org> wrote: > > The Aspeed SoCs have a dual boot function for firmware fail-over > recovery. The system auto-reboots from the second flash if the main > flash does not boot sucessfully within a certain amount of time. This > function is called alternate boot (ABR) in the FMC controllers. > > On AST2400/AST2500, ABR is enabled by hardware strapping in SCU70 to > enable the 2nd watchdog timer, on AST2600, through register SCU510. > If the boot on the the main flash succeeds, the firmware should > disable the 2nd watchdog timer. If not, the BMC is reset and the CE0 > and CE1 mappings are swapped to restart the BMC from the 2nd flash. > > On the AST2600, the registers controlling the 2nd watchdog timer were > moved from the watchdog register to the FMC controller. Add simple > read/write handlers for these to let the FW disable the 2nd watchdog > without error. > > Reported-by: Peter Delevoryas <p...@fb.com> > Signed-off-by: Cédric Le Goater <c...@kaod.org> > --- > hw/ssi/aspeed_smc.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c > index 331a2c544635..c9990f069ea4 100644 > --- a/hw/ssi/aspeed_smc.c > +++ b/hw/ssi/aspeed_smc.c > @@ -124,6 +124,13 @@ > /* SPI dummy cycle data */ > #define R_DUMMY_DATA (0x54 / 4) > > +/* FMC_WDT2 Control/Status Register for Alternate Boot (AST2600) */ > +#define R_FMC_WDT2_CTRL (0x64 / 4) > +#define FMC_WDT2_CTRL_ALT_BOOT_MODE BIT(6) /* O: 2 chips 1: 1 chip */ > +#define FMC_WDT2_CTRL_SINGLE_BOOT_MODE BIT(5) > +#define FMC_WDT2_CTRL_BOOT_SOURCE BIT(4) /* O: primary 1: alternate */ > +#define FMC_WDT2_CTRL_EN BIT(0) > + > /* DMA Control/Status Register */ > #define R_DMA_CTRL (0x80 / 4) > #define DMA_CTRL_REQUEST (1 << 31) > @@ -263,12 +270,18 @@ static void aspeed_2600_smc_dma_ctrl(AspeedSMCState *s, > uint32_t value); > > #define ASPEED_SMC_FEATURE_DMA 0x1 > #define ASPEED_SMC_FEATURE_DMA_GRANT 0x2 > +#define ASPEED_SMC_FEATURE_WDT_CONTROL 0x4 > > static inline bool aspeed_smc_has_dma(const AspeedSMCState *s) > { > return !!(s->ctrl->features & ASPEED_SMC_FEATURE_DMA); > } > > +static inline bool aspeed_smc_has_wdt_control(const AspeedSMCState *s) > +{ > + return !!(s->ctrl->features & ASPEED_SMC_FEATURE_WDT_CONTROL); > +} > + > static const AspeedSMCController controllers[] = { > { > .name = "aspeed.smc-ast2400", > @@ -1019,6 +1032,7 @@ static uint64_t aspeed_smc_read(void *opaque, hwaddr > addr, unsigned int size) > addr == R_CE_CMD_CTRL || > addr == R_INTR_CTRL || > addr == R_DUMMY_DATA || > + (aspeed_smc_has_wdt_control(s) && addr == R_FMC_WDT2_CTRL) || > (aspeed_smc_has_dma(s) && addr == R_DMA_CTRL) || > (aspeed_smc_has_dma(s) && addr == R_DMA_FLASH_ADDR) || > (aspeed_smc_has_dma(s) && addr == R_DMA_DRAM_ADDR) || > @@ -1350,6 +1364,8 @@ static void aspeed_smc_write(void *opaque, hwaddr addr, > uint64_t data, > s->regs[addr] = value & 0xff; > } else if (addr == R_DUMMY_DATA) { > s->regs[addr] = value & 0xff; > + } else if (aspeed_smc_has_wdt_control(s) && addr == R_FMC_WDT2_CTRL) { > + s->regs[addr] = value & 0xb; > } else if (addr == R_INTR_CTRL) { > s->regs[addr] = value; > } else if (aspeed_smc_has_dma(s) && addr == R_DMA_CTRL) { > -- > 2.31.1 >
Looks good to me! Reviewed-by: Peter Delevoryas <p...@fb.com> One thing: should we enable this feature (ASPEED_SMC_FEATURE_WDT_CONTROL) on any of the SMC controller models? I wanted to test this with the Fuji image and machine type I added, and I needed the following diff to enable this: diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c index 331a2c5446..b5d835d488 100644 --- a/hw/ssi/aspeed_smc.c +++ b/hw/ssi/aspeed_smc.c @@ -388,7 +388,7 @@ static const AspeedSMCController controllers[] = { .segments = aspeed_segments_ast2600_fmc, .flash_window_base = ASPEED26_SOC_FMC_FLASH_BASE, .flash_window_size = 0x10000000, - .features = ASPEED_SMC_FEATURE_DMA, + .features = ASPEED_SMC_FEATURE_DMA | ASPEED_SMC_FEATURE_WDT_CONTROL, .dma_flash_mask = 0x0FFFFFFC, .dma_dram_mask = 0x3FFFFFFC, .nregs = ASPEED_SMC_R_MAX, Without this, Fuji would try to clear BIT(0) of R_FMC_WDT2_CTRL, but then the default aspeed_smc_read() value would return 0xFFFFFFF. Error: unable to disable the 2nd watchdog: FMC_WDT2=0xFFFFFFFF And then with these changes added, the writes and reads work so that BIT(0) appears to have been cleared: Disabled the 2nd watchdog (FMC_WDT2) successfully. root@bmc-oob:~# devmem 0x1e620064 0x00000000 root@bmc-oob:~# devmem 0x1e620064 32 0xffffffff root@bmc-oob:~# devmem 0x1e620064 0x0000000B I don’t totally understand why the mask for the register is 0xb though? >>> bin(0xb) ‘0b1011' This doesn’t seem to match the macro BIT(...) #define’s included. Those #define’s are correct (I cross-referenced with the datasheet to double check), wouldn’t it be something like this? (zero’s for the reserved bits?) >>> hex(0b1111111101110001) '0xff71' Thanks, Peter