Re: [Qemu-arm] [PATCH v10 5/7] hw/ptimer: Legalize running with delta = load = 0

[Peter Crosthwaite]

On Sat, Jan 09, 2016 at 08:39:53PM +0300, Dmitry Osipenko wrote:
> Currently ptimer would print error message and clear enable flag for an
> arming timer that has delta = load = 0. That actually could be a valid case
> for some hardware, like instant IRQ trigger for oneshot timer or continuous
> in periodic mode. Support those cases by printing error message only when
> period = 0.
> 

Isn't the continuous-periodic the same as period = 0, so if we were to really
support this, there should be no error message. This would simplify as we
can remove the conditionals of 0 period completely and rely only on the
too-fast clamps you add in previous patches.

Regards,
Peter

> In addition, don't load one-shot timer when delta = 0 and actually stop the
> timer by timer_del().
> 
> Signed-off-by: Dmitry Osipenko <address@hidden>
> ---
>  hw/core/ptimer.c | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/core/ptimer.c b/hw/core/ptimer.c
> index 6960738..42e44f9 100644
> --- a/hw/core/ptimer.c
> +++ b/hw/core/ptimer.c
> @@ -36,13 +36,20 @@ static void ptimer_reload(ptimer_state *s)
>  {
>      uint32_t period_frac = s->period_frac;
>      uint64_t period = s->period;
> +    int periodic = (s->enabled == 1);
>  
> -    if (s->delta == 0) {
> +    if (s->delta == 0 && period != 0) {
>          ptimer_trigger(s);
> -        s->delta = s->limit;
> +        if (periodic) {
> +            s->delta = s->limit;
> +        }
>      }
> -    if (s->delta == 0 || s->period == 0) {
> -        fprintf(stderr, "Timer with period zero, disabling\n");
> +    if (s->delta == 0 || period == 0) {
> +        if (period == 0) {
> +            fprintf(stderr, "Timer with period zero, disabling\n");
> +            s->delta = 0;
> +        }
> +        timer_del(s->timer);
>          s->enabled = 0;
>          return;
>      }
> @@ -56,7 +63,7 @@ static void ptimer_reload(ptimer_state *s)
>       * on the current generation of host machines.
>       */
>  
> -    if ((s->enabled == 1) && !use_icount && (s->delta * period < 10000)) {
> +    if (periodic && !use_icount && (s->delta * period < 10000)) {
>          period = 10000 / s->delta;
>          period_frac = 0;
>      }
> @@ -86,14 +93,14 @@ uint64_t ptimer_get_count(ptimer_state *s)
>      int enabled = s->enabled;
>      uint64_t counter;
>  
> -    if (enabled) {
> +    if (enabled && s->delta != 0) {
>          int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>          int64_t next = s->next_event;
>          int expired = (now - next >= 0);
>          int oneshot = (enabled == 2);
>  
>          /* Figure out the current counter value.  */
> -        if (s->period == 0 || (expired && (use_icount || oneshot))) {
> +        if (expired && (use_icount || oneshot)) {
>              /* Prevent timer underflowing if it should already have
>                 triggered.  */
>              counter = 0;
> -- 
> 2.6.4
>