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

[Laurent Vivier]

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-ayland@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 <laurent@vivier.eu>