On Tue, Nov 16, 2010 at 05:26:07PM +0900, Isaku Yamahata wrote: > This patch refine the initialization/reset of > pci status registers. > > Signed-off-by: Isaku Yamahata <yamah...@valinux.co.jp>
This one seems good. Applied with some tweaks: I cut down the comment: we don't really need to repeat what code does IMO, rather why it does it, and clarified the comment text. Also split init_w1c out to a function so that each function does one thing. > --- > hw/pci.c | 41 +++++++++++++++++++++++++++++++++++++++-- > 1 files changed, 39 insertions(+), 2 deletions(-) > > diff --git a/hw/pci.c b/hw/pci.c > index 52fe655..fba765b 100644 > --- a/hw/pci.c > +++ b/hw/pci.c > @@ -145,6 +145,9 @@ static void pci_device_reset(PCIDevice *dev) > pci_word_test_and_clear_mask(dev->config + PCI_COMMAND, > pci_get_word(dev->wmask + PCI_COMMAND) | > pci_get_word(dev->w1cmask + PCI_COMMAND)); > + pci_word_test_and_clear_mask(dev->config + PCI_STATUS, > + pci_get_word(dev->wmask + PCI_STATUS) | > + pci_get_word(dev->w1cmask + PCI_STATUS)); > dev->config[PCI_CACHE_LINE_SIZE] = 0x0; > dev->config[PCI_INTERRUPT_LINE] = 0x0; > for (r = 0; r < PCI_NUM_REGIONS; ++r) { > @@ -540,7 +543,7 @@ static void pci_init_cmask(PCIDevice *dev) > dev->cmask[PCI_CAPABILITY_LIST] = 0xff; > } > > -static void pci_init_wmask(PCIDevice *dev) > +static void pci_init_wmask_w1cmask(PCIDevice *dev) > { > int config_size = pci_config_size(dev); > > @@ -595,6 +598,40 @@ static void pci_init_wmask(PCIDevice *dev) > PCI_COMMAND_MASTER | PCI_COMMAND_PARITY | PCI_COMMAND_SERR | > PCI_COMMAND_INTX_DISABLE); > > + /* > + * bit 0-2: reserved > + * bit 3: PCI_STATUS_INTERRUPT: RO > + * bit 4: PCI_STATUS_CAP_LIST: RO > + * bit 5: PCI_STATUS_66MHZ: RO > + * bit 6: PCI_STATUS_UDF: reserved (PCI 2.2-) > + * bit 7: PCI_STATUS_FAST_BACK: RO > + * bit 8: PCI_STATUS_PARITY > + * type 0: RW for bus master > + * type 1: RW1C > + * bit 9-10: PCI_STATUS_DEVSEL: RO > + * bit 11: PCI_STATUS_SIG_TARGET_ABORT > + * type 0: RW1C for targets that is capable of terminating > + * a transaction. > + * type 1: RW1C > + * bit 12: PCI_STATUS_REC_TARGET_ABORT > + * type 0: RW1C for masters > + * type 1: RW1C > + * bit 13: PCI_STATUS_REC_MASTER_ABORT > + * type 0: RW1C for masters > + * type 1: RW1C > + * bit 14: PCI_STATUS_SIG_SYSTEM_ERROR > + * type 0: RW1C with execptions > + * type 1: RW1C > + * bit : PCI_STATUS_DETECTED_PARITY: RW1C > + * > + * It's okay to set w1mask even for RO=0(i.e. reserved) because > + * writing value 1 to w1c bit whose value is 0 has no effect. > + */ > + pci_set_word(dev->w1cmask + PCI_STATUS, > + PCI_STATUS_PARITY | PCI_STATUS_SIG_TARGET_ABORT | > + PCI_STATUS_REC_TARGET_ABORT | PCI_STATUS_REC_MASTER_ABORT | > + PCI_STATUS_SIG_SYSTEM_ERROR | PCI_STATUS_DETECTED_PARITY); > + > memset(dev->wmask + PCI_CONFIG_HEADER_SIZE, 0xff, > config_size - PCI_CONFIG_HEADER_SIZE); > } > @@ -725,7 +762,7 @@ static PCIDevice *do_pci_register_device(PCIDevice > *pci_dev, PCIBus *bus, > pci_set_default_subsystem_id(pci_dev); > } > pci_init_cmask(pci_dev); > - pci_init_wmask(pci_dev); > + pci_init_wmask_w1cmask(pci_dev); > if (is_bridge) { > pci_init_wmask_bridge(pci_dev); > } > -- > 1.7.1.1