Message ID | 20191017132905.5604-8-peter.maydell@linaro.org |
---|---|
State | Superseded |
Headers | show |
Series | Convert misc-arch devices to new ptimer API | expand |
On 10/17/19 6:29 AM, Peter Maydell wrote: > Switch the mcf5206 code away from bottom-half based ptimers to > the new transaction-based ptimer API. This just requires adding > begin/commit calls around the various places that modify the ptimer > state, and using the new ptimer_init() function to create the timer. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > hw/m68k/mcf5206.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On 10/17/19 3:29 PM, Peter Maydell wrote: > Switch the mcf5206 code away from bottom-half based ptimers to > the new transaction-based ptimer API. This just requires adding > begin/commit calls around the various places that modify the ptimer > state, and using the new ptimer_init() function to create the timer. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > hw/m68k/mcf5206.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/hw/m68k/mcf5206.c b/hw/m68k/mcf5206.c > index a49096367cb..c05401e0e50 100644 > --- a/hw/m68k/mcf5206.c > +++ b/hw/m68k/mcf5206.c > @@ -8,7 +8,6 @@ > > #include "qemu/osdep.h" > #include "qemu/error-report.h" > -#include "qemu/main-loop.h" > #include "cpu.h" > #include "hw/hw.h" > #include "hw/irq.h" > @@ -57,6 +56,7 @@ static void m5206_timer_recalibrate(m5206_timer_state *s) > int prescale; > int mode; > > + ptimer_transaction_begin(s->timer); > ptimer_stop(s->timer); > > if ((s->tmr & TMR_RST) == 0) > @@ -78,6 +78,7 @@ static void m5206_timer_recalibrate(m5206_timer_state *s) > ptimer_set_limit(s->timer, s->trr, 0); > > ptimer_run(s->timer, 0); > + ptimer_transaction_commit(s->timer); > } > > static void m5206_timer_trigger(void *opaque) > @@ -123,7 +124,9 @@ static void m5206_timer_write(m5206_timer_state *s, uint32_t addr, uint32_t val) > s->tcr = val; > break; > case 0xc: > + ptimer_transaction_begin(s->timer); > ptimer_set_count(s->timer, val); > + ptimer_transaction_commit(s->timer); > break; > case 0x11: > s->ter &= ~val; > @@ -137,11 +140,9 @@ static void m5206_timer_write(m5206_timer_state *s, uint32_t addr, uint32_t val) > static m5206_timer_state *m5206_timer_init(qemu_irq irq) > { > m5206_timer_state *s; > - QEMUBH *bh; > > s = g_new0(m5206_timer_state, 1); > - bh = qemu_bh_new(m5206_timer_trigger, s); > - s->timer = ptimer_init_with_bh(bh, PTIMER_POLICY_DEFAULT); > + s->timer = ptimer_init(m5206_timer_trigger, s, PTIMER_POLICY_DEFAULT); > s->irq = irq; > m5206_timer_reset(s); > return s; > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Am Thu, 17 Oct 2019 14:29:04 +0100 schrieb Peter Maydell <peter.maydell@linaro.org>: > Switch the mcf5206 code away from bottom-half based ptimers to > the new transaction-based ptimer API. This just requires adding > begin/commit calls around the various places that modify the ptimer > state, and using the new ptimer_init() function to create the timer. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > hw/m68k/mcf5206.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) After applying the patch, I now get an assertion: $ qemu-system-m68k -nographic -kernel ~/tmp/images/image-an5206-big-20000706.bin -M an5206 uClinux/COLDFIRE(m5206) COLDFIRE port done by Greg Ungerer, gerg@moreton.com.au Flat model support (C) 1998,1999 Kenneth Albanowski, D. Jeff Dionne KERNEL -> TEXT=0x010000-0x077cb8 DATA=0x077cb8-0x08b0d0 BSS=0x08b0d0-0x0a2d58 KERNEL -> ROMFS=0x0a2d58-0x183b10 MEM=0x183b10-0x1fff000 STACK=0x1fff000-0x2000000 qemu-system-m68k: hw/core/ptimer.c:410: ptimer_transaction_begin: Assertion `!s->in_transaction || !s->callback' failed. Looks like something is still wrong here... Thomas
Am Sat, 19 Oct 2019 12:48:59 +0200 schrieb Thomas Huth <huth@tuxfamily.org>: > Am Thu, 17 Oct 2019 14:29:04 +0100 > schrieb Peter Maydell <peter.maydell@linaro.org>: > > > Switch the mcf5206 code away from bottom-half based ptimers to > > the new transaction-based ptimer API. This just requires adding > > begin/commit calls around the various places that modify the ptimer > > state, and using the new ptimer_init() function to create the timer. > > > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > > --- > > hw/m68k/mcf5206.c | 9 +++++---- > > 1 file changed, 5 insertions(+), 4 deletions(-) > > After applying the patch, I now get an assertion: > > $ qemu-system-m68k -nographic -kernel > ~/tmp/images/image-an5206-big-20000706.bin -M an5206 > > uClinux/COLDFIRE(m5206) > COLDFIRE port done by Greg Ungerer, gerg@moreton.com.au > Flat model support (C) 1998,1999 Kenneth Albanowski, D. Jeff Dionne > KERNEL -> TEXT=0x010000-0x077cb8 DATA=0x077cb8-0x08b0d0 > BSS=0x08b0d0-0x0a2d58 KERNEL -> ROMFS=0x0a2d58-0x183b10 > MEM=0x183b10-0x1fff000 STACK=0x1fff000-0x2000000 qemu-system-m68k: > hw/core/ptimer.c:410: ptimer_transaction_begin: Assertion > `!s->in_transaction || !s->callback' failed. > > Looks like something is still wrong here... FWIW, the image for testing can be found here: https://web.archive.org/web/20181018135822/http://www.uclinux.org/ports/coldfire/image-an5206-big-20000706.bin.gz Thomas
Am Sat, 19 Oct 2019 13:10:27 +0200 schrieb Thomas Huth <huth@tuxfamily.org>: > Am Sat, 19 Oct 2019 12:48:59 +0200 > schrieb Thomas Huth <huth@tuxfamily.org>: > > > Am Thu, 17 Oct 2019 14:29:04 +0100 > > schrieb Peter Maydell <peter.maydell@linaro.org>: > > > > > Switch the mcf5206 code away from bottom-half based ptimers to > > > the new transaction-based ptimer API. This just requires adding > > > begin/commit calls around the various places that modify the > > > ptimer state, and using the new ptimer_init() function to create > > > the timer. > > > > > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > > > --- > > > hw/m68k/mcf5206.c | 9 +++++---- > > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > After applying the patch, I now get an assertion: > > > > $ qemu-system-m68k -nographic -kernel > > ~/tmp/images/image-an5206-big-20000706.bin -M an5206 > > > > uClinux/COLDFIRE(m5206) > > COLDFIRE port done by Greg Ungerer, gerg@moreton.com.au > > Flat model support (C) 1998,1999 Kenneth Albanowski, D. Jeff Dionne > > KERNEL -> TEXT=0x010000-0x077cb8 DATA=0x077cb8-0x08b0d0 > > BSS=0x08b0d0-0x0a2d58 KERNEL -> ROMFS=0x0a2d58-0x183b10 > > MEM=0x183b10-0x1fff000 STACK=0x1fff000-0x2000000 qemu-system-m68k: > > hw/core/ptimer.c:410: ptimer_transaction_begin: Assertion > > `!s->in_transaction || !s->callback' failed. > > > > Looks like something is still wrong here... Looking at the code a little bit, I think you missed the early return in m5206_timer_recalibrate() : if ((s->tmr & TMR_RST) == 0) return; That needs a ptimer_transaction_commit() now, too. Thomas
diff --git a/hw/m68k/mcf5206.c b/hw/m68k/mcf5206.c index a49096367cb..c05401e0e50 100644 --- a/hw/m68k/mcf5206.c +++ b/hw/m68k/mcf5206.c @@ -8,7 +8,6 @@ #include "qemu/osdep.h" #include "qemu/error-report.h" -#include "qemu/main-loop.h" #include "cpu.h" #include "hw/hw.h" #include "hw/irq.h" @@ -57,6 +56,7 @@ static void m5206_timer_recalibrate(m5206_timer_state *s) int prescale; int mode; + ptimer_transaction_begin(s->timer); ptimer_stop(s->timer); if ((s->tmr & TMR_RST) == 0) @@ -78,6 +78,7 @@ static void m5206_timer_recalibrate(m5206_timer_state *s) ptimer_set_limit(s->timer, s->trr, 0); ptimer_run(s->timer, 0); + ptimer_transaction_commit(s->timer); } static void m5206_timer_trigger(void *opaque) @@ -123,7 +124,9 @@ static void m5206_timer_write(m5206_timer_state *s, uint32_t addr, uint32_t val) s->tcr = val; break; case 0xc: + ptimer_transaction_begin(s->timer); ptimer_set_count(s->timer, val); + ptimer_transaction_commit(s->timer); break; case 0x11: s->ter &= ~val; @@ -137,11 +140,9 @@ static void m5206_timer_write(m5206_timer_state *s, uint32_t addr, uint32_t val) static m5206_timer_state *m5206_timer_init(qemu_irq irq) { m5206_timer_state *s; - QEMUBH *bh; s = g_new0(m5206_timer_state, 1); - bh = qemu_bh_new(m5206_timer_trigger, s); - s->timer = ptimer_init_with_bh(bh, PTIMER_POLICY_DEFAULT); + s->timer = ptimer_init(m5206_timer_trigger, s, PTIMER_POLICY_DEFAULT); s->irq = irq; m5206_timer_reset(s); return s;
Switch the mcf5206 code away from bottom-half based ptimers to the new transaction-based ptimer API. This just requires adding begin/commit calls around the various places that modify the ptimer state, and using the new ptimer_init() function to create the timer. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/m68k/mcf5206.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) -- 2.20.1