diff mbox

[v4,5/6] hw/arm/sysbus-fdt: helpers for platform bus nodes addition

Message ID 1414763612-4939-6-git-send-email-eric.auger@linaro.org
State New
Headers show

Commit Message

Auger Eric Oct. 31, 2014, 1:53 p.m. UTC
This new C module will be used by ARM machine files to generate
platform bus node and their dynamic sysbus device tree nodes.

Dynamic sysbus device node addition is done in a machine init
done notifier. arm_register_platform_bus_fdt_creator does the
registration of this latter and is supposed to be called by
ARM machine files that support platform bus and their dynamic
sysbus. Addition of dynamic sysbus nodes is done only if the
user did not provide any dtb.

Signed-off-by: Alexander Graf <agraf@suse.de>
Signed-off-by: Eric Auger <eric.auger@linaro.org>

---

v3 -> v4:
- dyn_sysbus_devtree.c renamed into sysbus-fdt.c
- use new PlatformBusDevice object
- the dtb upgrade is done through modify_dtb. Before the fdt
  was recreated from scratch. When the user provided a dtb this
  latter was overwritten which was not correct.
- an array contains the association between device type names
  and their node creation function
- I must aknowledge I did not find any cleaner way to implement
  a FDT_BUILDER interface, as suggested by Paolo. The class method
  would need to be initialized somewhere and since it cannot
  happen in the device itself - according to Alex & Peter comments -,
  I don't see when I shall associate the device type and its
  interface implementation.

v2 -> v3:
- add arm_ prefix
- arm_sysbus_device_create_devtree becomes static

v1 -> v2:
- Code moved in an arch specific file to accomodate architecture
  dependent specificities.
- remove platform_bus_base from PlatformDevtreeData

v1: code originally written by Alex Graf in e500.c and reused for
ARM [Eric Auger]
---
 hw/arm/Makefile.objs        |   1 +
 hw/arm/sysbus-fdt.c         | 181 ++++++++++++++++++++++++++++++++++++++++++++
 include/hw/arm/sysbus-fdt.h |  50 ++++++++++++
 3 files changed, 232 insertions(+)
 create mode 100644 hw/arm/sysbus-fdt.c
 create mode 100644 include/hw/arm/sysbus-fdt.h

Comments

Alexander Graf Nov. 5, 2014, 10:19 a.m. UTC | #1
On 31.10.14 14:53, Eric Auger wrote:
> This new C module will be used by ARM machine files to generate
> platform bus node and their dynamic sysbus device tree nodes.
> 
> Dynamic sysbus device node addition is done in a machine init
> done notifier. arm_register_platform_bus_fdt_creator does the
> registration of this latter and is supposed to be called by
> ARM machine files that support platform bus and their dynamic
> sysbus. Addition of dynamic sysbus nodes is done only if the
> user did not provide any dtb.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> Signed-off-by: Eric Auger <eric.auger@linaro.org>
> 
> ---
> 
> v3 -> v4:
> - dyn_sysbus_devtree.c renamed into sysbus-fdt.c
> - use new PlatformBusDevice object
> - the dtb upgrade is done through modify_dtb. Before the fdt
>   was recreated from scratch. When the user provided a dtb this
>   latter was overwritten which was not correct.
> - an array contains the association between device type names
>   and their node creation function
> - I must aknowledge I did not find any cleaner way to implement
>   a FDT_BUILDER interface, as suggested by Paolo. The class method
>   would need to be initialized somewhere and since it cannot
>   happen in the device itself - according to Alex & Peter comments -,
>   I don't see when I shall associate the device type and its
>   interface implementation.
> 
> v2 -> v3:
> - add arm_ prefix
> - arm_sysbus_device_create_devtree becomes static
> 
> v1 -> v2:
> - Code moved in an arch specific file to accomodate architecture
>   dependent specificities.
> - remove platform_bus_base from PlatformDevtreeData
> 
> v1: code originally written by Alex Graf in e500.c and reused for
> ARM [Eric Auger]
> ---
>  hw/arm/Makefile.objs        |   1 +
>  hw/arm/sysbus-fdt.c         | 181 ++++++++++++++++++++++++++++++++++++++++++++
>  include/hw/arm/sysbus-fdt.h |  50 ++++++++++++
>  3 files changed, 232 insertions(+)
>  create mode 100644 hw/arm/sysbus-fdt.c
>  create mode 100644 include/hw/arm/sysbus-fdt.h
> 
> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
> index 6088e53..0cc63e1 100644
> --- a/hw/arm/Makefile.objs
> +++ b/hw/arm/Makefile.objs
> @@ -3,6 +3,7 @@ obj-$(CONFIG_DIGIC) += digic_boards.o
>  obj-y += integratorcp.o kzm.o mainstone.o musicpal.o nseries.o
>  obj-y += omap_sx1.o palm.o realview.o spitz.o stellaris.o
>  obj-y += tosa.o versatilepb.o vexpress.o virt.o xilinx_zynq.o z2.o
> +obj-y += sysbus-fdt.o
>  
>  obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
>  obj-$(CONFIG_DIGIC) += digic.o
> diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
> new file mode 100644
> index 0000000..d5476f1
> --- /dev/null
> +++ b/hw/arm/sysbus-fdt.c
> @@ -0,0 +1,181 @@
> +/*
> + * ARM Platform Bus device tree generation helpers
> + *
> + * Copyright (c) 2014 Linaro Limited
> + *
> + * Authors:
> + *  Alex Graf <agraf@suse.de>
> + *  Eric Auger <eric.auger@linaro.org>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + *
> + */
> +
> +#include "hw/arm/sysbus-fdt.h"
> +#include "qemu/error-report.h"
> +#include "sysemu/device_tree.h"
> +#include "hw/platform-bus.h"
> +#include "sysemu/sysemu.h"
> +#include "hw/platform-bus.h"
> +
> +/*
> + * internal struct that contains the information to create dynamic
> + * sysbus device node
> + */
> +typedef struct PlatformBusFdtData {
> +    void *fdt; /* device tree handle */
> +    int irq_start; /* index of the first IRQ usable by platform bus devices */
> +    const char *pbus_node_name; /* name of the platform bus node */
> +    PlatformBusDevice *pbus;
> +} PlatformBusFdtData;
> +
> +/*
> + * struct used when calling the machine init done notifier
> + * that constructs the fdt nodes of platform bus devices
> + */
> +typedef struct PlatformBusFdtNotifierParams {
> +    ARMPlatformBusFdtParams *fdt_params;
> +    Notifier notifier;
> +} PlatformBusFdtNotifierParams;
> +
> +/* struct that associates a device type name and a node creation function */
> +typedef struct NodeCreationPair {
> +    const char *typename;
> +    int (*add_fdt_node_fn)(SysBusDevice *sbdev, void *opaque);
> +} NodeCreationPair;
> +
> +/* list of supported dynamic sysbus devices */
> +NodeCreationPair add_fdt_node_functions[] = {
> +        {"", NULL}, /*last element*/

s/    // :)


Alex
Auger Eric Nov. 27, 2014, 12:25 p.m. UTC | #2
On 11/27/2014 01:07 PM, Shannon Zhao wrote:
> On 2014/10/31 21:53, Eric Auger wrote:
>> This new C module will be used by ARM machine files to generate
>> platform bus node and their dynamic sysbus device tree nodes.
>>
>> Dynamic sysbus device node addition is done in a machine init
>> done notifier. arm_register_platform_bus_fdt_creator does the
>> registration of this latter and is supposed to be called by
>> ARM machine files that support platform bus and their dynamic
>> sysbus. Addition of dynamic sysbus nodes is done only if the
>> user did not provide any dtb.
>>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> Signed-off-by: Eric Auger <eric.auger@linaro.org>
>>
>> ---
>>
>> v3 -> v4:
>> - dyn_sysbus_devtree.c renamed into sysbus-fdt.c
>> - use new PlatformBusDevice object
>> - the dtb upgrade is done through modify_dtb. Before the fdt
>>   was recreated from scratch. When the user provided a dtb this
>>   latter was overwritten which was not correct.
>> - an array contains the association between device type names
>>   and their node creation function
>> - I must aknowledge I did not find any cleaner way to implement
>>   a FDT_BUILDER interface, as suggested by Paolo. The class method
>>   would need to be initialized somewhere and since it cannot
>>   happen in the device itself - according to Alex & Peter comments -,
>>   I don't see when I shall associate the device type and its
>>   interface implementation.
>>
>> v2 -> v3:
>> - add arm_ prefix
>> - arm_sysbus_device_create_devtree becomes static
>>
>> v1 -> v2:
>> - Code moved in an arch specific file to accomodate architecture
>>   dependent specificities.
>> - remove platform_bus_base from PlatformDevtreeData
>>
>> v1: code originally written by Alex Graf in e500.c and reused for
>> ARM [Eric Auger]
>> ---
>>  hw/arm/Makefile.objs        |   1 +
>>  hw/arm/sysbus-fdt.c         | 181 ++++++++++++++++++++++++++++++++++++++++++++
>>  include/hw/arm/sysbus-fdt.h |  50 ++++++++++++
>>  3 files changed, 232 insertions(+)
>>  create mode 100644 hw/arm/sysbus-fdt.c
>>  create mode 100644 include/hw/arm/sysbus-fdt.h
>>
>> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
>> index 6088e53..0cc63e1 100644
>> --- a/hw/arm/Makefile.objs
>> +++ b/hw/arm/Makefile.objs
>> @@ -3,6 +3,7 @@ obj-$(CONFIG_DIGIC) += digic_boards.o
>>  obj-y += integratorcp.o kzm.o mainstone.o musicpal.o nseries.o
>>  obj-y += omap_sx1.o palm.o realview.o spitz.o stellaris.o
>>  obj-y += tosa.o versatilepb.o vexpress.o virt.o xilinx_zynq.o z2.o
>> +obj-y += sysbus-fdt.o
>>  
>>  obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
>>  obj-$(CONFIG_DIGIC) += digic.o
>> diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
>> new file mode 100644
>> index 0000000..d5476f1
>> --- /dev/null
>> +++ b/hw/arm/sysbus-fdt.c
>> @@ -0,0 +1,181 @@
>> +/*
>> + * ARM Platform Bus device tree generation helpers
>> + *
>> + * Copyright (c) 2014 Linaro Limited
>> + *
>> + * Authors:
>> + *  Alex Graf <agraf@suse.de>
>> + *  Eric Auger <eric.auger@linaro.org>
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2 or later, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
>> + * more details.
>> + *
>> + * You should have received a copy of the GNU General Public License along with
>> + * this program.  If not, see <http://www.gnu.org/licenses/>.
>> + *
>> + */
>> +
>> +#include "hw/arm/sysbus-fdt.h"
>> +#include "qemu/error-report.h"
>> +#include "sysemu/device_tree.h"
>> +#include "hw/platform-bus.h"
> [*]
>> +#include "sysemu/sysemu.h"
>> +#include "hw/platform-bus.h"
> [*]
> Duplicate include "hw/platform-bus.h"
Hi Shannon,

thanks for pointing this out
>> +
>> +/*
>> + * internal struct that contains the information to create dynamic
>> + * sysbus device node
>> + */
>> +typedef struct PlatformBusFdtData {
>> +    void *fdt; /* device tree handle */
>> +    int irq_start; /* index of the first IRQ usable by platform bus devices */
>> +    const char *pbus_node_name; /* name of the platform bus node */
>> +    PlatformBusDevice *pbus;
>> +} PlatformBusFdtData;
>> +
>> +/*
>> + * struct used when calling the machine init done notifier
>> + * that constructs the fdt nodes of platform bus devices
>> + */
>> +typedef struct PlatformBusFdtNotifierParams {
>> +    ARMPlatformBusFdtParams *fdt_params;
>> +    Notifier notifier;
>> +} PlatformBusFdtNotifierParams;
>> +
>> +/* struct that associates a device type name and a node creation function */
>> +typedef struct NodeCreationPair {
>> +    const char *typename;
>> +    int (*add_fdt_node_fn)(SysBusDevice *sbdev, void *opaque);
>> +} NodeCreationPair;
>> +
>> +/* list of supported dynamic sysbus devices */
>> +NodeCreationPair add_fdt_node_functions[] = {
>> +        {"", NULL}, /*last element*/
>> +};
> 
> Eric, I have a question about how to use this.
> If I want to dynamically add a device, I must add a member in above array, such as {TYPE_PFD, pfd_add_fdt_node}.
> And the "pfd_add_fdt_node" is defined by myself which is used to dynamically generate the device fdt node.
> 
> Am I right ?
yes that's correct. You can find an example (Calxeda midway xgmac) in
[PATCH v7 12/16] hw/arm/sysbus-fdt: enable vfio-calxeda-xgmac dynamic
(https://patches.linaro.org/39910/).

I am currently reworking this according to Alex comments but the spirit
remains the same.

Please do not hesitate to come back to me if you have other questions.

Best Regards

Eric


> 
> Thanks,
> Shannon
> 
>> +
>> +/**
>> + * add_fdt_node - add the device tree node of a dynamic sysbus device
>> + *
>> + * @sbdev: handle to the sysbus device
>> + * @opaque: handle to the PlatformBusFdtData
>> + *
>> + * Checks the sysbus type belongs to the list of device types that
>> + * are dynamically instantiable and in the positive call the node
>> + * creation function.
>> + */
>> +static int add_fdt_node(SysBusDevice *sbdev, void *opaque)
>> +{
>> +    int i;
>> +
>> +    for (i = 0; i < ARRAY_SIZE(add_fdt_node_functions); i++) {
>> +        if (!strcmp(object_get_typename(OBJECT(sbdev)),
>> +                    add_fdt_node_functions[i].typename)) {
>> +            add_fdt_node_functions[i].add_fdt_node_fn(sbdev, opaque);
>> +            return 0;
>> +        }
>> +    }
>> +    error_report("Device %s can not be dynamically instantiated",
>> +                     qdev_fw_name(DEVICE(sbdev)));
>> +    return 1;
>> +}
>> +
>> +/**
>> + * add_all_platform_bus_fdt_nodes - create all the platform bus nodes
>> + *
>> + * builds the parent platform bus node and all the nodes of dynamic
>> + * sysbus devices attached * to it. It is a modify_dtb() function, ie.
>> + * called * by arm_load_dtb()
>> + */
>> +static void add_all_platform_bus_fdt_nodes(const struct arm_boot_info *info,
>> +                                           void *fdt)
>> +{
>> +    const char platcomp[] = "qemu,platform\0simple-bus";
>> +    ARMPlatformBusSystemParams *system_params;
>> +    ARMPlatformBusFdtParams *fdt_params = info->modify_dtb_opaque;
>> +    PlatformBusDevice *pbus;
>> +    DeviceState *dev;
>> +    gchar *node;
>> +    uint64_t addr, size;
>> +    int irq_start;
>> +
>> +    if (!fdt_params) {
>> +        return;
>> +    }
>> +
>> +    system_params = fdt_params->system_params;
>> +    node = g_strdup_printf("/platform@%"PRIx64,
>> +                               system_params->platform_bus_base);
>> +    addr = system_params->platform_bus_base;
>> +    size = system_params->platform_bus_size;
>> +    irq_start = system_params->platform_bus_first_irq;
>> +
>> +    /* Create a /platform node that we can put all devices into */
>> +    qemu_fdt_add_subnode(fdt, node);
>> +    qemu_fdt_setprop(fdt, node, "compatible", platcomp, sizeof(platcomp));
>> +
>> +    /* Our platform bus region is less than 32bit big, so 1 cell is enough for
>> +       address and size */
>> +    qemu_fdt_setprop_cells(fdt, node, "#size-cells", 1);
>> +    qemu_fdt_setprop_cells(fdt, node, "#address-cells", 1);
>> +    qemu_fdt_setprop_cells(fdt, node, "ranges", 0, addr >> 32, addr, size);
>> +
>> +    qemu_fdt_setprop_phandle(fdt, node, "interrupt-parent", fdt_params->intc);
>> +
>> +    dev = qdev_find_recursive(sysbus_get_default(), TYPE_PLATFORM_BUS_DEVICE);
>> +    pbus = PLATFORM_BUS_DEVICE(dev);
>> +
>> +    /* We can only create dt nodes for dynamic devices when they're ready */
>> +    if (pbus->done_gathering) {
>> +        PlatformBusFdtData data = {
>> +            .fdt = fdt,
>> +            .irq_start = irq_start,
>> +            .pbus_node_name = node,
>> +            .pbus = pbus,
>> +        };
>> +
>> +        /* Loop through all dynamic sysbus devices and create their node */
>> +        foreach_dynamic_sysbus_device(add_fdt_node, &data);
>> +    }
>> +
>> +    g_free(node);
>> +}
>> +
>> +static void upgrade_dtb(ARMPlatformBusFdtParams *params)
>> +{
>> +    struct arm_boot_info *binfo = params->binfo;
>> +
>> +    /*
>> +     * In case the user provided a dtb, we assume he already integrated the
>> +     * dynamic sysbus nodes. This corresponds to a use case where the dynamic
>> +     * sysbus nodes are complex and their generation is not yet supported. In
>> +     * case the use can take in charge the guest dt while qemu takes in charge
>> +     * the qom stuff.
>> +     */
>> +    if (!binfo->dtb_filename) {
>> +        binfo->modify_dtb_opaque = params;
>> +        binfo->modify_dtb = add_all_platform_bus_fdt_nodes;
>> +        arm_load_dtb(binfo);
>> +    }
>> +}
>> +
>> +static void platform_bus_fdt_notify(Notifier *notifier, void *data)
>> +{
>> +    PlatformBusFdtNotifierParams *p =
>> +        container_of(notifier, PlatformBusFdtNotifierParams, notifier);
>> +    upgrade_dtb(p->fdt_params);
>> +}
>> +
>> +void arm_register_platform_bus_fdt_creator(ARMPlatformBusFdtParams *fdt_params)
>> +{
>> +    PlatformBusFdtNotifierParams *p = g_new(PlatformBusFdtNotifierParams, 1);
>> +
>> +    p->fdt_params = fdt_params;
>> +    p->notifier.notify = platform_bus_fdt_notify;
>> +    qemu_add_machine_init_done_notifier(&p->notifier);
>> +}
>> diff --git a/include/hw/arm/sysbus-fdt.h b/include/hw/arm/sysbus-fdt.h
>> new file mode 100644
>> index 0000000..b9602f1
>> --- /dev/null
>> +++ b/include/hw/arm/sysbus-fdt.h
>> @@ -0,0 +1,50 @@
>> +/*
>> + * Dynamic sysbus device tree node generation API
>> + *
>> + * Copyright Linaro Limited, 2014
>> + *
>> + * Authors:
>> + *  Alex Graf <agraf@suse.de>
>> + *  Eric Auger <eric.auger@linaro.org>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2.  See
>> + * the COPYING file in the top-level directory.
>> + */
>> +
>> +#ifndef HW_ARM_SYSBUS_FDT_H
>> +#define HW_ARM_SYSBUS_FDT_H
>> +
>> +#include "hw/arm/arm.h"
>> +#include "qemu-common.h"
>> +#include "hw/sysbus.h"
>> +
>> +/*
>> + * struct that contains dimensioning parameters of the platform bus
>> + */
>> +typedef struct {
>> +    hwaddr platform_bus_base; /* start address of the bus */
>> +    hwaddr platform_bus_size; /* size of the bus */
>> +    int platform_bus_first_irq; /* first hwirq assigned to the bus */
>> +    int platform_bus_num_irqs; /* number of hwirq assigned to the bus */
>> +} ARMPlatformBusSystemParams;
>> +
>> +/*
>> + * struct that contains all relevant info to build the fdt nodes of
>> + * platform bus and attached dynamic sysbus devices
>> + * in the future might be augmented with additional info
>> + * such as PHY, CLK handles ...
>> + */
>> +typedef struct {
>> +    ARMPlatformBusSystemParams *system_params;
>> +    struct arm_boot_info *binfo;
>> +    const char *intc; /* parent interrupt controller name */
>> +} ARMPlatformBusFdtParams;
>> +
>> +/**
>> + * arm_register_platform_bus_fdt_creator - register a machine init done
>> + * notifier that creates the device tree nodes of the platform bus and
>> + * associated dynamic sysbus devices
>> + */
>> +void arm_register_platform_bus_fdt_creator(ARMPlatformBusFdtParams *fdt_params);
>> +
>> +#endif
>>
> 
>
Auger Eric Nov. 27, 2014, 1:16 p.m. UTC | #3
On 11/27/2014 01:56 PM, Shannon Zhao wrote:
> On 2014/11/27 20:25, Eric Auger wrote:
>> On 11/27/2014 01:07 PM, Shannon Zhao wrote:
>>> On 2014/10/31 21:53, Eric Auger wrote:
>>>> This new C module will be used by ARM machine files to generate
>>>> platform bus node and their dynamic sysbus device tree nodes.
>>>>
>>>> Dynamic sysbus device node addition is done in a machine init
>>>> done notifier. arm_register_platform_bus_fdt_creator does the
>>>> registration of this latter and is supposed to be called by
>>>> ARM machine files that support platform bus and their dynamic
>>>> sysbus. Addition of dynamic sysbus nodes is done only if the
>>>> user did not provide any dtb.
>>>>
>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>> Signed-off-by: Eric Auger <eric.auger@linaro.org>
>>>>
>>>> ---
>>>>
>>>> v3 -> v4:
>>>> - dyn_sysbus_devtree.c renamed into sysbus-fdt.c
>>>> - use new PlatformBusDevice object
>>>> - the dtb upgrade is done through modify_dtb. Before the fdt
>>>>   was recreated from scratch. When the user provided a dtb this
>>>>   latter was overwritten which was not correct.
>>>> - an array contains the association between device type names
>>>>   and their node creation function
>>>> - I must aknowledge I did not find any cleaner way to implement
>>>>   a FDT_BUILDER interface, as suggested by Paolo. The class method
>>>>   would need to be initialized somewhere and since it cannot
>>>>   happen in the device itself - according to Alex & Peter comments -,
>>>>   I don't see when I shall associate the device type and its
>>>>   interface implementation.
>>>>
>>>> v2 -> v3:
>>>> - add arm_ prefix
>>>> - arm_sysbus_device_create_devtree becomes static
>>>>
>>>> v1 -> v2:
>>>> - Code moved in an arch specific file to accomodate architecture
>>>>   dependent specificities.
>>>> - remove platform_bus_base from PlatformDevtreeData
>>>>
>>>> v1: code originally written by Alex Graf in e500.c and reused for
>>>> ARM [Eric Auger]
>>>> ---
>>>>  hw/arm/Makefile.objs        |   1 +
>>>>  hw/arm/sysbus-fdt.c         | 181 ++++++++++++++++++++++++++++++++++++++++++++
>>>>  include/hw/arm/sysbus-fdt.h |  50 ++++++++++++
>>>>  3 files changed, 232 insertions(+)
>>>>  create mode 100644 hw/arm/sysbus-fdt.c
>>>>  create mode 100644 include/hw/arm/sysbus-fdt.h
>>>>
>>>> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
>>>> index 6088e53..0cc63e1 100644
>>>> --- a/hw/arm/Makefile.objs
>>>> +++ b/hw/arm/Makefile.objs
>>>> @@ -3,6 +3,7 @@ obj-$(CONFIG_DIGIC) += digic_boards.o
>>>>  obj-y += integratorcp.o kzm.o mainstone.o musicpal.o nseries.o
>>>>  obj-y += omap_sx1.o palm.o realview.o spitz.o stellaris.o
>>>>  obj-y += tosa.o versatilepb.o vexpress.o virt.o xilinx_zynq.o z2.o
>>>> +obj-y += sysbus-fdt.o
>>>>  
>>>>  obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
>>>>  obj-$(CONFIG_DIGIC) += digic.o
>>>> diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
>>>> new file mode 100644
>>>> index 0000000..d5476f1
>>>> --- /dev/null
>>>> +++ b/hw/arm/sysbus-fdt.c
>>>> @@ -0,0 +1,181 @@
>>>> +/*
>>>> + * ARM Platform Bus device tree generation helpers
>>>> + *
>>>> + * Copyright (c) 2014 Linaro Limited
>>>> + *
>>>> + * Authors:
>>>> + *  Alex Graf <agraf@suse.de>
>>>> + *  Eric Auger <eric.auger@linaro.org>
>>>> + *
>>>> + * This program is free software; you can redistribute it and/or modify it
>>>> + * under the terms and conditions of the GNU General Public License,
>>>> + * version 2 or later, as published by the Free Software Foundation.
>>>> + *
>>>> + * This program is distributed in the hope it will be useful, but WITHOUT
>>>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>>>> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
>>>> + * more details.
>>>> + *
>>>> + * You should have received a copy of the GNU General Public License along with
>>>> + * this program.  If not, see <http://www.gnu.org/licenses/>.
>>>> + *
>>>> + */
>>>> +
>>>> +#include "hw/arm/sysbus-fdt.h"
>>>> +#include "qemu/error-report.h"
>>>> +#include "sysemu/device_tree.h"
>>>> +#include "hw/platform-bus.h"
>>> [*]
>>>> +#include "sysemu/sysemu.h"
>>>> +#include "hw/platform-bus.h"
>>> [*]
>>> Duplicate include "hw/platform-bus.h"
>> Hi Shannon,
>>
>> thanks for pointing this out
>>>> +
>>>> +/*
>>>> + * internal struct that contains the information to create dynamic
>>>> + * sysbus device node
>>>> + */
>>>> +typedef struct PlatformBusFdtData {
>>>> +    void *fdt; /* device tree handle */
>>>> +    int irq_start; /* index of the first IRQ usable by platform bus devices */
>>>> +    const char *pbus_node_name; /* name of the platform bus node */
>>>> +    PlatformBusDevice *pbus;
>>>> +} PlatformBusFdtData;
>>>> +
>>>> +/*
>>>> + * struct used when calling the machine init done notifier
>>>> + * that constructs the fdt nodes of platform bus devices
>>>> + */
>>>> +typedef struct PlatformBusFdtNotifierParams {
>>>> +    ARMPlatformBusFdtParams *fdt_params;
>>>> +    Notifier notifier;
>>>> +} PlatformBusFdtNotifierParams;
>>>> +
>>>> +/* struct that associates a device type name and a node creation function */
>>>> +typedef struct NodeCreationPair {
>>>> +    const char *typename;
>>>> +    int (*add_fdt_node_fn)(SysBusDevice *sbdev, void *opaque);
>>>> +} NodeCreationPair;
>>>> +
>>>> +/* list of supported dynamic sysbus devices */
>>>> +NodeCreationPair add_fdt_node_functions[] = {
>>>> +        {"", NULL}, /*last element*/
>>>> +};
>>>
>>> Eric, I have a question about how to use this.
>>> If I want to dynamically add a device, I must add a member in above array, such as {TYPE_PFD, pfd_add_fdt_node}.
>>> And the "pfd_add_fdt_node" is defined by myself which is used to dynamically generate the device fdt node.
>>>
>>> Am I right ?
>> yes that's correct. You can find an example (Calxeda midway xgmac) in
>> [PATCH v7 12/16] hw/arm/sysbus-fdt: enable vfio-calxeda-xgmac dynamic
>> (https://patches.linaro.org/39910/).
>>
> Hi Eric,
> 
> Thanks for your reply.
> 
> Assuming that I want to add a device "TYPE_PFD". I create a pfd.c in hw/arm/.
> 
> As the "pfd_add_fdt_node" is associated with device "TYPE_PFD", so I just want to let it in pfd.c.
> But the struct PlatformBusFdtData is an internal struct which means I have to move "pfd_add_fdt_node"
> to sysbus-fdt.c. If so, I think it's a little interleaving. And the file sysbus-fdt.c is a little messy.
> 
> What do you think about moving the associated struct to sysbus-fdt.h ?


> 
> And I think the array add_fdt_node_functions is not convenient to add a new add_fdt_node_fn for a new device.
> Maybe a global register function would be better.

Hi Shannon,

Sorry but I don't know what the pfd device correspond to. Please could
you elaborate on it or provide me with a link?

Besides I just wanted to warn you about the fact for the time being
quite few sysbus devices are supposed to be instantiated from command
line. We need to make sure this is the case for pfd. Also in the past we
discussed about the the relevance of putting the device node generation
in the (sysbus) device and I did some patch for that and Alex and Peter
convinced me this was a bad idea overall. Now we are talking about
devices that were generic (not arch specific), like vfio. In case of an
ARM specific device it might be worth to submit this point on the ML.
Besides the current philosophy was to put the device tree node creation
function in sysbus-fdt.c and not in the device file. Already there is
some uncertainty about relevance of putting this outside of the machine
file, according to Alex.

Eric


> 
> Thanks,
> Shannon
> 
>> I am currently reworking this according to Alex comments but the spirit
>> remains the same.
>>
>> Please do not hesitate to come back to me if you have other questions.
>>
>> Best Regards
>>
>> Eric
>>
>>
> 
>
diff mbox

Patch

diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index 6088e53..0cc63e1 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -3,6 +3,7 @@  obj-$(CONFIG_DIGIC) += digic_boards.o
 obj-y += integratorcp.o kzm.o mainstone.o musicpal.o nseries.o
 obj-y += omap_sx1.o palm.o realview.o spitz.o stellaris.o
 obj-y += tosa.o versatilepb.o vexpress.o virt.o xilinx_zynq.o z2.o
+obj-y += sysbus-fdt.o
 
 obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
 obj-$(CONFIG_DIGIC) += digic.o
diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
new file mode 100644
index 0000000..d5476f1
--- /dev/null
+++ b/hw/arm/sysbus-fdt.c
@@ -0,0 +1,181 @@ 
+/*
+ * ARM Platform Bus device tree generation helpers
+ *
+ * Copyright (c) 2014 Linaro Limited
+ *
+ * Authors:
+ *  Alex Graf <agraf@suse.de>
+ *  Eric Auger <eric.auger@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "hw/arm/sysbus-fdt.h"
+#include "qemu/error-report.h"
+#include "sysemu/device_tree.h"
+#include "hw/platform-bus.h"
+#include "sysemu/sysemu.h"
+#include "hw/platform-bus.h"
+
+/*
+ * internal struct that contains the information to create dynamic
+ * sysbus device node
+ */
+typedef struct PlatformBusFdtData {
+    void *fdt; /* device tree handle */
+    int irq_start; /* index of the first IRQ usable by platform bus devices */
+    const char *pbus_node_name; /* name of the platform bus node */
+    PlatformBusDevice *pbus;
+} PlatformBusFdtData;
+
+/*
+ * struct used when calling the machine init done notifier
+ * that constructs the fdt nodes of platform bus devices
+ */
+typedef struct PlatformBusFdtNotifierParams {
+    ARMPlatformBusFdtParams *fdt_params;
+    Notifier notifier;
+} PlatformBusFdtNotifierParams;
+
+/* struct that associates a device type name and a node creation function */
+typedef struct NodeCreationPair {
+    const char *typename;
+    int (*add_fdt_node_fn)(SysBusDevice *sbdev, void *opaque);
+} NodeCreationPair;
+
+/* list of supported dynamic sysbus devices */
+NodeCreationPair add_fdt_node_functions[] = {
+        {"", NULL}, /*last element*/
+};
+
+/**
+ * add_fdt_node - add the device tree node of a dynamic sysbus device
+ *
+ * @sbdev: handle to the sysbus device
+ * @opaque: handle to the PlatformBusFdtData
+ *
+ * Checks the sysbus type belongs to the list of device types that
+ * are dynamically instantiable and in the positive call the node
+ * creation function.
+ */
+static int add_fdt_node(SysBusDevice *sbdev, void *opaque)
+{
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(add_fdt_node_functions); i++) {
+        if (!strcmp(object_get_typename(OBJECT(sbdev)),
+                    add_fdt_node_functions[i].typename)) {
+            add_fdt_node_functions[i].add_fdt_node_fn(sbdev, opaque);
+            return 0;
+        }
+    }
+    error_report("Device %s can not be dynamically instantiated",
+                     qdev_fw_name(DEVICE(sbdev)));
+    return 1;
+}
+
+/**
+ * add_all_platform_bus_fdt_nodes - create all the platform bus nodes
+ *
+ * builds the parent platform bus node and all the nodes of dynamic
+ * sysbus devices attached * to it. It is a modify_dtb() function, ie.
+ * called * by arm_load_dtb()
+ */
+static void add_all_platform_bus_fdt_nodes(const struct arm_boot_info *info,
+                                           void *fdt)
+{
+    const char platcomp[] = "qemu,platform\0simple-bus";
+    ARMPlatformBusSystemParams *system_params;
+    ARMPlatformBusFdtParams *fdt_params = info->modify_dtb_opaque;
+    PlatformBusDevice *pbus;
+    DeviceState *dev;
+    gchar *node;
+    uint64_t addr, size;
+    int irq_start;
+
+    if (!fdt_params) {
+        return;
+    }
+
+    system_params = fdt_params->system_params;
+    node = g_strdup_printf("/platform@%"PRIx64,
+                               system_params->platform_bus_base);
+    addr = system_params->platform_bus_base;
+    size = system_params->platform_bus_size;
+    irq_start = system_params->platform_bus_first_irq;
+
+    /* Create a /platform node that we can put all devices into */
+    qemu_fdt_add_subnode(fdt, node);
+    qemu_fdt_setprop(fdt, node, "compatible", platcomp, sizeof(platcomp));
+
+    /* Our platform bus region is less than 32bit big, so 1 cell is enough for
+       address and size */
+    qemu_fdt_setprop_cells(fdt, node, "#size-cells", 1);
+    qemu_fdt_setprop_cells(fdt, node, "#address-cells", 1);
+    qemu_fdt_setprop_cells(fdt, node, "ranges", 0, addr >> 32, addr, size);
+
+    qemu_fdt_setprop_phandle(fdt, node, "interrupt-parent", fdt_params->intc);
+
+    dev = qdev_find_recursive(sysbus_get_default(), TYPE_PLATFORM_BUS_DEVICE);
+    pbus = PLATFORM_BUS_DEVICE(dev);
+
+    /* We can only create dt nodes for dynamic devices when they're ready */
+    if (pbus->done_gathering) {
+        PlatformBusFdtData data = {
+            .fdt = fdt,
+            .irq_start = irq_start,
+            .pbus_node_name = node,
+            .pbus = pbus,
+        };
+
+        /* Loop through all dynamic sysbus devices and create their node */
+        foreach_dynamic_sysbus_device(add_fdt_node, &data);
+    }
+
+    g_free(node);
+}
+
+static void upgrade_dtb(ARMPlatformBusFdtParams *params)
+{
+    struct arm_boot_info *binfo = params->binfo;
+
+    /*
+     * In case the user provided a dtb, we assume he already integrated the
+     * dynamic sysbus nodes. This corresponds to a use case where the dynamic
+     * sysbus nodes are complex and their generation is not yet supported. In
+     * case the use can take in charge the guest dt while qemu takes in charge
+     * the qom stuff.
+     */
+    if (!binfo->dtb_filename) {
+        binfo->modify_dtb_opaque = params;
+        binfo->modify_dtb = add_all_platform_bus_fdt_nodes;
+        arm_load_dtb(binfo);
+    }
+}
+
+static void platform_bus_fdt_notify(Notifier *notifier, void *data)
+{
+    PlatformBusFdtNotifierParams *p =
+        container_of(notifier, PlatformBusFdtNotifierParams, notifier);
+    upgrade_dtb(p->fdt_params);
+}
+
+void arm_register_platform_bus_fdt_creator(ARMPlatformBusFdtParams *fdt_params)
+{
+    PlatformBusFdtNotifierParams *p = g_new(PlatformBusFdtNotifierParams, 1);
+
+    p->fdt_params = fdt_params;
+    p->notifier.notify = platform_bus_fdt_notify;
+    qemu_add_machine_init_done_notifier(&p->notifier);
+}
diff --git a/include/hw/arm/sysbus-fdt.h b/include/hw/arm/sysbus-fdt.h
new file mode 100644
index 0000000..b9602f1
--- /dev/null
+++ b/include/hw/arm/sysbus-fdt.h
@@ -0,0 +1,50 @@ 
+/*
+ * Dynamic sysbus device tree node generation API
+ *
+ * Copyright Linaro Limited, 2014
+ *
+ * Authors:
+ *  Alex Graf <agraf@suse.de>
+ *  Eric Auger <eric.auger@linaro.org>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ */
+
+#ifndef HW_ARM_SYSBUS_FDT_H
+#define HW_ARM_SYSBUS_FDT_H
+
+#include "hw/arm/arm.h"
+#include "qemu-common.h"
+#include "hw/sysbus.h"
+
+/*
+ * struct that contains dimensioning parameters of the platform bus
+ */
+typedef struct {
+    hwaddr platform_bus_base; /* start address of the bus */
+    hwaddr platform_bus_size; /* size of the bus */
+    int platform_bus_first_irq; /* first hwirq assigned to the bus */
+    int platform_bus_num_irqs; /* number of hwirq assigned to the bus */
+} ARMPlatformBusSystemParams;
+
+/*
+ * struct that contains all relevant info to build the fdt nodes of
+ * platform bus and attached dynamic sysbus devices
+ * in the future might be augmented with additional info
+ * such as PHY, CLK handles ...
+ */
+typedef struct {
+    ARMPlatformBusSystemParams *system_params;
+    struct arm_boot_info *binfo;
+    const char *intc; /* parent interrupt controller name */
+} ARMPlatformBusFdtParams;
+
+/**
+ * arm_register_platform_bus_fdt_creator - register a machine init done
+ * notifier that creates the device tree nodes of the platform bus and
+ * associated dynamic sysbus devices
+ */
+void arm_register_platform_bus_fdt_creator(ARMPlatformBusFdtParams *fdt_params);
+
+#endif