[Linux-kernel-mentees] [RFC PATCH v5 18/23] PCI: Remove .aspm_* from struct pcie_link_state

Saheed O. Bolarinwa refactormyself at gmail.com
Sat Aug 22 20:03:53 UTC 2020


 - Remove initiations of pcie_link_state.aspm_*
 - Replace all access to pcie_link_state.aspm_* with pci_pdev.aspm_*
 - Remove pcie_link_state.aspm_*
 - Do few cleaups

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

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 27a590fe6b23..8d5a38081753 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -46,13 +46,6 @@ struct pcie_link_state {
 	struct pci_dev *downstream;	/* Downstream component, function 0 */
 	struct pcie_link_state *root;	/* pointer to the root port link */
 	struct pcie_link_state *parent;	/* pointer to the parent Link state */
-
-	/* 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 */
-	u32 aspm_disable:7;		/* Disabled ASPM state */
 };
 
 static int aspm_disabled, aspm_force;
@@ -97,7 +90,7 @@ static int policy_to_aspm_state(struct pci_dev *pdev)
 		/* Enable Everything */
 		return ASPM_STATE_ALL;
 	case POLICY_DEFAULT:
-		return pdev->link_state->aspm_default;
+		return pdev->aspm_default;
 	}
 	return 0;
 }
@@ -373,18 +366,14 @@ static void pcie_aspm_check_latency(struct pci_dev *endpoint)
 
 	while (link) {
 		/* Check upstream direction L0s latency */
-		if ((link->aspm_capable & ASPM_STATE_L0S_UP) &&
-		    (link->pdev->latency_up.l0s > acceptable->l0s)) {
-			link->aspm_capable &= ~ASPM_STATE_L0S_UP;
+		if ((link->pdev->aspm_capable & ASPM_STATE_L0S_UP) &&
+		    (link->pdev->latency_up.l0s > acceptable->l0s))
 			pdev->aspm_capable &= ~ASPM_STATE_L0S_UP;
-		}
 
 		/* Check downstream direction L0s latency */
-		if ((link->aspm_capable & ASPM_STATE_L0S_DW) &&
-		    (link->pdev->latency_dw.l0s > acceptable->l0s)) {
-			link->aspm_capable &= ~ASPM_STATE_L0S_DW;
+		if ((link->pdev->aspm_capable & ASPM_STATE_L0S_DW) &&
+		    (link->pdev->latency_dw.l0s > acceptable->l0s))
 			pdev->aspm_capable &= ~ASPM_STATE_L0S_DW;
-		}
 
 		/*
 		 * Check L1 latency.
@@ -401,11 +390,9 @@ static void pcie_aspm_check_latency(struct pci_dev *endpoint)
 		 */
 		latency = max_t(u32, link->pdev->latency_up.l1,
 						link->pdev->latency_dw.l1);
-		if ((link->aspm_capable & ASPM_STATE_L1) &&
-		    (latency + l1_switch_latency > acceptable->l1)) {
-			link->aspm_capable &= ~ASPM_STATE_L1;
+		if ((link->pdev->aspm_capable & ASPM_STATE_L1) &&
+		    (latency + l1_switch_latency > acceptable->l1))
 			pdev->aspm_capable &= ~ASPM_STATE_L1;
-		}
 
 		l1_switch_latency += 1000;
 
@@ -438,7 +425,7 @@ static void aspm_calc_l1ss_ctl_values(struct pci_dev *pdev,
 	struct pci_dev *dw_pdev = link->downstream;
 	struct pci_dev *up_pdev = link->pdev;
 
-	if (!(link->aspm_support & ASPM_STATE_L1_2_MASK))
+	if (!(link->pdev->aspm_support & ASPM_STATE_L1_2_MASK))
 		return;
 
 	/* Choose the greater of the two Port Common_Mode_Restore_Times */
@@ -493,8 +480,6 @@ static void pcie_aspm_cap_init(struct pci_dev *pdev, int blacklist)
 
 	if (blacklist) {
 		/* Set enabled/disable so that we will disable ASPM later */
-		link->aspm_enabled = ASPM_STATE_ALL;
-		link->aspm_disable = ASPM_STATE_ALL;
 		parent->aspm_enabled = ASPM_STATE_ALL;
 		parent->aspm_disable = ASPM_STATE_ALL;
 		return;
@@ -525,20 +510,14 @@ static void pcie_aspm_cap_init(struct pci_dev *pdev, int blacklist)
 	 */
 	if (((parent->lnkcap & PCI_EXP_LNKCAP_ASPMS) >> 10) &
 			((child->lnkcap & PCI_EXP_LNKCAP_ASPMS) >> 10) &
-			PCIE_LINK_STATE_L0S) {
-		link->aspm_support |= ASPM_STATE_L0S;
+			PCIE_LINK_STATE_L0S)
 		parent->aspm_support |= ASPM_STATE_L0S;
-	}
 
-	if (get_aspm_enable(child) & PCIE_LINK_STATE_L0S) {
-		link->aspm_enabled |= ASPM_STATE_L0S_UP;
+	if (get_aspm_enable(child) & PCIE_LINK_STATE_L0S)
 		parent->aspm_enabled |= ASPM_STATE_L0S_UP;
-	}
 
-	if (get_aspm_enable(parent) & PCIE_LINK_STATE_L0S) {
-		link->aspm_enabled |= ASPM_STATE_L0S_DW;
+	if (get_aspm_enable(parent) & PCIE_LINK_STATE_L0S)
 		parent->aspm_enabled |= ASPM_STATE_L0S_DW;
-	}
 
 	parent->latency_up.l0s = calc_l0s_latency(parent);
 	parent->latency_dw.l0s = calc_l0s_latency(child);
@@ -546,61 +525,39 @@ static void pcie_aspm_cap_init(struct pci_dev *pdev, int blacklist)
 	/* Setup L1 state */
 	if (((parent->lnkcap & PCI_EXP_LNKCAP_ASPMS) >> 10) &
 			((child->lnkcap & PCI_EXP_LNKCAP_ASPMS) >> 10) &
-			PCIE_LINK_STATE_L1) {
-		link->aspm_support |= ASPM_STATE_L1;
+			PCIE_LINK_STATE_L1)
 		parent->aspm_support |= ASPM_STATE_L1;
-	}
 
 	if (get_aspm_enable(parent) & get_aspm_enable(child) &
-			    PCIE_LINK_STATE_L1) {
-		link->aspm_enabled |= ASPM_STATE_L1;
+			    PCIE_LINK_STATE_L1)
 		parent->aspm_enabled |= ASPM_STATE_L1;
-	}
 
 	parent->latency_up.l1 = calc_l1_latency(parent);
 	parent->latency_dw.l1 = calc_l1_latency(child);
 
 	/* Setup L1 substate */
-	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_1)
 		parent->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_ASPM_L1_2)
 		parent->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_1)
 		parent->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 & child->l1ss_cap & PCI_L1SS_CAP_PCIPM_L1_2)
 		parent->aspm_support |= ASPM_STATE_L1_2_PCIPM;
-	}
 
-	if (up_l1ss_ctl1 & dw_l1ss_ctl1 & PCI_L1SS_CTL1_ASPM_L1_1) {
-		link->aspm_enabled |= ASPM_STATE_L1_1;
+	if (up_l1ss_ctl1 & dw_l1ss_ctl1 & PCI_L1SS_CTL1_ASPM_L1_1)
 		parent->aspm_enabled |= ASPM_STATE_L1_1;
-	}
-	if (up_l1ss_ctl1 & dw_l1ss_ctl1 & PCI_L1SS_CTL1_ASPM_L1_2) {
-		link->aspm_enabled |= ASPM_STATE_L1_2;
+	if (up_l1ss_ctl1 & dw_l1ss_ctl1 & PCI_L1SS_CTL1_ASPM_L1_2)
 		parent->aspm_enabled |= ASPM_STATE_L1_2;
-	}
-	if (up_l1ss_ctl1 & dw_l1ss_ctl1 & PCI_L1SS_CTL1_PCIPM_L1_1) {
-		link->aspm_enabled |= ASPM_STATE_L1_1_PCIPM;
+	if (up_l1ss_ctl1 & dw_l1ss_ctl1 & PCI_L1SS_CTL1_PCIPM_L1_1)
 		parent->aspm_enabled |= ASPM_STATE_L1_1_PCIPM;
-	}
-	if (up_l1ss_ctl1 & dw_l1ss_ctl1 & PCI_L1SS_CTL1_PCIPM_L1_2) {
-		link->aspm_enabled |= ASPM_STATE_L1_2_PCIPM;
+	if (up_l1ss_ctl1 & dw_l1ss_ctl1 & PCI_L1SS_CTL1_PCIPM_L1_2)
 		parent->aspm_enabled |= ASPM_STATE_L1_2_PCIPM;
-	}
 
 	/* Save default state */
-	link->aspm_default = link->aspm_enabled;
 	parent->aspm_default = parent->aspm_enabled;
 
 	/* Setup initial capable state. Will be updated later */
-	link->aspm_capable = link->aspm_support;
 	parent->aspm_capable = parent->aspm_support;
 
 	/* Get and check endpoint acceptable latencies */
@@ -729,7 +686,7 @@ static void pcie_config_aspm_link(struct pci_dev *pdev, u32 state)
 	struct pci_bus *linkbus = parent->subordinate;
 
 	/* Enable only the states that were not explicitly disabled */
-	state &= (link->aspm_capable & ~link->aspm_disable);
+	state &= (link->pdev->aspm_capable & ~link->pdev->aspm_disable);
 
 	/* Can't enable any substates if L1 is not enabled */
 	if (!(state & ASPM_STATE_L1))
@@ -738,11 +695,11 @@ static void pcie_config_aspm_link(struct pci_dev *pdev, 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 |= (link->pdev->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 (link->pdev->aspm_enabled == state)
 		return;
 	/* Convert ASPM state to upstream/downstream ASPM register state */
 	if (state & ASPM_STATE_L0S_UP)
@@ -754,7 +711,7 @@ static void pcie_config_aspm_link(struct pci_dev *pdev, u32 state)
 		dwstream |= PCI_EXP_LNKCTL_ASPM_L1;
 	}
 
-	if (link->aspm_capable & ASPM_STATE_L1SS)
+	if (link->pdev->aspm_capable & ASPM_STATE_L1SS)
 		pcie_config_aspm_l1ss(parent, state);
 
 	/*
@@ -770,7 +727,6 @@ static void pcie_config_aspm_link(struct pci_dev *pdev, u32 state)
 	if (!(state & ASPM_STATE_L1))
 		pcie_config_aspm_dev(parent, upstream);
 
-	link->aspm_enabled = state;
 	parent->aspm_enabled = state;
 }
 
@@ -1085,31 +1041,20 @@ static int __pci_disable_link_state(struct pci_dev *pdev, int state, bool sem)
 	if (sem)
 		down_read(&pci_bus_sem);
 	mutex_lock(&aspm_lock);
-	if (state & PCIE_LINK_STATE_L0S) {
-		link->aspm_disable |= ASPM_STATE_L0S;
+	if (state & PCIE_LINK_STATE_L0S)
 		bridge->aspm_disable |= ASPM_STATE_L0S;
-	}
-	if (state & PCIE_LINK_STATE_L1) {
+	if (state & PCIE_LINK_STATE_L1)
 		/* L1 PM substates require L1 */
-		link->aspm_disable |= ASPM_STATE_L1 | ASPM_STATE_L1SS;
 		bridge->aspm_disable |= ASPM_STATE_L1 | ASPM_STATE_L1SS;
-	}
-	if (state & PCIE_LINK_STATE_L1_1) {
-		link->aspm_disable |= ASPM_STATE_L1_1;
+	if (state & PCIE_LINK_STATE_L1_1)
 		bridge->aspm_disable |= ASPM_STATE_L1_1;
-	}
-	if (state & PCIE_LINK_STATE_L1_2) {
-		link->aspm_disable |= ASPM_STATE_L1_2;
+	if (state & PCIE_LINK_STATE_L1_2)
 		bridge->aspm_disable |= ASPM_STATE_L1_2;
-	}
-	if (state & PCIE_LINK_STATE_L1_1_PCIPM) {
-		link->aspm_disable |= ASPM_STATE_L1_1_PCIPM;
+	if (state & PCIE_LINK_STATE_L1_1_PCIPM)
 		bridge->aspm_disable |= ASPM_STATE_L1_1_PCIPM;
-	}
-	if (state & PCIE_LINK_STATE_L1_2_PCIPM) {
-		link->aspm_disable |= ASPM_STATE_L1_2_PCIPM;
+	if (state & PCIE_LINK_STATE_L1_2_PCIPM)
 		bridge->aspm_disable |= ASPM_STATE_L1_2_PCIPM;
-	}
+
 	pcie_config_aspm_link(link->pdev, policy_to_aspm_state(link->pdev));
 
 	if (state & PCIE_LINK_STATE_CLKPM) {
@@ -1200,7 +1145,7 @@ bool pcie_aspm_enabled(struct pci_dev *pdev)
 	if (!dev)
 		return false;
 
-	return dev->link_state->aspm_enabled;
+	return dev->aspm_enabled;
 }
 EXPORT_SYMBOL_GPL(pcie_aspm_enabled);
 
@@ -1211,7 +1156,7 @@ static ssize_t aspm_attr_show_common(struct device *dev,
 	struct pci_dev *pdev = pcie_aspm_get_link(to_pci_dev(dev));
 
 	return sprintf(buf, "%d\n",
-		(pdev->link_state->aspm_enabled & state) ? 1 : 0);
+		(pdev->aspm_enabled & state) ? 1 : 0);
 }
 
 static ssize_t aspm_attr_store_common(struct device *dev,
@@ -1230,17 +1175,12 @@ static ssize_t aspm_attr_store_common(struct device *dev,
 	mutex_lock(&aspm_lock);
 
 	if (state_enable) {
-		link->aspm_disable &= ~state;
 		bridge->aspm_disable &= ~state;
 		/* need to enable L1 for substates */
-		if (state & ASPM_STATE_L1SS) {
-			link->aspm_disable &= ~ASPM_STATE_L1;
+		if (state & ASPM_STATE_L1SS)
 			bridge->aspm_disable &= ~ASPM_STATE_L1;
-		}
-	} else {
-		link->aspm_disable |= state;
+	} else
 		bridge->aspm_disable |= state;
-	}
 
 	pcie_config_aspm_link(link->pdev, policy_to_aspm_state(link->pdev));
 
@@ -1322,7 +1262,6 @@ static umode_t aspm_ctrl_attrs_are_visible(struct kobject *kobj,
 {
 	struct device *dev = kobj_to_dev(kobj);
 	struct pci_dev *pdev = pcie_aspm_get_link(to_pci_dev(dev));
-	struct pcie_link_state *link = pdev->link_state;
 	static const u8 aspm_state_map[] = {
 		ASPM_STATE_L0S,
 		ASPM_STATE_L1,
@@ -1332,13 +1271,13 @@ static umode_t aspm_ctrl_attrs_are_visible(struct kobject *kobj,
 		ASPM_STATE_L1_2_PCIPM,
 	};
 
-	if (aspm_disabled || !link)
+	if (aspm_disabled || !pdev)
 		return 0;
 
 	if (n == 0)
 		return pdev->clkpm_capable ? a->mode : 0;
 
-	return link->aspm_capable & aspm_state_map[n - 1] ? a->mode : 0;
+	return pdev->aspm_capable & aspm_state_map[n - 1] ? a->mode : 0;
 }
 
 const struct attribute_group aspm_ctrl_attr_group = {
-- 
2.18.4



More information about the Linux-kernel-mentees mailing list