Damien,
On Wed, 2021-01-13 at 11:19 +0100, Damien Hedde wrote:
> 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)
I concur that a general solution would be preferable. My patch was only
meant to highlight the issue by providing an ad-hoc solution.
Thank you for picking up the topic.
Best regards,
Michael
>
> 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
