[Qemu-devel] [RFC v1 0/2] ARM MPTimer fixes and refactoring

[Qemu-devel] [RFC v1 0/2] ARM MPTimer fixes and refactoring

[Peter Crosthwaite]

Hi Dmitry,

Based on my comment earlier, this is what I came up with RE consolidation of
those arm_mptimer code paths that were giving you problems. I have not done the
interrupt mask fix, as that one from your series is reasonably independent.

Regards,
Peter

Peter Crosthwaite (2):
  timer: arm_mp: Factor out timer value calculation
  timer: arm_mp: consolidate control and counter write logic

 hw/timer/arm_mptimer.c | 73 +++++++++++++++++++++++++++++---------------------
 1 file changed, 42 insertions(+), 31 deletions(-)

-- 
1.9.1

[Qemu-devel] [RFC v1 1/2] timer: arm_mp: Factor out timer value calculation

[Peter Crosthwaite]

Factor out the code that calculates the runtime value of the timer.
Updates tb->count to the calculated value. Prepares support for pausing
the timer where the timer disable event should sync the counter to its
current value.

Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
 hw/timer/arm_mptimer.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/hw/timer/arm_mptimer.c b/hw/timer/arm_mptimer.c
index 8b93b3c..04dfb63 100644
--- a/hw/timer/arm_mptimer.c
+++ b/hw/timer/arm_mptimer.c
@@ -72,25 +72,32 @@ static void timerblock_tick(void *opaque)
     timerblock_update_irq(tb);
 }
 
+static void timerblock_sync(TimerBlock *tb)
+{
+    int64_t val;
+
+    if (((tb->control & 1) == 0) || (tb->count == 0)) {
+        return;
+    }
+    /* Slow and ugly, but hopefully won't happen too often.  */
+    val = tb->tick - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+    val /= timerblock_scale(tb);
+    if (val < 0) {
+        val = 0;
+    }
+    tb->count = val;
+}
+
 static uint64_t timerblock_read(void *opaque, hwaddr addr,
                                 unsigned size)
 {
     TimerBlock *tb = (TimerBlock *)opaque;
-    int64_t val;
     switch (addr) {
     case 0: /* Load */
         return tb->load;
     case 4: /* Counter.  */
-        if (((tb->control & 1) == 0) || (tb->count == 0)) {
-            return 0;
-        }
-        /* Slow and ugly, but hopefully won't happen too often.  */
-        val = tb->tick - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-        val /= timerblock_scale(tb);
-        if (val < 0) {
-            val = 0;
-        }
-        return val;
+        timerblock_sync(tb);
+        return tb->count;
     case 8: /* Control.  */
         return tb->control;
     case 12: /* Interrupt status.  */
-- 
1.9.1

[Qemu-devel] [RFC v1 2/2] timer: arm_mp: consolidate control and counter write logic

[Peter Crosthwaite]

Writing to any of the load, counter or control registers can require a
reload of the timer. Currently load and counter share a code path, but
the control logic is separate. Consolidate them by reducing the switch
to only sync the timer state. For load/counter this just means setting
tb->count to the new value. For control, this means setting tb->count
to the current value. Then outside the switch, any old timers are
discarded, and if the timer is (still is, or has just become) enabled,
setup a new one.

A fast escape path is added to control writes. If it is detected that
the timer was, and still is running without change of prescaler, don't
do the restart. This avoid an un-needed restart that could potentially
cause timer rounding errors.

For further consolidation, move the auto-load refresh logic to
timerblock_reload.

This change fixes two bugs and implements a missing feature.

Previously a running timer could not be stopped, the commonifying of
the timer_del call now means clearing the enable bit in the control
register now correctly stops a running timer.

There was a bug where if starting a timer in periodic mode after
one-shot expiration, the timer would not restart. This was because of
a bad conditional for the old do-nothing fast path.

This implements pause and resumption of the timer. Clearing the enable
bit, and the setting it again later will cause the timer to pick up
where it left off. The paused value of the timer can be read by the
guest. Another use of this stop and restart feature, is it also now
models a change of prescaler for a running timer.

Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
 hw/timer/arm_mptimer.c | 44 ++++++++++++++++++++++++--------------------
 1 file changed, 24 insertions(+), 20 deletions(-)

diff --git a/hw/timer/arm_mptimer.c b/hw/timer/arm_mptimer.c
index 04dfb63..69899cf 100644
--- a/hw/timer/arm_mptimer.c
+++ b/hw/timer/arm_mptimer.c
@@ -50,7 +50,11 @@ static inline uint32_t timerblock_scale(TimerBlock *tb)
 static void timerblock_reload(TimerBlock *tb, int restart)
 {
     if (tb->count == 0) {
-        return;
+        if (tb->control & 2) {
+            tb->count = tb->load;
+        } else {
+            return;
+        }
     }
     if (restart) {
         tb->tick = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
@@ -63,12 +67,8 @@ static void timerblock_tick(void *opaque)
 {
     TimerBlock *tb = (TimerBlock *)opaque;
     tb->status = 1;
-    if (tb->control & 2) {
-        tb->count = tb->load;
-        timerblock_reload(tb, 0);
-    } else {
-        tb->count = 0;
-    }
+    tb->count = 0;
+    timerblock_reload(tb, 0);
     timerblock_update_irq(tb);
 }
 
@@ -113,33 +113,37 @@ static void timerblock_write(void *opaque, hwaddr addr,
     TimerBlock *tb = (TimerBlock *)opaque;
     int64_t old;
     switch (addr) {
+    /* Breaking from this switch implies that timer needs to be refreshed.
+     * Operations that do not affect the running timer must return directly
+     * to avoid a spurious reload of the timer.
+     */
     case 0: /* Load */
         tb->load = value;
         /* Fall through.  */
     case 4: /* Counter.  */
-        if ((tb->control & 1) && tb->count) {
-            /* Cancel the previous timer.  */
-            timer_del(tb->timer);
-        }
         tb->count = value;
-        if (tb->control & 1) {
-            timerblock_reload(tb, 1);
-        }
         break;
     case 8: /* Control.  */
         old = tb->control;
         tb->control = value;
-        if (((old & 1) == 0) && (value & 1)) {
-            if (tb->count == 0 && (tb->control & 2)) {
-                tb->count = tb->load;
-            }
-            timerblock_reload(tb, 1);
+        if ((value & 1) && (old & 1) && tb->count != 0 &&
+            !extract64(value ^ old, 8, 8)) {
+            /* Timer was running, and still is, without prescale change */
+            return;
         }
+        timerblock_sync(tb);
         break;
     case 12: /* Interrupt status.  */
         tb->status &= ~value;
         timerblock_update_irq(tb);
-        break;
+        return;
+    }
+
+    /* Cancel any previous timer.  */
+    timer_del(tb->timer);
+
+    if (tb->control & 1) {
+        timerblock_reload(tb, 1);
     }
 }
 
-- 
1.9.1

Re: [Qemu-devel] [RFC v1 0/2] ARM MPTimer fixes and refactoring

[Dmitry Osipenko]

05.07.2015 23:26, Peter Crosthwaite пишет:
> Hi Dmitry,
>
> Based on my comment earlier, this is what I came up with RE consolidation of
> those arm_mptimer code paths that were giving you problems. I have not done the
> interrupt mask fix, as that one from your series is reasonably independent.
>
> Regards,
> Peter
>
> Peter Crosthwaite (2):
>    timer: arm_mp: Factor out timer value calculation
>    timer: arm_mp: consolidate control and counter write logic
>
>   hw/timer/arm_mptimer.c | 73 +++++++++++++++++++++++++++++---------------------
>   1 file changed, 42 insertions(+), 31 deletions(-)
>

Hi Peter, thanks a lot! Generally, I don't have any trouble with currently 
missed functionality, just noticed it while was hacking my NVIDIA Tegra2 
emulation pet-project and decided to contribute =).

It looks like you are trying to duplicate what generic ptimer is already doing, 
isn't it?

-- 
Dmitry

Re: [Qemu-devel] [RFC v1 0/2] ARM MPTimer fixes and refactoring

[Peter Crosthwaite]

On Sun, Jul 5, 2015 at 1:47 PM, Dmitry Osipenko <digetx@gmail.com> wrote:
> 05.07.2015 23:26, Peter Crosthwaite пишет:
>
>> Hi Dmitry,
>>
>> Based on my comment earlier, this is what I came up with RE consolidation
>> of
>> those arm_mptimer code paths that were giving you problems. I have not
>> done the
>> interrupt mask fix, as that one from your series is reasonably
>> independent.
>>
>> Regards,
>> Peter
>>
>> Peter Crosthwaite (2):
>>    timer: arm_mp: Factor out timer value calculation
>>    timer: arm_mp: consolidate control and counter write logic
>>
>>   hw/timer/arm_mptimer.c | 73
>> +++++++++++++++++++++++++++++---------------------
>>   1 file changed, 42 insertions(+), 31 deletions(-)
>>
>
> Hi Peter, thanks a lot! Generally, I don't have any trouble with currently
> missed functionality, just noticed it while was hacking my NVIDIA Tegra2
> emulation pet-project and decided to contribute =).
>
> It looks like you are trying to duplicate what generic ptimer is already
> doing, isn't it?
>

Yes, ptimer was probably the correct way to do this in the first
place. Some of the new code structures introduced in this patch series
are directly applicable though for that conversion effort.

Regards,
Peter

> --
> Dmitry
>

Re: [Qemu-devel] [RFC v1 0/2] ARM MPTimer fixes and refactoring

[Peter Crosthwaite]

On Sun, Jul 5, 2015 at 1:58 PM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Sun, Jul 5, 2015 at 1:47 PM, Dmitry Osipenko <digetx@gmail.com> wrote:
>> 05.07.2015 23:26, Peter Crosthwaite пишет:
>>
>>> Hi Dmitry,
>>>
>>> Based on my comment earlier, this is what I came up with RE consolidation
>>> of
>>> those arm_mptimer code paths that were giving you problems. I have not
>>> done the
>>> interrupt mask fix, as that one from your series is reasonably
>>> independent.
>>>
>>> Regards,
>>> Peter
>>>
>>> Peter Crosthwaite (2):
>>>    timer: arm_mp: Factor out timer value calculation
>>>    timer: arm_mp: consolidate control and counter write logic
>>>
>>>   hw/timer/arm_mptimer.c | 73
>>> +++++++++++++++++++++++++++++---------------------
>>>   1 file changed, 42 insertions(+), 31 deletions(-)
>>>
>>
>> Hi Peter, thanks a lot! Generally, I don't have any trouble with currently
>> missed functionality, just noticed it while was hacking my NVIDIA Tegra2
>> emulation pet-project and decided to contribute =).
>>
>> It looks like you are trying to duplicate what generic ptimer is already
>> doing, isn't it?
>>
>
> Yes, ptimer was probably the correct way to do this in the first
> place. Some of the new code structures introduced in this patch series
> are directly applicable though for that conversion effort.
>

Conversion also comes at the price of a VMSD version bump if Peter is
ok with that.

Regards,
Peter

> Regards,
> Peter
>
>> --
>> Dmitry
>>

Re: [Qemu-devel] [RFC v1 0/2] ARM MPTimer fixes and refactoring

[Peter Maydell]

On 5 July 2015 at 22:01, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
> On Sun, Jul 5, 2015 at 1:58 PM, Peter Crosthwaite
> <peter.crosthwaite@xilinx.com> wrote:
>> On Sun, Jul 5, 2015 at 1:47 PM, Dmitry Osipenko <digetx@gmail.com> wrote:
>>> Hi Peter, thanks a lot! Generally, I don't have any trouble with currently
>>> missed functionality, just noticed it while was hacking my NVIDIA Tegra2
>>> emulation pet-project and decided to contribute =).

Given that it's the eve of hardfreeze for 2.4, if these
aren't causing actual problems for you I'm wondering if maybe
we should leave master as is for 2.4 and put in a proper
rework patchset after that is released? (This is a bugfix
though so certainly in scope for 2.4 still if we want.)

>>> It looks like you are trying to duplicate what generic ptimer is already
>>> doing, isn't it?
>>>
>>
>> Yes, ptimer was probably the correct way to do this in the first
>> place. Some of the new code structures introduced in this patch series
>> are directly applicable though for that conversion effort.
>>
>
> Conversion also comes at the price of a VMSD version bump if Peter is
> ok with that.

Yes, we don't currently provide cross-version migration on
ARM so version bumps are OK.

thanks
-- PMM

Re: [Qemu-devel] [RFC v1 0/2] ARM MPTimer fixes and refactoring

[Peter Crosthwaite]

On Sun, Jul 5, 2015 at 2:06 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 5 July 2015 at 22:01, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
>> On Sun, Jul 5, 2015 at 1:58 PM, Peter Crosthwaite
>> <peter.crosthwaite@xilinx.com> wrote:
>>> On Sun, Jul 5, 2015 at 1:47 PM, Dmitry Osipenko <digetx@gmail.com> wrote:
>>>> Hi Peter, thanks a lot! Generally, I don't have any trouble with currently
>>>> missed functionality, just noticed it while was hacking my NVIDIA Tegra2
>>>> emulation pet-project and decided to contribute =).
>
> Given that it's the eve of hardfreeze for 2.4, if these
> aren't causing actual problems for you I'm wondering if maybe
> we should leave master as is for 2.4 and put in a proper
> rework patchset after that is released? (This is a bugfix
> though so certainly in scope for 2.4 still if we want.)
>

I think bugfixes should go in, which probably means apply Dmitrys
series, and i'll rebase my refactorings on. His patches are correct
and I was just exploring alternatives which is probably not
soft-freeze appropriate. Putting an RB.

Regards,
Peter

>>>> It looks like you are trying to duplicate what generic ptimer is already
>>>> doing, isn't it?
>>>>
>>>
>>> Yes, ptimer was probably the correct way to do this in the first
>>> place. Some of the new code structures introduced in this patch series
>>> are directly applicable though for that conversion effort.
>>>
>>
>> Conversion also comes at the price of a VMSD version bump if Peter is
>> ok with that.
>
> Yes, we don't currently provide cross-version migration on
> ARM so version bumps are OK.
>
> thanks
> -- PMM
>

Re: [Qemu-devel] [RFC v1 0/2] ARM MPTimer fixes and refactoring

[Dmitry Osipenko]

06.07.2015 00:06, Peter Maydell пишет:
> On 5 July 2015 at 22:01, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
>> On Sun, Jul 5, 2015 at 1:58 PM, Peter Crosthwaite
>> <peter.crosthwaite@xilinx.com> wrote:
>>> On Sun, Jul 5, 2015 at 1:47 PM, Dmitry Osipenko <digetx@gmail.com> wrote:
>>>> Hi Peter, thanks a lot! Generally, I don't have any trouble with currently
>>>> missed functionality, just noticed it while was hacking my NVIDIA Tegra2
>>>> emulation pet-project and decided to contribute =).
>
> Given that it's the eve of hardfreeze for 2.4, if these
> aren't causing actual problems for you I'm wondering if maybe
> we should leave master as is for 2.4 and put in a proper
> rework patchset after that is released? (This is a bugfix
> though so certainly in scope for 2.4 still if we want.)
>

Sure, I'm totally ok with leaving master as-is. However, my recent two patches 
looks quite simple and still fixing issues. Peter, you decide.

Potentially, shutdown issue might cause troubles with Linux kernel in SMP QEMU 
setup.

-- 
Dmitry

Re: [Qemu-devel] [RFC v1 0/2] ARM MPTimer fixes and refactoring

[Peter Crosthwaite]

On Sun, Jul 5, 2015 at 1:58 PM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Sun, Jul 5, 2015 at 1:47 PM, Dmitry Osipenko <digetx@gmail.com> wrote:
>> 05.07.2015 23:26, Peter Crosthwaite пишет:
>>
>>> Hi Dmitry,
>>>
>>> Based on my comment earlier, this is what I came up with RE consolidation
>>> of
>>> those arm_mptimer code paths that were giving you problems. I have not
>>> done the
>>> interrupt mask fix, as that one from your series is reasonably
>>> independent.
>>>
>>> Regards,
>>> Peter
>>>
>>> Peter Crosthwaite (2):
>>>    timer: arm_mp: Factor out timer value calculation
>>>    timer: arm_mp: consolidate control and counter write logic
>>>
>>>   hw/timer/arm_mptimer.c | 73
>>> +++++++++++++++++++++++++++++---------------------
>>>   1 file changed, 42 insertions(+), 31 deletions(-)
>>>
>>
>> Hi Peter, thanks a lot! Generally, I don't have any trouble with currently
>> missed functionality, just noticed it while was hacking my NVIDIA Tegra2
>> emulation pet-project and decided to contribute =).
>>
>> It looks like you are trying to duplicate what generic ptimer is already
>> doing, isn't it?
>>
>
> Yes, ptimer was probably the correct way to do this in the first
> place. Some of the new code structures introduced in this patch series
> are directly applicable though for that conversion effort.
>

I looked into doing this with ptimer, and pitmer doesn't really play
nice with periodic down counters. You could do the subtractions
against an up counter with the load value but that seems just as
complex as the current solution IMO. The hardest part is setting the
counter value on an already running timer.

The alternative is to patch ptimer to handle down timers.

Regards,
Peter

> Regards,
> Peter
>
>> --
>> Dmitry
>>

Re: [Qemu-devel] [RFC v1 0/2] ARM MPTimer fixes and refactoring

[Dmitry Osipenko]

20.09.2015 20:48, Peter Crosthwaite пишет:
> On Sun, Jul 5, 2015 at 1:58 PM, Peter Crosthwaite
> <peter.crosthwaite@xilinx.com> wrote:
>> On Sun, Jul 5, 2015 at 1:47 PM, Dmitry Osipenko <digetx@gmail.com> wrote:
>>> 05.07.2015 23:26, Peter Crosthwaite пишет:
>>>
>>>> Hi Dmitry,
>>>>
>>>> Based on my comment earlier, this is what I came up with RE consolidation
>>>> of
>>>> those arm_mptimer code paths that were giving you problems. I have not
>>>> done the
>>>> interrupt mask fix, as that one from your series is reasonably
>>>> independent.
>>>>
>>>> Regards,
>>>> Peter
>>>>
>>>> Peter Crosthwaite (2):
>>>>     timer: arm_mp: Factor out timer value calculation
>>>>     timer: arm_mp: consolidate control and counter write logic
>>>>
>>>>    hw/timer/arm_mptimer.c | 73
>>>> +++++++++++++++++++++++++++++---------------------
>>>>    1 file changed, 42 insertions(+), 31 deletions(-)
>>>>
>>>
>>> Hi Peter, thanks a lot! Generally, I don't have any trouble with currently
>>> missed functionality, just noticed it while was hacking my NVIDIA Tegra2
>>> emulation pet-project and decided to contribute =).
>>>
>>> It looks like you are trying to duplicate what generic ptimer is already
>>> doing, isn't it?
>>>
>>
>> Yes, ptimer was probably the correct way to do this in the first
>> place. Some of the new code structures introduced in this patch series
>> are directly applicable though for that conversion effort.
>>
>
> I looked into doing this with ptimer, and pitmer doesn't really play
> nice with periodic down counters. You could do the subtractions
> against an up counter with the load value but that seems just as
> complex as the current solution IMO. The hardest part is setting the
> counter value on an already running timer.
>
> The alternative is to patch ptimer to handle down timers.
>
> Regards,
> Peter
>

Hmm, I think you missed something. Ptimer is doing downcount, not up. 
ptimer_get_count() might be a misnomer, it returns current timer value (it goes 
down to 0), not a passed ticks count number. Ptimer also should handle reloading 
of a running timer just fine with ptimer_set_count(), don't see any trouble 
here. Anyway, I'll try to re-implement MPtimer using ptimer ASAP and see how it 
really fits.

-- 
Dmitry

Re: [Qemu-devel] [RFC v1 0/2] ARM MPTimer fixes and refactoring

[Dmitry Osipenko]

03.10.2015 16:11, Dmitry Osipenko пишет:
> 20.09.2015 20:48, Peter Crosthwaite пишет:
>> On Sun, Jul 5, 2015 at 1:58 PM, Peter Crosthwaite
>> <peter.crosthwaite@xilinx.com> wrote:
>>> On Sun, Jul 5, 2015 at 1:47 PM, Dmitry Osipenko <digetx@gmail.com> wrote:
>>>> 05.07.2015 23:26, Peter Crosthwaite пишет:
>>>>
>>>>> Hi Dmitry,
>>>>>
>>>>> Based on my comment earlier, this is what I came up with RE consolidation
>>>>> of
>>>>> those arm_mptimer code paths that were giving you problems. I have not
>>>>> done the
>>>>> interrupt mask fix, as that one from your series is reasonably
>>>>> independent.
>>>>>
>>>>> Regards,
>>>>> Peter
>>>>>
>>>>> Peter Crosthwaite (2):
>>>>>     timer: arm_mp: Factor out timer value calculation
>>>>>     timer: arm_mp: consolidate control and counter write logic
>>>>>
>>>>>    hw/timer/arm_mptimer.c | 73
>>>>> +++++++++++++++++++++++++++++---------------------
>>>>>    1 file changed, 42 insertions(+), 31 deletions(-)
>>>>>
>>>>
>>>> Hi Peter, thanks a lot! Generally, I don't have any trouble with currently
>>>> missed functionality, just noticed it while was hacking my NVIDIA Tegra2
>>>> emulation pet-project and decided to contribute =).
>>>>
>>>> It looks like you are trying to duplicate what generic ptimer is already
>>>> doing, isn't it?
>>>>
>>>
>>> Yes, ptimer was probably the correct way to do this in the first
>>> place. Some of the new code structures introduced in this patch series
>>> are directly applicable though for that conversion effort.
>>>
>>
>> I looked into doing this with ptimer, and pitmer doesn't really play
>> nice with periodic down counters. You could do the subtractions
>> against an up counter with the load value but that seems just as
>> complex as the current solution IMO. The hardest part is setting the
>> counter value on an already running timer.
>>
>> The alternative is to patch ptimer to handle down timers.
>>
>> Regards,
>> Peter
>>
>
> Hmm, I think you missed something. Ptimer is doing downcount, not up.
> ptimer_get_count() might be a misnomer, it returns current timer value (it goes
> down to 0), not a passed ticks count number. Ptimer also should handle reloading
> of a running timer just fine with ptimer_set_count(), don't see any trouble
> here. Anyway, I'll try to re-implement MPtimer using ptimer ASAP and see how it
> really fits.
>

Finally, I tried ptimer and everything seem to work perfect. Pausing, 
re-loading, etc works as it should. Will send the patch soon.

-- 
Dmitry

Re: [Qemu-devel] [RFC v1 0/2] ARM MPTimer fixes and refactoring

[Peter Crosthwaite]

On Mon, Oct 5, 2015 at 9:07 AM, Dmitry Osipenko <digetx@gmail.com> wrote:
> 03.10.2015 16:11, Dmitry Osipenko пишет:
>
>> 20.09.2015 20:48, Peter Crosthwaite пишет:
>>>
>>> On Sun, Jul 5, 2015 at 1:58 PM, Peter Crosthwaite
>>> <peter.crosthwaite@xilinx.com> wrote:
>>>>
>>>> On Sun, Jul 5, 2015 at 1:47 PM, Dmitry Osipenko <digetx@gmail.com>
>>>> wrote:
>>>>>
>>>>> 05.07.2015 23:26, Peter Crosthwaite пишет:
>>>>>
>>>>>> Hi Dmitry,
>>>>>>
>>>>>> Based on my comment earlier, this is what I came up with RE
>>>>>> consolidation
>>>>>> of
>>>>>> those arm_mptimer code paths that were giving you problems. I have not
>>>>>> done the
>>>>>> interrupt mask fix, as that one from your series is reasonably
>>>>>> independent.
>>>>>>
>>>>>> Regards,
>>>>>> Peter
>>>>>>
>>>>>> Peter Crosthwaite (2):
>>>>>>     timer: arm_mp: Factor out timer value calculation
>>>>>>     timer: arm_mp: consolidate control and counter write logic
>>>>>>
>>>>>>    hw/timer/arm_mptimer.c | 73
>>>>>> +++++++++++++++++++++++++++++---------------------
>>>>>>    1 file changed, 42 insertions(+), 31 deletions(-)
>>>>>>
>>>>>
>>>>> Hi Peter, thanks a lot! Generally, I don't have any trouble with
>>>>> currently
>>>>> missed functionality, just noticed it while was hacking my NVIDIA
>>>>> Tegra2
>>>>> emulation pet-project and decided to contribute =).
>>>>>
>>>>> It looks like you are trying to duplicate what generic ptimer is
>>>>> already
>>>>> doing, isn't it?
>>>>>
>>>>
>>>> Yes, ptimer was probably the correct way to do this in the first
>>>> place. Some of the new code structures introduced in this patch series
>>>> are directly applicable though for that conversion effort.
>>>>
>>>
>>> I looked into doing this with ptimer, and pitmer doesn't really play
>>> nice with periodic down counters. You could do the subtractions
>>> against an up counter with the load value but that seems just as
>>> complex as the current solution IMO. The hardest part is setting the
>>> counter value on an already running timer.
>>>
>>> The alternative is to patch ptimer to handle down timers.
>>>
>>> Regards,
>>> Peter
>>>
>>
>> Hmm, I think you missed something. Ptimer is doing downcount, not up.
>> ptimer_get_count() might be a misnomer, it returns current timer value (it
>> goes
>> down to 0), not a passed ticks count number. Ptimer also should handle
>> reloading
>> of a running timer just fine with ptimer_set_count(), don't see any
>> trouble
>> here. Anyway, I'll try to re-implement MPtimer using ptimer ASAP and see
>> how it
>> really fits.
>>
>
> Finally, I tried ptimer and everything seem to work perfect. Pausing,
> re-loading, etc works as it should. Will send the patch soon.
>

Great!

Thanks for the efforts. I'll take a CC on the review.

Do on the fly prescaler changes work?

Regards,
Peter

> --
> Dmitry

Re: [Qemu-devel] [RFC v1 0/2] ARM MPTimer fixes and refactoring

[Dmitry Osipenko]

05.10.2015 19:27, Peter Crosthwaite пишет:
> On Mon, Oct 5, 2015 at 9:07 AM, Dmitry Osipenko <digetx@gmail.com> wrote:
>> 03.10.2015 16:11, Dmitry Osipenko пишет:
>>
>>> 20.09.2015 20:48, Peter Crosthwaite пишет:
>>>>
>>>> On Sun, Jul 5, 2015 at 1:58 PM, Peter Crosthwaite
>>>> <peter.crosthwaite@xilinx.com> wrote:
>>>>>
>>>>> On Sun, Jul 5, 2015 at 1:47 PM, Dmitry Osipenko <digetx@gmail.com>
>>>>> wrote:
>>>>>>
>>>>>> 05.07.2015 23:26, Peter Crosthwaite пишет:
>>>>>>
>>>>>>> Hi Dmitry,
>>>>>>>
>>>>>>> Based on my comment earlier, this is what I came up with RE
>>>>>>> consolidation
>>>>>>> of
>>>>>>> those arm_mptimer code paths that were giving you problems. I have not
>>>>>>> done the
>>>>>>> interrupt mask fix, as that one from your series is reasonably
>>>>>>> independent.
>>>>>>>
>>>>>>> Regards,
>>>>>>> Peter
>>>>>>>
>>>>>>> Peter Crosthwaite (2):
>>>>>>>      timer: arm_mp: Factor out timer value calculation
>>>>>>>      timer: arm_mp: consolidate control and counter write logic
>>>>>>>
>>>>>>>     hw/timer/arm_mptimer.c | 73
>>>>>>> +++++++++++++++++++++++++++++---------------------
>>>>>>>     1 file changed, 42 insertions(+), 31 deletions(-)
>>>>>>>
>>>>>>
>>>>>> Hi Peter, thanks a lot! Generally, I don't have any trouble with
>>>>>> currently
>>>>>> missed functionality, just noticed it while was hacking my NVIDIA
>>>>>> Tegra2
>>>>>> emulation pet-project and decided to contribute =).
>>>>>>
>>>>>> It looks like you are trying to duplicate what generic ptimer is
>>>>>> already
>>>>>> doing, isn't it?
>>>>>>
>>>>>
>>>>> Yes, ptimer was probably the correct way to do this in the first
>>>>> place. Some of the new code structures introduced in this patch series
>>>>> are directly applicable though for that conversion effort.
>>>>>
>>>>
>>>> I looked into doing this with ptimer, and pitmer doesn't really play
>>>> nice with periodic down counters. You could do the subtractions
>>>> against an up counter with the load value but that seems just as
>>>> complex as the current solution IMO. The hardest part is setting the
>>>> counter value on an already running timer.
>>>>
>>>> The alternative is to patch ptimer to handle down timers.
>>>>
>>>> Regards,
>>>> Peter
>>>>
>>>
>>> Hmm, I think you missed something. Ptimer is doing downcount, not up.
>>> ptimer_get_count() might be a misnomer, it returns current timer value (it
>>> goes
>>> down to 0), not a passed ticks count number. Ptimer also should handle
>>> reloading
>>> of a running timer just fine with ptimer_set_count(), don't see any
>>> trouble
>>> here. Anyway, I'll try to re-implement MPtimer using ptimer ASAP and see
>>> how it
>>> really fits.
>>>
>>
>> Finally, I tried ptimer and everything seem to work perfect. Pausing,
>> re-loading, etc works as it should. Will send the patch soon.
>>
>
> Great!
>
> Thanks for the efforts. I'll take a CC on the review.
>
> Do on the fly prescaler changes work?
>
> Regards,
> Peter
>
>> --
>> Dmitry

Yes, prescaler works on the fly. I'll post my MPtimer tests somewhere.

-- 
Dmitry