From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MBQr7-0005rr-QY for qemu-devel@nongnu.org; Tue, 02 Jun 2009 06:04:29 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MBQr2-0005r5-4I for qemu-devel@nongnu.org; Tue, 02 Jun 2009 06:04:28 -0400 Received: from [199.232.76.173] (port=54180 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MBQr1-0005r2-Tn for qemu-devel@nongnu.org; Tue, 02 Jun 2009 06:04:23 -0400 Received: from mx2.redhat.com ([66.187.237.31]:53201) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MBQr1-0002U8-99 for qemu-devel@nongnu.org; Tue, 02 Jun 2009 06:04:23 -0400 Date: Tue, 2 Jun 2009 13:01:21 +0300 From: "Michael S. Tsirkin" Message-ID: <20090602100121.GA3397@redhat.com> References: <1243924970-17545-1-git-send-email-yamahata@valinux.co.jp> <1243924970-17545-4-git-send-email-yamahata@valinux.co.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1243924970-17545-4-git-send-email-yamahata@valinux.co.jp> Subject: [Qemu-devel] Re: [PATCH 3/7] pci: pci_default_config_write() clean up. List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Isaku Yamahata Cc: mtosatti@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com, paul@codesourcery.com, avi@redhat.com On Tue, Jun 02, 2009 at 03:42:46PM +0900, Isaku Yamahata wrote: > +struct PCIConfigReg { > + uint8_t wmask; > + /* offset of registers in bits for 2/4 bytes function register */ > + uint8_t reg_offset; Sorry about being dense, but the comment still doesn't help me much. Can't we simply use the index in the array as offset? > + pci_config_written_t callback; > +}; > > struct PCIDevice { > DeviceState qdev; > /* PCI config space */ > uint8_t config[PCI_CONFIG_SPACE_SIZE]; > - > - /* Used to implement R/W bytes */ > - uint8_t mask[PCI_CONFIG_SPACE_SIZE]; > + struct PCIConfigReg config_regs[PCI_CONFIG_SPACE_SIZE]; I still think separate mask/config/callback arrays are better - they are easier for devices to use. E.g. a single memset can make a range of register writeable, and a single function call does everything necessary to save a range the whole config space. Add a callback array if you like, and be done with it. > > /* the following fields are read only */ > PCIBus *bus; > @@ -180,6 +261,21 @@ struct PCIDevice { > int irq_state[4]; > }; > > +typedef void(*pci_conf_init_t)(struct PCIConfigReg*); > + > +void pci_conf_initb(struct PCIConfigReg *config_regs, uint32_t addr, > + pci_config_written_t callback, uint32_t wmask); > +void pci_conf_initw(struct PCIConfigReg *config_regs, uint32_t addr, > + pci_config_written_t callback, uint32_t wmask); > +void pci_conf_initl(struct PCIConfigReg *config_regs, uint32_t addr, > + pci_config_written_t callback, uint32_t wmask); If we got rid of reg_offset, I think we won't need these. We'd just do dev->callback[REGISTER] = my_callback. -- MST