From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NmolH-0004VW-0c for qemu-devel@nongnu.org; Wed, 03 Mar 2010 08:37:15 -0500 Received: from [199.232.76.173] (port=33392 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NmolG-0004Uz-9v for qemu-devel@nongnu.org; Wed, 03 Mar 2010 08:37:14 -0500 Received: from Debian-exim by monty-python.gnu.org with spam-scanned (Exim 4.60) (envelope-from ) id 1NmolF-0001IV-D7 for qemu-devel@nongnu.org; Wed, 03 Mar 2010 08:37:14 -0500 Received: from mx1.redhat.com ([209.132.183.28]:56488) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NmolE-0001IC-K7 for qemu-devel@nongnu.org; Wed, 03 Mar 2010 08:37:13 -0500 Date: Wed, 3 Mar 2010 15:33:48 +0200 From: "Michael S. Tsirkin" Message-ID: <20100303133348.GA21663@redhat.com> References: <4B7821AC.6080400@mail.berlios.de> <1266164189-21062-1-git-send-email-weil@mail.berlios.de> <1266164189-21062-2-git-send-email-weil@mail.berlios.de> <1266164189-21062-3-git-send-email-weil@mail.berlios.de> <1266164189-21062-4-git-send-email-weil@mail.berlios.de> <1266164189-21062-5-git-send-email-weil@mail.berlios.de> <20100221170021.GA5185@redhat.com> <4B81A0E1.5030300@mail.berlios.de> <20100303115111.GD15278@redhat.com> <4B8E64C9.10406@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4B8E64C9.10406@redhat.com> Subject: [Qemu-devel] Re: [PATCH 05/20] eepro100: Add all supported devices to pci.c List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: QEMU Developers On Wed, Mar 03, 2010 at 02:31:53PM +0100, Gerd Hoffmann wrote: > On 03/03/10 12:51, Michael S. Tsirkin wrote: >> On Sun, Feb 21, 2010 at 10:08:49PM +0100, Stefan Weil wrote: >>>>> static const char * const pci_nic_models[] = { >>>>> "ne2k_pci", >>>>> + "i82550", >>>>> "i82551", >>>>> + "i82557a", >>>>> "i82557b", >>>>> + "i82557c", >>>>> + "i82558a", >>>>> + "i82558b", >>>>> + "i82559a", >>>>> + "i82559b", >>>>> + "i82559c", >>>>> "i82559er", >>>>> + "i82562", >>>>> "rtl8139", >>>>> "e1000", >>>>> "pcnet", > >>>> One wonders: would it be cleaner to have a single eepro100 device >>>> with specific model as qdev option? > > No. I think it would be cleaner to use qdev even more. For that > specific driver it would make sense to create a private Info struct, > i.e. something like this: > > E100PCIDeviceInfo { > PCIDeviceInfo pci; > [ model specific stuff such as has_extended_tcb_support here ] > }; > > Then go > > static E100PCIDeviceInfo eepro100_info[] = { > { > .pci.qdev.name = "i82550", > [ usual qdev stuff here ] > has_extended_tcb_support = 0 | 1; > [ more e100 device description bits here ] > }, > [ ... ] > }; > > Then you can simply zap all the one-liner wrapper functions around > nic_init(). nic_init() can get a pointer using something like this: > > E100PCIDeviceInfo *einfo = DO_UPCAST(E100PCIDeviceInfo, > pci.qdev, pci_dev->qdev.info); > > and setup the device accordingly. > >>> I prefer the "official" device names which are also >>> used in the Intel documentation. eepro100 or e100 >>> are marketing names (more of them exist). > > qdev has aliases. You can add .qdev.alias = "eepro100" to one of the > variants, then the eepro100 name will work as well. > > Adding the marketing name to the .desc test is probably a good idea too > because alot of people know the marketing name only. That's my thinking as well. >>> Please note that this patch was marked as optional. >>> The arrays pci_nic_names and pci_nic_models are >>> not really needed, and as soon as the table of available >>> nics is automatically derived from the device information, >>> all variants of i825xx are supported automatically. > >> Gerd, any input on this patch? > > Looks fine to me. When we switch over to walking the list of registered > devices instead of having a hard-coded list of pci nic drivers all the > eepro100 variants will show up in the list anyway. > > cheers, > Gerd -- MST