Re: [Qemu-devel] [PATCH v2] arm_mptimer: Fix timer shutdown

[Dmitry Osipenko <digetx@gmail.com>]

02.07.2015 21:09, Peter Maydell пишет:
> On 2 July 2015 at 18:52, Dmitry Osipenko <digetx@gmail.com> wrote:
>> 02.07.2015 20:34, Peter Maydell пишет:
>>>
>>>
>>> This will now cause us to do the "reload the timer"
>>> logic if you write a 1 to the control bit when it was
>>> already 1, which we didn't do before.
>>>
>>> The logic I suggested in my previous review
>>> comment gets this right...
>>>
>>> -- PMM
>>>
>>
>> The problem with code you suggested is that won't start periodic count after
>> one-shot tick was completed.
>
> Can you give more detail? This code is only for when
> the guest writes to the control register, so it doesn't
> get run when a one-shot tick completes.
>
> In any case, the code currently in master does:
>    old value   new value    action
>        0           0          nothing
>        0           1          reload timer
>        1           0          nothing
>        1           1          nothing
>
> Your first patch did:
>
>    old value   new value    action
>        0           0          delete timer
>        0           1          reload timer
>        1           0          delete timer
>        1           1          nothing
>
> Your second patch does:
>
>    old value   new value    action
>        0           0          nothing
>        0           1          reload timer
>        1           0          delete timer
>        1           1          reload timer
>
> My suggestion was:
>
>    old value   new value    action
>        0           0          nothing
>        0           1          reload timer
>        1           0          delete timer
>        1           1          nothing
>
> If you think that's wrong, then surely your first
> patch also has the same problem?
>
> thanks
> -- PMM
>

Yes, my first patch has same problem. Noticed that issue couple hours ago, it's 
separate bug as I see now... Will make patch for it too.

To clarify new issue:
	1) load TIMER_LOAD with some value
	2) write (TIMER_CONTROL_ENABLE | TIMER_CONTROL_ONESHOT | 
TIMER_CONTROL_IT_ENABLE) to TIMER_CONTROL
	3) wait for one-shot complete
	4) re-load TIMER_LOAD
	5) write (TIMER_CONTROL_ENABLE | TIMER_CONTROL_PERIODIC | 
TIMER_CONTROL_IT_ENABLE) to TIMER_CONTROL <---- it won't start, bug


Oh, and just noticed that timer code doesn't handle IT(interrupt enable) bit. 
Will fix it too.

-- 
Dmitry