Re: [Qemu-devel] [PATCH v16 07/16] hw/ptimer: Add "continuous trigger" policy

[Dmitry Osipenko]

On 20.09.2016 20:21, Peter Maydell wrote:
> On 7 September 2016 at 14:22, Dmitry Osipenko <address@hidden> wrote:
>> Currently, periodic timer that has load = delta = 0 performs trigger
>> on timer reload and stops, printing a "period zero" error message.
>> Introduce new policy that makes periodic timer to continuously trigger
>> with a period interval in case of load = 0.
>>
>> Signed-off-by: Dmitry Osipenko <address@hidden>
>> ---
>>  hw/core/ptimer.c    | 18 +++++++++++++++++-
>>  include/hw/ptimer.h |  4 ++++
>>  2 files changed, 21 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/core/ptimer.c b/hw/core/ptimer.c
>> index e9b2e15..97ce8ae 100644
>> --- a/hw/core/ptimer.c
>> +++ b/hw/core/ptimer.c
>> @@ -45,7 +45,8 @@ static void ptimer_reload(ptimer_state *s, int 
>> delta_adjust)
>>          ptimer_trigger(s);
>>          delta = s->delta = s->limit;
>>      }
>> -    if (delta == 0 || s->period == 0) {
>> +
>> +    if (s->period == 0) {
>>          if (!qtest_enabled()) {
>>              fprintf(stderr, "Timer with period zero, disabling\n");
>>          }
>> @@ -58,6 +59,21 @@ static void ptimer_reload(ptimer_state *s, int 
>> delta_adjust)
>>          delta += delta_adjust;
>>      }
>>
>> +    if (delta == 0 && (s->policy_mask & PTIMER_POLICY_CONTINUOUS_TRIGGER)) {
>> +        if (s->enabled == 1) {
>> +            delta = 1;
>> +        }
>> +    }
>> +
>> +    if (delta == 0) {
>> +        if (!qtest_enabled()) {
>> +            fprintf(stderr, "Timer with delta zero, disabling\n");
>> +        }
>> +        timer_del(s->timer);
>> +        s->enabled = 0;
>> +        return;
>> +    }
>> +
> 
> Again, this looks like it's affecting behaviour even when the
> policy flag isn't set.
> 

No, the old behaviour is "to error out" when the delta = 0 and it's still the
same with this patch applied. This patch splits the "(delta == 0 || s->period ==
0)" check, so it won't error out if delta was changed to 1 in case of the
"continuous trigger" policy being used.

-- 
Dmitry