Re: [Qemu-devel] [PATCH v3 5/6] hw/core: explicit OFW unit address property for SysBusDevice

[Marcel Apfelbaum <marcel@redhat.com>]

On 06/11/2015 03:38 AM, Laszlo Ersek wrote:
> The sysbus_get_fw_dev_path() function formats OpenFirmware device path
> nodes ("driver-name@unit-address") for sysbus devices. The first choice
> for "unit-address" is the base address of the device's first MMIO region.
> The second choice is its first IO port.
>
> However, if two sysbus devices with the same "driver-name" lack both MMIO
> and PIO resources, then there is no good way to distinguish them based on
> their OFW nodes, because in this case unit-address is omitted completely
> for both devices.
>
> For such devices, delegate a fallback property,
> "explicit_ofw_unit_address", to whoever creates the device. The creator
> can set the property to any appropriate value.
>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>
> Notes:
>      v3:
>      - new in v3
>      - new approach
>
>   include/hw/sysbus.h |  8 ++++++++
>   hw/core/sysbus.c    | 11 +++++++++++
>   2 files changed, 19 insertions(+)
>
> diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h
> index d1f3f00..de41b06 100644
> --- a/include/hw/sysbus.h
> +++ b/include/hw/sysbus.h
> @@ -55,6 +55,14 @@ struct SysBusDevice {
>       } mmio[QDEV_MAX_MMIO];
>       int num_pio;
>       pio_addr_t pio[QDEV_MAX_PIO];
> +
> +    /*
> +     * Sometimes a SysBusDevice has neither MMIO nor PIO resources, yet it
> +     * would like to distinguish itself, in OpenFirmware device paths, from
> +     * other instances of the same class on the same sysbus. For that end we
> +     * expose this property.
> +     */
> +    char *explicit_ofw_unit_address;
>   };
>
>   typedef int FindSysbusDeviceFunc(SysBusDevice *sbdev, void *opaque);
> diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
> index 0ebb4e2..6e9af1c 100644
> --- a/hw/core/sysbus.c
> +++ b/hw/core/sysbus.c
> @@ -289,6 +289,10 @@ static char *sysbus_get_fw_dev_path(DeviceState *dev)
>       if (s->num_pio) {
>           return g_strdup_printf("%s@i%04x", qdev_fw_name(dev), s->pio[0]);
>       }
> +    if (s->explicit_ofw_unit_address) {
> +        return g_strdup_printf("%s@%s", qdev_fw_name(dev),
> +                               s->explicit_ofw_unit_address);
> +    }
>       return g_strdup(qdev_fw_name(dev));
>   }
>
> @@ -303,11 +307,18 @@ MemoryRegion *sysbus_address_space(SysBusDevice *dev)
>       return get_system_memory();
>   }
>
> +static Property sysbus_device_properties[] = {
> +    DEFINE_PROP_STRING("explicit_ofw_unit_address", SysBusDevice,
> +                       explicit_ofw_unit_address),
> +    DEFINE_PROP_END_OF_LIST()
> +};
> +
>   static void sysbus_device_class_init(ObjectClass *klass, void *data)
>   {
>       DeviceClass *k = DEVICE_CLASS(klass);
>       k->init = sysbus_device_init;
>       k->bus_type = TYPE_SYSTEM_BUS;
> +    k->props = sysbus_device_properties;
>   }
>
>   static const TypeInfo sysbus_device_type_info = {
>
Better than previous approach.

Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>