[qom-next,42/59] xtensa_pic: Pass XtensaCPU to xtensa_ccompare_cb()

Needed for cpu_has_work().

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/xtensa_pic.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

Am 23.05.2012 05:08, schrieb Andreas Färber:
> Needed for cpu_has_work().
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>

Max, could you ack this trivial patch please? It still applies.

I notice that you were originally not cc'ed: Probably this file was/is
not yet documented in MAINTAINERS.

Thanks,
Andreas

> ---
>  hw/xtensa_pic.c |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/xtensa_pic.c b/hw/xtensa_pic.c
> index 653ded6..8b9c051 100644
> --- a/hw/xtensa_pic.c
> +++ b/hw/xtensa_pic.c
> @@ -125,7 +125,8 @@ void xtensa_rearm_ccompare_timer(CPUXtensaState *env)
>  
>  static void xtensa_ccompare_cb(void *opaque)
>  {
> -    CPUXtensaState *env = opaque;
> +    XtensaCPU *cpu = opaque;
> +    CPUXtensaState *env = &cpu->env;
>  
>      if (env->halted) {
>          env->halt_clock = qemu_get_clock_ns(vm_clock);
> @@ -139,12 +140,14 @@ static void xtensa_ccompare_cb(void *opaque)
>  
>  void xtensa_irq_init(CPUXtensaState *env)
>  {
> +    XtensaCPU *cpu = xtensa_env_get_cpu(env);
> +
>      env->irq_inputs = (void **)qemu_allocate_irqs(
>              xtensa_set_irq, env, env->config->ninterrupt);
>      if (xtensa_option_enabled(env->config, XTENSA_OPTION_TIMER_INTERRUPT) &&
>              env->config->nccompare > 0) {
>          env->ccompare_timer =
> -            qemu_new_timer_ns(vm_clock, &xtensa_ccompare_cb, env);
> +            qemu_new_timer_ns(vm_clock, &xtensa_ccompare_cb, cpu);
>      }
>  }
>

On Wed, Oct 10, 2012 at 7:15 PM, Andreas Färber <afaerber@suse.de> wrote:
> Am 23.05.2012 05:08, schrieb Andreas Färber:
>> Needed for cpu_has_work().
>>
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>
> Max, could you ack this trivial patch please? It still applies.

Well, it does but why do you want to add a level of indirection here?
Does that mean that cpu->env may change during cpu lifetime?
Commit message is not very helpful here.

> I notice that you were originally not cc'ed: Probably this file was/is
> not yet documented in MAINTAINERS.
>
> Thanks,
> Andreas
>
>> ---
>>  hw/xtensa_pic.c |    7 +++++--
>>  1 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/xtensa_pic.c b/hw/xtensa_pic.c
>> index 653ded6..8b9c051 100644
>> --- a/hw/xtensa_pic.c
>> +++ b/hw/xtensa_pic.c
>> @@ -125,7 +125,8 @@ void xtensa_rearm_ccompare_timer(CPUXtensaState *env)
>>
>>  static void xtensa_ccompare_cb(void *opaque)
>>  {
>> -    CPUXtensaState *env = opaque;
>> +    XtensaCPU *cpu = opaque;
>> +    CPUXtensaState *env = &cpu->env;
>>
>>      if (env->halted) {
>>          env->halt_clock = qemu_get_clock_ns(vm_clock);
>> @@ -139,12 +140,14 @@ static void xtensa_ccompare_cb(void *opaque)
>>
>>  void xtensa_irq_init(CPUXtensaState *env)
>>  {
>> +    XtensaCPU *cpu = xtensa_env_get_cpu(env);
>> +
>>      env->irq_inputs = (void **)qemu_allocate_irqs(
>>              xtensa_set_irq, env, env->config->ninterrupt);
>>      if (xtensa_option_enabled(env->config, XTENSA_OPTION_TIMER_INTERRUPT) &&
>>              env->config->nccompare > 0) {
>>          env->ccompare_timer =
>> -            qemu_new_timer_ns(vm_clock, &xtensa_ccompare_cb, env);
>> +            qemu_new_timer_ns(vm_clock, &xtensa_ccompare_cb, cpu);
>>      }
>>  }
>>
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>

Am 10.10.2012 17:35, schrieb Max Filippov:
> On Wed, Oct 10, 2012 at 7:15 PM, Andreas Färber <afaerber@suse.de> wrote:
>> Am 23.05.2012 05:08, schrieb Andreas Färber:
>>> Needed for cpu_has_work().
>>>
>>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>>
>> Max, could you ack this trivial patch please? It still applies.
> 
> Well, it does but why do you want to add a level of indirection here?
> Does that mean that cpu->env may change during cpu lifetime?
> Commit message is not very helpful here.

Patch 43/59 in that series updates cpu_has_work() argument to CPUState*,
thus this patch prepares xtensa_pic as one of the callers.

This is the only xtensa patch I have in my queue, so I wanted to avoid
hitting you with a large resend.

I see now that xtensa_set_irq could get an XtensaCPU opaque as well, but
so far there was no need apparently, so that can be changed later.

For target-specific code my general rule of thumb is, use FooCPU rather
than CPUFooState arguments and opaques wherever possible since the need
is growing. Also, any new fields that are not accessed by TCG should be
placed into FooCPU rather than CPUFooState.

Andreas

>>> @@ -139,12 +140,14 @@ static void xtensa_ccompare_cb(void *opaque)
>>>
>>>  void xtensa_irq_init(CPUXtensaState *env)
>>>  {
>>> +    XtensaCPU *cpu = xtensa_env_get_cpu(env);
>>> +
>>>      env->irq_inputs = (void **)qemu_allocate_irqs(
>>>              xtensa_set_irq, env, env->config->ninterrupt);
>>>      if (xtensa_option_enabled(env->config, XTENSA_OPTION_TIMER_INTERRUPT) &&
>>>              env->config->nccompare > 0) {
>>>          env->ccompare_timer =
>>> -            qemu_new_timer_ns(vm_clock, &xtensa_ccompare_cb, env);
>>> +            qemu_new_timer_ns(vm_clock, &xtensa_ccompare_cb, cpu);
>>>      }
>>>  }

On Wed, Oct 10, 2012 at 8:33 PM, Andreas Färber <afaerber@suse.de> wrote:
> Am 10.10.2012 17:35, schrieb Max Filippov:
>> On Wed, Oct 10, 2012 at 7:15 PM, Andreas Färber <afaerber@suse.de> wrote:
>>> Am 23.05.2012 05:08, schrieb Andreas Färber:
>>>> Needed for cpu_has_work().
>>>>
>>>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>>>
>>> Max, could you ack this trivial patch please? It still applies.
>>
>> Well, it does but why do you want to add a level of indirection here?
>> Does that mean that cpu->env may change during cpu lifetime?
>> Commit message is not very helpful here.
>
> Patch 43/59 in that series updates cpu_has_work() argument to CPUState*,
> thus this patch prepares xtensa_pic as one of the callers.
>
> This is the only xtensa patch I have in my queue, so I wanted to avoid
> hitting you with a large resend.
>
> I see now that xtensa_set_irq could get an XtensaCPU opaque as well, but
> so far there was no need apparently, so that can be changed later.
>
> For target-specific code my general rule of thumb is, use FooCPU rather
> than CPUFooState arguments and opaques wherever possible since the need
> is growing. Also, any new fields that are not accessed by TCG should be
> placed into FooCPU rather than CPUFooState.

Ok,
Acked-by: Max Filippov <jcmvbkbc@gmail.com>