Re: [PATCH v4 27/37] leon3: use qdev gpio facilities for the PIL

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 11/21/19 2:51 PM, Peter Maydell wrote:
> On Wed, 20 Nov 2019 at 15:30, Marc-André Lureau
> <marcandre.lureau@redhat.com> wrote:
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> ---
>>   hw/sparc/leon3.c   | 6 ++++--
>>   target/sparc/cpu.h | 1 -
>>   2 files changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
>> index cac987373e..1a89d44e57 100644
>> --- a/hw/sparc/leon3.c
>> +++ b/hw/sparc/leon3.c
>> @@ -230,8 +230,10 @@ static void leon3_generic_hw_init(MachineState *machine)
>>
>>       /* Allocate IRQ manager */
>>       dev = qdev_create(NULL, TYPE_GRLIB_IRQMP);
>> -    env->pil_irq = qemu_allocate_irq(leon3_set_pil_in, env, 0);
>> -    qdev_connect_gpio_out_named(dev, "grlib-irq", 0, env->pil_irq);
>> +    qdev_init_gpio_in_named_with_opaque(DEVICE(env), leon3_set_pil_in,
>> +                                        env, "pil", 1);
>> +    qdev_connect_gpio_out_named(dev, "grlib-irq", 0,
>> +                                qdev_get_gpio_in_named(DEVICE(env), "pil", 0));
>>       qdev_init_nofail(dev);
>>       sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, LEON3_IRQMP_OFFSET);
>>       env->irq_manager = dev;
> 
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> 
> Creating a gpio pin on some object that isn't yourself
> looks a bit odd, but all this leon3 code is modifying
> the CPU object from the outside anyway. Someday we might
> tidy it up, but not today.

Watch out, while SPARCCPU inherits DeviceState from CPUState,
env does not: it is not QDEV but a CPUSPARCState object.

If you replace both DEVICE(env) uses by DEVICE(cpu), your patch looks safe.

If you agree with the change:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>