Re: [PATCH] Initialize Zynq7000 UART clocks on reset

[Michael Peter]

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.hedde@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.peter@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.peter@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