| Message ID | 20191018154212.13458-7-marcandre.lureau@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Clean-ups: remove QDEV_PROP_PTR | expand |
On Fri, 18 Oct 2019 at 16:43, Marc-André Lureau <marcandre.lureau@redhat.com> wrote: > > "set_pil_in" and "set_pil_in" are used to set a callback, but have a > single user and cannot be modified by the user. > > Simplify the code by calling directly into leon3_set_pil_in(), and use > a "cpu" link property. > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > hw/intc/grlib_irqmp.c | 20 ++++++-------------- > hw/sparc/leon3.c | 7 +++---- > target/sparc/cpu.h | 1 + > 3 files changed, 10 insertions(+), 18 deletions(-) > > diff --git a/hw/intc/grlib_irqmp.c b/hw/intc/grlib_irqmp.c > index bc78e1a14f..34b7e1b4e1 100644 > --- a/hw/intc/grlib_irqmp.c > +++ b/hw/intc/grlib_irqmp.c > @@ -58,10 +58,8 @@ typedef struct IRQMP { > > MemoryRegion iomem; > > - void *set_pil_in; > - void *set_pil_in_opaque; > - > IRQMPState *state; > + SPARCCPU *cpu; > } IRQMP; > > struct IRQMPState { > @@ -82,7 +80,6 @@ static void grlib_irqmp_check_irqs(IRQMPState *state) > uint32_t pend = 0; > uint32_t level0 = 0; > uint32_t level1 = 0; > - set_pil_in_fn set_pil_in; > > assert(state != NULL); > assert(state->parent != NULL); > @@ -97,13 +94,11 @@ static void grlib_irqmp_check_irqs(IRQMPState *state) > trace_grlib_irqmp_check_irqs(state->pending, state->force[0], > state->mask[0], level1, level0); > > - set_pil_in = (set_pil_in_fn)state->parent->set_pil_in; > - > /* Trigger level1 interrupt first and level0 if there is no level1 */ > if (level1 != 0) { > - set_pil_in(state->parent->set_pil_in_opaque, level1); > + leon3_set_pil_in(state->parent->cpu, level1); > } else { > - set_pil_in(state->parent->set_pil_in_opaque, level0); > + leon3_set_pil_in(state->parent->cpu, level0); > } > } > > @@ -348,14 +343,13 @@ static void grlib_irqmp_realize(DeviceState *dev, Error **errp) > IRQMP *irqmp = GRLIB_IRQMP(dev); > > /* Check parameters */ > - if (irqmp->set_pil_in == NULL) { > - error_setg(errp, "set_pil_in cannot be NULL."); > + if (irqmp->cpu == NULL) { > + error_setg(errp, "cpu cannot be NULL."); > } > } > static Property grlib_irqmp_properties[] = { > - DEFINE_PROP_PTR("set_pil_in", IRQMP, set_pil_in), > - DEFINE_PROP_PTR("set_pil_in_opaque", IRQMP, set_pil_in_opaque), > + DEFINE_PROP_LINK("cpu", IRQMP, cpu, TYPE_SPARC_CPU, SPARCCPU *), > DEFINE_PROP_END_OF_LIST(), > }; This is using ptr properties to define a callback mechanism where the device says "call the callback function, passing it an opaque cookie and a 32-bit value". We already have a generic mechanism for doing that, which is the qemu_irq. So we should just use that instead of adding a link property between the interrupt controller and the CPU. (Bonus further cleanup: the code currently in leon3_set_pil_in() should probably be part of the SPARC CPU object itself, as a handler for an inbound gpio line, as then you could just wire the qemu_irq from the interrupt controller to the CPU. But you can leave it as ad-hoc code in leon3.c for now.) thanks -- PMM
diff --git a/hw/intc/grlib_irqmp.c b/hw/intc/grlib_irqmp.c index bc78e1a14f..34b7e1b4e1 100644 --- a/hw/intc/grlib_irqmp.c +++ b/hw/intc/grlib_irqmp.c @@ -58,10 +58,8 @@ typedef struct IRQMP { MemoryRegion iomem; - void *set_pil_in; - void *set_pil_in_opaque; - IRQMPState *state; + SPARCCPU *cpu; } IRQMP; struct IRQMPState { @@ -82,7 +80,6 @@ static void grlib_irqmp_check_irqs(IRQMPState *state) uint32_t pend = 0; uint32_t level0 = 0; uint32_t level1 = 0; - set_pil_in_fn set_pil_in; assert(state != NULL); assert(state->parent != NULL); @@ -97,13 +94,11 @@ static void grlib_irqmp_check_irqs(IRQMPState *state) trace_grlib_irqmp_check_irqs(state->pending, state->force[0], state->mask[0], level1, level0); - set_pil_in = (set_pil_in_fn)state->parent->set_pil_in; - /* Trigger level1 interrupt first and level0 if there is no level1 */ if (level1 != 0) { - set_pil_in(state->parent->set_pil_in_opaque, level1); + leon3_set_pil_in(state->parent->cpu, level1); } else { - set_pil_in(state->parent->set_pil_in_opaque, level0); + leon3_set_pil_in(state->parent->cpu, level0); } } @@ -348,14 +343,13 @@ static void grlib_irqmp_realize(DeviceState *dev, Error **errp) IRQMP *irqmp = GRLIB_IRQMP(dev); /* Check parameters */ - if (irqmp->set_pil_in == NULL) { - error_setg(errp, "set_pil_in cannot be NULL."); + if (irqmp->cpu == NULL) { + error_setg(errp, "cpu cannot be NULL."); } } static Property grlib_irqmp_properties[] = { - DEFINE_PROP_PTR("set_pil_in", IRQMP, set_pil_in), - DEFINE_PROP_PTR("set_pil_in_opaque", IRQMP, set_pil_in_opaque), + DEFINE_PROP_LINK("cpu", IRQMP, cpu, TYPE_SPARC_CPU, SPARCCPU *), DEFINE_PROP_END_OF_LIST(), }; @@ -365,8 +359,6 @@ static void grlib_irqmp_class_init(ObjectClass *klass, void *data) dc->reset = grlib_irqmp_reset; dc->props = grlib_irqmp_properties; - /* Reason: pointer properties "set_pil_in", "set_pil_in_opaque" */ - dc->user_creatable = false; dc->realize = grlib_irqmp_realize; } diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c index c5f1b1ee72..fa32936ca4 100644 --- a/hw/sparc/leon3.c +++ b/hw/sparc/leon3.c @@ -143,9 +143,9 @@ void leon3_irq_ack(void *irq_manager, int intno) grlib_irqmp_ack((DeviceState *)irq_manager, intno); } -static void leon3_set_pil_in(void *opaque, uint32_t pil_in) +void leon3_set_pil_in(SPARCCPU *cpu, uint32_t pil_in) { - CPUSPARCState *env = (CPUSPARCState *)opaque; + CPUSPARCState *env = &cpu->env; CPUState *cs; assert(env != NULL); @@ -225,8 +225,7 @@ static void leon3_generic_hw_init(MachineState *machine) /* Allocate IRQ manager */ dev = qdev_create(NULL, TYPE_GRLIB_IRQMP); - qdev_prop_set_ptr(dev, "set_pil_in", leon3_set_pil_in); - qdev_prop_set_ptr(dev, "set_pil_in_opaque", env); + object_property_set_link(OBJECT(dev), OBJECT(cpu), "cpu", &error_abort); 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 778aa8e073..5f8e6ec6e8 100644 --- a/target/sparc/cpu.h +++ b/target/sparc/cpu.h @@ -592,6 +592,7 @@ void cpu_check_irqs(CPUSPARCState *env); /* leon3.c */ void leon3_irq_ack(void *irq_manager, int intno); +void leon3_set_pil_in(SPARCCPU *cpu, uint32_t pil_in); #if defined (TARGET_SPARC64)
"set_pil_in" and "set_pil_in" are used to set a callback, but have a single user and cannot be modified by the user. Simplify the code by calling directly into leon3_set_pil_in(), and use a "cpu" link property. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- hw/intc/grlib_irqmp.c | 20 ++++++-------------- hw/sparc/leon3.c | 7 +++---- target/sparc/cpu.h | 1 + 3 files changed, 10 insertions(+), 18 deletions(-)