From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=50886 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OOxmo-0005k3-5L for qemu-devel@nongnu.org; Wed, 16 Jun 2010 14:56:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OOxml-00011f-Js for qemu-devel@nongnu.org; Wed, 16 Jun 2010 14:56:29 -0400 Received: from mx1.redhat.com ([209.132.183.28]:40136) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OOxml-00011J-AW for qemu-devel@nongnu.org; Wed, 16 Jun 2010 14:56:27 -0400 Date: Wed, 16 Jun 2010 21:51:34 +0300 From: "Michael S. Tsirkin" Message-ID: <20100616185134.GC7183@redhat.com> References: <20100615091207.GA1365@redhat.com> <20100616022002.GB7932@valinux.co.jp> <20100616085425.GA4637@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: Content-Transfer-Encoding: quoted-printable Subject: [Qemu-devel] Re: [PATCH 2/2] pci: don't overwrite pci header type. List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: Isaku Yamahata , qemu-devel@nongnu.org On Wed, Jun 16, 2010 at 06:41:22PM +0000, Blue Swirl wrote: > On Wed, Jun 16, 2010 at 8:54 AM, Michael S. Tsirkin wr= ote: > > On Wed, Jun 16, 2010 at 11:20:02AM +0900, Isaku Yamahata wrote: > >> On Tue, Jun 15, 2010 at 12:12:07PM +0300, Michael S. Tsirkin wrote: > >> > On Tue, Jun 15, 2010 at 02:06:46PM +0900, Isaku Yamahata wrote: > >> > > Don't overwrite pci header type. > >> > > Otherwise, multi function bit which pci_init_header_type() sets > >> > > appropriately is lost. > >> > > Anyway PCI_HEADER_TYPE_NORMAL is zero, so it is unnecessary to z= ero > >> > > which is already zero cleared. > >> > > > >> > > Signed-off-by: Isaku Yamahata > >> > > >> > ... > >> > > >> > > diff --git a/hw/apb_pci.c b/hw/apb_pci.c > >> > > index 31c8d70..cdf3bc2 100644 > >> > > --- a/hw/apb_pci.c > >> > > +++ b/hw/apb_pci.c > >> > > @@ -428,7 +428,8 @@ static int pbm_pci_host_init(PCIDevice *d) > >> > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 PCI_STATUS_DEVSEL_MEDIUM); > >> > > =A0 =A0 =A0pci_config_set_class(d->config, PCI_CLASS_BRIDGE_HOST= ); > >> > > =A0 =A0 =A0pci_set_byte(d->config + PCI_HEADER_TYPE, > >> > > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 PCI_HEADER_TYPE_NORMAL); > >> > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (pci_get_byte(d->config + PCI_= HEADER_TYPE) & > >> > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0PCI_HEADER_TYPE_MULTI_FUNCT= ION) | PCI_HEADER_TYPE_NORMAL); > >> > > >> > what is this doing? > >> > >> It changes the header type to normal device(bit 1-7) without overwri= ting > >> multi function bit(bit 8). > > > > Don't we know what the multi function bit value is? > > > >> Apb host bridge specifies PCI_HEADER_TYPE_BRIDGE in PCIDeviceInfo, > >> on the other hand pbc_pci_host_init() sets the register > >> to PCI_HEADER_TYPE_NORMAL. > >> To be honest I don't know why it does so, but that is what Blue want= s. > > > > BTW I think it would be prettier to have is_bridge instead of header_= type > > as a qdev property. Agree? >=20 > Good idea. >=20 > >> So I touch only multi function bit(bit 8) and leave other bit (bit 1= -7) > >> unchanged. > >> > >> If you don't like this hunk, I'll drop this hunk and leave it to Blu= e. > >> What do you think? > > > > Blue Swirl, could you comment on this please? >=20 > I'd go for is_bridge and drop the override for header type in apb_pci.c= then. Yes, but what header type does it need? > >> static PCIDeviceInfo pbm_pci_host_info =3D { > >> =A0 =A0 .qdev.name =3D "pbm", > >> =A0 =A0 .qdev.size =3D sizeof(PCIDevice), > >> =A0 =A0 .init =A0 =A0 =A0=3D pbm_pci_host_init, > >> =A0 =A0 .header_type =A0=3D PCI_HEADER_TYPE_BRIDGE, =A0 <<<<< Here > >> }; > >> > >> -- > >> yamahata > >