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? 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.
