diff mbox series

[RFC,PATCH-for-6.1,9/9] hw/core/machine: Reset machine clocks using qemu_register_reset()

Message ID 20210409062401.2350436-10-f4bug@amsat.org
State New
Headers show
Series [RFC,PATCH-for-6.1,1/9] hw/core/clock: Increase clock propagation trace events verbosity | expand

Commit Message

Philippe Mathieu-Daudé April 9, 2021, 6:24 a.m. UTC 
While the documentation mentions:

  Note that if you are creating a clock with a fixed period which
  will never change (for example the main clock source of a board),
  then you'll have nothing else to do. This value will be propagated
  to other clocks when connecting the clocks together and devices
  will fetch the right value during the first reset.

the clocks created in machine_init() aren't propagating their value
because they are never reset (not part of the reset tree, such
TYPE_DEVICE).

Register a generic reset handler to have them properly reset.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/core/machine.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Peter Maydell April 19, 2021, 2:27 p.m. UTC | #1 
On Fri, 9 Apr 2021 at 07:24, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> While the documentation mentions:
>
>   Note that if you are creating a clock with a fixed period which
>   will never change (for example the main clock source of a board),
>   then you'll have nothing else to do. This value will be propagated
>   to other clocks when connecting the clocks together and devices
>   will fetch the right value during the first reset.
>
> the clocks created in machine_init() aren't propagating their value
> because they are never reset (not part of the reset tree, such
> TYPE_DEVICE).
>
> Register a generic reset handler to have them properly reset.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/core/machine.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index e8bdcd10854..2817fe6a567 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -1234,6 +1234,13 @@ void machine_run_board_init(MachineState *machine)
>      phase_advance(PHASE_MACHINE_INITIALIZED);
>  }
>
> +static void constant_clock_reset(void *opaque)
> +{
> +    Clock *clk = opaque;
> +
> +    clock_propagate(clk);
> +}
> +
>  Clock *machine_create_constant_clock(MachineState *machine,
>                                       const char *name, unsigned freq_hz)
>  {
> @@ -1241,6 +1248,7 @@ Clock *machine_create_constant_clock(MachineState *machine,
>
>      clk = clock_new(OBJECT(machine), name);
>      clock_set_hz(clk, freq_hz);
> +    qemu_register_reset(constant_clock_reset, clk);

You mention this in the cover letter, but I agree that this
isn't really very nice. The machine's reset method ought to
reset the clocks (either explicitly or maybe some day implicitly).

thanks
-- PMM
diff mbox series

Patch

diff --git a/hw/core/machine.c b/hw/core/machine.c
index e8bdcd10854..2817fe6a567 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1234,6 +1234,13 @@  void machine_run_board_init(MachineState *machine)
     phase_advance(PHASE_MACHINE_INITIALIZED);
 }
 
+static void constant_clock_reset(void *opaque)
+{
+    Clock *clk = opaque;
+
+    clock_propagate(clk);
+}
+
 Clock *machine_create_constant_clock(MachineState *machine,
                                      const char *name, unsigned freq_hz)
 {
@@ -1241,6 +1248,7 @@  Clock *machine_create_constant_clock(MachineState *machine,
 
     clk = clock_new(OBJECT(machine), name);
     clock_set_hz(clk, freq_hz);
+    qemu_register_reset(constant_clock_reset, clk);
 
     return clk;
 }