From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Nebau-0001NL-V5 for qemu-devel@nongnu.org; Mon, 08 Feb 2010 16:56:36 -0500 Received: from [199.232.76.173] (port=56552 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Nebau-0001ND-LI for qemu-devel@nongnu.org; Mon, 08 Feb 2010 16:56:36 -0500 Received: from Debian-exim by monty-python.gnu.org with spam-scanned (Exim 4.60) (envelope-from ) id 1Nebat-000196-1L for qemu-devel@nongnu.org; Mon, 08 Feb 2010 16:56:36 -0500 Received: from mail-iw0-f185.google.com ([209.85.223.185]:44610) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Nebar-00018m-8W for qemu-devel@nongnu.org; Mon, 08 Feb 2010 16:56:33 -0500 Received: by iwn15 with SMTP id 15so4369485iwn.19 for ; Mon, 08 Feb 2010 13:56:32 -0800 (PST) Message-ID: <4B70888D.7000702@codemonkey.ws> Date: Mon, 08 Feb 2010 15:56:29 -0600 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] Re: [PATCH] pci: initialize header type register. References: <20100208173204.GA10716@redhat.com> <4B704BE5.9010205@redhat.com> <20100208173741.GB10716@redhat.com> <20100208182624.GD10716@redhat.com> <4B706C4E.2010508@redhat.com> <20100208201919.GA17088@redhat.com> <4B7074DA.10408@codemonkey.ws> <20100208203459.GC17088@redhat.com> <4B707A00.2000706@codemonkey.ws> <20100208210147.GA17248@redhat.com> In-Reply-To: <20100208210147.GA17248@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: Blue Swirl , Isaku Yamahata , Gerd Hoffmann , qemu-devel@nongnu.org On 02/08/2010 03:01 PM, Michael S. Tsirkin wrote: >> Sorry, but: >> >> versatile_pci.c: d->config[0x04] = 0x00; >> versatile_pci.c: d->config[0x05] = 0x00; >> versatile_pci.c: d->config[0x06] = 0x20; >> versatile_pci.c: d->config[0x07] = 0x02; >> >> To: >> >> pci_config_set_command(d, 0); >> pci_config_set_status(d, PCI_STATUS_DEVSEL_MEDIUM | PCI_STATUS_66MHZ); >> >> Is a huge improvement. >> > > Yes but > > pci_set_word(d->config + PCI_COMMAND, 0); > pci_set_word(d->config + PCI_STATUS, PCI_STATUS_DEVSEL_MEDIUM | PCI_STATUS_66MHZ); > > is not much worse, and that API is already there. > And advatage is it uses macros from linux which have > higher chance to be correct than what we come up with. > Oh, pci_set_word() is certainly an improvement. Personally, I prefer passing the PCIDevice as a context and adding individual accessors but anything is better than open coded config. >> I'm staring at a PCI config space diagram right >> now and I'm *still* not even sure I'm interpreting the versatile_pci >> code correctly :-) >> > I spent time cleaning up devices, just did not get to bridges. > What I did is write patches and verify that compiled code > did not change at all. This guarantees no breakage. > Care to volunteer to complete that work? > Separately people that are familiar with device can clean it up. > It's on my radar but I've got another PCI series in flight. I've got a branch pci-cleanup on staging that's a work in progress for adding a proper region API along with PCI memory accessors. Once I find a little more time to finish converting VGA devices, I'll post. Regards, Anthony Liguori