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@xxxxxxx> >>> Signed-off-by: Eric Auger <eric.auger@xxxxxxxxxx> >>> >>> --- >>> >>> 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@xxxxxxx> >>> + * Eric Auger <eric.auger@xxxxxxxxxx> >>> + * >>> + * 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. 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 > > -- Shannon _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
