Re: [PATCH 1/2]MSI(X) save/restore for suspend/resume

[Matthew Wilcox <matthew@wil.cx>]

On Thu, Jan 05, 2006 at 08:58:00AM +0800, Shaohua Li wrote:
> +	if ((pos = pci_find_capability(dev, PCI_CAP_ID_MSIX)) <= 0 ||
> +		dev->no_msi)

No assignments within conditionals, please.

	pos = pci_find_capability(dev, PCI_CAP_ID_MSIX);
	if (pos <= 0 || dev->no_msi)

>  	u32		saved_config_space[16]; /* config space saved at suspend time */
> +	void		*saved_cap_space[PCI_CAP_ID_MAX + 1]; /* ext config space saved at suspend time */
>  	struct bin_attribute *rom_attr; /* attribute descriptor for sysfs ROM entry */
...
>  #define  PCI_CAP_ID_MSIX	0x11	/* MSI-X */
> +#define PCI_CAP_ID_MAX		PCI_CAP_ID_MSIX
>  #define PCI_CAP_LIST_NEXT	1	/* Next capability in the list */

Rather than taking all this space in the pci_dev structure (even
without CONFIG_PM), how about:

struct pci_cap_saved_state {
	struct pci_cap_saved_state *next;
	char cap_nr;
	char data[0];
}

and then just add:

	struct pci_cap_saved_state *saved_cap_space;

to the struct pci_dev?  One pointer, rather than (currently!) 12.
That's an 88 byte saving per PCI device on 64-bit machines!
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/