From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48313) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XF8QH-0005mN-26 for qemu-devel@nongnu.org; Wed, 06 Aug 2014 17:07:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XF8QB-0006lP-9E for qemu-devel@nongnu.org; Wed, 06 Aug 2014 17:07:00 -0400 Received: from mx1.redhat.com ([209.132.183.28]:63443) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XF8QB-0006lL-1u for qemu-devel@nongnu.org; Wed, 06 Aug 2014 17:06:55 -0400 Date: Wed, 6 Aug 2014 23:07:14 +0200 From: "Michael S. Tsirkin" Message-ID: <20140806210714.GD22307@redhat.com> References: <1407307835-9692-1-git-send-email-tiejun.chen@intel.com> <1407307835-9692-4-git-send-email-tiejun.chen@intel.com> <20140806094509.GB22307@redhat.com> <53E2009E.4080107@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <53E2009E.4080107@intel.com> Subject: Re: [Qemu-devel] [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Chen, Tiejun" Cc: pbonzini@redhat.com, xen-devel@lists.xensource.com, qemu-devel@nongnu.org, stefano.stabellini@eu.citrix.com On Wed, Aug 06, 2014 at 06:17:02PM +0800, Chen, Tiejun wrote: > On 2014/8/6 17:45, Michael S. Tsirkin wrote: > >On Wed, Aug 06, 2014 at 02:50:33PM +0800, Tiejun Chen wrote: > >>We need to use this index to reuse this macro later > >> > >>Signed-off-by: Tiejun Chen > > > >Which index? > >Most users don't need to change. > >Just open-code OBJECT_CHECK where necessary, or add > >a new wrapper. > > Okay so what about this? > > hw:pci-host:piix: define I440FX_PCI_DEVICE_FROM_TYPE > > We need to introduce I440FX_PCI_DEVICE_FROM_TYPE to get > object with type, then we can reuse i440fx_init() simply. > > Signed-off-by: Tiejun Chen > > diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c > index 0cd82b8..8c74653 100644 > --- a/hw/pci-host/piix.c > +++ b/hw/pci-host/piix.c > @@ -93,6 +93,9 @@ typedef struct PIIX3State { > #define I440FX_PCI_DEVICE(obj) \ > OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE) > > +#define I440FX_PCI_DEVICE_FROM_TYPE(obj, type) \ > + OBJECT_CHECK(PCII440FXState, (obj), type) This is just wrong. If you are casting to PCII440FXState, there is no reason not to use TYPE_I440FX_PCI_DEVICE. > + > struct PCII440FXState { > /*< private >*/ > PCIDevice parent_obj; > @@ -333,7 +336,7 @@ PCIBus *i440fx_init(const char *host_type, const char > *pci_type, > qdev_init_nofail(dev); > > d = pci_create_simple(b, 0, pci_type); > - *pi440fx_state = I440FX_PCI_DEVICE(d); > + *pi440fx_state = I440FX_PCI_DEVICE_FROM_TYPE(d, pci_type); > f = *pi440fx_state; > f->system_memory = address_space_mem; > f->pci_address_space = pci_address_space; > > > Thanks > Tiejun > > > > >>--- > >> hw/pci-host/piix.c | 10 +++++----- > >> 1 file changed, 5 insertions(+), 5 deletions(-) > >> > >>v4: > >> > >>* New patch to extend I440FX_PCI_DEVICE > >> > >>diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c > >>index 0cd82b8..4330599 100644 > >>--- a/hw/pci-host/piix.c > >>+++ b/hw/pci-host/piix.c > >>@@ -90,8 +90,8 @@ typedef struct PIIX3State { > >> MemoryRegion rcr_mem; > >> } PIIX3State; > >> > >>-#define I440FX_PCI_DEVICE(obj) \ > >>- OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE) > >>+#define I440FX_PCI_DEVICE(obj, type) \ > >>+ OBJECT_CHECK(PCII440FXState, (obj), type) > >> > >> struct PCII440FXState { > >> /*< private >*/ > >>@@ -155,7 +155,7 @@ static void i440fx_set_smm(int val, void *arg) > >> static void i440fx_write_config(PCIDevice *dev, > >> uint32_t address, uint32_t val, int len) > >> { > >>- PCII440FXState *d = I440FX_PCI_DEVICE(dev); > >>+ PCII440FXState *d = I440FX_PCI_DEVICE(dev, TYPE_I440FX_PCI_DEVICE); > >> > >> /* XXX: implement SMRAM.D_LOCK */ > >> pci_default_write_config(dev, address, val, len); > >>@@ -295,7 +295,7 @@ static void i440fx_pcihost_realize(DeviceState *dev, Error **errp) > >> > >> static int i440fx_initfn(PCIDevice *dev) > >> { > >>- PCII440FXState *d = I440FX_PCI_DEVICE(dev); > >>+ PCII440FXState *d = I440FX_PCI_DEVICE(dev, TYPE_I440FX_PCI_DEVICE); > >> > >> dev->config[I440FX_SMRAM] = 0x02; > >> > >>@@ -333,7 +333,7 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type, > >> qdev_init_nofail(dev); > >> > >> d = pci_create_simple(b, 0, pci_type); > >>- *pi440fx_state = I440FX_PCI_DEVICE(d); > >>+ *pi440fx_state = I440FX_PCI_DEVICE(d, pci_type); > >> f = *pi440fx_state; > >> f->system_memory = address_space_mem; > >> f->pci_address_space = pci_address_space; > >>-- > >>1.9.1 > > > > From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index Date: Wed, 6 Aug 2014 23:07:14 +0200 Message-ID: <20140806210714.GD22307@redhat.com> References: <1407307835-9692-1-git-send-email-tiejun.chen@intel.com> <1407307835-9692-4-git-send-email-tiejun.chen@intel.com> <20140806094509.GB22307@redhat.com> <53E2009E.4080107@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <53E2009E.4080107@intel.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: "Chen, Tiejun" Cc: pbonzini@redhat.com, xen-devel@lists.xensource.com, qemu-devel@nongnu.org, stefano.stabellini@eu.citrix.com List-Id: xen-devel@lists.xenproject.org On Wed, Aug 06, 2014 at 06:17:02PM +0800, Chen, Tiejun wrote: > On 2014/8/6 17:45, Michael S. Tsirkin wrote: > >On Wed, Aug 06, 2014 at 02:50:33PM +0800, Tiejun Chen wrote: > >>We need to use this index to reuse this macro later > >> > >>Signed-off-by: Tiejun Chen > > > >Which index? > >Most users don't need to change. > >Just open-code OBJECT_CHECK where necessary, or add > >a new wrapper. > > Okay so what about this? > > hw:pci-host:piix: define I440FX_PCI_DEVICE_FROM_TYPE > > We need to introduce I440FX_PCI_DEVICE_FROM_TYPE to get > object with type, then we can reuse i440fx_init() simply. > > Signed-off-by: Tiejun Chen > > diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c > index 0cd82b8..8c74653 100644 > --- a/hw/pci-host/piix.c > +++ b/hw/pci-host/piix.c > @@ -93,6 +93,9 @@ typedef struct PIIX3State { > #define I440FX_PCI_DEVICE(obj) \ > OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE) > > +#define I440FX_PCI_DEVICE_FROM_TYPE(obj, type) \ > + OBJECT_CHECK(PCII440FXState, (obj), type) This is just wrong. If you are casting to PCII440FXState, there is no reason not to use TYPE_I440FX_PCI_DEVICE. > + > struct PCII440FXState { > /*< private >*/ > PCIDevice parent_obj; > @@ -333,7 +336,7 @@ PCIBus *i440fx_init(const char *host_type, const char > *pci_type, > qdev_init_nofail(dev); > > d = pci_create_simple(b, 0, pci_type); > - *pi440fx_state = I440FX_PCI_DEVICE(d); > + *pi440fx_state = I440FX_PCI_DEVICE_FROM_TYPE(d, pci_type); > f = *pi440fx_state; > f->system_memory = address_space_mem; > f->pci_address_space = pci_address_space; > > > Thanks > Tiejun > > > > >>--- > >> hw/pci-host/piix.c | 10 +++++----- > >> 1 file changed, 5 insertions(+), 5 deletions(-) > >> > >>v4: > >> > >>* New patch to extend I440FX_PCI_DEVICE > >> > >>diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c > >>index 0cd82b8..4330599 100644 > >>--- a/hw/pci-host/piix.c > >>+++ b/hw/pci-host/piix.c > >>@@ -90,8 +90,8 @@ typedef struct PIIX3State { > >> MemoryRegion rcr_mem; > >> } PIIX3State; > >> > >>-#define I440FX_PCI_DEVICE(obj) \ > >>- OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE) > >>+#define I440FX_PCI_DEVICE(obj, type) \ > >>+ OBJECT_CHECK(PCII440FXState, (obj), type) > >> > >> struct PCII440FXState { > >> /*< private >*/ > >>@@ -155,7 +155,7 @@ static void i440fx_set_smm(int val, void *arg) > >> static void i440fx_write_config(PCIDevice *dev, > >> uint32_t address, uint32_t val, int len) > >> { > >>- PCII440FXState *d = I440FX_PCI_DEVICE(dev); > >>+ PCII440FXState *d = I440FX_PCI_DEVICE(dev, TYPE_I440FX_PCI_DEVICE); > >> > >> /* XXX: implement SMRAM.D_LOCK */ > >> pci_default_write_config(dev, address, val, len); > >>@@ -295,7 +295,7 @@ static void i440fx_pcihost_realize(DeviceState *dev, Error **errp) > >> > >> static int i440fx_initfn(PCIDevice *dev) > >> { > >>- PCII440FXState *d = I440FX_PCI_DEVICE(dev); > >>+ PCII440FXState *d = I440FX_PCI_DEVICE(dev, TYPE_I440FX_PCI_DEVICE); > >> > >> dev->config[I440FX_SMRAM] = 0x02; > >> > >>@@ -333,7 +333,7 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type, > >> qdev_init_nofail(dev); > >> > >> d = pci_create_simple(b, 0, pci_type); > >>- *pi440fx_state = I440FX_PCI_DEVICE(d); > >>+ *pi440fx_state = I440FX_PCI_DEVICE(d, pci_type); > >> f = *pi440fx_state; > >> f->system_memory = address_space_mem; > >> f->pci_address_space = pci_address_space; > >>-- > >>1.9.1 > > > >