From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58065) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UiESZ-0001AD-Vp for qemu-devel@nongnu.org; Thu, 30 May 2013 21:48:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UiESY-0007oF-FY for qemu-devel@nongnu.org; Thu, 30 May 2013 21:48:51 -0400 Received: from [222.73.24.84] (port=22492 helo=song.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UiESY-0007mM-52 for qemu-devel@nongnu.org; Thu, 30 May 2013 21:48:50 -0400 Date: Fri, 31 May 2013 09:47:57 +0800 From: Hu Tao Message-ID: <20130531014757.GE9509@localhost.localdomain> References: <1369920427-18274-1-git-send-email-mst@redhat.com> <1369920427-18274-3-git-send-email-mst@redhat.com> MIME-Version: 1.0 In-Reply-To: <1369920427-18274-3-git-send-email-mst@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Subject: Re: [Qemu-devel] [PATCH 2/3] fw_cfg: add API to find FW cfg object List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: qemu-devel@nongnu.org On Thu, May 30, 2013 at 04:28:01PM +0300, Michael S. Tsirkin wrote: > Remove some code duplication by adding a > function to look up the fw cfg file. > This way, we don't need to duplicate same strings everywhere. > Use by both fw cfg and pvpanic device. > > Signed-off-by: Michael S. Tsirkin > --- > hw/misc/pvpanic.c | 2 +- > hw/nvram/fw_cfg.c | 15 ++++++++++++--- > include/hw/nvram/fw_cfg.h | 2 ++ > 3 files changed, 15 insertions(+), 4 deletions(-) > > diff --git a/hw/misc/pvpanic.c b/hw/misc/pvpanic.c > index 1483f27..910e44f 100644 > --- a/hw/misc/pvpanic.c > +++ b/hw/misc/pvpanic.c > @@ -96,7 +96,7 @@ static int pvpanic_isa_initfn(ISADevice *dev) > isa_register_ioport(dev, &s->io, s->ioport); > > if (!port_configured) { > - fw_cfg = object_resolve_path("/machine/fw_cfg", NULL); > + fw_cfg = fw_cfg_find(); > if (fw_cfg) { > fw_cfg_add_file(fw_cfg, "etc/pvpanic-port", > g_memdup(&s->ioport, sizeof(s->ioport)), > diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c > index 479113b..df3f089 100644 > --- a/hw/nvram/fw_cfg.c > +++ b/hw/nvram/fw_cfg.c > @@ -32,6 +32,9 @@ > > #define FW_CFG_SIZE 2 > #define FW_CFG_DATA_SIZE 1 > +#define TYPE_FW_CFG "fw_cfg" > +#define FW_CFG_NAME "fw_cfg" > +#define FW_CFG_PATH "/machine/" FW_CFG_NAME > > typedef struct FWCfgEntry { > uint32_t len; > @@ -493,8 +496,8 @@ FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port, > > s = DO_UPCAST(FWCfgState, busdev.qdev, dev); > > - if (!object_resolve_path("/machine/fw_cfg", NULL)) { > - object_property_add_child(qdev_get_machine(), "fw_cfg", OBJECT(s), > + if (!object_resolve_path(FW_CFG_PATH, NULL)) { > + object_property_add_child(qdev_get_machine(), FW_CFG_NAME, OBJECT(s), > NULL); > } > > @@ -553,6 +556,12 @@ static Property fw_cfg_properties[] = { > DEFINE_PROP_END_OF_LIST(), > }; > > +FWCfgState *fw_cfg_find(void) > +{ > + return OBJECT_CHECK(FWCfgState, object_resolve_path(FW_CFG_PATH, NULL), > + TYPE_FW_CFG); > +} > + > static void fw_cfg_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > @@ -566,7 +575,7 @@ static void fw_cfg_class_init(ObjectClass *klass, void *data) > } > > static const TypeInfo fw_cfg_info = { > - .name = "fw_cfg", > + .name = TYPE_FW_CFG, > .parent = TYPE_SYS_BUS_DEVICE, > .instance_size = sizeof(FWCfgState), > .class_init = fw_cfg_class_init, > diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h > index f37714e..f60dd67 100644 > --- a/include/hw/nvram/fw_cfg.h > +++ b/include/hw/nvram/fw_cfg.h > @@ -73,6 +73,8 @@ void fw_cfg_add_file(FWCfgState *s, const char *filename, void *data, > FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port, > hwaddr crl_addr, hwaddr data_addr); > > +FWCfgState *fw_cfg_find(void); How about fw_cfg_get()? It makes me more comfortable than fw_cfg_find(). > + > #endif /* NO_QEMU_PROTOS */ > > #endif > -- > MST >