Re: [PATCH v4 05/20] nubus: move slot bitmap checks from NubusDevice realize() to BusClass check_address()

[Mark Cave-Ayland]

On 20/09/2021 20:56, Laurent Vivier wrote:

Le 17/09/2021 à 09:50, Mark Cave-Ayland a écrit :

Allow Nubus to manage the slot allocations itself using the BusClass 
check_address()
virtual function rather than managing this during NubusDevice realize().

Signed-off-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk>
---
  hw/nubus/nubus-bus.c    | 37 +++++++++++++++++++++++++++++++++++++
  hw/nubus/nubus-device.c | 29 -----------------------------
  2 files changed, 37 insertions(+), 29 deletions(-)

diff --git a/hw/nubus/nubus-bus.c b/hw/nubus/nubus-bus.c
index 404c1032e0..141876b579 100644
--- a/hw/nubus/nubus-bus.c
+++ b/hw/nubus/nubus-bus.c
@@ -87,11 +87,48 @@ static void nubus_init(Object *obj)
      nubus->slot_available_mask = MAKE_64BIT_MASK(0, 16);
  }

+static bool nubus_check_address(BusState *bus, DeviceState *dev, Error **errp)

+{
+    NubusDevice *nd = NUBUS_DEVICE(dev);
+    NubusBus *nubus = NUBUS_BUS(bus);
+    uint16_t s;
+
+    if (nd->slot == -1) {
+        /* No slot specified, find first available free slot */
+        s = ctz32(nubus->slot_available_mask);
+        if (s != 32) {
+            nd->slot = s;
+        } else {
+            error_setg(errp, "Cannot register nubus card, no free slot "
+                             "available");
+            return false;
+        }
+    } else {
+        /* Slot specified, make sure the slot is available */
+        if (!(nubus->slot_available_mask & BIT(nd->slot))) {
+            error_setg(errp, "Cannot register nubus card, slot %d is "
+                             "unavailable or already occupied", nd->slot);
+            return false;
+        }
+    }
+
+    if (nd->slot < NUBUS_FIRST_SLOT || nd->slot > NUBUS_LAST_SLOT) {
+        error_setg(errp, "Cannot register nubus card, slot must be "
+                         "between %d and %d", NUBUS_FIRST_SLOT,
+                         NUBUS_LAST_SLOT);
+        return false;
+    }
+
+    nubus->slot_available_mask &= ~BIT(nd->slot);
+    return true;
+}
+
  static void nubus_class_init(ObjectClass *oc, void *data)
  {
      BusClass *bc = BUS_CLASS(oc);

bc->realize = nubus_realize;

+    bc->check_address = nubus_check_address;
  }

static const TypeInfo nubus_bus_info = {

diff --git a/hw/nubus/nubus-device.c b/hw/nubus/nubus-device.c
index d91a1e4af3..7a32c8c95b 100644
--- a/hw/nubus/nubus-device.c
+++ b/hw/nubus/nubus-device.c
@@ -160,35 +160,6 @@ static void nubus_device_realize(DeviceState *dev, Error 
**errp)
      NubusDevice *nd = NUBUS_DEVICE(dev);
      char *name;
      hwaddr slot_offset;
-    uint16_t s;
-
-    if (nd->slot == -1) {
-        /* No slot specified, find first available free slot */
-        s = ctz32(nubus->slot_available_mask);
-        if (s != 32) {
-            nd->slot = s;
-        } else {
-            error_setg(errp, "Cannot register nubus card, no free slot "
-                             "available");
-            return;
-        }
-    } else {
-        /* Slot specified, make sure the slot is available */
-        if (!(nubus->slot_available_mask & BIT(nd->slot))) {
-            error_setg(errp, "Cannot register nubus card, slot %d is "
-                             "unavailable or already occupied", nd->slot);
-            return;
-        }
-    }
-
-    if (nd->slot < NUBUS_FIRST_SLOT || nd->slot > NUBUS_LAST_SLOT) {
-        error_setg(errp, "Cannot register nubus card, slot must be "
-                         "between %d and %d", NUBUS_FIRST_SLOT,
-                         NUBUS_LAST_SLOT);
-        return;
-    }
-
-    nubus->slot_available_mask &= ~BIT(nd->slot);

/* Super */

      slot_offset = (nd->slot - 6) * NUBUS_SUPER_SLOT_SIZE;

For the code move (but I think it needs some update in patch 4):

Reviewed-by: Laurent Vivier <laur...@vivier.eu>

Yes, that will be resolved by rebasing onto an updated patch 2.


ATB,

Mark.