This is ok but I'm afraid we may end up doing this kind of thing in a lot of devices. So maybe we should consider changing the behavior of device_is_in_reset() so that it returns false in the reset-exit case. What do you think ? (I've a patch for this, which make this one useless)
But this patch does not harm so, anyway: Reviewed-by: Damien Hedde <damien.he...@greensocs.com> On 1/7/21 9:00 PM, Peter Maydell wrote: > Alistair/Edgar/Damien -- could I get a review from one of you > for this Xilinx clock-gen related patch, please? > > thanks > -- PMM > > On Tue, 24 Nov 2020 at 18:54, Michael Peter > <michael.pe...@hensoldt-cyber.de> wrote: >> >> Pass an additional argument to zynq_slcr_compute_clocks that indicates >> whether an reset-exit condition >> applies. If called from zynq_slcr_reset_exit, external clocks are assumed to >> be active, even if the >> device state indicates a reset state. >> >> Signed-off-by: Michael Peter <michael.pe...@hensoldt-cyber.de> >> --- >> hw/misc/zynq_slcr.c | 12 ++++++------ >> 1 file changed, 6 insertions(+), 6 deletions(-) >> >> diff --git a/hw/misc/zynq_slcr.c b/hw/misc/zynq_slcr.c >> index a2b28019e3..073122b934 100644 >> --- a/hw/misc/zynq_slcr.c >> +++ b/hw/misc/zynq_slcr.c >> @@ -269,12 +269,12 @@ static uint64_t zynq_slcr_compute_clock(const uint64_t >> periods[], >> * But do not propagate them further. Connected clocks >> * will not receive any updates (See zynq_slcr_compute_clocks()) >> */ >> -static void zynq_slcr_compute_clocks(ZynqSLCRState *s) >> +static void zynq_slcr_compute_clocks(ZynqSLCRState *s, bool ignore_reset) >> { >> uint64_t ps_clk = clock_get(s->ps_clk); >> >> /* consider outputs clocks are disabled while in reset */ >> - if (device_is_in_reset(DEVICE(s))) { >> + if (!ignore_reset && device_is_in_reset(DEVICE(s))) { >> ps_clk = 0; >> } >> >> @@ -305,7 +305,7 @@ static void zynq_slcr_propagate_clocks(ZynqSLCRState *s) >> static void zynq_slcr_ps_clk_callback(void *opaque) >> { >> ZynqSLCRState *s = (ZynqSLCRState *) opaque; >> - zynq_slcr_compute_clocks(s); >> + zynq_slcr_compute_clocks(s, false); >> zynq_slcr_propagate_clocks(s); >> } >> >> @@ -410,7 +410,7 @@ static void zynq_slcr_reset_hold(Object *obj) >> ZynqSLCRState *s = ZYNQ_SLCR(obj); >> >> /* will disable all output clocks */ >> - zynq_slcr_compute_clocks(s); >> + zynq_slcr_compute_clocks(s, false); >> zynq_slcr_propagate_clocks(s); >> } >> >> @@ -419,7 +419,7 @@ static void zynq_slcr_reset_exit(Object *obj) >> ZynqSLCRState *s = ZYNQ_SLCR(obj); >> >> /* will compute output clocks according to ps_clk and registers */ >> - zynq_slcr_compute_clocks(s); >> + zynq_slcr_compute_clocks(s, true); >> zynq_slcr_propagate_clocks(s); >> } >> >> @@ -558,7 +558,7 @@ static void zynq_slcr_write(void *opaque, hwaddr offset, >> case R_ARM_PLL_CTRL: >> case R_DDR_PLL_CTRL: >> case R_UART_CLK_CTRL: >> - zynq_slcr_compute_clocks(s); >> + zynq_slcr_compute_clocks(s, false); >> zynq_slcr_propagate_clocks(s); >> break; >> } >> -- >> 2.17.1 >