On Wed, Apr 08, 2009 at 01:15:49PM +0800, Yu Zhao wrote: > PCIe 1.1 spec neither requires the endpoint to implement the entire > PCIe capability structure nor specifies default value of registers > that are not implemented by the device. So we only save and restore > registers that must be implemented by different device types if the > device PCIe capability version is 1. > > PCIe 1.1 Capability Structure Expansion ENC and PCIe 2.0 requires ECN, I think, not ENC? > all registers in the PCIe capability to be either implemented or > hardwired to 0. Their PCIe capability version is 2. > + pci_read_config_word(dev, pos + PCI_EXP_FLAGS, &flags); > + cap_version = flags & PCI_EXP_FLAGS_VERS; > + > pci_read_config_word(dev, pos + PCI_EXP_DEVCTL, &cap[i++]); > - pci_read_config_word(dev, pos + PCI_EXP_LNKCTL, &cap[i++]); > - pci_read_config_word(dev, pos + PCI_EXP_SLTCTL, &cap[i++]); > - pci_read_config_word(dev, pos + PCI_EXP_RTCTL, &cap[i++]); > + > + if (cap_version > 1 || > + (dev->pcie_type == PCI_EXP_TYPE_ROOT_PORT || > + dev->pcie_type == PCI_EXP_TYPE_ENDPOINT || > + dev->pcie_type == PCI_EXP_TYPE_LEG_END)) > + pci_read_config_word(dev, pos + PCI_EXP_LNKCTL, &cap[i++]); It's too much effort to check that these two things are in sync between the save and restore code. How about doing it this way? #define pcie_has_link_regs(dev) \ (dev->pcie_type == PCI_EXP_TYPE_ROOT_PORT || \ dev->pcie_type == PCI_EXP_TYPE_ENDPOINT || \ dev->pcie_type == PCI_EXP_TYPE_LEG_END) #define pcie_has_slot_regs(dev, flags) \ ((dev->pcie_type == PCI_EXP_TYPE_ROOT_PORT) || \ (dev->pcie_type == PCI_EXP_TYPE_DOWNSTREAM && \ (flags & PCI_EXP_FLAGS_SLOT))) #define pcie_has_root_regs(dev) \ (dev->pcie_type == PCI_EXP_TYPE_ROOT_PORT || \ dev->pcie_type == PCI_EXP_TYPE_RC_EC) ... if (cap_version > 1 || pcie_has_link_regs(dev)) pci_read_config_word(dev, pos + PCI_EXP_LNKCTL, &cap[i++]); if (cap_version > 1 || pcie_has_slot_regs(dev, flags)) pci_read_config_word(dev, pos + PCI_EXP_SLTCTL, &cap[i++]); if (cap_version > 1 || pcie_has_root_regs(dev)) pci_read_config_word(dev, pos + PCI_EXP_RTCTL, &cap[i++]); ... if (cap_version > 1 || pcie_has_link_regs(dev)) pci_write_config_word(dev, pos + PCI_EXP_LNKCTL, cap[i++]); if (cap_version > 1 || pcie_has_slot_regs(dev, flags)) pci_write_config_word(dev, pos + PCI_EXP_SLTCTL, cap[i++]); if (cap_version > 1 || pcie_has_root_regs(dev)) pci_write_config_word(dev, pos + PCI_EXP_RTCTL, cap[i++]); > > + pci_read_config_word(dev, pos + PCI_EXP_FLAGS, &flags); > + cap_version = flags & PCI_EXP_FLAGS_VERS; > + > pci_write_config_word(dev, pos + PCI_EXP_DEVCTL, cap[i++]); > - pci_write_config_word(dev, pos + PCI_EXP_LNKCTL, cap[i++]); > - pci_write_config_word(dev, pos + PCI_EXP_SLTCTL, cap[i++]); > - pci_write_config_word(dev, pos + PCI_EXP_RTCTL, cap[i++]); > + > + if (cap_version > 1 || > + (dev->pcie_type == PCI_EXP_TYPE_ROOT_PORT || > + dev->pcie_type == PCI_EXP_TYPE_ENDPOINT || > + dev->pcie_type == PCI_EXP_TYPE_LEG_END)) > + pci_read_config_word(dev, pos + PCI_EXP_LNKCTL, &cap[i++]); You made a copy and paste error here ;-) -- Matthew Wilcox Intel Open Source Technology Centre "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step." -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
