[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-arm] [PATCH v14 1/3] hw/ptimer: Support running with counter =
From: |
Dmitry Osipenko |
Subject: |
Re: [Qemu-arm] [PATCH v14 1/3] hw/ptimer: Support running with counter = 0 by introducing new policy feature |
Date: |
Thu, 23 Jun 2016 20:05:40 +0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 |
On 23.06.2016 19:43, Peter Maydell wrote:
> On 23 June 2016 at 17:32, Dmitry Osipenko <address@hidden> wrote:
>> On 23.06.2016 16:49, Peter Maydell wrote:
>>> On 17 June 2016 at 14:17, Dmitry Osipenko <address@hidden> wrote:
>>>> Currently ptimer prints error message and stops the running timer that has
>>>> delta (counter) = 0, this is an incorrect behaviour for some of the ptimer
>>>> users. There are different variants of how particular timer could handle
>>>> that
>>>> case besides stopping the timer, like immediate or deferred IRQ trigger.
>>>> Introduce policy feature that provides ptimer with an information about the
>>>> correct behaviour.
>>>>
>>>> Implement the "counter = 0 triggers IRQ after one period" policy, as it is
>>>> known to be used by the ARM MPTimer and set "default" policy to all ptimer
>>>> users, maintaining old behaviour till they get fixed.
>>>
>>> Could you split this into:
>>> (1) a patch which just adds the new argument to ptimer_init() and
>>> updates all its callers
>>> (2) a patch which adds support for setting the policy option to
>>> something other than the default value
>>>
>>> and also make sure that we only do one change per patch -- there
>>> seem to be several different behaviour changes tangled up in
>>> one patch here.
>>>
>>> I think that will be easier to review.
>>>
>>
>> This patch isn't supposed to change behaviour for any of the current timers.
>> I
>> think it is clearly expressed in the last sentence of the commit message.
>> There
>> is one unintended behaviour change in this patch, it's my overlook [see
>> below].
>
> Right, but my point is that it is hard to tell that when I read
> the patch to review it, and splitting this will make it easier.
>
> thanks
> -- PMM
>
Ah, you are meaning to derive adding *new* "deferred trigger" policy into a
separate patch. I'll do it.
--
Dmitry
[Qemu-arm] [PATCH v14 2/3] hw/ptimer: Fix counter - 1 returned by ptimer_get_count for the active timer, Dmitry Osipenko, 2016/06/17