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

login
register
mail settings
Submitter Myron Stowe
Date June 1, 2012, 9:16 p.m.
Message ID <20120601211643.20328.26918.stgit@amt.stowe>
Download mbox | patch
Permalink /patch/162372/
State Accepted
Headers show

Comments

Myron Stowe - June 1, 2012, 9:16 p.m.
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(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Don Dutile - June 1, 2012, 10 p.m.
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++]);
> +	}
>   }
>
>
>

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

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)) {
 		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)) {
 		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++]);
+	}
 }