[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-arm] [PATCH v2] hw/ptimer: Don't wrap around counter for expir

From: Dmitry Osipenko
Subject: Re: [Qemu-arm] [PATCH v2] hw/ptimer: Don't wrap around counter for expired timer that uses tick handler
Date: Thu, 7 Jul 2016 15:20:21 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1

On 07.07.2016 13:53, Peter Maydell wrote:
> On 4 July 2016 at 10:55, Peter Maydell <address@hidden> wrote:
>> On 1 July 2016 at 18:49, Dmitry Osipenko <address@hidden> wrote:
>>> On 01.07.2016 19:36, Peter Maydell wrote:
>>>> On 30 June 2016 at 20:01, Dmitry Osipenko <address@hidden> wrote:
>>>>> On 30.06.2016 18:02, Peter Maydell wrote:
>>>>>> What I meant was: ptimer_get_count() is typically called to generate
>>>>>> a value to return from a register. That's a separate thing, conceptually,
>>>>>> from whether the device happens to also trigger an interrupt on timer
>>>>>> expiry by passing a bh to ptimer_init(). So it's very odd for a detail
>>>>>> of interrupt-on-timer-expiry (that there is a bottom half) to affect
>>>>>> the value returned when you read the timer count register.
>>>>> In order to handle wraparound correctly, software needs to track the 
>>>>> moment of
>>>>> the wraparound - the interrupt. If software reads wrapped around counter 
>>>>> value
>>>>> before IRQ triggered (ptimer expired), then it would assume that no 
>>>>> wraparound
>>>>> happened and won't perform counter value correction, resulting in periodic
>>>>> counter "jumping" backwards.
>>>> That just says you need particular behaviour between counter reads
>>>> and IRQ triggers; it doesn't say that you need the behaviour to be
>>>> different if the ptimer code doesn't know about the IRQ trigger.
>>> Okay, I already explained the reason for having two different behaviours - 
>>> to
>>> make polled counter value more distributed when possible. If I understand 
>>> you
>>> correctly, you don't like it because it is "odd" and I agree that it's a 
>>> bit clumsy.
>>> So, what we are going to do now? Would you just revert the offending commit 
>>> or
>>> you have some other suggestions?
>> Well, we need to fix the regression, but basically I'm kind of
>> confused at the moment. I haven't invested a lot of time in
>> trying to understand the timer code, so all I can really do
>> is say "this does not look like the right thing" and ask you
>> to come up with a different fix for it.

I'm currently leaning to the revert solution. Unfortunately, I haven't had a
chance to look at it yet, will do it today and send patch after.

> My current best guess is that this condition should simply be
> "if (expired) {".

Since you are insisted that wraparound isn't a needed feature - yes, that's
nearly the same what the original code did :)


reply via email to

[Prev in Thread] Current Thread [Next in Thread]