qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Baptiste Reynal
Subject: Re: [Qemu-devel] [RFC PATCH 1/3] hw/vfio/sysbus-fdt: generic add_generic_fdt_node
Date: Wed, 17 Dec 2014 15:55:57 +0100



On Wed, Dec 17, 2014 at 2:32 PM, Eric Auger <address@hidden> wrote:
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.

I'm not sure I totally understand your point, but creating a new  add_pl330_fdt_node function which will call to add_calxeda_midway_xgmac_fdt_node and perform amba specific process (multiple compatible string and clocks) seems to be a good solution for you ?

>
> 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 = ""> >      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

From my point of view, formatting the string directly in VFIO AMBA device will mess a lot with string function, but I can use an array of string in VFIOPlatformDevice to avoid it.

Best Regards,

Baptiste

> +
>      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*/
>  };
>
>


reply via email to

[Prev in Thread] Current Thread [Next in Thread]