On 2014/11/27 21:16, Eric Auger wrote: > 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@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. > > 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? > Hi Eric, The pfd device is not a real device, it's just an alias. > 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. > Ok, I understand that. Thanks for your explaining :-) Shannon > 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 >>> >>> >> >> > > > . > -- Shannon _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm