Re: [PATCH v5 18/20] nubus: add support for slot IRQs

[Mark Cave-Ayland]

On 23/09/2021 10:49, Philippe Mathieu-Daudé wrote:

> On 9/23/21 11:13, Mark Cave-Ayland wrote:
> > Each Nubus slot has an IRQ line that can be used to request service from the
> > CPU. Connect the IRQs to the Nubus bridge so that they can be wired up using 
> > qdev
> > gpios accordingly, and introduce a new nubus_set_irq() function that can be used
> > by Nubus devices to control the slot IRQ.
> >
> > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> > Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> > ---
> >   hw/nubus/nubus-bridge.c  | 2 ++
> >   hw/nubus/nubus-device.c  | 8 ++++++++
> >   include/hw/nubus/nubus.h | 6 ++++++
> >   3 files changed, 16 insertions(+)
>
> >   static Property nubus_bridge_properties[] = {
> > diff --git a/hw/nubus/nubus-device.c b/hw/nubus/nubus-device.c
> > index 280f40e88a..0f1852f671 100644
> > --- a/hw/nubus/nubus-device.c
> > +++ b/hw/nubus/nubus-device.c
> > @@ -10,12 +10,20 @@
> >   #include "qemu/osdep.h"
> >   #include "qemu/datadir.h"
> > +#include "hw/irq.h"
> >   #include "hw/loader.h"
> >   #include "hw/nubus/nubus.h"
> >   #include "qapi/error.h"
> >   #include "qemu/error-report.h"
> > +void nubus_set_irq(NubusDevice *nd, int level)
> > +{
> > +    NubusBus *nubus = NUBUS_BUS(qdev_get_parent_bus(DEVICE(nd)));
> > +
>
> A trace-event could be helpful here, otherwise:
>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
> > +    qemu_set_irq(nubus->irqs[nd->slot], level);
> > +}

I think adding a trace event here would just be too verbose (particularly if you have 
more than one nubus device) and not particularly helpful: normally the part you want 
to debug is the in the device itself looking at which constituent flags combine to 
raise/lower the interrupt line. And once you've done that then you've already got a 
suitable trace-event in place...


ATB,

Mark.