[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC PATCH-for-6.1 0/9] hw/clock: Strengthen machine (non-qdev) cloc

From: Philippe Mathieu-Daudé
Subject: Re: [RFC PATCH-for-6.1 0/9] hw/clock: Strengthen machine (non-qdev) clock propagation
Date: Fri, 9 Apr 2021 15:12:02 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1

On 4/9/21 8:23 AM, Philippe Mathieu-Daudé wrote:
> Hi Damian, Luc, Peter.
> I've been debugging some odd issue with the clocks:
> a clock created in the machine (IOW, not a qdev clock) isn't
> always resetted, thus propagating its value.
> "not always" is the odd part. In the MPS2 board, the machine
> clock is propagated. Apparently because the peripherals are
> created directly in the machine_init() handler. When moving
> them out in a SoC QOM container, the clock isn't... I'm still
> having hard time to understand what is going on.
> Alternatively I tried to strengthen the clock API by reducing
> the clock creation in 2 cases: machine/device. This way clocks
> aren't left dangling around alone. The qdev clocks are properly
> resetted, and for the machine clocks I register a generic reset
> handler. This way is safer, but I don't think we want to keep
> adding generic reset handlers, instead we'd like to remove them.
> I'll keep debugging to understand. Meanwhile posting this series
> as RFC to get feedback on the approach and start discussing on
> this issue.

I wonder if this could be the culprit:

  commit 96250eab904261b31d9d1ac3abbdb36737635ffa
  Author: Philippe Mathieu-Daudé <f4bug@amsat.org>
  Date:   Fri Aug 28 10:02:44 2020 +0100

      hw/clock: Only propagate clock changes if the clock is changed

      Avoid propagating the clock change when the clock does not change.

  diff --git a/include/hw/clock.h b/include/hw/clock.h
  index d85af45c967..9ecd78b2c30 100644
  --- a/include/hw/clock.h
  +++ b/include/hw/clock.h
  @@ -165,8 +165,9 @@ void clock_propagate(Clock *clk);
   static inline void clock_update(Clock *clk, uint64_t value)
  -    clock_set(clk, value);
  -    clock_propagate(clk);
  +    if (clock_set(clk, value)) {
  +        clock_propagate(clk);
  +    }


- first use clock_set() to set the new period
- then call clock_update() with the same "new period"

-> the clock parent already has the new period, so the
   children are not updated.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]