[RFC PATCH v1 0/6] Remove struct pcie_link_state.aspm_*

[RFC PATCH v1 0/6] Remove struct pcie_link_state.aspm_*

[Saheed O. Bolarinwa]

aspm_{enable, support} in struct pcie_link_state do not need to be
stored. This series removes them and creates functions to calculate
them.

MERGE NOTICE:
These series are based on
»       'commit e4e737bb5c17 ("Linux 5.15-rc2")'


Saheed O. Bolarinwa (6):
  PCI/ASPM: Extract out L1SS_CAP calculations
  PCI/ASPM: Extract the calculation of link->aspm_support
  PCI/ASPM: Extract the calculation of link->aspm_enabled
  PCI/ASPM: Don't cache struct pcie_link_state->aspm_support
  PCI/ASPM: Move pcie_aspm_sanity_check() upwards
  PCI/ASPM: Don't cache struct pcie_link_state->aspm_enabled

 drivers/pci/pcie/aspm.c | 291 +++++++++++++++++++++++-----------------
 1 file changed, 170 insertions(+), 121 deletions(-)

-- 
2.20.1

[RFC PATCH v1 1/6] PCI/ASPM: Extract out L1SS_CAP calculations

[Saheed O. Bolarinwa]

Inside pcie_aspm_cap_init() the L1SS_CAP of both ends of the link is
calculated. The values are used to calculate link->aspm_support and
link->aspm_enabled. Isolating this calcution with simplify
pcie_aspm_cap_init().

Extract the calculations of L1SS_CAP on both ends into
aspm_calc_both_l1ss_caps().

Signed-off-by: Saheed O. Bolarinwa <refactormyself@gmail.com>
---
 drivers/pci/pcie/aspm.c | 42 ++++++++++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 013a47f587ce..057c6768fb7b 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -540,6 +540,30 @@ static void aspm_calc_l1ss_info(struct pcie_link_state *link,
 	}
 }
 
+static void aspm_calc_both_l1ss_caps(struct pcie_link_state *link,
+				    u32 *up_l1ss_cap, u32 *dwn_l1ss_cap)
+{
+	struct pci_dev *child = link->downstream, *parent = link->pdev;
+
+	pci_read_config_dword(parent, parent->l1ss + PCI_L1SS_CAP,
+			      up_l1ss_cap);
+	pci_read_config_dword(child, child->l1ss + PCI_L1SS_CAP,
+			      dwn_l1ss_cap);
+
+	if (!(*up_l1ss_cap & PCI_L1SS_CAP_L1_PM_SS))
+		*up_l1ss_cap = 0;
+	if (!(*dwn_l1ss_cap & PCI_L1SS_CAP_L1_PM_SS))
+		*dwn_l1ss_cap = 0;
+
+	/*
+	 * If we don't have LTR for the entire path from the Root Complex
+	 * to this device, we can't use ASPM L1.2 because it relies on the
+	 * LTR_L1.2_THRESHOLD.  See PCIe r4.0, secs 5.5.4, 6.18.
+	 */
+	if (!child->ltr_path)
+		*dwn_l1ss_cap &= ~PCI_L1SS_CAP_ASPM_L1_2;
+}
+
 static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 {
 	struct pci_dev *child = link->downstream, *parent = link->pdev;
@@ -606,23 +630,7 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 	link->latency_dw.l1 = calc_l1_latency(child_lnkcap);
 
 	/* Setup L1 substate */
-	pci_read_config_dword(parent, parent->l1ss + PCI_L1SS_CAP,
-			      &parent_l1ss_cap);
-	pci_read_config_dword(child, child->l1ss + PCI_L1SS_CAP,
-			      &child_l1ss_cap);
-
-	if (!(parent_l1ss_cap & PCI_L1SS_CAP_L1_PM_SS))
-		parent_l1ss_cap = 0;
-	if (!(child_l1ss_cap & PCI_L1SS_CAP_L1_PM_SS))
-		child_l1ss_cap = 0;
-
-	/*
-	 * If we don't have LTR for the entire path from the Root Complex
-	 * to this device, we can't use ASPM L1.2 because it relies on the
-	 * LTR_L1.2_THRESHOLD.  See PCIe r4.0, secs 5.5.4, 6.18.
-	 */
-	if (!child->ltr_path)
-		child_l1ss_cap &= ~PCI_L1SS_CAP_ASPM_L1_2;
+	aspm_calc_both_l1ss_caps(link, &parent_l1ss_cap, &child_l1ss_cap);
 
 	if (parent_l1ss_cap & child_l1ss_cap & PCI_L1SS_CAP_ASPM_L1_1)
 		link->aspm_support |= ASPM_STATE_L1_1;
-- 
2.20.1

[RFC PATCH v1 2/6] PCI/ASPM: Extract the calculation of link->aspm_support

[Saheed O. Bolarinwa]

struct pcie_link_state->aspm_support hold the initial capable
state of the link. This value is calculated inside
pcie_aspm_init_cap(). Isolating this calculation will simplify
pcie_aspm_init_cap().

Extract the calculation of link->aspm_support into
aspm_calc_init_linkcap().

Signed-off-by: Saheed O. Bolarinwa <refactormyself@gmail.com>
---
 drivers/pci/pcie/aspm.c | 60 ++++++++++++++++++++++++-----------------
 1 file changed, 35 insertions(+), 25 deletions(-)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 057c6768fb7b..23441a32f604 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -564,6 +564,33 @@ static void aspm_calc_both_l1ss_caps(struct pcie_link_state *link,
 		*dwn_l1ss_cap &= ~PCI_L1SS_CAP_ASPM_L1_2;
 }
 
+static u32 aspm_calc_init_linkcap(u32 up_lnkcap, u32 dwn_lnkcap,
+				  u32 up_l1ss_cap, u32 dwn_l1ss_cap)
+{
+	u32 link_cap = 0;
+
+	/*
+	 * Note that we must not enable L0s in either direction on a
+	 * given link unless components on both sides of the link each
+	 * support L0s.
+	 */
+	if (up_lnkcap & dwn_lnkcap & PCI_EXP_LNKCAP_ASPM_L0S)
+		link_cap |= ASPM_STATE_L0S;
+
+	if (up_lnkcap & dwn_lnkcap & PCI_EXP_LNKCAP_ASPM_L1)
+		link_cap |= ASPM_STATE_L1;
+	if (up_l1ss_cap & dwn_l1ss_cap & PCI_L1SS_CAP_ASPM_L1_1)
+		link_cap |= ASPM_STATE_L1_1;
+	if (up_l1ss_cap & dwn_l1ss_cap & PCI_L1SS_CAP_ASPM_L1_2)
+		link_cap |= ASPM_STATE_L1_2;
+	if (up_l1ss_cap & dwn_l1ss_cap & PCI_L1SS_CAP_PCIPM_L1_1)
+		link_cap |= ASPM_STATE_L1_1_PCIPM;
+	if (up_l1ss_cap & dwn_l1ss_cap & PCI_L1SS_CAP_PCIPM_L1_2)
+		link_cap |= ASPM_STATE_L1_2_PCIPM;
+
+	return link_cap;
+}
+
 static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 {
 	struct pci_dev *child = link->downstream, *parent = link->pdev;
@@ -603,16 +630,7 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 	pcie_capability_read_word(parent, PCI_EXP_LNKCTL, &parent_lnkctl);
 	pcie_capability_read_word(child, PCI_EXP_LNKCTL, &child_lnkctl);
 
-	/*
-	 * Setup L0s state
-	 *
-	 * Note that we must not enable L0s in either direction on a
-	 * given link unless components on both sides of the link each
-	 * support L0s.
-	 */
-	if (parent_lnkcap & child_lnkcap & PCI_EXP_LNKCAP_ASPM_L0S)
-		link->aspm_support |= ASPM_STATE_L0S;
-
+	/* Setup L0s state */
 	if (child_lnkctl & PCI_EXP_LNKCTL_ASPM_L0S)
 		link->aspm_enabled |= ASPM_STATE_L0S_UP;
 	if (parent_lnkctl & PCI_EXP_LNKCTL_ASPM_L0S)
@@ -621,9 +639,6 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 	link->latency_dw.l0s = calc_l0s_latency(child_lnkcap);
 
 	/* Setup L1 state */
-	if (parent_lnkcap & child_lnkcap & PCI_EXP_LNKCAP_ASPM_L1)
-		link->aspm_support |= ASPM_STATE_L1;
-
 	if (parent_lnkctl & child_lnkctl & PCI_EXP_LNKCTL_ASPM_L1)
 		link->aspm_enabled |= ASPM_STATE_L1;
 	link->latency_up.l1 = calc_l1_latency(parent_lnkcap);
@@ -632,15 +647,6 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 	/* Setup L1 substate */
 	aspm_calc_both_l1ss_caps(link, &parent_l1ss_cap, &child_l1ss_cap);
 
-	if (parent_l1ss_cap & child_l1ss_cap & PCI_L1SS_CAP_ASPM_L1_1)
-		link->aspm_support |= ASPM_STATE_L1_1;
-	if (parent_l1ss_cap & child_l1ss_cap & PCI_L1SS_CAP_ASPM_L1_2)
-		link->aspm_support |= ASPM_STATE_L1_2;
-	if (parent_l1ss_cap & child_l1ss_cap & PCI_L1SS_CAP_PCIPM_L1_1)
-		link->aspm_support |= ASPM_STATE_L1_1_PCIPM;
-	if (parent_l1ss_cap & child_l1ss_cap & PCI_L1SS_CAP_PCIPM_L1_2)
-		link->aspm_support |= ASPM_STATE_L1_2_PCIPM;
-
 	if (parent_l1ss_cap)
 		pci_read_config_dword(parent, parent->l1ss + PCI_L1SS_CTL1,
 				      &parent_l1ss_ctl1);
@@ -657,12 +663,16 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 	if (parent_l1ss_ctl1 & child_l1ss_ctl1 & PCI_L1SS_CTL1_PCIPM_L1_2)
 		link->aspm_enabled |= ASPM_STATE_L1_2_PCIPM;
 
-	if (link->aspm_support & ASPM_STATE_L1SS)
-		aspm_calc_l1ss_info(link, parent_l1ss_cap, child_l1ss_cap);
-
 	/* Save default state */
 	link->aspm_default = link->aspm_enabled;
 
+	link->aspm_support = aspm_calc_init_linkcap(parent_lnkcap,
+						    child_lnkcap,
+						    parent_l1ss_cap,
+						    child_l1ss_cap);
+	if (link->aspm_support & ASPM_STATE_L1SS)
+		aspm_calc_l1ss_info(link, parent_l1ss_cap, child_l1ss_cap);
+
 	/* Setup initial capable state. Will be updated later */
 	link->aspm_capable = link->aspm_support;
 
-- 
2.20.1

[RFC PATCH v1 3/6] PCI/ASPM: Extract the calculation of link->aspm_enabled

[Saheed O. Bolarinwa]

Inside pcie_aspm_init_cap() the initial enabled state is read
from the firmware and calculated. It is stored in
struct pcie_link_state->aspm_enabled and also backed up in
struct pcie_link_state->aspm_default. pcie_aspm_init_cap() can
be simplified by isolation this calculation.

Extract the calculation of struct pcie_link_state->aspm_enabled
into aspm_calc_enabled_states().

Signed-off-by: Saheed O. Bolarinwa <refactormyself@gmail.com>
---
 drivers/pci/pcie/aspm.c | 66 ++++++++++++++++++++++++-----------------
 1 file changed, 38 insertions(+), 28 deletions(-)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 23441a32f604..5e8613cb5db6 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -591,13 +591,47 @@ static u32 aspm_calc_init_linkcap(u32 up_lnkcap, u32 dwn_lnkcap,
 	return link_cap;
 }
 
+static u32 aspm_calc_enabled_states(struct pcie_link_state *link,
+				    u32 up_l1ss_cap, u32 dwn_l1ss_cap)
+{
+	struct pci_dev *child = link->downstream, *parent = link->pdev;
+	u16 parent_lnkctl, child_lnkctl;
+	u32 aspm_enabled = 0, parent_l1ss_ctl1 = 0, child_l1ss_ctl1 = 0;
+
+	pcie_capability_read_word(parent, PCI_EXP_LNKCTL, &parent_lnkctl);
+	pcie_capability_read_word(child, PCI_EXP_LNKCTL, &child_lnkctl);
+
+	if (child_lnkctl & PCI_EXP_LNKCTL_ASPM_L0S)
+		aspm_enabled |= ASPM_STATE_L0S_UP;
+	if (parent_lnkctl & PCI_EXP_LNKCTL_ASPM_L0S)
+		aspm_enabled |= ASPM_STATE_L0S_DW;
+	if (parent_lnkctl & child_lnkctl & PCI_EXP_LNKCTL_ASPM_L1)
+		aspm_enabled |= ASPM_STATE_L1;
+
+	if (up_l1ss_cap)
+		pci_read_config_dword(parent, parent->l1ss + PCI_L1SS_CTL1,
+				      &parent_l1ss_ctl1);
+	if (dwn_l1ss_cap)
+		pci_read_config_dword(child, child->l1ss + PCI_L1SS_CTL1,
+				      &child_l1ss_ctl1);
+
+	if (parent_l1ss_ctl1 & child_l1ss_ctl1 & PCI_L1SS_CTL1_ASPM_L1_1)
+		aspm_enabled |= ASPM_STATE_L1_1;
+	if (parent_l1ss_ctl1 & child_l1ss_ctl1 & PCI_L1SS_CTL1_ASPM_L1_2)
+		aspm_enabled |= ASPM_STATE_L1_2;
+	if (parent_l1ss_ctl1 & child_l1ss_ctl1 & PCI_L1SS_CTL1_PCIPM_L1_1)
+		aspm_enabled |= ASPM_STATE_L1_1_PCIPM;
+	if (parent_l1ss_ctl1 & child_l1ss_ctl1 & PCI_L1SS_CTL1_PCIPM_L1_2)
+		aspm_enabled |= ASPM_STATE_L1_2_PCIPM;
+
+	return aspm_enabled;
+}
+
 static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 {
 	struct pci_dev *child = link->downstream, *parent = link->pdev;
 	u32 parent_lnkcap, child_lnkcap;
-	u16 parent_lnkctl, child_lnkctl;
 	u32 parent_l1ss_cap, child_l1ss_cap;
-	u32 parent_l1ss_ctl1 = 0, child_l1ss_ctl1 = 0;
 	struct pci_bus *linkbus = parent->subordinate;
 
 	if (blacklist) {
@@ -627,41 +661,17 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 	 */
 	pcie_capability_read_dword(parent, PCI_EXP_LNKCAP, &parent_lnkcap);
 	pcie_capability_read_dword(child, PCI_EXP_LNKCAP, &child_lnkcap);
-	pcie_capability_read_word(parent, PCI_EXP_LNKCTL, &parent_lnkctl);
-	pcie_capability_read_word(child, PCI_EXP_LNKCTL, &child_lnkctl);
 
-	/* Setup L0s state */
-	if (child_lnkctl & PCI_EXP_LNKCTL_ASPM_L0S)
-		link->aspm_enabled |= ASPM_STATE_L0S_UP;
-	if (parent_lnkctl & PCI_EXP_LNKCTL_ASPM_L0S)
-		link->aspm_enabled |= ASPM_STATE_L0S_DW;
 	link->latency_up.l0s = calc_l0s_latency(parent_lnkcap);
 	link->latency_dw.l0s = calc_l0s_latency(child_lnkcap);
-
-	/* Setup L1 state */
-	if (parent_lnkctl & child_lnkctl & PCI_EXP_LNKCTL_ASPM_L1)
-		link->aspm_enabled |= ASPM_STATE_L1;
 	link->latency_up.l1 = calc_l1_latency(parent_lnkcap);
 	link->latency_dw.l1 = calc_l1_latency(child_lnkcap);
 
 	/* Setup L1 substate */
 	aspm_calc_both_l1ss_caps(link, &parent_l1ss_cap, &child_l1ss_cap);
 
-	if (parent_l1ss_cap)
-		pci_read_config_dword(parent, parent->l1ss + PCI_L1SS_CTL1,
-				      &parent_l1ss_ctl1);
-	if (child_l1ss_cap)
-		pci_read_config_dword(child, child->l1ss + PCI_L1SS_CTL1,
-				      &child_l1ss_ctl1);
-
-	if (parent_l1ss_ctl1 & child_l1ss_ctl1 & PCI_L1SS_CTL1_ASPM_L1_1)
-		link->aspm_enabled |= ASPM_STATE_L1_1;
-	if (parent_l1ss_ctl1 & child_l1ss_ctl1 & PCI_L1SS_CTL1_ASPM_L1_2)
-		link->aspm_enabled |= ASPM_STATE_L1_2;
-	if (parent_l1ss_ctl1 & child_l1ss_ctl1 & PCI_L1SS_CTL1_PCIPM_L1_1)
-		link->aspm_enabled |= ASPM_STATE_L1_1_PCIPM;
-	if (parent_l1ss_ctl1 & child_l1ss_ctl1 & PCI_L1SS_CTL1_PCIPM_L1_2)
-		link->aspm_enabled |= ASPM_STATE_L1_2_PCIPM;
+	link->aspm_enabled = aspm_calc_enabled_states(link, parent_l1ss_cap,
+						      child_l1ss_cap);
 
 	/* Save default state */
 	link->aspm_default = link->aspm_enabled;
-- 
2.20.1

[RFC PATCH v1 4/6] PCI/ASPM: Don't cache struct pcie_link_state->aspm_support

[Saheed O. Bolarinwa]

aspm_support in struct pcie_link_state is used to cache the
initial supported ASPM capabilty states as calculated within
aspm_calc_init_linkcap(). This value can be accessed directly
by calling aspm_calc_init_linkcap().

- Remove aspm_support from struct pcie_link_state;
- Create a wrapper function for aspm_calc_init_linkcap()
- Call the wrapper function outside pcie_aspm_cap_init()
- Within pcie_aspm_cap_init() use a local variable to replace
  struct pcie_link_state->aspm_support.

Signed-off-by: Saheed O. Bolarinwa <refactormyself@gmail.com>
---
 drivers/pci/pcie/aspm.c | 37 +++++++++++++++++++++++++------------
 1 file changed, 25 insertions(+), 12 deletions(-)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 5e8613cb5db6..9aaae476ea31 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -54,7 +54,6 @@ struct pcie_link_state {
 	struct list_head sibling;	/* node in link_list */
 
 	/* ASPM state */
-	u32 aspm_support:7;		/* Supported ASPM state */
 	u32 aspm_enabled:7;		/* Enabled ASPM state */
 	u32 aspm_capable:7;		/* Capable ASPM state with latency */
 	u32 aspm_default:7;		/* Default ASPM state by BIOS */
@@ -450,7 +449,8 @@ static void pci_clear_and_set_dword(struct pci_dev *pdev, int pos,
 
 /* Calculate L1.2 PM substate timing parameters */
 static void aspm_calc_l1ss_info(struct pcie_link_state *link,
-				u32 parent_l1ss_cap, u32 child_l1ss_cap)
+				u32 aspm_support, u32 parent_l1ss_cap,
+				u32 child_l1ss_cap)
 {
 	struct pci_dev *child = link->downstream, *parent = link->pdev;
 	u32 val1, val2, scale1, scale2;
@@ -459,7 +459,7 @@ static void aspm_calc_l1ss_info(struct pcie_link_state *link,
 	u32 pctl1, pctl2, cctl1, cctl2;
 	u32 pl1_2_enables, cl1_2_enables;
 
-	if (!(link->aspm_support & ASPM_STATE_L1_2_MASK))
+	if (!(aspm_support & ASPM_STATE_L1_2_MASK))
 		return;
 
 	/* Choose the greater of the two Port Common_Mode_Restore_Times */
@@ -591,6 +591,20 @@ static u32 aspm_calc_init_linkcap(u32 up_lnkcap, u32 dwn_lnkcap,
 	return link_cap;
 }
 
+static u32 aspm_get_init_cap(struct pcie_link_state *link)
+{
+	struct pci_dev *child = link->downstream, *parent = link->pdev;
+	u32 parent_lnkcap, child_lnkcap;
+	u32 parent_l1ss_cap, child_l1ss_cap;
+
+	pcie_capability_read_dword(parent, PCI_EXP_LNKCAP, &parent_lnkcap);
+	pcie_capability_read_dword(child, PCI_EXP_LNKCAP, &child_lnkcap);
+	aspm_calc_both_l1ss_caps(link, &parent_l1ss_cap, &child_l1ss_cap);
+
+	return aspm_calc_init_linkcap(parent_lnkcap, child_lnkcap,
+				      parent_l1ss_cap, child_l1ss_cap);
+}
+
 static u32 aspm_calc_enabled_states(struct pcie_link_state *link,
 				    u32 up_l1ss_cap, u32 dwn_l1ss_cap)
 {
@@ -630,7 +644,7 @@ static u32 aspm_calc_enabled_states(struct pcie_link_state *link,
 static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 {
 	struct pci_dev *child = link->downstream, *parent = link->pdev;
-	u32 parent_lnkcap, child_lnkcap;
+	u32 parent_lnkcap, child_lnkcap, aspm_support;
 	u32 parent_l1ss_cap, child_l1ss_cap;
 	struct pci_bus *linkbus = parent->subordinate;
 
@@ -676,15 +690,14 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 	/* Save default state */
 	link->aspm_default = link->aspm_enabled;
 
-	link->aspm_support = aspm_calc_init_linkcap(parent_lnkcap,
-						    child_lnkcap,
-						    parent_l1ss_cap,
-						    child_l1ss_cap);
-	if (link->aspm_support & ASPM_STATE_L1SS)
-		aspm_calc_l1ss_info(link, parent_l1ss_cap, child_l1ss_cap);
+	aspm_support = aspm_calc_init_linkcap(parent_lnkcap, child_lnkcap,
+					      parent_l1ss_cap, child_l1ss_cap);
+	if (aspm_support & ASPM_STATE_L1SS)
+		aspm_calc_l1ss_info(link, aspm_support, parent_l1ss_cap,
+				    child_l1ss_cap);
 
 	/* Setup initial capable state. Will be updated later */
-	link->aspm_capable = link->aspm_support;
+	link->aspm_capable = aspm_support;
 
 	/* Get and check endpoint acceptable latencies */
 	list_for_each_entry(child, &linkbus->devices, bus_list) {
@@ -994,7 +1007,7 @@ static void pcie_update_aspm_capable(struct pcie_link_state *root)
 	list_for_each_entry(link, &link_list, sibling) {
 		if (link->root != root)
 			continue;
-		link->aspm_capable = link->aspm_support;
+		link->aspm_capable = aspm_get_init_cap(link);
 	}
 	list_for_each_entry(link, &link_list, sibling) {
 		struct pci_dev *child;
-- 
2.20.1

[RFC PATCH v1 5/6] PCI/ASPM: Move pcie_aspm_sanity_check() upwards

[Saheed O. Bolarinwa]

Move pcie_aspm_sanity_check() upwards to make more accessible.

Signed-off-by: Saheed O. Bolarinwa <refactormyself@gmail.com>
---
 drivers/pci/pcie/aspm.c | 70 ++++++++++++++++++++---------------------
 1 file changed, 35 insertions(+), 35 deletions(-)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 9aaae476ea31..05ca165380e1 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -447,6 +447,41 @@ static void pci_clear_and_set_dword(struct pci_dev *pdev, int pos,
 	pci_write_config_dword(pdev, pos, val);
 }
 
+static int pcie_aspm_sanity_check(struct pci_dev *pdev)
+{
+	struct pci_dev *child;
+	u32 reg32;
+
+	/*
+	 * Some functions in a slot might not all be PCIe functions,
+	 * very strange. Disable ASPM for the whole slot
+	 */
+	list_for_each_entry(child, &pdev->subordinate->devices, bus_list) {
+		if (!pci_is_pcie(child))
+			return -EINVAL;
+
+		/*
+		 * If ASPM is disabled then we're not going to change
+		 * the BIOS state. It's safe to continue even if it's a
+		 * pre-1.1 device
+		 */
+
+		if (aspm_disabled)
+			continue;
+
+		/*
+		 * Disable ASPM for pre-1.1 PCIe device, we follow MS to use
+		 * RBER bit to determine if a function is 1.1 version device
+		 */
+		pcie_capability_read_dword(child, PCI_EXP_DEVCAP, &reg32);
+		if (!(reg32 & PCI_EXP_DEVCAP_RBER) && !aspm_force) {
+			pci_info(child, "disabling ASPM on pre-1.1 PCIe device.  You can enable it with 'pcie_aspm=force'\n");
+			return -EINVAL;
+		}
+	}
+	return 0;
+}
+
 /* Calculate L1.2 PM substate timing parameters */
 static void aspm_calc_l1ss_info(struct pcie_link_state *link,
 				u32 aspm_support, u32 parent_l1ss_cap,
@@ -846,41 +881,6 @@ static void free_link_state(struct pcie_link_state *link)
 	kfree(link);
 }
 
-static int pcie_aspm_sanity_check(struct pci_dev *pdev)
-{
-	struct pci_dev *child;
-	u32 reg32;
-
-	/*
-	 * Some functions in a slot might not all be PCIe functions,
-	 * very strange. Disable ASPM for the whole slot
-	 */
-	list_for_each_entry(child, &pdev->subordinate->devices, bus_list) {
-		if (!pci_is_pcie(child))
-			return -EINVAL;
-
-		/*
-		 * If ASPM is disabled then we're not going to change
-		 * the BIOS state. It's safe to continue even if it's a
-		 * pre-1.1 device
-		 */
-
-		if (aspm_disabled)
-			continue;
-
-		/*
-		 * Disable ASPM for pre-1.1 PCIe device, we follow MS to use
-		 * RBER bit to determine if a function is 1.1 version device
-		 */
-		pcie_capability_read_dword(child, PCI_EXP_DEVCAP, &reg32);
-		if (!(reg32 & PCI_EXP_DEVCAP_RBER) && !aspm_force) {
-			pci_info(child, "disabling ASPM on pre-1.1 PCIe device.  You can enable it with 'pcie_aspm=force'\n");
-			return -EINVAL;
-		}
-	}
-	return 0;
-}
-
 static struct pcie_link_state *alloc_pcie_link_state(struct pci_dev *pdev)
 {
 	struct pcie_link_state *link;
-- 
2.20.1

[RFC PATCH v1 6/6] PCI/ASPM: Don't cache struct pcie_link_state->aspm_enabled

[Saheed O. Bolarinwa]

aspm_enabled in struct pcie_link_state usually holds the current
enabled states of the link. This value is set in two places:
1. pcie_aspm_cap_init(): if the passed in blacklist value holds true,
   then `link->aspm_enabled = ASPM_STATE_ALL` otherwise values are
   read from the register and link->aspm_enabled is calculated.
   This calculation has been extracted into aspm_calc_enabled_states()
   in an earlier patch in this series.
2. pcie_config_aspm_link(): when a valid state is calculated from the
   value passed in. The result is later written into the register. The
   calculated valid state is then store in struct
   pcie_link_state->aspm_enabled.

The calculations in aspm_calc_enabled_states() reads and uses the
current state in the register. This can be called whenever
link->aspm_enabled is needed. We don't need to store the state.

For the case when blacklist value holds in pcie_aspm_cap_init(), this
value comes from pcie_aspm_init_link_state(). We can obtain this value
whenever link->aspm_enabled is needed and determine if the current
enabled states should be set to "ASPM_STATE_ALL". So also in this case
we don't need to store the enabled states, it can be obtained on the
fly.

 - Remove aspm_enabled from struct pcie_link_state.
 - Create a wrapper function arround aspm_calc_enabled_states().
 - Replace references to aspm_enabled with a function call.

Signed-off-by: Saheed O. Bolarinwa <refactormyself@gmail.com>
---
 drivers/pci/pcie/aspm.c | 36 ++++++++++++++++++++++--------------
 1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 05ca165380e1..dce0851c6ab5 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -54,7 +54,6 @@ struct pcie_link_state {
 	struct list_head sibling;	/* node in link_list */
 
 	/* ASPM state */
-	u32 aspm_enabled:7;		/* Enabled ASPM state */
 	u32 aspm_capable:7;		/* Capable ASPM state with latency */
 	u32 aspm_default:7;		/* Default ASPM state by BIOS */
 	u32 aspm_disable:7;		/* Disabled ASPM state */
@@ -676,6 +675,18 @@ static u32 aspm_calc_enabled_states(struct pcie_link_state *link,
 	return aspm_enabled;
 }
 
+static u32 aspm_get_enabled_states(struct pcie_link_state *link)
+{
+	u32 up_l1ss_cap, dwn_l1ss_cap;
+
+	if (pcie_aspm_sanity_check(link->pdev))
+		return ASPM_STATE_ALL;
+
+	aspm_calc_both_l1ss_caps(link, &up_l1ss_cap, &dwn_l1ss_cap);
+
+	return aspm_calc_enabled_states(link, up_l1ss_cap, dwn_l1ss_cap);
+}
+
 static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 {
 	struct pci_dev *child = link->downstream, *parent = link->pdev;
@@ -684,8 +695,7 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 	struct pci_bus *linkbus = parent->subordinate;
 
 	if (blacklist) {
-		/* Set enabled/disable so that we will disable ASPM later */
-		link->aspm_enabled = ASPM_STATE_ALL;
+		/* Set disable so that we will disable ASPM later */
 		link->aspm_disable = ASPM_STATE_ALL;
 		return;
 	}
@@ -719,11 +729,9 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 	/* Setup L1 substate */
 	aspm_calc_both_l1ss_caps(link, &parent_l1ss_cap, &child_l1ss_cap);
 
-	link->aspm_enabled = aspm_calc_enabled_states(link, parent_l1ss_cap,
-						      child_l1ss_cap);
-
 	/* Save default state */
-	link->aspm_default = link->aspm_enabled;
+	link->aspm_default = aspm_calc_enabled_states(link, parent_l1ss_cap,
+						      child_l1ss_cap);
 
 	aspm_support = aspm_calc_init_linkcap(parent_lnkcap, child_lnkcap,
 					      parent_l1ss_cap, child_l1ss_cap);
@@ -762,7 +770,7 @@ static void pcie_config_aspm_l1ss(struct pcie_link_state *link, u32 state)
 	u32 val, enable_req;
 	struct pci_dev *child = link->downstream, *parent = link->pdev;
 
-	enable_req = (link->aspm_enabled ^ state) & state;
+	enable_req = (aspm_get_enabled_states(link) ^ state) & state;
 
 	/*
 	 * Here are the rules specified in the PCIe spec for enabling L1SS:
@@ -821,6 +829,7 @@ static void pcie_config_aspm_link(struct pcie_link_state *link, u32 state)
 	u32 upstream = 0, dwstream = 0;
 	struct pci_dev *child = link->downstream, *parent = link->pdev;
 	struct pci_bus *linkbus = parent->subordinate;
+	u32 aspm_enabled = aspm_get_enabled_states(link);
 
 	/* Enable only the states that were not explicitly disabled */
 	state &= (link->aspm_capable & ~link->aspm_disable);
@@ -832,11 +841,11 @@ static void pcie_config_aspm_link(struct pcie_link_state *link, u32 state)
 	/* Spec says both ports must be in D0 before enabling PCI PM substates*/
 	if (parent->current_state != PCI_D0 || child->current_state != PCI_D0) {
 		state &= ~ASPM_STATE_L1_SS_PCIPM;
-		state |= (link->aspm_enabled & ASPM_STATE_L1_SS_PCIPM);
+		state |= (aspm_enabled & ASPM_STATE_L1_SS_PCIPM);
 	}
 
 	/* Nothing to do if the link is already in the requested state */
-	if (link->aspm_enabled == state)
+	if (aspm_enabled == state)
 		return;
 	/* Convert ASPM state to upstream/downstream ASPM register state */
 	if (state & ASPM_STATE_L0S_UP)
@@ -863,8 +872,6 @@ static void pcie_config_aspm_link(struct pcie_link_state *link, u32 state)
 		pcie_config_aspm_dev(child, dwstream);
 	if (!(state & ASPM_STATE_L1))
 		pcie_config_aspm_dev(parent, upstream);
-
-	link->aspm_enabled = state;
 }
 
 static void pcie_config_aspm_path(struct pcie_link_state *link)
@@ -1238,7 +1245,7 @@ bool pcie_aspm_enabled(struct pci_dev *pdev)
 	if (!link)
 		return false;
 
-	return link->aspm_enabled;
+	return aspm_get_enabled_states(link);
 }
 EXPORT_SYMBOL_GPL(pcie_aspm_enabled);
 
@@ -1249,7 +1256,8 @@ static ssize_t aspm_attr_show_common(struct device *dev,
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct pcie_link_state *link = pcie_aspm_get_link(pdev);
 
-	return sysfs_emit(buf, "%d\n", (link->aspm_enabled & state) ? 1 : 0);
+	return sysfs_emit(buf, "%d\n",
+			  (aspm_get_enabled_states(link) & state) ? 1 : 0);
 }
 
 static ssize_t aspm_attr_store_common(struct device *dev,
-- 
2.20.1