On Tue, Apr 07, 2009 at 09:31:06AM +0800, Yu Zhao wrote:
> -#define PCI_EXP_SAVE_REGS 7
> +static const int pcie_save_regs[] = {
> + PCI_EXP_DEVCTL,
> + PCI_EXP_LNKCTL,
> + PCI_EXP_SLTCTL,
> + PCI_EXP_RTCTL,
> + PCI_EXP_DEVCTL2,
> + PCI_EXP_LNKCTL2,
> + PCI_EXP_SLTCTL2
> +};
We could save a bit of space and store these as unsigned char, right?
> @@ -698,20 +706,15 @@ static int pci_save_pcie_state(struct pci_dev *dev)
> }
> cap = (u16 *)&save_state->data[0];
>
> - 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++]);
> - pci_read_config_word(dev, pos + PCI_EXP_DEVCTL2, &cap[i++]);
> - pci_read_config_word(dev, pos + PCI_EXP_LNKCTL2, &cap[i++]);
> - pci_read_config_word(dev, pos + PCI_EXP_SLTCTL2, &cap[i++]);
> + for (i = 0; i < ARRAY_SIZE(pcie_save_regs); i++)
> + pci_read_config_word(dev, pos + pcie_save_regs[i], &cap[i]);
I never looked at this code before ... but this is terribly wrong. None
of the capabilities after DEVCTL are guaranteed to be there. We could
be writing to some other register entirely.
Unfortunately, I think we need something like:
pci_read_config_word(dev, pos + PCI_EXP_DEVCTL, &cap[i++]);
if (has_link)
pci_read_config_word(dev, pos + PCI_EXP_LNKCTL, &cap[i++]);
if (has_slot)
pci_read_config_word(dev, pos + PCI_EXP_SLTCTL, &cap[i++]);
if (root_port || root event collector)
pci_read_config_word(dev, pos + PCI_EXP_RTCTL, &cap[i++]);
if (cap_version < 2)
return;
... and so on ...
and that means that your rather good patch looks a lot harder to
implement. We could maybe implement an array with pairs of (feature,
address), but that's probably more complex than it's worth.
--
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
