Message ID | 20100208064503.GE22624@valinux.co.jp |
---|---|
State | New |
Headers | show |
On Mon, Feb 08, 2010 at 03:45:03PM +0900, Isaku Yamahata wrote: > The change set of 776e1bbb6cf4fe66a93c1a5dd814bbb650deca00 > specifies pbm pci host bridge is type of bridge. > It contradicts with pbm_pci_host_init(). > > Blue Swirl, could you please check this patch? > To be honest I don't know about pbm pci host bridge so that > I don't know which is correct, pbm_pci_host_init() or pbm_pci_host_info. > I just took the older code. > > Cc: Blue Swirl <blauwirbel@gmail.com> > Cc: "Michael S. Tsirkin" <mst@redhat.com> > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp> Blue Swirl, can you Ack please? > --- > hw/apb_pci.c | 3 +-- > 1 files changed, 1 insertions(+), 2 deletions(-) > > diff --git a/hw/apb_pci.c b/hw/apb_pci.c > index 46d5b0e..359a84f 100644 > --- a/hw/apb_pci.c > +++ b/hw/apb_pci.c > @@ -471,7 +471,7 @@ static int pbm_pci_host_init(PCIDevice *d) > d->config[0x09] = 0x00; // programming i/f > pci_config_set_class(d->config, PCI_CLASS_BRIDGE_HOST); > d->config[0x0D] = 0x10; // latency_timer Looks like apb_pci needs another sweep of getting rid of hard-coded constants. Any takers? > - d->config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type > + /* header type is initialized by do_pci_register_device() */ Let's not put such comments around code: that function can get renamed or removed or moved to another file and no one will remember to update this comment. This belongs in commit message. > return 0; > } > > @@ -479,7 +479,6 @@ static PCIDeviceInfo pbm_pci_host_info = { > .qdev.name = "pbm", > .qdev.size = sizeof(PCIDevice), > .init = pbm_pci_host_init, > - .header_type = PCI_HEADER_TYPE_BRIDGE, > }; > > static SysBusDeviceInfo pbm_host_info = { > -- > 1.6.6.1
On Mon, Feb 08, 2010 at 03:45:03PM +0900, Isaku Yamahata wrote: > The change set of 776e1bbb6cf4fe66a93c1a5dd814bbb650deca00 > specifies pbm pci host bridge is type of bridge. > It contradicts with pbm_pci_host_init(). By the way, the next below (cover letter) should be put after --- rather than here, so that it does not end up in commit message. > Blue Swirl, could you please check this patch? > To be honest I don't know about pbm pci host bridge so that > I don't know which is correct, pbm_pci_host_init() or pbm_pci_host_info. > I just took the older code. > > Cc: Blue Swirl <blauwirbel@gmail.com> > Cc: "Michael S. Tsirkin" <mst@redhat.com> > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp> > --- > hw/apb_pci.c | 3 +-- > 1 files changed, 1 insertions(+), 2 deletions(-) > > diff --git a/hw/apb_pci.c b/hw/apb_pci.c > index 46d5b0e..359a84f 100644 > --- a/hw/apb_pci.c > +++ b/hw/apb_pci.c > @@ -471,7 +471,7 @@ static int pbm_pci_host_init(PCIDevice *d) > d->config[0x09] = 0x00; // programming i/f > pci_config_set_class(d->config, PCI_CLASS_BRIDGE_HOST); > d->config[0x0D] = 0x10; // latency_timer > - d->config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type > + /* header type is initialized by do_pci_register_device() */ > return 0; > } > > @@ -479,7 +479,6 @@ static PCIDeviceInfo pbm_pci_host_info = { > .qdev.name = "pbm", > .qdev.size = sizeof(PCIDevice), > .init = pbm_pci_host_init, > - .header_type = PCI_HEADER_TYPE_BRIDGE, > }; > > static SysBusDeviceInfo pbm_host_info = { > -- > 1.6.6.1
On Mon, Feb 8, 2010 at 8:45 AM, Isaku Yamahata <yamahata@valinux.co.jp> wrote: > The change set of 776e1bbb6cf4fe66a93c1a5dd814bbb650deca00 > specifies pbm pci host bridge is type of bridge. > It contradicts with pbm_pci_host_init(). Bridge header type in qdev info is needed so that the write masks are correct. Otherwise the masks make for example PCI_SECONDARY_BUS readonly. But the device uses PCI_HEADER_TYPE_NORMAL. So both are correct.
diff --git a/hw/apb_pci.c b/hw/apb_pci.c index 46d5b0e..359a84f 100644 --- a/hw/apb_pci.c +++ b/hw/apb_pci.c @@ -471,7 +471,7 @@ static int pbm_pci_host_init(PCIDevice *d) d->config[0x09] = 0x00; // programming i/f pci_config_set_class(d->config, PCI_CLASS_BRIDGE_HOST); d->config[0x0D] = 0x10; // latency_timer - d->config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type + /* header type is initialized by do_pci_register_device() */ return 0; } @@ -479,7 +479,6 @@ static PCIDeviceInfo pbm_pci_host_info = { .qdev.name = "pbm", .qdev.size = sizeof(PCIDevice), .init = pbm_pci_host_init, - .header_type = PCI_HEADER_TYPE_BRIDGE, }; static SysBusDeviceInfo pbm_host_info = {
The change set of 776e1bbb6cf4fe66a93c1a5dd814bbb650deca00 specifies pbm pci host bridge is type of bridge. It contradicts with pbm_pci_host_init(). Blue Swirl, could you please check this patch? To be honest I don't know about pbm pci host bridge so that I don't know which is correct, pbm_pci_host_init() or pbm_pci_host_info. I just took the older code. Cc: Blue Swirl <blauwirbel@gmail.com> Cc: "Michael S. Tsirkin" <mst@redhat.com> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp> --- hw/apb_pci.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-)