On 19:06 Thu 21 Jan , Peter Maydell wrote:
> Switch the CMSDK APB dualtimer device over to using its Clock input;
> the pclk-frq property is now ignored.
>
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
> ---
> hw/timer/cmsdk-apb-dualtimer.c | 42 ++++++++++++++++++++++++++++++----
> 1 file changed, 37 insertions(+), 5 deletions(-)
>
> diff --git a/hw/timer/cmsdk-apb-dualtimer.c b/hw/timer/cmsdk-apb-dualtimer.c
> index 781b496037b..828127b366f 100644
> --- a/hw/timer/cmsdk-apb-dualtimer.c
> +++ b/hw/timer/cmsdk-apb-dualtimer.c
> @@ -106,6 +106,22 @@ static void cmsdk_apb_dualtimer_update(CMSDKAPBDualTimer
> *s)
> qemu_set_irq(s->timerintc, timintc);
> }
>
> +static int cmsdk_dualtimermod_divisor(CMSDKAPBDualTimerModule *m)
> +{
> + /* Return the divisor set by the current CONTROL.PRESCALE value */
> + switch (FIELD_EX32(m->control, CONTROL, PRESCALE)) {
> + case 0:
> + return 1;
> + case 1:
> + return 16;
> + case 2:
> + case 3: /* UNDEFINED, we treat like 2 (and complained when it was set) */
> + return 256;
> + default:
> + g_assert_not_reached();
> + }
> +}
> +
> static void cmsdk_dualtimermod_write_control(CMSDKAPBDualTimerModule *m,
> uint32_t newctrl)
> {
> @@ -146,7 +162,7 @@ static void
> cmsdk_dualtimermod_write_control(CMSDKAPBDualTimerModule *m,
> default:
> g_assert_not_reached();
> }
> - ptimer_set_freq(m->timer, m->parent->pclk_frq / divisor);
> + ptimer_set_period_from_clock(m->timer, m->parent->timclk, divisor);
Just a small cosmetic note, maybe you can use your new
cmsdk_dualtimermod_divisor function to factor out the switch above?
Something like:
if (changed & R_CONTROL_PRESCALE_MASK) {
if (FIELD_EX32(newctrl, CONTROL, PRESCALE) == 3) {
qemu_log_mask(LOG_GUEST_ERROR,
"CMSDK APB dual-timer: CONTROL.PRESCALE==0b11"
" is undefined behaviour\n");
}
ptimer_set_period_from_clock(m->timer, m->parent->timclk,
cmsdk_dualtimermod_divisor(m));
}
With or without this modification:
Reviewed-by: Luc Michel <l...@lmichel.fr>
Luc
> }
>
> if (changed & R_CONTROL_MODE_MASK) {
> @@ -414,7 +430,8 @@ static void
> cmsdk_dualtimermod_reset(CMSDKAPBDualTimerModule *m)
> * limit must both be set to 0xffff, so we wrap at 16 bits.
> */
> ptimer_set_limit(m->timer, 0xffff, 1);
> - ptimer_set_freq(m->timer, m->parent->pclk_frq);
> + ptimer_set_period_from_clock(m->timer, m->parent->timclk,
> + cmsdk_dualtimermod_divisor(m));
> ptimer_transaction_commit(m->timer);
> }
>
> @@ -432,6 +449,20 @@ static void cmsdk_apb_dualtimer_reset(DeviceState *dev)
> s->timeritop = 0;
> }
>
> +static void cmsdk_apb_dualtimer_clk_update(void *opaque)
> +{
> + CMSDKAPBDualTimer *s = CMSDK_APB_DUALTIMER(opaque);
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(s->timermod); i++) {
> + CMSDKAPBDualTimerModule *m = &s->timermod[i];
> + ptimer_transaction_begin(m->timer);
> + ptimer_set_period_from_clock(m->timer, m->parent->timclk,
> + cmsdk_dualtimermod_divisor(m));
> + ptimer_transaction_commit(m->timer);
> + }
> +}
> +
> static void cmsdk_apb_dualtimer_init(Object *obj)
> {
> SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> @@ -446,7 +477,8 @@ static void cmsdk_apb_dualtimer_init(Object *obj)
> for (i = 0; i < ARRAY_SIZE(s->timermod); i++) {
> sysbus_init_irq(sbd, &s->timermod[i].timerint);
> }
> - s->timclk = qdev_init_clock_in(DEVICE(s), "TIMCLK", NULL, NULL);
> + s->timclk = qdev_init_clock_in(DEVICE(s), "TIMCLK",
> + cmsdk_apb_dualtimer_clk_update, s);
> }
>
> static void cmsdk_apb_dualtimer_realize(DeviceState *dev, Error **errp)
> @@ -454,8 +486,8 @@ static void cmsdk_apb_dualtimer_realize(DeviceState *dev,
> Error **errp)
> CMSDKAPBDualTimer *s = CMSDK_APB_DUALTIMER(dev);
> int i;
>
> - if (s->pclk_frq == 0) {
> - error_setg(errp, "CMSDK APB timer: pclk-frq property must be set");
> + if (!clock_has_source(s->timclk)) {
> + error_setg(errp, "CMSDK APB dualtimer: TIMCLK clock must be
> connected");
> return;
> }
>
> --
> 2.20.1
>
--
