Re: [PATCH 4/4] PCI: Remove redundant capabilities checking in pci_{save, restore}_pcie_state

[Don Dutile <ddutile@redhat.com>]

On 06/01/2012 05:16 PM, Myron Stowe wrote:
> Unlike PCI Express v1's Capabilities Structure, v2's requires the entire
> structure to be implemented.  In v2 structures, register fields that are
> not necessarly implemented, are present but hardwired to 0x0.  These may
> include: Link Capabilities, Status, and Control; Slot Capabilities,
> Status, and Control; Root Capabilities, Status, and Control; and all of
> the '2' (Device, Link, and Slot) Capabilities, Status, and Control
> registers.
>
> This patch removes the redundant capability checks corresponding to the
> Link 2's and Slot 2's, Capabilities, Status, and Control registers as they
> will be present if Device Capabilities 2's registers are (which explains
> why the macros for each of the three are identical).
>
> Signed-off-by: Myron Stowe<myron.stowe@redhat.com>
> ---
>
>   drivers/pci/pci.c |   10 ++++------
>   1 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 26933ff..f9f8036 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -903,12 +903,11 @@ static int pci_save_pcie_state(struct pci_dev *dev)
>   		pci_read_config_word(dev, pos + PCI_EXP_SLTCTL,&cap[i++]);
>   	if (pcie_cap_has_rtctl(dev->pcie_type, flags))
>   		pci_read_config_word(dev, pos + PCI_EXP_RTCTL,&cap[i++]);
> -	if (pcie_cap_has_devctl2(dev->pcie_type, flags))
> +	if (pcie_cap_has_devctl2(dev->pcie_type, flags)) {
and why not use your new function:
+	pos = pci_pcie_cap2(bridge);
+	if (!pos)
instead of this devctl2-specific check, so it's obvious it's the whole cap-struct (additional regs)?
  
>   		pci_read_config_word(dev, pos + PCI_EXP_DEVCTL2,&cap[i++]);
> -	if (pcie_cap_has_lnkctl2(dev->pcie_type, flags))
>   		pci_read_config_word(dev, pos + PCI_EXP_LNKCTL2,&cap[i++]);
> -	if (pcie_cap_has_sltctl2(dev->pcie_type, flags))
>   		pci_read_config_word(dev, pos + PCI_EXP_SLTCTL2,&cap[i++]);
> +	}
>
>   	return 0;
>   }
> @@ -936,12 +935,11 @@ static void pci_restore_pcie_state(struct pci_dev *dev)
>   		pci_write_config_word(dev, pos + PCI_EXP_SLTCTL, cap[i++]);
>   	if (pcie_cap_has_rtctl(dev->pcie_type, flags))
>   		pci_write_config_word(dev, pos + PCI_EXP_RTCTL, cap[i++]);
> -	if (pcie_cap_has_devctl2(dev->pcie_type, flags))
> +	if (pcie_cap_has_devctl2(dev->pcie_type, flags)) {
ditto.

>   		pci_write_config_word(dev, pos + PCI_EXP_DEVCTL2, cap[i++]);
> -	if (pcie_cap_has_lnkctl2(dev->pcie_type, flags))
>   		pci_write_config_word(dev, pos + PCI_EXP_LNKCTL2, cap[i++]);
> -	if (pcie_cap_has_sltctl2(dev->pcie_type, flags))
>   		pci_write_config_word(dev, pos + PCI_EXP_SLTCTL2, cap[i++]);
> +	}
>   }
>
>
>