Patchwork [2/4] powerpc: Make the decr interrupt type overridable

login
register
mail settings
Submitter Edgar Iglesias
Date Sept. 11, 2010, 2:09 p.m.
Message ID <1284214197-27140-3-git-send-email-edgar.iglesias@gmail.com>
Download mbox | patch
Permalink /patch/64518/
State New
Headers show

Comments

Edgar Iglesias - Sept. 11, 2010, 2:09 p.m.
Make it possible for boards to override the kind of interrupt
to be signaled when the decr timer hits. The 405's signal PIT
interrupts while the 440's signal DECR.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>
---
 hw/ppc.c |   22 ++++++++++++++++++++--
 hw/ppc.h |    2 ++
 2 files changed, 22 insertions(+), 2 deletions(-)
Alexander Graf - Sept. 20, 2010, 10:42 a.m.
Edgar E. Iglesias wrote:
> Make it possible for boards to override the kind of interrupt
> to be signaled when the decr timer hits. The 405's signal PIT
> interrupts while the 440's signal DECR.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> ---
>  hw/ppc.c |   22 ++++++++++++++++++++--
>  hw/ppc.h |    2 ++
>  2 files changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/hw/ppc.c b/hw/ppc.c
> index 55e3808..3df7801 100644
> --- a/hw/ppc.c
> +++ b/hw/ppc.c
> @@ -769,6 +769,9 @@ struct ppcemb_timer_t {
>      struct QEMUTimer *fit_timer;
>      uint64_t wdt_next;    /* Tick for next WDT interrupt  */
>      struct QEMUTimer *wdt_timer;
> +
> +    /* 405 have the PIT, 440 have a DECR.  */
> +    unsigned int decr_excp;
>  };
>  
>  /* Fixed interval timer */
> @@ -851,7 +854,7 @@ static void cpu_4xx_pit_cb (void *opaque)
>      ppcemb_timer = tb_env->opaque;
>      env->spr[SPR_40x_TSR] |= 1 << 27;
>      if ((env->spr[SPR_40x_TCR] >> 26) & 0x1)
>   

Do these registers also apply to 440? If so, they should probably be
renamed to 4xx. Also while you're at it - I'd love to have readable
#define's for magic numbers :).

> -        ppc_set_irq(env, PPC_INTERRUPT_PIT, 1);
> +        ppc_set_irq(env, ppcemb_timer->decr_excp, 1);
>      start_stop_pit(env, tb_env, 1);
>      LOG_TB("%s: ar %d ir %d TCR " TARGET_FMT_lx " TSR " TARGET_FMT_lx " "
>             "%016" PRIx64 "\n", __func__,
> @@ -948,10 +951,15 @@ target_ulong load_40x_pit (CPUState *env)
>  
>  void store_booke_tsr (CPUState *env, target_ulong val)
>  {
> +    ppc_tb_t *tb_env = env->tb_env;
> +    ppcemb_timer_t *ppcemb_timer;
> +
> +    ppcemb_timer = tb_env->opaque;
> +
>      LOG_TB("%s: val " TARGET_FMT_lx "\n", __func__, val);
>      env->spr[SPR_40x_TSR] &= ~(val & 0xFC000000);
>      if (val & 0x80000000)
> -        ppc_set_irq(env, PPC_INTERRUPT_PIT, 0);
> +        ppc_set_irq(env, ppcemb_timer->decr_excp, 0);
>  }
>  
>  void store_booke_tcr (CPUState *env, target_ulong val)
> @@ -977,6 +985,15 @@ static void ppc_emb_set_tb_clk (void *opaque, uint32_t freq)
>      /* XXX: we should also update all timers */
>  }
>  
> +void ppc_emb_timers_set_decr_excp(CPUState *env, unsigned int excp)
> +{
> +    ppc_tb_t *tb_env = env->tb_env;
> +    ppcemb_timer_t *ppcemb_timer;
> +
> +    ppcemb_timer = tb_env->opaque;
> +    ppcemb_timer->decr_excp = excp;
>   

Why do you need this? Shouldn't the decrementor type be set by the CPU core?

> +}
> +
>  clk_setup_cb ppc_emb_timers_init (CPUState *env, uint32_t freq)
>  {
>      ppc_tb_t *tb_env;
> @@ -996,6 +1013,7 @@ clk_setup_cb ppc_emb_timers_init (CPUState *env, uint32_t freq)
>              qemu_new_timer(vm_clock, &cpu_4xx_fit_cb, env);
>          ppcemb_timer->wdt_timer =
>              qemu_new_timer(vm_clock, &cpu_4xx_wdt_cb, env);
> +        ppcemb_timer->decr_excp = PPC_INTERRUPT_PIT;
>   

If anything, it should be a parameter here.


Alex
Edgar Iglesias - Sept. 20, 2010, 12:11 p.m.
On Mon, Sep 20, 2010 at 12:42:41PM +0200, Alexander Graf wrote:
> Edgar E. Iglesias wrote:
> > Make it possible for boards to override the kind of interrupt
> > to be signaled when the decr timer hits. The 405's signal PIT
> > interrupts while the 440's signal DECR.
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> > ---
> >  hw/ppc.c |   22 ++++++++++++++++++++--
> >  hw/ppc.h |    2 ++
> >  2 files changed, 22 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/ppc.c b/hw/ppc.c
> > index 55e3808..3df7801 100644
> > --- a/hw/ppc.c
> > +++ b/hw/ppc.c
> > @@ -769,6 +769,9 @@ struct ppcemb_timer_t {
> >      struct QEMUTimer *fit_timer;
> >      uint64_t wdt_next;    /* Tick for next WDT interrupt  */
> >      struct QEMUTimer *wdt_timer;
> > +
> > +    /* 405 have the PIT, 440 have a DECR.  */
> > +    unsigned int decr_excp;
> >  };
> >  
> >  /* Fixed interval timer */
> > @@ -851,7 +854,7 @@ static void cpu_4xx_pit_cb (void *opaque)
> >      ppcemb_timer = tb_env->opaque;
> >      env->spr[SPR_40x_TSR] |= 1 << 27;
> >      if ((env->spr[SPR_40x_TCR] >> 26) & 0x1)
> >   
> 
> Do these registers also apply to 440? If so, they should probably be
> renamed to 4xx. Also while you're at it - I'd love to have readable
> #define's for magic numbers :).

Sure, but it sounds to me like follow-up patches :)
The code is already full with mixed use of 4xx and 40x.

> > -        ppc_set_irq(env, PPC_INTERRUPT_PIT, 1);
> > +        ppc_set_irq(env, ppcemb_timer->decr_excp, 1);
> >      start_stop_pit(env, tb_env, 1);
> >      LOG_TB("%s: ar %d ir %d TCR " TARGET_FMT_lx " TSR " TARGET_FMT_lx " "
> >             "%016" PRIx64 "\n", __func__,
> > @@ -948,10 +951,15 @@ target_ulong load_40x_pit (CPUState *env)
> >  
> >  void store_booke_tsr (CPUState *env, target_ulong val)
> >  {
> > +    ppc_tb_t *tb_env = env->tb_env;
> > +    ppcemb_timer_t *ppcemb_timer;
> > +
> > +    ppcemb_timer = tb_env->opaque;
> > +
> >      LOG_TB("%s: val " TARGET_FMT_lx "\n", __func__, val);
> >      env->spr[SPR_40x_TSR] &= ~(val & 0xFC000000);
> >      if (val & 0x80000000)
> > -        ppc_set_irq(env, PPC_INTERRUPT_PIT, 0);
> > +        ppc_set_irq(env, ppcemb_timer->decr_excp, 0);
> >  }
> >  
> >  void store_booke_tcr (CPUState *env, target_ulong val)
> > @@ -977,6 +985,15 @@ static void ppc_emb_set_tb_clk (void *opaque, uint32_t freq)
> >      /* XXX: we should also update all timers */
> >  }
> >  
> > +void ppc_emb_timers_set_decr_excp(CPUState *env, unsigned int excp)
> > +{
> > +    ppc_tb_t *tb_env = env->tb_env;
> > +    ppcemb_timer_t *ppcemb_timer;
> > +
> > +    ppcemb_timer = tb_env->opaque;
> > +    ppcemb_timer->decr_excp = excp;
> >   
> 
> Why do you need this? Shouldn't the decrementor type be set by the CPU core?

Not the way things are modelled today. These blocks are indirectly
instantiated by the boards. But lets make the decr_excp ...

> 
> > +}
> > +
> >  clk_setup_cb ppc_emb_timers_init (CPUState *env, uint32_t freq)
> >  {
> >      ppc_tb_t *tb_env;
> > @@ -996,6 +1013,7 @@ clk_setup_cb ppc_emb_timers_init (CPUState *env, uint32_t freq)
> >              qemu_new_timer(vm_clock, &cpu_4xx_fit_cb, env);
> >          ppcemb_timer->wdt_timer =
> >              qemu_new_timer(vm_clock, &cpu_4xx_wdt_cb, env);
> > +        ppcemb_timer->decr_excp = PPC_INTERRUPT_PIT;
> >   
> 
> If anything, it should be a parameter here.

... an argument to ppc_emb_timers_init and ppc_emb_timers_set_decr_excp
can go away. Does it sound good enough?

Thanks
Alexander Graf - Sept. 20, 2010, 2:06 p.m.
Am 20.09.2010 um 14:11 schrieb "Edgar E. Iglesias" <edgar.iglesias@gmail.com>:

> On Mon, Sep 20, 2010 at 12:42:41PM +0200, Alexander Graf wrote:
>> Edgar E. Iglesias wrote:
>>> Make it possible for boards to override the kind of interrupt
>>> to be signaled when the decr timer hits. The 405's signal PIT
>>> interrupts while the 440's signal DECR.
>>> 
>>> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>
>>> ---
>>> hw/ppc.c |   22 ++++++++++++++++++++--
>>> hw/ppc.h |    2 ++
>>> 2 files changed, 22 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/hw/ppc.c b/hw/ppc.c
>>> index 55e3808..3df7801 100644
>>> --- a/hw/ppc.c
>>> +++ b/hw/ppc.c
>>> @@ -769,6 +769,9 @@ struct ppcemb_timer_t {
>>>     struct QEMUTimer *fit_timer;
>>>     uint64_t wdt_next;    /* Tick for next WDT interrupt  */
>>>     struct QEMUTimer *wdt_timer;
>>> +
>>> +    /* 405 have the PIT, 440 have a DECR.  */
>>> +    unsigned int decr_excp;
>>> };
>>> 
>>> /* Fixed interval timer */
>>> @@ -851,7 +854,7 @@ static void cpu_4xx_pit_cb (void *opaque)
>>>     ppcemb_timer = tb_env->opaque;
>>>     env->spr[SPR_40x_TSR] |= 1 << 27;
>>>     if ((env->spr[SPR_40x_TCR] >> 26) & 0x1)
>>> 
>> 
>> Do these registers also apply to 440? If so, they should probably be
>> renamed to 4xx. Also while you're at it - I'd love to have readable
>> #define's for magic numbers :).
> 
> Sure, but it sounds to me like follow-up patches :)
> The code is already full with mixed use of 4xx and 40x.

I agree :).

> 
>>> -        ppc_set_irq(env, PPC_INTERRUPT_PIT, 1);
>>> +        ppc_set_irq(env, ppcemb_timer->decr_excp, 1);
>>>     start_stop_pit(env, tb_env, 1);
>>>     LOG_TB("%s: ar %d ir %d TCR " TARGET_FMT_lx " TSR " TARGET_FMT_lx " "
>>>            "%016" PRIx64 "\n", __func__,
>>> @@ -948,10 +951,15 @@ target_ulong load_40x_pit (CPUState *env)
>>> 
>>> void store_booke_tsr (CPUState *env, target_ulong val)
>>> {
>>> +    ppc_tb_t *tb_env = env->tb_env;
>>> +    ppcemb_timer_t *ppcemb_timer;
>>> +
>>> +    ppcemb_timer = tb_env->opaque;
>>> +
>>>     LOG_TB("%s: val " TARGET_FMT_lx "\n", __func__, val);
>>>     env->spr[SPR_40x_TSR] &= ~(val & 0xFC000000);
>>>     if (val & 0x80000000)
>>> -        ppc_set_irq(env, PPC_INTERRUPT_PIT, 0);
>>> +        ppc_set_irq(env, ppcemb_timer->decr_excp, 0);
>>> }
>>> 
>>> void store_booke_tcr (CPUState *env, target_ulong val)
>>> @@ -977,6 +985,15 @@ static void ppc_emb_set_tb_clk (void *opaque, uint32_t freq)
>>>     /* XXX: we should also update all timers */
>>> }
>>> 
>>> +void ppc_emb_timers_set_decr_excp(CPUState *env, unsigned int excp)
>>> +{
>>> +    ppc_tb_t *tb_env = env->tb_env;
>>> +    ppcemb_timer_t *ppcemb_timer;
>>> +
>>> +    ppcemb_timer = tb_env->opaque;
>>> +    ppcemb_timer->decr_excp = excp;
>>> 
>> 
>> Why do you need this? Shouldn't the decrementor type be set by the CPU core?
> 
> Not the way things are modelled today. These blocks are indirectly
> instantiated by the boards. But lets make the decr_excp ...
> 
>> 
>>> +}
>>> +
>>> clk_setup_cb ppc_emb_timers_init (CPUState *env, uint32_t freq)
>>> {
>>>     ppc_tb_t *tb_env;
>>> @@ -996,6 +1013,7 @@ clk_setup_cb ppc_emb_timers_init (CPUState *env, uint32_t freq)
>>>             qemu_new_timer(vm_clock, &cpu_4xx_fit_cb, env);
>>>         ppcemb_timer->wdt_timer =
>>>             qemu_new_timer(vm_clock, &cpu_4xx_wdt_cb, env);
>>> +        ppcemb_timer->decr_excp = PPC_INTERRUPT_PIT;
>>> 
>> 
>> If anything, it should be a parameter here.
> 
> ... an argument to ppc_emb_timers_init and ppc_emb_timers_set_decr_excp
> can go away. Does it sound good enough?

Perfect, yeah :)

Alex

> 
> Thanks

Patch

diff --git a/hw/ppc.c b/hw/ppc.c
index 55e3808..3df7801 100644
--- a/hw/ppc.c
+++ b/hw/ppc.c
@@ -769,6 +769,9 @@  struct ppcemb_timer_t {
     struct QEMUTimer *fit_timer;
     uint64_t wdt_next;    /* Tick for next WDT interrupt  */
     struct QEMUTimer *wdt_timer;
+
+    /* 405 have the PIT, 440 have a DECR.  */
+    unsigned int decr_excp;
 };
 
 /* Fixed interval timer */
@@ -851,7 +854,7 @@  static void cpu_4xx_pit_cb (void *opaque)
     ppcemb_timer = tb_env->opaque;
     env->spr[SPR_40x_TSR] |= 1 << 27;
     if ((env->spr[SPR_40x_TCR] >> 26) & 0x1)
-        ppc_set_irq(env, PPC_INTERRUPT_PIT, 1);
+        ppc_set_irq(env, ppcemb_timer->decr_excp, 1);
     start_stop_pit(env, tb_env, 1);
     LOG_TB("%s: ar %d ir %d TCR " TARGET_FMT_lx " TSR " TARGET_FMT_lx " "
            "%016" PRIx64 "\n", __func__,
@@ -948,10 +951,15 @@  target_ulong load_40x_pit (CPUState *env)
 
 void store_booke_tsr (CPUState *env, target_ulong val)
 {
+    ppc_tb_t *tb_env = env->tb_env;
+    ppcemb_timer_t *ppcemb_timer;
+
+    ppcemb_timer = tb_env->opaque;
+
     LOG_TB("%s: val " TARGET_FMT_lx "\n", __func__, val);
     env->spr[SPR_40x_TSR] &= ~(val & 0xFC000000);
     if (val & 0x80000000)
-        ppc_set_irq(env, PPC_INTERRUPT_PIT, 0);
+        ppc_set_irq(env, ppcemb_timer->decr_excp, 0);
 }
 
 void store_booke_tcr (CPUState *env, target_ulong val)
@@ -977,6 +985,15 @@  static void ppc_emb_set_tb_clk (void *opaque, uint32_t freq)
     /* XXX: we should also update all timers */
 }
 
+void ppc_emb_timers_set_decr_excp(CPUState *env, unsigned int excp)
+{
+    ppc_tb_t *tb_env = env->tb_env;
+    ppcemb_timer_t *ppcemb_timer;
+
+    ppcemb_timer = tb_env->opaque;
+    ppcemb_timer->decr_excp = excp;
+}
+
 clk_setup_cb ppc_emb_timers_init (CPUState *env, uint32_t freq)
 {
     ppc_tb_t *tb_env;
@@ -996,6 +1013,7 @@  clk_setup_cb ppc_emb_timers_init (CPUState *env, uint32_t freq)
             qemu_new_timer(vm_clock, &cpu_4xx_fit_cb, env);
         ppcemb_timer->wdt_timer =
             qemu_new_timer(vm_clock, &cpu_4xx_wdt_cb, env);
+        ppcemb_timer->decr_excp = PPC_INTERRUPT_PIT;
     }
 
     return &ppc_emb_set_tb_clk;
diff --git a/hw/ppc.h b/hw/ppc.h
index 1251932..a0fc651 100644
--- a/hw/ppc.h
+++ b/hw/ppc.h
@@ -20,6 +20,8 @@  int ppc_dcr_init (CPUState *env, int (*dcr_read_error)(int dcrn),
 int ppc_dcr_register (CPUState *env, int dcrn, void *opaque,
                       dcr_read_cb drc_read, dcr_write_cb dcr_write);
 clk_setup_cb ppc_emb_timers_init (CPUState *env, uint32_t freq);
+void ppc_emb_timers_set_decr_excp(CPUState *env, unsigned int excp);
+
 /* Embedded PowerPC reset */
 void ppc40x_core_reset (CPUState *env);
 void ppc40x_chip_reset (CPUState *env);