The following patchset tries to address an issue where the watchdog
counter was running as soon as the device is out of reset. This
created a few problems with the firmware under test. It was pointed
out that the firmware under test was already working on an emulator
using the real RTL. Further review showed that the WDOGCONTROL.INTEN
is also expected to be the counter enable as it is defined in this
page from ARM:

https://developer.arm.com/documentation/ddi0479/d/apb-components/apb-watchdog/programmers-model?lang=en

     |       | Enable the interrupt event, WDOGINT. Set to 1 to
     |       | enable the counter and the interrupt, or to 0 to
 [0] | INTEN | disable the counter and interrupt. Reloads the
     |       | counter from the value in WDOGLOAD when the interrupt
     |       | is enabled, after previously being disabled.

The second half of the patch is focused on adding tests for this
behavior. It was required to modify the test to be able to run some
test cases in the MPS2 since the the stellaris machine had 2 issues:

  - The stellaris machine did not run the counter out of reset
    because of the way it is reset. The function
    ssys_calculate_system_clock is not called until the reset phase
    stellaris_sys_reset_hold which is after the watchdog executes its
    reset, so the watchdog would not start the counter due to the
    clock not being set up at the time of reset.

  - The Stellaris machine uses a derived watchdog and not the
    original CMSDK one which has a special behavior where it cannot
    be stopped.

The MPS2 machine on the other hand uses the standard CMSDK watchdog
and could expose the problems we were dealing with.

Roque Arcudia Hernandez (5):
  hw/watchdog/cmsdk_apb_watchdog: Fix broken link
  hw/watchdog/cmsdk_apb_watchdog: Fix INTEN issues
  tests/qtest/cmsdk-apb-watchdog-test: Parameterize tests
  tests/qtest/cmsdk-apb-watchdog-test: Don't abort on assertion failure
  tests/qtest/cmsdk-apb-watchdog-test: Test INTEN as counter enable

 hw/watchdog/cmsdk-apb-watchdog.c      |  38 ++-
 tests/qtest/cmsdk-apb-watchdog-test.c | 328 +++++++++++++++++++++++---
 tests/qtest/meson.build               |   3 +-
 3 files changed, 329 insertions(+), 40 deletions(-)

-- 
2.47.0.338.g60cca15819-goog

On Fri, 15 Nov 2024 at 16:03, Roque Arcudia Hernandez <roqueh@google.com> wrote:
>
> The following patchset tries to address an issue where the watchdog
> counter was running as soon as the device is out of reset. This
> created a few problems with the firmware under test. It was pointed
> out that the firmware under test was already working on an emulator
> using the real RTL. Further review showed that the WDOGCONTROL.INTEN
> is also expected to be the counter enable as it is defined in this
> page from ARM:
>
> https://developer.arm.com/documentation/ddi0479/d/apb-components/apb-watchdog/programmers-model?lang=en
>
>      |       | Enable the interrupt event, WDOGINT. Set to 1 to
>      |       | enable the counter and the interrupt, or to 0 to
>  [0] | INTEN | disable the counter and interrupt. Reloads the
>      |       | counter from the value in WDOGLOAD when the interrupt
>      |       | is enabled, after previously being disabled.
>
> The second half of the patch is focused on adding tests for this
> behavior. It was required to modify the test to be able to run some
> test cases in the MPS2 since the the stellaris machine had 2 issues:
>
>   - The stellaris machine did not run the counter out of reset
>     because of the way it is reset. The function
>     ssys_calculate_system_clock is not called until the reset phase
>     stellaris_sys_reset_hold which is after the watchdog executes its
>     reset, so the watchdog would not start the counter due to the
>     clock not being set up at the time of reset.
>
>   - The Stellaris machine uses a derived watchdog and not the
>     original CMSDK one which has a special behavior where it cannot
>     be stopped.
>
> The MPS2 machine on the other hand uses the standard CMSDK watchdog
> and could expose the problems we were dealing with.
>
> Roque Arcudia Hernandez (5):
>   hw/watchdog/cmsdk_apb_watchdog: Fix broken link
>   hw/watchdog/cmsdk_apb_watchdog: Fix INTEN issues
>   tests/qtest/cmsdk-apb-watchdog-test: Parameterize tests
>   tests/qtest/cmsdk-apb-watchdog-test: Don't abort on assertion failure
>   tests/qtest/cmsdk-apb-watchdog-test: Test INTEN as counter enable

Applied to target-arm.next for 9.2, thanks.

-- PMM

Hello,

I forgot to add the -v 2 to this patch series. This is the second
version after Peter's feedback. Shall I resend it again or is it fine
like this?

Thanks

Roque

On Fri, Nov 15, 2024 at 8:03 AM Roque Arcudia Hernandez
<roqueh@google.com> wrote:
>
> The following patchset tries to address an issue where the watchdog
> counter was running as soon as the device is out of reset. This
> created a few problems with the firmware under test. It was pointed
> out that the firmware under test was already working on an emulator
> using the real RTL. Further review showed that the WDOGCONTROL.INTEN
> is also expected to be the counter enable as it is defined in this
> page from ARM:
>
> https://developer.arm.com/documentation/ddi0479/d/apb-components/apb-watchdog/programmers-model?lang=en
>
>      |       | Enable the interrupt event, WDOGINT. Set to 1 to
>      |       | enable the counter and the interrupt, or to 0 to
>  [0] | INTEN | disable the counter and interrupt. Reloads the
>      |       | counter from the value in WDOGLOAD when the interrupt
>      |       | is enabled, after previously being disabled.
>
> The second half of the patch is focused on adding tests for this
> behavior. It was required to modify the test to be able to run some
> test cases in the MPS2 since the the stellaris machine had 2 issues:
>
>   - The stellaris machine did not run the counter out of reset
>     because of the way it is reset. The function
>     ssys_calculate_system_clock is not called until the reset phase
>     stellaris_sys_reset_hold which is after the watchdog executes its
>     reset, so the watchdog would not start the counter due to the
>     clock not being set up at the time of reset.
>
>   - The Stellaris machine uses a derived watchdog and not the
>     original CMSDK one which has a special behavior where it cannot
>     be stopped.
>
> The MPS2 machine on the other hand uses the standard CMSDK watchdog
> and could expose the problems we were dealing with.
>
> Roque Arcudia Hernandez (5):
>   hw/watchdog/cmsdk_apb_watchdog: Fix broken link
>   hw/watchdog/cmsdk_apb_watchdog: Fix INTEN issues
>   tests/qtest/cmsdk-apb-watchdog-test: Parameterize tests
>   tests/qtest/cmsdk-apb-watchdog-test: Don't abort on assertion failure
>   tests/qtest/cmsdk-apb-watchdog-test: Test INTEN as counter enable
>
>  hw/watchdog/cmsdk-apb-watchdog.c      |  38 ++-
>  tests/qtest/cmsdk-apb-watchdog-test.c | 328 +++++++++++++++++++++++---
>  tests/qtest/meson.build               |   3 +-
>  3 files changed, 329 insertions(+), 40 deletions(-)
>
> --
> 2.47.0.338.g60cca15819-goog
>

On Fri, 15 Nov 2024 at 17:21, Roque Arcudia Hernandez <roqueh@google.com> wrote:
>
> Hello,
>
> I forgot to add the -v 2 to this patch series. This is the second
> version after Peter's feedback. Shall I resend it again or is it fine
> like this?

It's fine like this, no need to resend just to add the v2 tag.

-- PMM