On 9/24/21 09:16, Mark Cave-Ayland wrote:
On 23/09/2021 15:16, BALATON Zoltan wrote:
On Thu, 23 Sep 2021, Mark Cave-Ayland wrote:
Convert nubus_device_realize() to use a bitmap to manage available
slots to allow
for future Nubus devices to be plugged into arbitrary slots from the
command line
using a new qdev "slot" parameter for nubus devices.
Update mac_nubus_bridge_init() to only allow slots 0x9 to 0xe on a
Macintosh
machines as documented in "Desigining Cards and Drivers for the
Macintosh Family".
Small typo: "a Macintosh machnies", either a or s is not needed.
Thanks - I've updated this for v6.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk>
---
hw/nubus/mac-nubus-bridge.c | 4 ++++
hw/nubus/nubus-bus.c | 5 +++--
hw/nubus/nubus-device.c | 32 +++++++++++++++++++++++------
include/hw/nubus/mac-nubus-bridge.h | 4 ++++
include/hw/nubus/nubus.h | 13 ++++++------
5 files changed, 43 insertions(+), 15 deletions(-)
struct NubusDevice {
DeviceState qdev;
- int slot;
+ int32_t slot;
Why uint32_t? Considering its max value even uint8_t would be enough
although maybe invalid value would be 255 instead of -1 then. As this
was added in previous patch you could avoid churn here by introducing
it with the right type in that patch already. (But feel free to ignore
it if you have no time for more changes, the current version works so
if you don't do another version for other reasons this probably don't
worth the effort alone.)
I think it makes sense to keep this signed since -1 is used for other
bus implementations to indicate that an explicit slot hasn't been
assigned. Potentially the slot number could be represented by an 8-bit
value, however it seems there is no DEFINE_PROP_INT8 or
DEFINE_PROP_INT16. Fortunately the slot number is restricted by the
available slots bitmask anyhow, so this shouldn't be an issue.
I wondered the same and noticed there is no DEFINE_PROP_INT8, so didn't
want to bother Mark furthermore :) Adding & using DEFINE_PROP_INT8 seems
a good idea, but to be fair with the repository we'd need to audit the
other places where DEFINE_PROP_INT32 isn't justified and update. Extra
work for not much gain, so I'm find with this patch. Can be improved on
top.