> -----Original Message-----
> From: [email protected] 
> [mailto:[email protected]] On Behalf Of Blue Swirl
> Sent: Tuesday, February 17, 2009 11:09 PM
> To: [email protected]
> Cc: [email protected]; [email protected]; Liu Yu-B13201; 
> [email protected]
> Subject: Re: [Qemu-devel] [PATCH 2/5] kvm/powerpc: Add 
> freescale pci controller's support
> 
> On 2/17/09, Liu Yu <[email protected]> wrote:
> > This patch add the emulation of freescale's pci controller 
> for E500 platform.
> >
> >  Signed-off-by: Liu Yu <[email protected]>
> 
> A reset function (registered with qemu_register_reset) would be nice.
> 
> >  + * Copyright (C) 2009 Freescale Semiconductor, Inc. All 
> rights reserved.
> 
> "All rights reserved" conflicts with GPL.

It's our company's policy, all the patches submitted to kernel comply with this 
as well.

> 
> >  +    d->config[0x00] = 0x57; // vendor_id
> >  +    d->config[0x01] = 0x19;
> >  +    d->config[0x02] = 0x30; // device_id
> >  +    d->config[0x03] = 0x00;
> 
> Please use pci_config_set_vendor_id and pci_config_set_device_id
> functions and add the ID #defines to hw/pci.h.

Fixed.

> 
> >  +    d->config[0x0a] = 0x20; // class_sub = other bridge type
> >  +    d->config[0x0b] = 0x0B; // class_base = PCI_bridge
> 
> I'd think these should be 0x06 (PCI_BASE_CLASS_BRIDGE) and 0x04
> (PCI_CLASS_BRIDGE_PCI). Are these correct?

Sorry for the wrong comments.
I confirmed it from user manual, and I also print the header out on MPC8544 
board.
it shows that these value are correct.
0x20 represent PowerPC and 0x0B represent processor.
Like "#define PCI_CLASS_PROCESSOR_CO           0x0b40" in hw/pci.h

Reply via email to