Am 20.09.2010 um 14:11 schrieb "Edgar E. Iglesias" <edgar.igles...@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.igles...@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
