1. Patchew
  2. QEMU
  3. [PATCH 0/6] reset: Add RESET_TYPE_SNAPSHOT_LOAD
  4. View patch

[PATCH 1/6] hw/misc: Don't special case RESET_TYPE_COLD in npcm7xx_clk, gcr

Peter Maydell posted 6 patches 1 year, 10 months ago
Maintainers: Alistair Francis <alistair@alistair23.me>, Peter Maydell <peter.maydell@linaro.org>, Tyrone Ting <kfting@nuvoton.com>, Hao Wu <wuhaotsh@google.com>, Eric Auger <eric.auger@redhat.com>, Laurent Vivier <laurent@vivier.eu>, Gerd Hoffmann <kraxel@redhat.com>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Bin Meng <bin.meng@windriver.com>, Palmer Dabbelt <palmer@dabbelt.com>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Yanan Wang <wangyanan55@huawei.com>, "Daniel P. Berrangé" <berrange@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Arnaud Minier <arnaud.minier@telecom-paris.fr>, "Inès Varhol" <ines.varhol@telecom-paris.fr>, "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>, Beniamino Galvani <b.galvani@gmail.com>, Strahinja Jankovic <strahinja.p.jankovic@gmail.com>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Nicholas Piggin <npiggin@gmail.com>, Daniel Henrique Barboza <danielhb413@gmail.com>, David Gibson <david@gibson.dropbear.id.au>, Harsh Prateek Bora <harshpb@linux.ibm.com>, Francisco Iglesias <francisco.iglesias@amd.com>, Vikram Garhwal <vikram.garhwal@amd.com>, Pavel Pisa <pisa@cmp.felk.cvut.cz>, Jason Wang <jasowang@redhat.com>, Dmitry Fleytman <dmitry.fleytman@gmail.com>, Akihiko Odaki <akihiko.odaki@daynix.com>, Sriram Yagnaraman <sriram.yagnaraman@est.tech>, Huacai Chen <chenhuacai@kernel.org>, Jiaxun Yang <jiaxun.yang@flygoat.com>, "Cédric Le Goater" <clg@kaod.org>, "Frédéric Barrat" <fbarrat@linux.ibm.com>, Halil Pasic <pasic@linux.ibm.com>, Christian Borntraeger <borntraeger@linux.ibm.com>, Eric Farman <farman@linux.ibm.com>, Thomas Huth <thuth@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, David Hildenbrand <david@redhat.com>, Ilya Leoshkevich <iii@linux.ibm.com>, Titus Rwantare <titusr@google.com>, Michael Rolnik <mrolnik@gmail.com>, Brian Cain <bcain@quicinc.com>, Song Gao <gaosong@loongson.cn>, Aurelien Jarno <aurelien@aurel32.net>, Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>, Chris Wulff <crwulff@gmail.com>, Marek Vasut <marex@denx.de>, Stafford Horne <shorne@gmail.com>, Weiwei Li <liwei1518@gmail.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, Yoshinori Sato <ysato@users.sourceforge.jp>, Artyom Tarasenko <atar4qemu@gmail.com>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, Max Filippov <jcmvbkbc@gmail.com>
[PATCH 1/6] hw/misc: Don't special case RESET_TYPE_COLD in npcm7xx_clk, gcr
Posted by Peter Maydell 1 year, 10 months ago 
The npcm7xx_clk and npcm7xx_gcr device reset methods look at
the ResetType argument and only handle RESET_TYPE_COLD,
producing a warning if another reset type is passed. This
is different from how every other three-phase-reset method
we have works, and makes it difficult to add new reset types.

A better pattern is "assume that any reset type you don't know
about should be handled like RESET_TYPE_COLD"; switch these
devices to do that. Then adding a new reset type will only
need to touch those devices where its behaviour really needs
to be different from the standard cold reset.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/misc/npcm7xx_clk.c | 13 +++----------
 hw/misc/npcm7xx_gcr.c | 12 ++++--------
 2 files changed, 7 insertions(+), 18 deletions(-)

diff --git a/hw/misc/npcm7xx_clk.c b/hw/misc/npcm7xx_clk.c
index ac1622c38aa..2098c85ee01 100644
--- a/hw/misc/npcm7xx_clk.c
+++ b/hw/misc/npcm7xx_clk.c
@@ -873,20 +873,13 @@ static void npcm7xx_clk_enter_reset(Object *obj, ResetType type)
 
     QEMU_BUILD_BUG_ON(sizeof(s->regs) != sizeof(cold_reset_values));
 
-    switch (type) {
-    case RESET_TYPE_COLD:
-        memcpy(s->regs, cold_reset_values, sizeof(cold_reset_values));
-        s->ref_ns = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-        npcm7xx_clk_update_all_clocks(s);
-        return;
-    }
-
+    memcpy(s->regs, cold_reset_values, sizeof(cold_reset_values));
+    s->ref_ns = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+    npcm7xx_clk_update_all_clocks(s);
     /*
      * A small number of registers need to be reset on a core domain reset,
      * but no such reset type exists yet.
      */
-    qemu_log_mask(LOG_UNIMP, "%s: reset type %d not implemented.",
-                  __func__, type);
 }
 
 static void npcm7xx_clk_init_clock_hierarchy(NPCM7xxCLKState *s)
diff --git a/hw/misc/npcm7xx_gcr.c b/hw/misc/npcm7xx_gcr.c
index 9252f9d1488..c4c4e246d7e 100644
--- a/hw/misc/npcm7xx_gcr.c
+++ b/hw/misc/npcm7xx_gcr.c
@@ -159,14 +159,10 @@ static void npcm7xx_gcr_enter_reset(Object *obj, ResetType type)
 
     QEMU_BUILD_BUG_ON(sizeof(s->regs) != sizeof(cold_reset_values));
 
-    switch (type) {
-    case RESET_TYPE_COLD:
-        memcpy(s->regs, cold_reset_values, sizeof(s->regs));
-        s->regs[NPCM7XX_GCR_PWRON] = s->reset_pwron;
-        s->regs[NPCM7XX_GCR_MDLR] = s->reset_mdlr;
-        s->regs[NPCM7XX_GCR_INTCR3] = s->reset_intcr3;
-        break;
-    }
+    memcpy(s->regs, cold_reset_values, sizeof(s->regs));
+    s->regs[NPCM7XX_GCR_PWRON] = s->reset_pwron;
+    s->regs[NPCM7XX_GCR_MDLR] = s->reset_mdlr;
+    s->regs[NPCM7XX_GCR_INTCR3] = s->reset_intcr3;
 }
 
 static void npcm7xx_gcr_realize(DeviceState *dev, Error **errp)
-- 
2.34.1
Re: [PATCH 1/6] hw/misc: Don't special case RESET_TYPE_COLD in npcm7xx_clk, gcr
Posted by Luc Michel 1 year, 9 months ago 
On 17:08 Fri 12 Apr     , Peter Maydell wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> The npcm7xx_clk and npcm7xx_gcr device reset methods look at
> the ResetType argument and only handle RESET_TYPE_COLD,
> producing a warning if another reset type is passed. This
> is different from how every other three-phase-reset method
> we have works, and makes it difficult to add new reset types.
> 
> A better pattern is "assume that any reset type you don't know
> about should be handled like RESET_TYPE_COLD"; switch these
> devices to do that. Then adding a new reset type will only
> need to touch those devices where its behaviour really needs
> to be different from the standard cold reset.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Luc Michel <luc.michel@amd.com>

> ---
>  hw/misc/npcm7xx_clk.c | 13 +++----------
>  hw/misc/npcm7xx_gcr.c | 12 ++++--------
>  2 files changed, 7 insertions(+), 18 deletions(-)
> 
> diff --git a/hw/misc/npcm7xx_clk.c b/hw/misc/npcm7xx_clk.c
> index ac1622c38aa..2098c85ee01 100644
> --- a/hw/misc/npcm7xx_clk.c
> +++ b/hw/misc/npcm7xx_clk.c
> @@ -873,20 +873,13 @@ static void npcm7xx_clk_enter_reset(Object *obj, ResetType type)
> 
>      QEMU_BUILD_BUG_ON(sizeof(s->regs) != sizeof(cold_reset_values));
> 
> -    switch (type) {
> -    case RESET_TYPE_COLD:
> -        memcpy(s->regs, cold_reset_values, sizeof(cold_reset_values));
> -        s->ref_ns = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> -        npcm7xx_clk_update_all_clocks(s);
> -        return;
> -    }
> -
> +    memcpy(s->regs, cold_reset_values, sizeof(cold_reset_values));
> +    s->ref_ns = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> +    npcm7xx_clk_update_all_clocks(s);
>      /*
>       * A small number of registers need to be reset on a core domain reset,
>       * but no such reset type exists yet.
>       */
> -    qemu_log_mask(LOG_UNIMP, "%s: reset type %d not implemented.",
> -                  __func__, type);
>  }
> 
>  static void npcm7xx_clk_init_clock_hierarchy(NPCM7xxCLKState *s)
> diff --git a/hw/misc/npcm7xx_gcr.c b/hw/misc/npcm7xx_gcr.c
> index 9252f9d1488..c4c4e246d7e 100644
> --- a/hw/misc/npcm7xx_gcr.c
> +++ b/hw/misc/npcm7xx_gcr.c
> @@ -159,14 +159,10 @@ static void npcm7xx_gcr_enter_reset(Object *obj, ResetType type)
> 
>      QEMU_BUILD_BUG_ON(sizeof(s->regs) != sizeof(cold_reset_values));
> 
> -    switch (type) {
> -    case RESET_TYPE_COLD:
> -        memcpy(s->regs, cold_reset_values, sizeof(s->regs));
> -        s->regs[NPCM7XX_GCR_PWRON] = s->reset_pwron;
> -        s->regs[NPCM7XX_GCR_MDLR] = s->reset_mdlr;
> -        s->regs[NPCM7XX_GCR_INTCR3] = s->reset_intcr3;
> -        break;
> -    }
> +    memcpy(s->regs, cold_reset_values, sizeof(s->regs));
> +    s->regs[NPCM7XX_GCR_PWRON] = s->reset_pwron;
> +    s->regs[NPCM7XX_GCR_MDLR] = s->reset_mdlr;
> +    s->regs[NPCM7XX_GCR_INTCR3] = s->reset_intcr3;
>  }
> 
>  static void npcm7xx_gcr_realize(DeviceState *dev, Error **errp)
> --
> 2.34.1
> 
> 

--
Re: [PATCH 1/6] hw/misc: Don't special case RESET_TYPE_COLD in npcm7xx_clk, gcr
Posted by Philippe Mathieu-Daudé 1 year, 10 months ago 
On 12/4/24 18:08, Peter Maydell wrote:
> The npcm7xx_clk and npcm7xx_gcr device reset methods look at
> the ResetType argument and only handle RESET_TYPE_COLD,
> producing a warning if another reset type is passed. This
> is different from how every other three-phase-reset method
> we have works, and makes it difficult to add new reset types.
> 
> A better pattern is "assume that any reset type you don't know
> about should be handled like RESET_TYPE_COLD"; switch these
> devices to do that. Then adding a new reset type will only
> need to touch those devices where its behaviour really needs
> to be different from the standard cold reset.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/misc/npcm7xx_clk.c | 13 +++----------
>   hw/misc/npcm7xx_gcr.c | 12 ++++--------
>   2 files changed, 7 insertions(+), 18 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Re: [PATCH 1/6] hw/misc: Don't special case RESET_TYPE_COLD in npcm7xx_clk, gcr
Posted by Richard Henderson 1 year, 10 months ago 
On 4/12/24 09:08, Peter Maydell wrote:
> The npcm7xx_clk and npcm7xx_gcr device reset methods look at
> the ResetType argument and only handle RESET_TYPE_COLD,
> producing a warning if another reset type is passed. This
> is different from how every other three-phase-reset method
> we have works, and makes it difficult to add new reset types.
> 
> A better pattern is "assume that any reset type you don't know
> about should be handled like RESET_TYPE_COLD"; switch these
> devices to do that. Then adding a new reset type will only
> need to touch those devices where its behaviour really needs
> to be different from the standard cold reset.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   hw/misc/npcm7xx_clk.c | 13 +++----------
>   hw/misc/npcm7xx_gcr.c | 12 ++++--------
>   2 files changed, 7 insertions(+), 18 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.