[PATCH-for-5.2 0/3] hw/clock: Only propagate clock changes if the clock is changed

[PATCH-for-5.2 0/3] hw/clock: Only propagate clock changes if the clock is changed

[Philippe Mathieu-Daudé]

Trivial patches for the clock API:
- remove unused code
- only propagate clock update when a clock change

Philippe Mathieu-Daudé (3):
  hw/clock: Remove unused clock_init*() functions
  hw/clock: Let clock_set() return boolean value
  hw/clock: Only propagate clock changes if the clock is changed

 include/hw/clock.h | 30 ++++++++++--------------------
 hw/core/clock.c    |  7 ++++++-
 2 files changed, 16 insertions(+), 21 deletions(-)

-- 
2.21.3

[PATCH-for-5.2 1/3] hw/clock: Remove unused clock_init*() functions

[Philippe Mathieu-Daudé]

clock_init*() inlined funtions are simple wrappers around
clock_set*() and are not used. Remove them in favor of clock_set*().

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/clock.h | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/include/hw/clock.h b/include/hw/clock.h
index f822a94220..468fed0996 100644
--- a/include/hw/clock.h
+++ b/include/hw/clock.h
@@ -209,17 +209,4 @@ static inline bool clock_is_enabled(const Clock *clk)
     return clock_get(clk) != 0;
 }
 
-static inline void clock_init(Clock *clk, uint64_t value)
-{
-    clock_set(clk, value);
-}
-static inline void clock_init_hz(Clock *clk, uint64_t value)
-{
-    clock_set_hz(clk, value);
-}
-static inline void clock_init_ns(Clock *clk, uint64_t value)
-{
-    clock_set_ns(clk, value);
-}
-
 #endif /* QEMU_HW_CLOCK_H */
-- 
2.21.3

Re: [PATCH-for-5.2 1/3] hw/clock: Remove unused clock_init*() functions

[Richard Henderson]

On 8/6/20 5:38 AM, Philippe Mathieu-Daudé wrote:
> clock_init*() inlined funtions are simple wrappers around
> clock_set*() and are not used. Remove them in favor of clock_set*().
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  include/hw/clock.h | 13 -------------
>  1 file changed, 13 deletions(-)

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


r~

[PATCH-for-5.2 2/3] hw/clock: Let clock_set() return boolean value

[Philippe Mathieu-Daudé]

Let clock_set() return a boolean value whether the clock
has been updated or not.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/clock.h | 12 +++++++-----
 hw/core/clock.c    |  7 ++++++-
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/include/hw/clock.h b/include/hw/clock.h
index 468fed0996..d85af45c96 100644
--- a/include/hw/clock.h
+++ b/include/hw/clock.h
@@ -127,17 +127,19 @@ void clock_set_source(Clock *clk, Clock *src);
  * @value: the clock's value, 0 means unclocked
  *
  * Set the local cached period value of @clk to @value.
+ *
+ * @return: true if the clock is changed.
  */
-void clock_set(Clock *clk, uint64_t value);
+bool clock_set(Clock *clk, uint64_t value);
 
-static inline void clock_set_hz(Clock *clk, unsigned hz)
+static inline bool clock_set_hz(Clock *clk, unsigned hz)
 {
-    clock_set(clk, CLOCK_PERIOD_FROM_HZ(hz));
+    return clock_set(clk, CLOCK_PERIOD_FROM_HZ(hz));
 }
 
-static inline void clock_set_ns(Clock *clk, unsigned ns)
+static inline bool clock_set_ns(Clock *clk, unsigned ns)
 {
-    clock_set(clk, CLOCK_PERIOD_FROM_NS(ns));
+    return clock_set(clk, CLOCK_PERIOD_FROM_NS(ns));
 }
 
 /**
diff --git a/hw/core/clock.c b/hw/core/clock.c
index 3c0daf7d4c..7066282f7b 100644
--- a/hw/core/clock.c
+++ b/hw/core/clock.c
@@ -34,11 +34,16 @@ void clock_clear_callback(Clock *clk)
     clock_set_callback(clk, NULL, NULL);
 }
 
-void clock_set(Clock *clk, uint64_t period)
+bool clock_set(Clock *clk, uint64_t period)
 {
+    if (clk->period == period) {
+        return false;
+    }
     trace_clock_set(CLOCK_PATH(clk), CLOCK_PERIOD_TO_NS(clk->period),
                     CLOCK_PERIOD_TO_NS(period));
     clk->period = period;
+
+    return true;
 }
 
 static void clock_propagate_period(Clock *clk, bool call_callbacks)
-- 
2.21.3

Re: [PATCH-for-5.2 2/3] hw/clock: Let clock_set() return boolean value

[Richard Henderson]

On 8/6/20 5:38 AM, Philippe Mathieu-Daudé wrote:
> Let clock_set() return a boolean value whether the clock
> has been updated or not.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  include/hw/clock.h | 12 +++++++-----
>  hw/core/clock.c    |  7 ++++++-
>  2 files changed, 13 insertions(+), 6 deletions(-)

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


r~

[PATCH-for-5.2 3/3] hw/clock: Only propagate clock changes if the clock is changed

[Philippe Mathieu-Daudé]

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

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/clock.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/hw/clock.h b/include/hw/clock.h
index d85af45c96..9ecd78b2c3 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);
+    }
 }
 
 static inline void clock_update_hz(Clock *clk, unsigned hz)
-- 
2.21.3

Re: [PATCH-for-5.2 3/3] hw/clock: Only propagate clock changes if the clock is changed

[Richard Henderson]

On 8/6/20 5:38 AM, Philippe Mathieu-Daudé wrote:
> Avoid propagating the clock change when the clock does not change.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  include/hw/clock.h | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

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


r~

Re: [PATCH-for-5.2 0/3] hw/clock: Only propagate clock changes if the clock is changed

[Peter Maydell]

On Thu, 6 Aug 2020 at 13:39, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Trivial patches for the clock API:
> - remove unused code
> - only propagate clock update when a clock change
>
> Philippe Mathieu-Daudé (3):
>   hw/clock: Remove unused clock_init*() functions
>   hw/clock: Let clock_set() return boolean value
>   hw/clock: Only propagate clock changes if the clock is changed
>
>  include/hw/clock.h | 30 ++++++++++--------------------
>  hw/core/clock.c    |  7 ++++++-
>  2 files changed, 16 insertions(+), 21 deletions(-)



Applied to target-arm.next, thanks.

-- PMM