| Message ID | 166636579128.26670.11954825054446993916-0@git.sr.ht (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [qemu.git] target/imx: reload cmp timer outside of the reload ptimer transaction | expand |
On Fri, 21 Oct 2022 at 20:18, ~axelheider <axelheider@git.sr.ht> wrote: > > From: Axel Heider <axel.heider@hensoldt.net> > > Signed-off-by: Axel Heider <axel.heider@hensoldt.net> > --- > See https://gitlab.com/qemu-project/qemu/-/issues/1263 > When running the seL4 tests > (https://docs.sel4.systems/projects/sel4test), on the sabrelight > platform the timer test fails (and thus it's disabled by default). > Investigation has shown that the arm/imx6 EPIT timer interrupt does not > fire properly, instead of a second in can take up to a minute to finally > see the interrupt. > > hw/timer/imx_epit.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/hw/timer/imx_epit.c b/hw/timer/imx_epit.c > index 2bf8c754b2..0b13c1eab0 100644 > --- a/hw/timer/imx_epit.c > +++ b/hw/timer/imx_epit.c > @@ -276,9 +276,12 @@ static void imx_epit_write(void *opaque, hwaddr offset, uint64_t value, > ptimer_set_count(s->timer_reload, s->lr); > } > > + // commit s->timer_reload before imx_epit_reload_compare_timer > + // as timer_reload is read in imx_epit_reload_compare_timer QEMU coding style requires /* ... */ for comments, not // (with the /* and */ on lines of their own if it's a multiline comment. > + ptimer_transaction_commit(s->timer_reload); > + > imx_epit_reload_compare_timer(s); > ptimer_transaction_commit(s->timer_cmp); > - ptimer_transaction_commit(s->timer_reload); > break; Yes, I see what's happening here. It's OK to commit the timer_reload timer transaction first because that timer doesn't care about the timer_cmp timer. Do we also need to change the other places that call imx_epit_reload_compare_timer() in the handling of CR register writes ? (Those are a little more tricky.) thanks -- PMM
On Mon, 24 Oct 2022 at 13:37, Peter Maydell <peter.maydell@linaro.org> wrote: > > On Fri, 21 Oct 2022 at 20:18, ~axelheider <axelheider@git.sr.ht> wrote: > > > > From: Axel Heider <axel.heider@hensoldt.net> > > > > Signed-off-by: Axel Heider <axel.heider@hensoldt.net> > > --- > > See https://gitlab.com/qemu-project/qemu/-/issues/1263 > > When running the seL4 tests > > (https://docs.sel4.systems/projects/sel4test), on the sabrelight > > platform the timer test fails (and thus it's disabled by default). > > Investigation has shown that the arm/imx6 EPIT timer interrupt does not > > fire properly, instead of a second in can take up to a minute to finally > > see the interrupt. Oh yes, this sort of information should ideally be in the commit message proper -- the commit message is the place to explain what you're changing and why. You can make the gitlab issue auto-close by putting a line Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1263 into the commit message. thanks -- PMM
diff --git a/hw/timer/imx_epit.c b/hw/timer/imx_epit.c index 2bf8c754b2..0b13c1eab0 100644 --- a/hw/timer/imx_epit.c +++ b/hw/timer/imx_epit.c @@ -276,9 +276,12 @@ static void imx_epit_write(void *opaque, hwaddr offset, uint64_t value, ptimer_set_count(s->timer_reload, s->lr); } + // commit s->timer_reload before imx_epit_reload_compare_timer + // as timer_reload is read in imx_epit_reload_compare_timer + ptimer_transaction_commit(s->timer_reload); + imx_epit_reload_compare_timer(s); ptimer_transaction_commit(s->timer_cmp); - ptimer_transaction_commit(s->timer_reload); break; case 3: /* CMP */