On 24/09/2021 10:01, Philippe Mathieu-Daudé wrote:
On 9/24/21 09:06, Mark Cave-Ayland wrote:
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-ayl...@ilande.co.uk>
Reviewed-by: Laurent Vivier <laur...@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é <f4...@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...
But devices accessing the bus are not aware of which devices are plugged
onto it. Wait, what is suppose to call nubus_set_irq()? Devices on the
bus, to propagate the interrupt up to the CPU?
Yes indeed, that is correct.
OK so then the trace
event is irrelevant indeed. I understood this API as any external device
could trigger an IRQ to device on the bus. I wonder if renaming as
nubus_device_set_irq() could make it clearer. Or even simpler, add
a comment in "hw/nubus/nubus.h" explaining what nubus_set_irq() is for
to avoid any confusion, and we are good.
The function name and signature nubus_set_irq(NubusDevice *nd, int level) was chosen
because it was intended to be the Nubus equivalent of PCI's pci_set_irq(PCIDevice *d,
int level) where the first parameter of both functions nicely indicates that this is
to be called by the device. I don't think we've had any reports of confusion with the
existing pci_set_irq() function usage?
Another thing that makes me less worried is that the next series after this contains
a number of changes for the macfb device, including the addition of VBL interrupts
implemented using nubus_set_irq() so at that point there will be a grep-able usage
example in the codebase.
ATB,
Mark.
