Patchwork [V5,07/29] pci/bridge: clean up of pci_bridge_initfn()

login
register
mail settings
Submitter Isaku Yamahata
Date Oct. 9, 2009, 6:28 a.m.
Message ID <1255069742-15724-8-git-send-email-yamahata@valinux.co.jp>
Download mbox | patch
Permalink /patch/35570/
State Under Review
Headers show

Comments

Isaku Yamahata - Oct. 9, 2009, 6:28 a.m.
- use symbolic constant
- use helper function pci_set_xxx()

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
 hw/pci.c |   23 ++++++++++++-----------
 1 files changed, 12 insertions(+), 11 deletions(-)
Michael S. Tsirkin - Oct. 9, 2009, 6:53 a.m.
On Fri, Oct 09, 2009 at 03:28:40PM +0900, Isaku Yamahata wrote:
> - use symbolic constant
> - use helper function pci_set_xxx()
> 
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> ---
>  hw/pci.c |   23 ++++++++++++-----------
>  1 files changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/pci.c b/hw/pci.c
> index a66e3de..eaf471a 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -923,17 +923,18 @@ static int pci_bridge_initfn(PCIDevice *dev)
>      pci_config_set_vendor_id(s->dev.config, s->vid);
>      pci_config_set_device_id(s->dev.config, s->did);
>  
> -    s->dev.config[0x04] = 0x06; // command = bus master, pci mem
> -    s->dev.config[0x05] = 0x00;
> -    s->dev.config[0x06] = 0xa0; // status = fast back-to-back, 66MHz, no error
> -    s->dev.config[0x07] = 0x00; // status = fast devsel
> -    s->dev.config[0x08] = 0x00; // revision
> -    s->dev.config[0x09] = 0x00; // programming i/f
> -    pci_config_set_class(s->dev.config, PCI_CLASS_BRIDGE_PCI);
> -    s->dev.config[0x0D] = 0x10; // latency_timer
> -    s->dev.config[PCI_HEADER_TYPE] =
> -        PCI_HEADER_TYPE_MULTI_FUNCTION | PCI_HEADER_TYPE_BRIDGE; // header_type
> -    s->dev.config[0x1E] = 0xa0; // secondary status
> +    pci_set_word(dev->config + PCI_COMMAND,
> +                 PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER);

BTW, I think this is a wrong reset value: should be disabled
by default. Fixing this needs some testing though, so
I am not suggesting we do it in this patch. Have some time
to fix this?

> +    pci_set_word(dev->config + PCI_STATUS,
> +                 PCI_STATUS_66MHZ | PCI_STATUS_FAST_BACK);
> +    dev->config[PCI_REVISION_ID] = 0x00;
> +    dev->config[PCI_CLASS_PROG] = 0x00;

I think we can remove these two: this is the default already.

> +    pci_config_set_class(dev->config, PCI_CLASS_BRIDGE_PCI);
> +    dev->config[PCI_LATENCY_TIMER] = 0x10;
> +    dev->config[PCI_HEADER_TYPE] =
> +        PCI_HEADER_TYPE_MULTI_FUNCTION | PCI_HEADER_TYPE_BRIDGE;
> +    pci_set_word(dev->config + PCI_SEC_STATUS,
> +                 PCI_STATUS_66MHZ | PCI_STATUS_FAST_BACK);
>      return 0;
>  }
>  
> -- 
> 1.6.0.2
Michael S. Tsirkin - Oct. 9, 2009, 6:54 a.m.
On Fri, Oct 09, 2009 at 03:28:40PM +0900, Isaku Yamahata wrote:
> - use symbolic constant
> - use helper function pci_set_xxx()
> 
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>

Posted some minor comments previously, but anyway, this is
much better than what we now have.

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  hw/pci.c |   23 ++++++++++++-----------
>  1 files changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/pci.c b/hw/pci.c
> index a66e3de..eaf471a 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -923,17 +923,18 @@ static int pci_bridge_initfn(PCIDevice *dev)
>      pci_config_set_vendor_id(s->dev.config, s->vid);
>      pci_config_set_device_id(s->dev.config, s->did);
>  
> -    s->dev.config[0x04] = 0x06; // command = bus master, pci mem
> -    s->dev.config[0x05] = 0x00;
> -    s->dev.config[0x06] = 0xa0; // status = fast back-to-back, 66MHz, no error
> -    s->dev.config[0x07] = 0x00; // status = fast devsel
> -    s->dev.config[0x08] = 0x00; // revision
> -    s->dev.config[0x09] = 0x00; // programming i/f
> -    pci_config_set_class(s->dev.config, PCI_CLASS_BRIDGE_PCI);
> -    s->dev.config[0x0D] = 0x10; // latency_timer
> -    s->dev.config[PCI_HEADER_TYPE] =
> -        PCI_HEADER_TYPE_MULTI_FUNCTION | PCI_HEADER_TYPE_BRIDGE; // header_type
> -    s->dev.config[0x1E] = 0xa0; // secondary status
> +    pci_set_word(dev->config + PCI_COMMAND,
> +                 PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER);
> +    pci_set_word(dev->config + PCI_STATUS,
> +                 PCI_STATUS_66MHZ | PCI_STATUS_FAST_BACK);
> +    dev->config[PCI_REVISION_ID] = 0x00;
> +    dev->config[PCI_CLASS_PROG] = 0x00;
> +    pci_config_set_class(dev->config, PCI_CLASS_BRIDGE_PCI);
> +    dev->config[PCI_LATENCY_TIMER] = 0x10;
> +    dev->config[PCI_HEADER_TYPE] =
> +        PCI_HEADER_TYPE_MULTI_FUNCTION | PCI_HEADER_TYPE_BRIDGE;
> +    pci_set_word(dev->config + PCI_SEC_STATUS,
> +                 PCI_STATUS_66MHZ | PCI_STATUS_FAST_BACK);
>      return 0;
>  }
>  
> -- 
> 1.6.0.2
Isaku Yamahata - Oct. 13, 2009, 1:20 p.m.
On Fri, Oct 09, 2009 at 08:53:10AM +0200, Michael S. Tsirkin wrote:
> On Fri, Oct 09, 2009 at 03:28:40PM +0900, Isaku Yamahata wrote:
> > - use symbolic constant
> > - use helper function pci_set_xxx()
> > 
> > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> > ---
> >  hw/pci.c |   23 ++++++++++++-----------
> >  1 files changed, 12 insertions(+), 11 deletions(-)
> > 
> > diff --git a/hw/pci.c b/hw/pci.c
> > index a66e3de..eaf471a 100644
> > --- a/hw/pci.c
> > +++ b/hw/pci.c
> > @@ -923,17 +923,18 @@ static int pci_bridge_initfn(PCIDevice *dev)
> >      pci_config_set_vendor_id(s->dev.config, s->vid);
> >      pci_config_set_device_id(s->dev.config, s->did);
> >  
> > -    s->dev.config[0x04] = 0x06; // command = bus master, pci mem
> > -    s->dev.config[0x05] = 0x00;
> > -    s->dev.config[0x06] = 0xa0; // status = fast back-to-back, 66MHz, no error
> > -    s->dev.config[0x07] = 0x00; // status = fast devsel
> > -    s->dev.config[0x08] = 0x00; // revision
> > -    s->dev.config[0x09] = 0x00; // programming i/f
> > -    pci_config_set_class(s->dev.config, PCI_CLASS_BRIDGE_PCI);
> > -    s->dev.config[0x0D] = 0x10; // latency_timer
> > -    s->dev.config[PCI_HEADER_TYPE] =
> > -        PCI_HEADER_TYPE_MULTI_FUNCTION | PCI_HEADER_TYPE_BRIDGE; // header_type
> > -    s->dev.config[0x1E] = 0xa0; // secondary status
> > +    pci_set_word(dev->config + PCI_COMMAND,
> > +                 PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER);
> 
> BTW, I think this is a wrong reset value: should be disabled
> by default. Fixing this needs some testing though, so
> I am not suggesting we do it in this patch. Have some time
> to fix this?

Hmm, the user of it is only apb_pci.c
I guess other magic values came from the real machine.
So one possible fix is to create apb_pci specific initialization function
and to move those initialization code into apb_pci.c leaving to
sparc guys. So we can avoid breakage.
What do you think of it?



> > +    pci_set_word(dev->config + PCI_STATUS,
> > +                 PCI_STATUS_66MHZ | PCI_STATUS_FAST_BACK);
> > +    dev->config[PCI_REVISION_ID] = 0x00;
> > +    dev->config[PCI_CLASS_PROG] = 0x00;
> 
> I think we can remove these two: this is the default already.

Okay.


> > +    pci_config_set_class(dev->config, PCI_CLASS_BRIDGE_PCI);
> > +    dev->config[PCI_LATENCY_TIMER] = 0x10;
> > +    dev->config[PCI_HEADER_TYPE] =
> > +        PCI_HEADER_TYPE_MULTI_FUNCTION | PCI_HEADER_TYPE_BRIDGE;
> > +    pci_set_word(dev->config + PCI_SEC_STATUS,
> > +                 PCI_STATUS_66MHZ | PCI_STATUS_FAST_BACK);
> >      return 0;
> >  }
> >  
>
Michael S. Tsirkin - Oct. 13, 2009, 2:17 p.m.
On Tue, Oct 13, 2009 at 10:20:07PM +0900, Isaku Yamahata wrote:
> On Fri, Oct 09, 2009 at 08:53:10AM +0200, Michael S. Tsirkin wrote:
> > On Fri, Oct 09, 2009 at 03:28:40PM +0900, Isaku Yamahata wrote:
> > > - use symbolic constant
> > > - use helper function pci_set_xxx()
> > > 
> > > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> > > ---
> > >  hw/pci.c |   23 ++++++++++++-----------
> > >  1 files changed, 12 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/hw/pci.c b/hw/pci.c
> > > index a66e3de..eaf471a 100644
> > > --- a/hw/pci.c
> > > +++ b/hw/pci.c
> > > @@ -923,17 +923,18 @@ static int pci_bridge_initfn(PCIDevice *dev)
> > >      pci_config_set_vendor_id(s->dev.config, s->vid);
> > >      pci_config_set_device_id(s->dev.config, s->did);
> > >  
> > > -    s->dev.config[0x04] = 0x06; // command = bus master, pci mem
> > > -    s->dev.config[0x05] = 0x00;
> > > -    s->dev.config[0x06] = 0xa0; // status = fast back-to-back, 66MHz, no error
> > > -    s->dev.config[0x07] = 0x00; // status = fast devsel
> > > -    s->dev.config[0x08] = 0x00; // revision
> > > -    s->dev.config[0x09] = 0x00; // programming i/f
> > > -    pci_config_set_class(s->dev.config, PCI_CLASS_BRIDGE_PCI);
> > > -    s->dev.config[0x0D] = 0x10; // latency_timer
> > > -    s->dev.config[PCI_HEADER_TYPE] =
> > > -        PCI_HEADER_TYPE_MULTI_FUNCTION | PCI_HEADER_TYPE_BRIDGE; // header_type
> > > -    s->dev.config[0x1E] = 0xa0; // secondary status
> > > +    pci_set_word(dev->config + PCI_COMMAND,
> > > +                 PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER);
> > 
> > BTW, I think this is a wrong reset value: should be disabled
> > by default. Fixing this needs some testing though, so
> > I am not suggesting we do it in this patch. Have some time
> > to fix this?
> 
> Hmm, the user of it is only apb_pci.c
> I guess other magic values came from the real machine.
> So one possible fix is to create apb_pci specific initialization function
> and to move those initialization code into apb_pci.c leaving to
> sparc guys. So we can avoid breakage.
> What do you think of it?

That'll work, but let's start with Cc relevant people.  The issue here is
that the bridge is inited with memory enabled which is out of spec, but
we are concerned what fixing this will do - in the past such fixes
sometimes triggered BIOS bugs.  Anyone can test a fix for this?

> 
> 
> > > +    pci_set_word(dev->config + PCI_STATUS,
> > > +                 PCI_STATUS_66MHZ | PCI_STATUS_FAST_BACK);
> > > +    dev->config[PCI_REVISION_ID] = 0x00;
> > > +    dev->config[PCI_CLASS_PROG] = 0x00;
> > 
> > I think we can remove these two: this is the default already.
> 
> Okay.
> 
> 
> > > +    pci_config_set_class(dev->config, PCI_CLASS_BRIDGE_PCI);
> > > +    dev->config[PCI_LATENCY_TIMER] = 0x10;
> > > +    dev->config[PCI_HEADER_TYPE] =
> > > +        PCI_HEADER_TYPE_MULTI_FUNCTION | PCI_HEADER_TYPE_BRIDGE;
> > > +    pci_set_word(dev->config + PCI_SEC_STATUS,
> > > +                 PCI_STATUS_66MHZ | PCI_STATUS_FAST_BACK);
> > >      return 0;
> > >  }
> > >  
> > 
> 
> -- 
> yamahata
Blue Swirl - Oct. 13, 2009, 3:12 p.m.
On Tue, Oct 13, 2009 at 4:20 PM, Isaku Yamahata <yamahata@valinux.co.jp> wrote:
> On Fri, Oct 09, 2009 at 08:53:10AM +0200, Michael S. Tsirkin wrote:
>> On Fri, Oct 09, 2009 at 03:28:40PM +0900, Isaku Yamahata wrote:
>> > - use symbolic constant
>> > - use helper function pci_set_xxx()
>> >
>> > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
>> > ---
>> >  hw/pci.c |   23 ++++++++++++-----------
>> >  1 files changed, 12 insertions(+), 11 deletions(-)
>> >
>> > diff --git a/hw/pci.c b/hw/pci.c
>> > index a66e3de..eaf471a 100644
>> > --- a/hw/pci.c
>> > +++ b/hw/pci.c
>> > @@ -923,17 +923,18 @@ static int pci_bridge_initfn(PCIDevice *dev)
>> >      pci_config_set_vendor_id(s->dev.config, s->vid);
>> >      pci_config_set_device_id(s->dev.config, s->did);
>> >
>> > -    s->dev.config[0x04] = 0x06; // command = bus master, pci mem
>> > -    s->dev.config[0x05] = 0x00;
>> > -    s->dev.config[0x06] = 0xa0; // status = fast back-to-back, 66MHz, no error
>> > -    s->dev.config[0x07] = 0x00; // status = fast devsel
>> > -    s->dev.config[0x08] = 0x00; // revision
>> > -    s->dev.config[0x09] = 0x00; // programming i/f
>> > -    pci_config_set_class(s->dev.config, PCI_CLASS_BRIDGE_PCI);
>> > -    s->dev.config[0x0D] = 0x10; // latency_timer
>> > -    s->dev.config[PCI_HEADER_TYPE] =
>> > -        PCI_HEADER_TYPE_MULTI_FUNCTION | PCI_HEADER_TYPE_BRIDGE; // header_type
>> > -    s->dev.config[0x1E] = 0xa0; // secondary status
>> > +    pci_set_word(dev->config + PCI_COMMAND,
>> > +                 PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER);
>>
>> BTW, I think this is a wrong reset value: should be disabled
>> by default. Fixing this needs some testing though, so
>> I am not suggesting we do it in this patch. Have some time
>> to fix this?
>
> Hmm, the user of it is only apb_pci.c
> I guess other magic values came from the real machine.
> So one possible fix is to create apb_pci specific initialization function
> and to move those initialization code into apb_pci.c leaving to
> sparc guys. So we can avoid breakage.
> What do you think of it?

Current code is buggy. According to manual (805-1251) the reset value
should be zero, unless the boot pin is tied high (which is true) and
thus it should be PCI_COMMAND_MEMORY.

I think the default value should be zero and this should be overridden
later in apb_pci.c.
Michael S. Tsirkin - Oct. 13, 2009, 3:26 p.m.
On Tue, Oct 13, 2009 at 06:12:17PM +0300, Blue Swirl wrote:
> On Tue, Oct 13, 2009 at 4:20 PM, Isaku Yamahata <yamahata@valinux.co.jp> wrote:
> > On Fri, Oct 09, 2009 at 08:53:10AM +0200, Michael S. Tsirkin wrote:
> >> On Fri, Oct 09, 2009 at 03:28:40PM +0900, Isaku Yamahata wrote:
> >> > - use symbolic constant
> >> > - use helper function pci_set_xxx()
> >> >
> >> > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> >> > ---
> >> >  hw/pci.c |   23 ++++++++++++-----------
> >> >  1 files changed, 12 insertions(+), 11 deletions(-)
> >> >
> >> > diff --git a/hw/pci.c b/hw/pci.c
> >> > index a66e3de..eaf471a 100644
> >> > --- a/hw/pci.c
> >> > +++ b/hw/pci.c
> >> > @@ -923,17 +923,18 @@ static int pci_bridge_initfn(PCIDevice *dev)
> >> >      pci_config_set_vendor_id(s->dev.config, s->vid);
> >> >      pci_config_set_device_id(s->dev.config, s->did);
> >> >
> >> > -    s->dev.config[0x04] = 0x06; // command = bus master, pci mem
> >> > -    s->dev.config[0x05] = 0x00;
> >> > -    s->dev.config[0x06] = 0xa0; // status = fast back-to-back, 66MHz, no error
> >> > -    s->dev.config[0x07] = 0x00; // status = fast devsel
> >> > -    s->dev.config[0x08] = 0x00; // revision
> >> > -    s->dev.config[0x09] = 0x00; // programming i/f
> >> > -    pci_config_set_class(s->dev.config, PCI_CLASS_BRIDGE_PCI);
> >> > -    s->dev.config[0x0D] = 0x10; // latency_timer
> >> > -    s->dev.config[PCI_HEADER_TYPE] =
> >> > -        PCI_HEADER_TYPE_MULTI_FUNCTION | PCI_HEADER_TYPE_BRIDGE; // header_type
> >> > -    s->dev.config[0x1E] = 0xa0; // secondary status
> >> > +    pci_set_word(dev->config + PCI_COMMAND,
> >> > +                 PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER);
> >>
> >> BTW, I think this is a wrong reset value: should be disabled
> >> by default. Fixing this needs some testing though, so
> >> I am not suggesting we do it in this patch. Have some time
> >> to fix this?
> >
> > Hmm, the user of it is only apb_pci.c
> > I guess other magic values came from the real machine.
> > So one possible fix is to create apb_pci specific initialization function
> > and to move those initialization code into apb_pci.c leaving to
> > sparc guys. So we can avoid breakage.
> > What do you think of it?
> 
> Current code is buggy. According to manual (805-1251)

Link? Maybe we should put spec links in comments
in relevant code ...

> the reset value
> should be zero, unless the boot pin is tied high (which is true) and
> thus it should be PCI_COMMAND_MEMORY.
> 
> I think the default value should be zero and this should be overridden
> later in apb_pci.c.

Makes sense.
Blue Swirl - Oct. 13, 2009, 4:32 p.m.
On Tue, Oct 13, 2009 at 6:26 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Tue, Oct 13, 2009 at 06:12:17PM +0300, Blue Swirl wrote:
>> On Tue, Oct 13, 2009 at 4:20 PM, Isaku Yamahata <yamahata@valinux.co.jp> wrote:
>> > On Fri, Oct 09, 2009 at 08:53:10AM +0200, Michael S. Tsirkin wrote:
>> >> On Fri, Oct 09, 2009 at 03:28:40PM +0900, Isaku Yamahata wrote:
>> >> > - use symbolic constant
>> >> > - use helper function pci_set_xxx()
>> >> >
>> >> > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
>> >> > ---
>> >> >  hw/pci.c |   23 ++++++++++++-----------
>> >> >  1 files changed, 12 insertions(+), 11 deletions(-)
>> >> >
>> >> > diff --git a/hw/pci.c b/hw/pci.c
>> >> > index a66e3de..eaf471a 100644
>> >> > --- a/hw/pci.c
>> >> > +++ b/hw/pci.c
>> >> > @@ -923,17 +923,18 @@ static int pci_bridge_initfn(PCIDevice *dev)
>> >> >      pci_config_set_vendor_id(s->dev.config, s->vid);
>> >> >      pci_config_set_device_id(s->dev.config, s->did);
>> >> >
>> >> > -    s->dev.config[0x04] = 0x06; // command = bus master, pci mem
>> >> > -    s->dev.config[0x05] = 0x00;
>> >> > -    s->dev.config[0x06] = 0xa0; // status = fast back-to-back, 66MHz, no error
>> >> > -    s->dev.config[0x07] = 0x00; // status = fast devsel
>> >> > -    s->dev.config[0x08] = 0x00; // revision
>> >> > -    s->dev.config[0x09] = 0x00; // programming i/f
>> >> > -    pci_config_set_class(s->dev.config, PCI_CLASS_BRIDGE_PCI);
>> >> > -    s->dev.config[0x0D] = 0x10; // latency_timer
>> >> > -    s->dev.config[PCI_HEADER_TYPE] =
>> >> > -        PCI_HEADER_TYPE_MULTI_FUNCTION | PCI_HEADER_TYPE_BRIDGE; // header_type
>> >> > -    s->dev.config[0x1E] = 0xa0; // secondary status
>> >> > +    pci_set_word(dev->config + PCI_COMMAND,
>> >> > +                 PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER);
>> >>
>> >> BTW, I think this is a wrong reset value: should be disabled
>> >> by default. Fixing this needs some testing though, so
>> >> I am not suggesting we do it in this patch. Have some time
>> >> to fix this?
>> >
>> > Hmm, the user of it is only apb_pci.c
>> > I guess other magic values came from the real machine.
>> > So one possible fix is to create apb_pci specific initialization function
>> > and to move those initialization code into apb_pci.c leaving to
>> > sparc guys. So we can avoid breakage.
>> > What do you think of it?
>>
>> Current code is buggy. According to manual (805-1251)
>
> Link?

http://www.sun.com/processors/manuals/805-1251.pdf

> Maybe we should put spec links in comments
> in relevant code ...

Yes. Maybe that should be mandatory for new code.

Patch

diff --git a/hw/pci.c b/hw/pci.c
index a66e3de..eaf471a 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -923,17 +923,18 @@  static int pci_bridge_initfn(PCIDevice *dev)
     pci_config_set_vendor_id(s->dev.config, s->vid);
     pci_config_set_device_id(s->dev.config, s->did);
 
-    s->dev.config[0x04] = 0x06; // command = bus master, pci mem
-    s->dev.config[0x05] = 0x00;
-    s->dev.config[0x06] = 0xa0; // status = fast back-to-back, 66MHz, no error
-    s->dev.config[0x07] = 0x00; // status = fast devsel
-    s->dev.config[0x08] = 0x00; // revision
-    s->dev.config[0x09] = 0x00; // programming i/f
-    pci_config_set_class(s->dev.config, PCI_CLASS_BRIDGE_PCI);
-    s->dev.config[0x0D] = 0x10; // latency_timer
-    s->dev.config[PCI_HEADER_TYPE] =
-        PCI_HEADER_TYPE_MULTI_FUNCTION | PCI_HEADER_TYPE_BRIDGE; // header_type
-    s->dev.config[0x1E] = 0xa0; // secondary status
+    pci_set_word(dev->config + PCI_COMMAND,
+                 PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER);
+    pci_set_word(dev->config + PCI_STATUS,
+                 PCI_STATUS_66MHZ | PCI_STATUS_FAST_BACK);
+    dev->config[PCI_REVISION_ID] = 0x00;
+    dev->config[PCI_CLASS_PROG] = 0x00;
+    pci_config_set_class(dev->config, PCI_CLASS_BRIDGE_PCI);
+    dev->config[PCI_LATENCY_TIMER] = 0x10;
+    dev->config[PCI_HEADER_TYPE] =
+        PCI_HEADER_TYPE_MULTI_FUNCTION | PCI_HEADER_TYPE_BRIDGE;
+    pci_set_word(dev->config + PCI_SEC_STATUS,
+                 PCI_STATUS_66MHZ | PCI_STATUS_FAST_BACK);
     return 0;
 }