From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NmogH-0001pM-Mh for qemu-devel@nongnu.org; Wed, 03 Mar 2010 08:32:05 -0500 Received: from [199.232.76.173] (port=33952 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NmogH-0001pB-7x for qemu-devel@nongnu.org; Wed, 03 Mar 2010 08:32:05 -0500 Received: from Debian-exim by monty-python.gnu.org with spam-scanned (Exim 4.60) (envelope-from ) id 1NmogG-0000w7-2F for qemu-devel@nongnu.org; Wed, 03 Mar 2010 08:32:05 -0500 Received: from mx1.redhat.com ([209.132.183.28]:15288) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NmogF-0000vm-LD for qemu-devel@nongnu.org; Wed, 03 Mar 2010 08:32:03 -0500 Message-ID: <4B8E64C9.10406@redhat.com> Date: Wed, 03 Mar 2010 14:31:53 +0100 From: Gerd Hoffmann MIME-Version: 1.0 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> In-Reply-To: <20100303115111.GD15278@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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: "Michael S. Tsirkin" Cc: QEMU Developers 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. >> 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