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