On 1/21/21 10:59 PM, Philippe Mathieu-Daudé wrote: > On 1/21/21 8:06 PM, Peter Maydell wrote: >> Create and connect the Clock input for the watchdog device on the >> Stellaris boards. Because the Stellaris boards model the ability to >> change the clock rate by programming PLL registers, we have to create >> an output Clock on the ssys_state device and wire it up to the >> watchdog. >> >> Note that the old comment on ssys_calculate_system_clock() got the >> units wrong -- system_clock_scale is in nanoseconds, not >> milliseconds. Improve the commentary to clarify how we are >> calculating the period. >> >> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org> >> --- >> hw/arm/stellaris.c | 43 +++++++++++++++++++++++++++++++------------ >> 1 file changed, 31 insertions(+), 12 deletions(-) > ... > >> /* >> - * Caculate the sys. clock period in ms. >> + * Calculate the system clock period. We only want to propagate >> + * this change to the rest of the system if we're not being called >> + * from migration post-load. > > This part was not trivial to understand. I read the Clock API > doc again then found: > > Care should be taken not to use ``clock_update[_ns|_hz]()`` or > ``clock_propagate()`` during the whole migration procedure because it > will trigger side effects to other devices in an unknown state. > >> */ >> -static void ssys_calculate_system_clock(ssys_state *s) >> +static void ssys_calculate_system_clock(ssys_state *s, bool propagate_clock) >> { >> + /* >> + * SYSDIV field specifies divisor: 0 == /1, 1 == /2, etc. Input >> + * clock is 200MHz, which is a period of 5 ns. Dividing the clock >> + * frequency by X is the same as multiplying the period by X. >> + */ >> if (ssys_use_rcc2(s)) { >> system_clock_scale = 5 * (((s->rcc2 >> 23) & 0x3f) + 1); >> } else { >> system_clock_scale = 5 * (((s->rcc >> 23) & 0xf) + 1); >> } >> + clock_set_ns(s->sysclk, system_clock_scale); >> + if (propagate_clock) { >> + clock_propagate(s->sysclk); >> + } >> } > ... >> static void stellaris_sys_reset_exit(Object *obj) >> @@ -690,7 +704,7 @@ static int stellaris_sys_post_load(void *opaque, int >> version_id) >> { >> ssys_state *s = opaque; >> >> - ssys_calculate_system_clock(s); >> + ssys_calculate_system_clock(s, false); > > So this makes sense. > > I'll keep reviewing and come back to this patch later.
Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org>