[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 27/37] leon3: use qdev gpio facilities for the PIL
From: |
Marc-André Lureau |
Subject: |
Re: [PATCH v4 27/37] leon3: use qdev gpio facilities for the PIL |
Date: |
Thu, 19 Dec 2019 16:44:00 +0400 |
On Sun, Dec 15, 2019 at 10:10 AM Philippe Mathieu-Daudé
<address@hidden> wrote:
>
> On 11/21/19 2:51 PM, Peter Maydell wrote:
> > On Wed, 20 Nov 2019 at 15:30, Marc-André Lureau
> > <address@hidden> wrote:
> >>
> >> Signed-off-by: Marc-André Lureau <address@hidden>
> >> ---
> >> 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 <address@hidden>
> >
> > 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é <address@hidden>
good catch (it was actually already fixed in my branch ;)
thanks