[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH 4/4] hw/arm/sysbus-fdt: arm, pl330 vfio devi
From: |
Eric Auger |
Subject: |
Re: [Qemu-devel] [RFC PATCH 4/4] hw/arm/sysbus-fdt: arm, pl330 vfio device property |
Date: |
Thu, 15 Jan 2015 17:16:25 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 |
Hi Baptiste,
On 01/12/2015 02:21 PM, Baptiste Reynal wrote:
> Adapt arm,pl330 function to use the vfio device property API.
>
> Clock apb-pclk is the default if a clock is needed by the device.
>
> Three optional parameters are taken into account :
> - #dma-cells
> - #dma-channels
> - #dma-requests
>
> Signed-off-by: Baptiste Reynal <address@hidden>
> ---
> hw/arm/sysbus-fdt.c | 120
> ++++++++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 103 insertions(+), 17 deletions(-)
>
> diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
> index 087e788..a0adc50 100644
> --- a/hw/arm/sysbus-fdt.c
> +++ b/hw/arm/sysbus-fdt.c
> @@ -96,13 +96,13 @@ static int set_interrupts_fdt_node(char *nodename,
> SysBusDevice *sbdev,
> irq_attr[3*i+2] = cpu_to_be32(((__u32 *) irq_prop->data)[3*i+2]);
> }
>
> - ret = qemu_fdt_setprop(fdt, nodename, "interrupts",
> - irq_attr, vbasedev->num_irqs*3*sizeof(uint32_t));
> + ret = qemu_fdt_setprop(fdt, nodename, "interrupts",
> + irq_attr, vbasedev->num_irqs*3*sizeof(uint32_t));
delta may be removed
>
> g_free(irq_attr);
> g_free(irq_prop);
>
> - return ret;
> + return ret;
delta may be removed
> }
>
> /**
> @@ -140,6 +140,54 @@ static int set_regions_fdt_node(char *nodename,
> SysBusDevice *sbdev,
> return ret;
> }
>
may deserve a comment and/or pointer to
Documentation/devicetree/bindings/arm/primecell.txt; don't know whether
this is the reference doc, sorry.
> +static int set_arm_primecell_node(char *nodename, SysBusDevice *sbdev, void
> + *opaque)
> +{
> + PlatformBusFdtData *data = opaque;
> + void *fdt = data->fdt;
> + int ret;
> + VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(sbdev);
> + VFIODevice *vbasedev = &vdev->vbasedev;
> + struct vfio_dev_property *property;
> +
Wouldn't it make sense to handle the required compatible prop here?
> + /* Optional properties */
> + property = vfio_get_dev_property(vbasedev->fd, "interrupts",
> + VFIO_DEV_PROPERTY_TYPE_U32);
> + if (property != NULL) {
> + ret = set_interrupts_fdt_node(nodename, sbdev, opaque);
> + if (ret < 0) {
> + error_report("could not set interrupts property of node %s",
> + nodename);
> + goto fail;
in case of fail the property is not released.
> + }
> + g_free(property);
> + }
> +
> + property = vfio_get_dev_property(vbasedev->fd, "clock-names",
> + VFIO_DEV_PROPERTY_TYPE_STRINGS);
> + if (property != NULL) {
> + /* If a clock is attached, we rely on apb-pclk */
> + /* Check clock existence */
> + ret = fdt_path_offset(fdt, "/apb-pclk");
> +
> + if (ret < 0) {
> + error_report("could not set clocks property of node %s",
> nodename);
> + goto fail;
release property
> + } else {
> + qemu_fdt_setprop_cells(fdt, nodename, "clocks",
> + qemu_fdt_getprop_cell(fdt, "/apb-pclk", "phandle"));
> + char clock_names[] = "apb_pclk";
> + qemu_fdt_setprop(fdt, nodename, "clock-names", clock_names,
> + sizeof(clock_names));
> + }
> + g_free(property);
do you intend to support other optional properties?
> + }
> +
> + return 0;
> +
> +fail:
> + return -1;
> +}
> /**
> * add_calxeda_midway_xgmac_fdt_node
> *
> @@ -213,6 +261,8 @@ static int add_arm_pl330_fdt_node(SysBusDevice *sbdev,
> void *opaque)
> VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(sbdev);
> VFIODevice *vbasedev = &vdev->vbasedev;
> Object *obj = OBJECT(sbdev);
> + struct vfio_dev_property *property;
> + int propint;
>
> mmio_base = object_property_get_int(obj, "mmio[0]", NULL);
>
> @@ -222,6 +272,13 @@ static int add_arm_pl330_fdt_node(SysBusDevice *sbdev,
> void *opaque)
>
> qemu_fdt_add_subnode(fdt, nodename);
>
> + ret = set_arm_primecell_node(nodename, sbdev, opaque);
> + if (ret < 0) {
> + error_report("could not set arm,primecell properties of node %s",
> + nodename);
> + goto fail;
> + }
> +
> /*
> * Process compatible string to deal with multiple strings
> * (; is replaced by \0)
> @@ -237,20 +294,6 @@ static int add_arm_pl330_fdt_node(SysBusDevice *sbdev,
> void *opaque)
> qemu_fdt_setprop(fdt, nodename, "compatible",
> compat, compat_str_len);
>
> - /* Setup clocks for AMBA device */
> - /* Check clock existence */
> - ret = fdt_path_offset(fdt, "/apb-pclk");
> -
> - if (ret < 0) {
> - error_report("could not set clocks property of node %s", nodename);
> - } else {
> - qemu_fdt_setprop_cells(fdt, nodename, "clocks",
> - qemu_fdt_getprop_cell(fdt, "/apb-pclk", "phandle"));
> - char clock_names[] = "apb_pclk";
> - qemu_fdt_setprop(fdt, nodename, "clock-names", clock_names,
> - sizeof(clock_names));
> - }
> -
> ret = set_regions_fdt_node(nodename, sbdev, opaque);
>
> if (ret < 0) {
> @@ -266,6 +309,49 @@ static int add_arm_pl330_fdt_node(SysBusDevice *sbdev,
> void *opaque)
> goto fail;
> }
>
> + /* Optional properties */
> + property = vfio_get_dev_property(vbasedev->fd, "#dma-cells",
> + VFIO_DEV_PROPERTY_TYPE_U32);
> + if (property != NULL) {
> + propint = cpu_to_be32(((__u32 *) property->data)[0]);
> + ret = qemu_fdt_setprop(fdt, nodename, "#dma-cells", &propint,
> + sizeof(int));
> + if (ret < 0) {
> + error_report("could not set #dma-cells property of node %s",
> + nodename);
> + goto fail;
property release
> + }
> + g_free(property);
> + }
> +
> + property = vfio_get_dev_property(vbasedev->fd, "#dma-channels",
> + VFIO_DEV_PROPERTY_TYPE_U32);
> + if (property != NULL) {
> + propint = cpu_to_be32(((__u32 *) property->data)[0]);
> + ret = qemu_fdt_setprop(fdt, nodename, "#dma-cells", &propint,
> + sizeof(int));
> + if (ret < 0) {
> + error_report("could not set #dma-cells property of node %s",
> + nodename);
> + goto fail;
> + }
> + g_free(property);
> + }
> +
> + property = vfio_get_dev_property(vbasedev->fd, "#dma-requests",
> + VFIO_DEV_PROPERTY_TYPE_U32);
> + if (property != NULL) {
> + propint = cpu_to_be32(((__u32 *) property->data)[0]);
> + ret = qemu_fdt_setprop(fdt, nodename, "#dma-cells", &propint,
> + sizeof(int));
> + if (ret < 0) {
> + error_report("could not set #dma-cells property of node %s",
> + nodename);
> + goto fail;
> + }
> + g_free(property);
> + }
> +
> g_free(nodename);
>
> return 0;
to me it looks a good and interesting illustration of the modality
Best Regards
Eric
>
- [Qemu-devel] [RFC PATCH 0/4] VFIO platform: Use device properties API, Baptiste Reynal, 2015/01/12
- [Qemu-devel] [RFC PATCH 1/4] linux-headers update, Baptiste Reynal, 2015/01/12
- [Qemu-devel] [RFC PATCH 2/4] hw/vfio/common.c : vfio_get_dev_property, Baptiste Reynal, 2015/01/12
- [Qemu-devel] [RFC PATCH 3/4] hw/arm/sysbus-fdt: vfio device property for interrupts, Baptiste Reynal, 2015/01/12
- [Qemu-devel] [RFC PATCH 4/4] hw/arm/sysbus-fdt: arm, pl330 vfio device property, Baptiste Reynal, 2015/01/12
- Re: [Qemu-devel] [RFC PATCH 4/4] hw/arm/sysbus-fdt: arm, pl330 vfio device property,
Eric Auger <=