| Message ID | 1340349744-23920-1-git-send-email-e.voevodin@samsung.com |
|---|---|
| State | New |
| Headers | show |
Hi Evgeny, Im just speculating here, but I recently ran into Linux hangs on Microblaze due to ptimer issues and I think you may be suffering the same base issue. The Microblaze timer (hw/xilinx_timer.c) has a similar implementation to the exynos (chained one-shot ptimer). Recently Peter Chubb put patch cf36b31db209a261ee3bc2737e788e1ced0a1bec through, which modified ptimer to not set excessively short periods. Problem is, that only works for periodic ptimers. Anyways, you may find that changing your set_count() calls to set_limit (i.e. the function designed for periodic timers) calls works for you, without changing the logic of your device. Heres the change pattern: - ptimer_set_count(ptimer, count); + ptimer_set_limit(ptimer, count, 1); More permanently (and a question for the cheif maintainers) can we look into ways of fixing ptimer properly? Regards, Peter On Fri, Jun 22, 2012 at 5:22 PM, Evgeny Voevodin <e.voevodin@samsung.com> wrote: > From: Stanislav Vorobiov <s.vorobiov@samsung.com> > > After some long period of time Linux kernel hanged due to > ptimer_get_count may return 0 before timer interrupt occurs, > thus, causing FRC to jump back in time > > Signed-off-by: Evgeny Voevodin <e.voevodin@samsung.com> Reviewed-by Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com> > --- > hw/exynos4210_mct.c | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/hw/exynos4210_mct.c b/hw/exynos4210_mct.c > index 7474fcf..7a22b1f 100644 > --- a/hw/exynos4210_mct.c > +++ b/hw/exynos4210_mct.c > @@ -376,10 +376,6 @@ static uint64_t exynos4210_gfrc_get_count(Exynos4210MCTGT *s) > { > uint64_t count = 0; > count = ptimer_get_count(s->ptimer_frc); > - if (!count) { > - /* Timer event was generated and s->reg.cnt holds adequate value */ > - return s->reg.cnt; > - } > count = s->count - count; > return s->reg.cnt + count; > } > -- > 1.7.9.5 > >
On 22.06.2012 11:56, Peter Crosthwaite wrote: > Hi Evgeny, > > Im just speculating here, but I recently ran into Linux hangs on > Microblaze due to ptimer issues and I think you may be suffering the > same base issue. > > The Microblaze timer (hw/xilinx_timer.c) has a similar implementation > to the exynos (chained one-shot ptimer). Recently Peter Chubb put > patch cf36b31db209a261ee3bc2737e788e1ced0a1bec through, which modified > ptimer to not set excessively short periods. Problem is, that only > works for periodic ptimers. No, that's another problem. MCT was developed earlier then this commit landed and it contains similar code to avoid such situation. But our patch fixes bug in logic. The thing is that since ptimer uses BHs, it can be the situation when ptimer is stopped but BH is not called yet. And exactly in this moment target reads counter value that was incorrect calculated. > Anyways, you may find that changing your set_count() calls to > set_limit (i.e. the function designed for periodic timers) calls works > for you, without changing the logic of your device. Heres the change > pattern: > > - ptimer_set_count(ptimer, count); > + ptimer_set_limit(ptimer, count, 1); > > More permanently (and a question for the cheif maintainers) can we > look into ways of fixing ptimer properly? > > Regards, > Peter > > On Fri, Jun 22, 2012 at 5:22 PM, Evgeny Voevodin<e.voevodin@samsung.com> wrote: >> From: Stanislav Vorobiov<s.vorobiov@samsung.com> >> >> After some long period of time Linux kernel hanged due to >> ptimer_get_count may return 0 before timer interrupt occurs, >> thus, causing FRC to jump back in time >> >> Signed-off-by: Evgeny Voevodin<e.voevodin@samsung.com> > Reviewed-by Peter A. G. Crosthwaite<peter.crosthwaite@petalogix.com> Thanks. >> --- >> hw/exynos4210_mct.c | 4 ---- >> 1 file changed, 4 deletions(-) >> >> diff --git a/hw/exynos4210_mct.c b/hw/exynos4210_mct.c >> index 7474fcf..7a22b1f 100644 >> --- a/hw/exynos4210_mct.c >> +++ b/hw/exynos4210_mct.c >> @@ -376,10 +376,6 @@ static uint64_t exynos4210_gfrc_get_count(Exynos4210MCTGT *s) >> { >> uint64_t count = 0; >> count = ptimer_get_count(s->ptimer_frc); >> - if (!count) { >> - /* Timer event was generated and s->reg.cnt holds adequate value */ >> - return s->reg.cnt; >> - } >> count = s->count - count; >> return s->reg.cnt + count; >> } >> -- >> 1.7.9.5 >> >>
diff --git a/hw/exynos4210_mct.c b/hw/exynos4210_mct.c index 7474fcf..7a22b1f 100644 --- a/hw/exynos4210_mct.c +++ b/hw/exynos4210_mct.c @@ -376,10 +376,6 @@ static uint64_t exynos4210_gfrc_get_count(Exynos4210MCTGT *s) { uint64_t count = 0; count = ptimer_get_count(s->ptimer_frc); - if (!count) { - /* Timer event was generated and s->reg.cnt holds adequate value */ - return s->reg.cnt; - } count = s->count - count; return s->reg.cnt + count; }