[Qemu-devel] Re: [PATCH V5 07/29] pci/bridge: clean up of pci_bridge_initfn()

["Michael S. Tsirkin" <mst@redhat.com>]

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