Re: [PATCH 2/2] hw/timer/armv7m_systick: Rewrite to use ptimers

[Philippe Mathieu-Daudé]

On 10/15/20 5:31 PM, Peter Maydell wrote:
> On Thu, 15 Oct 2020 at 16:18, Peter Maydell <peter.maydell@linaro.org> wrote:
> > The armv7m systick timer is a 24-bit decrementing, wrap-on-zero,
> > clear-on-write counter. Our current implementation has various
> > bugs and dubious workarounds in it (for instance see
> > https://bugs.launchpad.net/qemu/+bug/1872237).

Nice cleanup :)

Nitpicking we should replace the 24-bit 0xffffff constant by
a definition.

> ...and 10 minutes after sending this I realized I'd forgotten
> to finish removing the no-longer-needed state struct fields.
> This should be squashed in:
>
> diff --git a/include/hw/timer/armv7m_systick.h
> b/include/hw/timer/armv7m_systick.h
> index 84496faaf96..d57e299fd89 100644
> --- a/include/hw/timer/armv7m_systick.h
> +++ b/include/hw/timer/armv7m_systick.h
> @@ -26,8 +26,6 @@ struct SysTickState {
>       /*< public >*/
>
>       uint32_t control;
> -    uint32_t reload;
> -    int64_t tick;
>       ptimer_state *ptimer;
>       MemoryRegion iomem;
>       qemu_irq irq;
> diff --git a/hw/timer/armv7m_systick.c b/hw/timer/armv7m_systick.c
> index 2f192011eb0..5ac3a8a7e81 100644
> --- a/hw/timer/armv7m_systick.c
> +++ b/hw/timer/armv7m_systick.c
> @@ -219,7 +219,6 @@ static const VMStateDescription vmstate_systick = {
>       .minimum_version_id = 2,
>       .fields = (VMStateField[]) {
>           VMSTATE_UINT32(control, SysTickState),
> -        VMSTATE_INT64(tick, SysTickState),
>           VMSTATE_PTIMER(ptimer, SysTickState),
>           VMSTATE_END_OF_LIST()
>       }

Patch + hunk:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>