Re: [PATCH 07/11] hw/timer: Renesas 8bit timer.

[Peter Maydell]

On Thu, 27 May 2021 at 06:30, Yoshinori Sato <ysato@users.sourceforge.jp> wrote:
>
> Rewrite timer api.
>
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> ---
>  include/hw/timer/renesas_tmr.h  |  58 ----
>  include/hw/timer/renesas_tmr8.h |  67 ++++
>  hw/timer/renesas_tmr.c          | 493 -----------------------------
>  hw/timer/renesas_tmr8.c         | 540 ++++++++++++++++++++++++++++++++
>  hw/timer/Kconfig                |   5 +-
>  hw/timer/meson.build            |   2 +-
>  6 files changed, 609 insertions(+), 556 deletions(-)
>  delete mode 100644 include/hw/timer/renesas_tmr.h
>  create mode 100644 include/hw/timer/renesas_tmr8.h
>  delete mode 100644 hw/timer/renesas_tmr.c
>  create mode 100644 hw/timer/renesas_tmr8.c

> +static const VMStateDescription vmstate_rtmr = {
> +    .name = "renesas-8tmr",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_END_OF_LIST()
> +    }
> +};

This seems to be missing the fields for the actual state ?

More generally, complete rewrites of devices are a pain
to code review. Could this be structured as incremental
improvements to the existing code ?


thanks
-- PMM