From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Nf67Y-0006NV-Qm for qemu-devel@nongnu.org; Wed, 10 Feb 2010 01:32:20 -0500 Received: from [199.232.76.173] (port=53838 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Nf67X-0006NK-3C for qemu-devel@nongnu.org; Wed, 10 Feb 2010 01:32:19 -0500 Received: from Debian-exim by monty-python.gnu.org with spam-scanned (Exim 4.60) (envelope-from ) id 1Nf67U-0004td-Kg for qemu-devel@nongnu.org; Wed, 10 Feb 2010 01:32:18 -0500 Received: from moutng.kundenserver.de ([212.227.126.186]:63400) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Nf67U-0004tV-44 for qemu-devel@nongnu.org; Wed, 10 Feb 2010 01:32:16 -0500 Message-ID: <4B7252EA.7000300@mail.berlios.de> Date: Wed, 10 Feb 2010 07:32:10 +0100 From: Stefan Weil MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 09/15] eepro100: convert to new pci interface References: <1265752899-26980-1-git-send-email-aliguori@us.ibm.com> <1265752899-26980-10-git-send-email-aliguori@us.ibm.com> In-Reply-To: <1265752899-26980-10-git-send-email-aliguori@us.ibm.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Michael Tsirkin , qemu-devel@nongnu.org See my inline comments. Anthony Liguori schrieb: > - Removed some dead defines for TARGET_I386 so that we could build once > > Signed-off-by: Anthony Liguori > --- > hw/eepro100.c | 238 ++++++++++++++------------------------------------------- > 1 files changed, 57 insertions(+), 181 deletions(-) > > diff --git a/hw/eepro100.c b/hw/eepro100.c > index b33dbb8..16230c9 100644 > --- a/hw/eepro100.c > +++ b/hw/eepro100.c > @@ -33,10 +33,6 @@ > * Open Source Software Developer Manual > */ > > -#if defined(TARGET_I386) > -# warning "PXE boot still not working!" > -#endif > - > You did not fix PXE boot here, did you? So the warning or a comment should stay there. > #include /* offsetof */ > #include > #include "hw.h" > ... > -static void ioport_write2(void *opaque, uint32_t addr, uint32_t val) > -{ > - EEPRO100State *s = opaque; > - eepro100_write2(s, addr - s->region[1], val); > -} > - > -static void ioport_write4(void *opaque, uint32_t addr, uint32_t val) > -{ > - EEPRO100State *s = opaque; > - eepro100_write4(s, addr - s->region[1], val); > -} > - > /***********************************************************/ > /* PCI EEPRO100 definitions */ > > -static void pci_map(PCIDevice * pci_dev, int region_num, > - pcibus_t addr, pcibus_t size, int type) > +static void pci_io_write(PCIDevice *dev, pcibus_t addr, int size, > + uint32_t value) > { > - EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, pci_dev); > - > - TRACE(OTHER, logout("region %d, addr=0x%08"FMT_PCIBUS", " > - "size=0x%08"FMT_PCIBUS", type=%d\n", > - region_num, addr, size, type)); > + EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, dev); > Please don't change the name of the PCIDevice pointer argument from pci_dev to dev. This dev, dev in DO_UPCAST is ugly and misleading. > > - assert(region_num == 1); > - register_ioport_write(addr, size, 1, ioport_write1, s); > - register_ioport_read(addr, size, 1, ioport_read1, s); > - register_ioport_write(addr, size, 2, ioport_write2, s); > - register_ioport_read(addr, size, 2, ioport_read2, s); > - register_ioport_write(addr, size, 4, ioport_write4, s); > - register_ioport_read(addr, size, 4, ioport_read4, s); > - > - s->region[region_num] = addr; > -} > - > -/***************************************************************************** > - * > - * Memory mapped I/O. > - * > - ****************************************************************************/ > - > -static void pci_mmio_writeb(void *opaque, target_phys_addr_t addr, uint32_t val) > -{ > - EEPRO100State *s = opaque; > - //~ logout("addr=%s val=0x%02x\n", regname(addr), val); > - eepro100_write1(s, addr, val); > -} > - > -static void pci_mmio_writew(void *opaque, target_phys_addr_t addr, uint32_t val) > -{ > - EEPRO100State *s = opaque; > - //~ logout("addr=%s val=0x%02x\n", regname(addr), val); > - eepro100_write2(s, addr, val); > -} > - > -static void pci_mmio_writel(void *opaque, target_phys_addr_t addr, uint32_t val) > -{ > - EEPRO100State *s = opaque; > - //~ logout("addr=%s val=0x%02x\n", regname(addr), val); > - eepro100_write4(s, addr, val); > -} > - > -static uint32_t pci_mmio_readb(void *opaque, target_phys_addr_t addr) > -{ > - EEPRO100State *s = opaque; > - //~ logout("addr=%s\n", regname(addr)); > - return eepro100_read1(s, addr); > -} > - > -static uint32_t pci_mmio_readw(void *opaque, target_phys_addr_t addr) > -{ > - EEPRO100State *s = opaque; > - //~ logout("addr=%s\n", regname(addr)); > - return eepro100_read2(s, addr); > -} > - > -static uint32_t pci_mmio_readl(void *opaque, target_phys_addr_t addr) > -{ > - EEPRO100State *s = opaque; > - //~ logout("addr=%s\n", regname(addr)); > - return eepro100_read4(s, addr); > + if (size == 1) { > + eepro100_write1(s, addr, value); > + } else if (size == 2) { > + eepro100_write2(s, addr, value); > + } else { > + eepro100_write4(s, addr, value); > + } > } > > -static CPUWriteMemoryFunc * const pci_mmio_write[] = { > - pci_mmio_writeb, > - pci_mmio_writew, > - pci_mmio_writel > -}; > - > -static CPUReadMemoryFunc * const pci_mmio_read[] = { > - pci_mmio_readb, > - pci_mmio_readw, > - pci_mmio_readl > -}; > - > -static void pci_mmio_map(PCIDevice * pci_dev, int region_num, > - pcibus_t addr, pcibus_t size, int type) > +static uint32_t pci_io_read(PCIDevice *dev, pcibus_t addr, int size) > { > Don't change pci_dev -> dev. See above. ...