Re: [Qemu-devel] [RFC PATCH 1/3] hw/vfio/sysbus-fdt: generic add_generic_fdt_node

[Eric Auger]

On 12/17/2014 11:09 AM, Baptiste Reynal wrote:
> Modify add_calxeda_midway_xgmac_fdt_node to make it more generic.
> Add multiple compatible strings support.
Hi Baptiste,

Although I understand you may be tempted to transform the Calxeda basic
node creation function into something more generic, we came to the
current result after long discussions with Alex Graf. This generic
function was something released in the past but transformed into
something specific at the end.

Typically the fact the IRQ is edge sensitive or level sensitive may
depend on the device and it would be arbitrary here to choose either in
a "generic" node creation.

So the current trend for new devices is to add another entry in the
add_fdt_node_functions array; genericity target was argued in the past
and rejected.

If you want to do something going in the direction of genericity I would
advise you to investigate using Antonios API ([RFC 0/4] VFIO: PLATFORM:
Return device tree info for a platform device node). This could
typically enable to get the egde/level sensitive characteritics.

> 
> Signed-off-by: Baptiste Reynal <address@hidden>
> ---
>  hw/arm/sysbus-fdt.c | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
> index 15bb50c..125ba37 100644
> --- a/hw/arm/sysbus-fdt.c
> +++ b/hw/arm/sysbus-fdt.c
> @@ -58,12 +58,12 @@ typedef struct NodeCreationPair {
>  /* Device Specific Code */
>  
>  /**
> - * add_calxeda_midway_xgmac_fdt_node
> + * add_device_fdt_node
>   *
>   * Generates a very simple node with following properties:
>   * compatible string, regs, interrupts
>   */
> -static int add_calxeda_midway_xgmac_fdt_node(SysBusDevice *sbdev, void 
> *opaque)
> +static int add_generic_fdt_node(SysBusDevice *sbdev, void *opaque)
>  {
>      PlatformBusFdtData *data = opaque;
>      PlatformBusDevice *pbus = data->pbus;
> @@ -80,6 +80,18 @@ static int add_calxeda_midway_xgmac_fdt_node(SysBusDevice 
> *sbdev, void *opaque)
>      VFIODevice *vbasedev = &vdev->vbasedev;
>      Object *obj = OBJECT(sbdev);
>  
> +    /*
> +     * Process compatible string to deal with multiple strings
> +     * (; is replaced by \0)
> +     */
> +    char *compat = g_strdup(vdev->compat);
> +    compat_str_len = strlen(compat) + 1;
> +
> +    char *semicolon = compat;
> +    while ((semicolon = strchr(semicolon, ';')) != NULL) {
> +        *semicolon = '\0';
> +    }
why can't you format the string directly in the corresponding VFIO AMBA
device? Formerly we had this problem of conversion since we passed the
format string in qemu command line.

Best Regards

Eric
> +
>      mmio_base = object_property_get_int(obj, "mmio[0]", NULL);
>  
>      nodename = g_strdup_printf("%s/address@hidden" PRIx64, parent_node,
> @@ -88,9 +100,8 @@ static int add_calxeda_midway_xgmac_fdt_node(SysBusDevice 
> *sbdev, void *opaque)
>  
>      qemu_fdt_add_subnode(fdt, nodename);
>  
> -    compat_str_len = strlen(vdev->compat) + 1;
>      qemu_fdt_setprop(fdt, nodename, "compatible",
> -                          vdev->compat, compat_str_len);
> +                          compat, compat_str_len);
>  
>      reg_attr = g_new(uint64_t, vbasedev->num_regions*4);
>  
> @@ -140,7 +151,7 @@ fail:
>  
>  /* list of supported dynamic sysbus devices */
>  static const NodeCreationPair add_fdt_node_functions[] = {
> -{TYPE_VFIO_CALXEDA_XGMAC, add_calxeda_midway_xgmac_fdt_node},
> +{TYPE_VFIO_CALXEDA_XGMAC, add_generic_fdt_node},
>  {"", NULL}, /*last element*/
>  };
>  
>