I'll be removing the extra line in the comment and adding a new patch modifying the link in version 2.
On Thu, Nov 14, 2024 at 4:53 AM Peter Maydell <[email protected]> wrote: > > On Fri, 8 Nov 2024 at 19:10, Roque Arcudia Hernandez <[email protected]> > wrote: > > > > Current watchdog is free running out of reset, this combined with the > > fact that current implementation also ensures the counter is running > > when programing WDOGLOAD creates issues when the firmware defer the > > programing of WDOGCONTROL.INTEN much later after WDOGLOAD. Arm > > Programmer's Model documentation states that INTEN is also the > > counter enable: > > > > > INTEN > > > > > > Enable the interrupt event, WDOGINT. Set HIGH to enable the counter > > > and the interrupt, or LOW to disable the counter and interrupt. > > > Reloads the counter from the value in WDOGLOAD when the interrupt > > > is enabled, after previously being disabled. > > > > Source of the time of writing: > > > > https://developer.arm.com/documentation/ddi0479/d/apb-components/apb-watchdog/programmers-model > > I see that the URL in the current sources > https://developer.arm.com/products/system-design/system-design-kits/cortex-m-system-design-kit > is no longer working -- would you mind including a patch that updates > the URL in the comment at the top of the file to the new one > https://developer.arm.com/documentation/ddi0479/ > > please? > > > Signed-off-by: Roque Arcudia Hernandez <[email protected]> > > Reviewed-by: Stephen Longfield <[email protected]> > > Reviewed-by: Joe Komlodi <[email protected]> > > --- > > hw/watchdog/cmsdk-apb-watchdog.c | 29 ++++++++++++++++++++++++++--- > > 1 file changed, 26 insertions(+), 3 deletions(-) > > > > diff --git a/hw/watchdog/cmsdk-apb-watchdog.c > > b/hw/watchdog/cmsdk-apb-watchdog.c > > index 7ad46f9410..e6ddc0a53b 100644 > > --- a/hw/watchdog/cmsdk-apb-watchdog.c > > +++ b/hw/watchdog/cmsdk-apb-watchdog.c > > @@ -202,10 +202,10 @@ static void cmsdk_apb_watchdog_write(void *opaque, > > hwaddr offset, > > */ > > ptimer_transaction_begin(s->timer); > > ptimer_set_limit(s->timer, value, 1); > > - ptimer_run(s->timer, 0); > > ptimer_transaction_commit(s->timer); > > break; > > This looks like a correct change, but the comment just > above here needs to be updated to match it (it currently > says "and make sure we're counting"). > > Otherwise this change looks good. > > -- PMM
