| Message ID | 20191120152442.26657-28-marcandre.lureau@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Clean-ups: qom-ify serial and remove QDEV_PROP_PTR | expand |
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. thanks -- PMM
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>
On Sun, Dec 15, 2019 at 10:10 AM Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > > 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> good catch (it was actually already fixed in my branch ;) thanks
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; diff --git a/target/sparc/cpu.h b/target/sparc/cpu.h index e70fec0133..ae97c7d9f7 100644 --- a/target/sparc/cpu.h +++ b/target/sparc/cpu.h @@ -541,7 +541,6 @@ struct CPUSPARCState { #endif sparc_def_t def; - qemu_irq pil_irq; void *irq_manager; void (*qemu_irq_ack)(CPUSPARCState *env, void *irq_manager, int intno);
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(-)