From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53935) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xtytu-0001j8-Gn for qemu-devel@nongnu.org; Thu, 27 Nov 2014 08:14:31 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xtytp-0005w5-Ma for qemu-devel@nongnu.org; Thu, 27 Nov 2014 08:14:26 -0500 Received: from cantor2.suse.de ([195.135.220.15]:60272 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xtytp-0005vb-CY for qemu-devel@nongnu.org; Thu, 27 Nov 2014 08:14:21 -0500 Message-ID: <547723A6.10900@suse.de> Date: Thu, 27 Nov 2014 14:14:14 +0100 From: Alexander Graf MIME-Version: 1.0 References: <1414763612-4939-1-git-send-email-eric.auger@linaro.org> <1414763612-4939-6-git-send-email-eric.auger@linaro.org> <54771406.5020008@huawei.com> <54771838.4080205@linaro.org> <54771F77.5090300@huawei.com> In-Reply-To: <54771F77.5090300@huawei.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v4 5/6] hw/arm/sysbus-fdt: helpers for platform bus nodes addition List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Shannon Zhao , Eric Auger , eric.auger@st.com, christoffer.dall@linaro.org, qemu-devel@nongnu.org, pbonzini@redhat.com, kim.phillips@freescale.com, a.rigo@virtualopensystems.com, manish.jaggi@caviumnetworks.com, joel.schopp@amd.com Cc: alex.williamson@redhat.com, patches@linaro.org, will.deacon@arm.com, kvmarm@lists.cs.columbia.edu, stuart.yoder@freescale.com On 27.11.14 13:56, 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 >>>> Signed-off-by: Eric Auger >>>> >>>> --- >>>> >>>> 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 >>>> + * Eric Auger >>>> + * >>>> + * 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 . >>>> + * >>>> + */ >>>> + >>>> +#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. It's not associated with TYPE_PFD at all. The device tree node is 100% machine owned and should be - only the machine knows about machine specific device tree semantics. If on top of that the machine needs to learn about device specific semantics to create a device tree node, so be it :). Alex