On Wed, 13 Jan 2021, 11:19 Damien Hedde, <damien.he...@greensocs.com> 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)
>

Thanks Damien,

IMO, a central fix for this would be better, I agree with you.

Thanks,
Edgar



> 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
> >
>

Reply via email to