[RFC PATCH v3 0/5] Remove unncessary linked list from aspm.c

[RFC PATCH v3 0/5] Remove unncessary linked list from aspm.c

[Saheed O. Bolarinwa]

An extra linked list is created inside aspm.c to keep track of devices
on which the link state was enabled. However, it is possible to access
them via existing device lists.

This series remove the extra linked list and other related members of
the struct pcie_link_state: `root`, `parent` and `downstream`. All
these are now either calculated or obtained directly when needed.


VERSION CHANGES:
 -v3:
	- Remane pci_get_parent() to pcie_upstream_link() and improve
	  the logic based previous review.
	- Improve the algorithm to iterate through the devices

 - v2:
»       - Avoid using BUG_ON()
»       - Create helper function pci_get_parent()
»       - Fix a bug from the previous version

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

Bolarinwa O. Saheed (4):
  PCI/ASPM: Remove struct pcie_link_state.parent
  PCI/ASPM: Remove struct pcie_link_state.root
  PCI/ASPM: Remove struct pcie_link_state.downstream
  PCI/ASPM: Remove unncessary linked list from aspm.c

Saheed O. Bolarinwa (1):
  PCI: Handle NULL value inside pci_upstream_bridge()

 drivers/pci/pcie/aspm.c | 152 ++++++++++++++++++++++++----------------
 include/linux/pci.h     |   3 +
 2 files changed, 94 insertions(+), 61 deletions(-)

-- 
2.20.1

[RFC PATCH v3 1/5] PCI: Handle NULL value inside pci_upstream_bridge()

[Saheed O. Bolarinwa]

Return NULL if a NULL pointer is passed into pci_upstream_bridge().

Signed-off-by: Saheed O. Bolarinwa <refactormyself@gmail.com>
---
 include/linux/pci.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/pci.h b/include/linux/pci.h
index cd8aa6fce204..b087e0b9814e 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -695,6 +695,9 @@ static inline bool pci_is_bridge(struct pci_dev *dev)
 
 static inline struct pci_dev *pci_upstream_bridge(struct pci_dev *dev)
 {
+	if (!dev)
+		return NULL;
+
 	dev = pci_physfn(dev);
 	if (pci_is_root_bus(dev->bus))
 		return NULL;
-- 
2.20.1

[RFC PATCH v3 2/5] PCI/ASPM: Remove struct pcie_link_state.parent

[Saheed O. Bolarinwa]

From: "Bolarinwa O. Saheed" <refactormyself@gmail.com>

Information cached in struct pcie_link_state.parent is accessible
via struct pci_dev.

 - remove *parent* from the struct pcie_link_state
 - creates pcie_upstream_link() to obtain this value directly
 - replaces references to pcie_link_state.parent with a call to
   pcie_upstream_link()
 - remove BUG_ON(root->parent), instead obtain the root of the
   device returned by pcie_upstream_link

NOTE/CLARIFICATION:
The logic of pcie_upstream_link() especially as expressed in its
use within alloc_pcie_link_state() assumes that:

pdev->bus->parent->self == pdev->bus->self->bus->self

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

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 013a47f587ce..75618302fb87 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -50,7 +50,6 @@ struct pcie_link_state {
 	struct pci_dev *pdev;		/* Upstream component of the Link */
 	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 */
 	struct list_head sibling;	/* node in link_list */
 
 	/* ASPM state */
@@ -139,6 +138,18 @@ static int policy_to_clkpm_state(struct pcie_link_state *link)
 	return 0;
 }
 
+static struct pcie_link_state *pcie_upstream_link(struct pci_dev *dev)
+{
+	struct pci_dev *bridge;
+
+	bridge = pci_upstream_bridge(dev);
+	if (!bridge)
+		return NULL;
+
+	bridge = pci_upstream_bridge(bridge);
+	return bridge ? bridge->link_state : NULL;
+}
+
 static void pcie_set_clkpm_nocheck(struct pcie_link_state *link, int enable)
 {
 	struct pci_dev *child;
@@ -419,7 +430,7 @@ static void pcie_aspm_check_latency(struct pci_dev *endpoint)
 			link->aspm_capable &= ~ASPM_STATE_L1;
 		l1_switch_latency += 1000;
 
-		link = link->parent;
+		link = pcie_upstream_link(link->pdev);
 	}
 }
 
@@ -795,7 +806,7 @@ static void pcie_config_aspm_path(struct pcie_link_state *link)
 {
 	while (link) {
 		pcie_config_aspm_link(link, policy_to_aspm_state(link));
-		link = link->parent;
+		link = pcie_upstream_link(link->pdev);
 	}
 }
 
@@ -864,16 +875,15 @@ static struct pcie_link_state *alloc_pcie_link_state(struct pci_dev *pdev)
 	    !pdev->bus->parent->self) {
 		link->root = link;
 	} else {
-		struct pcie_link_state *parent;
+		struct pcie_link_state *uplink_bridge;
 
-		parent = pdev->bus->parent->self->link_state;
-		if (!parent) {
+		uplink_bridge = pcie_upstream_link(pdev);
+		if (!uplink_bridge) {
 			kfree(link);
 			return NULL;
 		}
 
-		link->parent = parent;
-		link->root = link->parent->root;
+		link->root = ulink_bridge->root;
 	}
 
 	list_add(&link->sibling, &link_list);
@@ -962,7 +972,10 @@ void pcie_aspm_init_link_state(struct pci_dev *pdev)
 static void pcie_update_aspm_capable(struct pcie_link_state *root)
 {
 	struct pcie_link_state *link;
-	BUG_ON(root->parent);
+	struct pcie_link_state *uplink = pcie_upstream_link(root->pdev);
+
+	root = uplink ? uplink->root : root;
+
 	list_for_each_entry(link, &link_list, sibling) {
 		if (link->root != root)
 			continue;
@@ -986,7 +999,7 @@ static void pcie_update_aspm_capable(struct pcie_link_state *root)
 void pcie_aspm_exit_link_state(struct pci_dev *pdev)
 {
 	struct pci_dev *parent = pdev->bus->self;
-	struct pcie_link_state *link, *root, *parent_link;
+	struct pcie_link_state *link, *root, *uplink_bridge;
 
 	if (!parent || !parent->link_state)
 		return;
@@ -1002,7 +1015,7 @@ void pcie_aspm_exit_link_state(struct pci_dev *pdev)
 
 	link = parent->link_state;
 	root = link->root;
-	parent_link = link->parent;
+	uplink_bridge = pcie_upstream_link(link->pdev);
 
 	/* All functions are removed, so just disable ASPM for the link */
 	pcie_config_aspm_link(link, 0);
@@ -1011,9 +1024,9 @@ void pcie_aspm_exit_link_state(struct pci_dev *pdev)
 	free_link_state(link);
 
 	/* Recheck latencies and configure upstream links */
-	if (parent_link) {
+	if (uplink_bridge) {
 		pcie_update_aspm_capable(root);
-		pcie_config_aspm_path(parent_link);
+		pcie_config_aspm_path(uplink_bridge);
 	}
 out:
 	mutex_unlock(&aspm_lock);
-- 
2.20.1

[RFC PATCH v3 3/5] PCI/ASPM: Remove struct pcie_link_state.root

[Saheed O. Bolarinwa]

From: "Bolarinwa O. Saheed" <refactormyself@gmail.com>

Information on the root device is calculated within
alloc_pcie_link_state() and stored in struct pcie_link_state.root.
If this calculation is extracted out, it make it possible to avoid
storing the value

 - extract the calculations of pcie_link_state->root into
   pcie_get_root().
 - remove *root* from the struct pcie_link_state.
 - replace references to struct pcie_link_state.root with
   a call to pcie_get_root().

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

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 75618302fb87..90c7a0b379f4 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -49,7 +49,6 @@ struct aspm_latency {
 struct pcie_link_state {
 	struct pci_dev *pdev;		/* Upstream component of the Link */
 	struct pci_dev *downstream;	/* Downstream component, function 0 */
-	struct pcie_link_state *root;	/* pointer to the root port link */
 	struct list_head sibling;	/* node in link_list */
 
 	/* ASPM state */
@@ -851,6 +850,25 @@ static int pcie_aspm_sanity_check(struct pci_dev *pdev)
 	return 0;
 }
 
+/*
+ * Root Ports and PCI/PCI-X to PCIe Bridges are roots of PCIe
+ * hierarchies.  Note that some PCIe host implementations omit
+ * the root ports entirely, in which case a downstream port on
+ * a switch may become the root of the link state chain for all
+ * its subordinate endpoints.
+ */
+static struct pci_dev *pcie_get_root(struct pci_dev *pdev)
+{
+	struct pcie_link_state *uplink_bridge = pcie_upstream_link(pdev);
+
+	if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT ||
+	    pci_pcie_type(pdev) == PCI_EXP_TYPE_PCIE_BRIDGE || !uplink_bridge) {
+		return pdev;
+	} else {
+		return pcie_get_root(uplink_bridge->pdev);
+	}
+}
+
 static struct pcie_link_state *alloc_pcie_link_state(struct pci_dev *pdev)
 {
 	struct pcie_link_state *link;
@@ -863,29 +881,6 @@ static struct pcie_link_state *alloc_pcie_link_state(struct pci_dev *pdev)
 	link->pdev = pdev;
 	link->downstream = pci_function_0(pdev->subordinate);
 
-	/*
-	 * Root Ports and PCI/PCI-X to PCIe Bridges are roots of PCIe
-	 * hierarchies.  Note that some PCIe host implementations omit
-	 * the root ports entirely, in which case a downstream port on
-	 * a switch may become the root of the link state chain for all
-	 * its subordinate endpoints.
-	 */
-	if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT ||
-	    pci_pcie_type(pdev) == PCI_EXP_TYPE_PCIE_BRIDGE ||
-	    !pdev->bus->parent->self) {
-		link->root = link;
-	} else {
-		struct pcie_link_state *uplink_bridge;
-
-		uplink_bridge = pcie_upstream_link(pdev);
-		if (!uplink_bridge) {
-			kfree(link);
-			return NULL;
-		}
-
-		link->root = ulink_bridge->root;
-	}
-
 	list_add(&link->sibling, &link_list);
 	pdev->link_state = link;
 	return link;
@@ -972,20 +967,26 @@ void pcie_aspm_init_link_state(struct pci_dev *pdev)
 static void pcie_update_aspm_capable(struct pcie_link_state *root)
 {
 	struct pcie_link_state *link;
-	struct pcie_link_state *uplink = pcie_upstream_link(root->pdev);
+	struct pci_dev *dev, *root_dev;
 
-	root = uplink ? uplink->root : root;
+	/* Ensure it is the root device */
+	root_dev = pcie_get_root(root->pdev);
+	root = root_dev ? root_dev->link_state : root;
 
 	list_for_each_entry(link, &link_list, sibling) {
-		if (link->root != root)
+		dev = pcie_get_root(link->pdev);
+		if (dev->link_state != root)
 			continue;
+
 		link->aspm_capable = link->aspm_support;
 	}
 	list_for_each_entry(link, &link_list, sibling) {
 		struct pci_dev *child;
 		struct pci_bus *linkbus = link->pdev->subordinate;
-		if (link->root != root)
+		dev = pcie_get_root(link->pdev);
+		if (dev->link_state != root)
 			continue;
+
 		list_for_each_entry(child, &linkbus->devices, bus_list) {
 			if ((pci_pcie_type(child) != PCI_EXP_TYPE_ENDPOINT) &&
 			    (pci_pcie_type(child) != PCI_EXP_TYPE_LEG_END))
@@ -998,8 +999,8 @@ static void pcie_update_aspm_capable(struct pcie_link_state *root)
 /* @pdev: the endpoint device */
 void pcie_aspm_exit_link_state(struct pci_dev *pdev)
 {
-	struct pci_dev *parent = pdev->bus->self;
-	struct pcie_link_state *link, *root, *uplink_bridge;
+	struct pci_dev *root_dev, *parent = pdev->bus->self;
+	struct pcie_link_state *link, *uplink_bridge;
 
 	if (!parent || !parent->link_state)
 		return;
@@ -1014,7 +1015,7 @@ void pcie_aspm_exit_link_state(struct pci_dev *pdev)
 		goto out;
 
 	link = parent->link_state;
-	root = link->root;
+	root_dev = pcie_get_root(link->pdev);
 	uplink_bridge = pcie_upstream_link(link->pdev);
 
 	/* All functions are removed, so just disable ASPM for the link */
@@ -1025,7 +1026,7 @@ void pcie_aspm_exit_link_state(struct pci_dev *pdev)
 
 	/* Recheck latencies and configure upstream links */
 	if (uplink_bridge) {
-		pcie_update_aspm_capable(root);
+		pcie_update_aspm_capable(root_dev->link_state);
 		pcie_config_aspm_path(uplink_bridge);
 	}
 out:
@@ -1037,6 +1038,7 @@ void pcie_aspm_exit_link_state(struct pci_dev *pdev)
 void pcie_aspm_pm_state_change(struct pci_dev *pdev)
 {
 	struct pcie_link_state *link = pdev->link_state;
+	struct pci_dev *root = pcie_get_root(pdev);
 
 	if (aspm_disabled || !link)
 		return;
@@ -1046,7 +1048,7 @@ void pcie_aspm_pm_state_change(struct pci_dev *pdev)
 	 */
 	down_read(&pci_bus_sem);
 	mutex_lock(&aspm_lock);
-	pcie_update_aspm_capable(link->root);
+	pcie_update_aspm_capable(root->link_state);
 	pcie_config_aspm_path(link);
 	mutex_unlock(&aspm_lock);
 	up_read(&pci_bus_sem);
-- 
2.20.1

[RFC PATCH v3 4/5] PCI/ASPM: Remove struct pcie_link_state.downstream

[Saheed O. Bolarinwa]

From: "Bolarinwa O. Saheed" <refactormyself@gmail.com>

Information on the downstream component is cached in
struct pcie_link_state.downstream it obtained within
alloc_pcie_link_state() by calling pci_function_0()

 - remove *downstream* from the struct pcie_link_state.
 - replaces references to pcie_link_state.downstream with
   a call to pci_function_0(pdev->subordinate).

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

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 90c7a0b379f4..12623556f750 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -48,7 +48,6 @@ struct aspm_latency {
 
 struct pcie_link_state {
 	struct pci_dev *pdev;		/* Upstream component of the Link */
-	struct pci_dev *downstream;	/* Downstream component, function 0 */
 	struct list_head sibling;	/* node in link_list */
 
 	/* ASPM state */
@@ -462,7 +461,8 @@ static void pci_clear_and_set_dword(struct pci_dev *pdev, int pos,
 static void aspm_calc_l1ss_info(struct pcie_link_state *link,
 				u32 parent_l1ss_cap, u32 child_l1ss_cap)
 {
-	struct pci_dev *child = link->downstream, *parent = link->pdev;
+	struct pci_dev *parent = link->pdev;
+	struct pci_dev *child = pci_function_0(link->pdev->subordinate);
 	u32 val1, val2, scale1, scale2;
 	u32 t_common_mode, t_power_on, l1_2_threshold, scale, value;
 	u32 ctl1 = 0, ctl2 = 0;
@@ -552,7 +552,8 @@ static void aspm_calc_l1ss_info(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;
+	struct pci_dev *parent = link->pdev;
+	struct pci_dev *child = pci_function_0(link->pdev->subordinate);
 	u32 parent_lnkcap, child_lnkcap;
 	u16 parent_lnkctl, child_lnkctl;
 	u32 parent_l1ss_cap, child_l1ss_cap;
@@ -694,7 +695,8 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 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;
+	struct pci_dev *parent = link->pdev;
+	struct pci_dev *child = pci_function_0(link->pdev->subordinate);
 
 	enable_req = (link->aspm_enabled ^ state) & state;
 
@@ -753,7 +755,8 @@ static void pcie_config_aspm_dev(struct pci_dev *pdev, u32 val)
 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_dev *parent = link->pdev;
+	struct pci_dev *child = pci_function_0(link->pdev->subordinate);
 	struct pci_bus *linkbus = parent->subordinate;
 
 	/* Enable only the states that were not explicitly disabled */
@@ -879,7 +882,6 @@ static struct pcie_link_state *alloc_pcie_link_state(struct pci_dev *pdev)
 
 	INIT_LIST_HEAD(&link->sibling);
 	link->pdev = pdev;
-	link->downstream = pci_function_0(pdev->subordinate);
 
 	list_add(&link->sibling, &link_list);
 	pdev->link_state = link;
-- 
2.20.1

[RFC PATCH v3 5/5] PCI/ASPM: Remove unncessary linked list from aspm.c

[Saheed O. Bolarinwa]

From: "Bolarinwa O. Saheed" <refactormyself@gmail.com>

aspm.c defines a linked list - `link_list` and stores each of
its node in struct pcie_link_state.sibling. This linked list
tracks devices for which the struct pcie_link_state object
was successfully created. It is used to loop through the list
for instance to set ASPM policy or update changes. However, it
is possible to access these devices via existing lists defined
inside pci.h

- removes link_list and struct pcie_link_state.sibling
- accesses child devices via struct pci_dev.bust_list
- create pcie_config_bus_devices() which walk across the device
  heirarchies linked to the bus
- accesses all PCI buses via pci_root_buses on struct pci_bus.node

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

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 12623556f750..65da034dc290 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -48,7 +48,6 @@ struct aspm_latency {
 
 struct pcie_link_state {
 	struct pci_dev *pdev;		/* Upstream component of the Link */
-	struct list_head sibling;	/* node in link_list */
 
 	/* ASPM state */
 	u32 aspm_support:7;		/* Supported ASPM state */
@@ -76,7 +75,6 @@ struct pcie_link_state {
 static int aspm_disabled, aspm_force;
 static bool aspm_support_enabled = true;
 static DEFINE_MUTEX(aspm_lock);
-static LIST_HEAD(link_list);
 
 #define POLICY_DEFAULT 0	/* BIOS default setting */
 #define POLICY_PERFORMANCE 1	/* high performance */
@@ -880,10 +878,7 @@ static struct pcie_link_state *alloc_pcie_link_state(struct pci_dev *pdev)
 	if (!link)
 		return NULL;
 
-	INIT_LIST_HEAD(&link->sibling);
 	link->pdev = pdev;
-
-	list_add(&link->sibling, &link_list);
 	pdev->link_state = link;
 	return link;
 }
@@ -970,24 +965,22 @@ static void pcie_update_aspm_capable(struct pcie_link_state *root)
 {
 	struct pcie_link_state *link;
 	struct pci_dev *dev, *root_dev;
+	struct pci_bus *rootbus = root->pdev->bus;
 
 	/* Ensure it is the root device */
 	root_dev = pcie_get_root(root->pdev);
 	root = root_dev ? root_dev->link_state : root;
 
-	list_for_each_entry(link, &link_list, sibling) {
-		dev = pcie_get_root(link->pdev);
-		if (dev->link_state != root)
+	list_for_each_entry(dev, &rootbus->devices, bus_list) {
+		if (!dev->link_state)
 			continue;
 
-		link->aspm_capable = link->aspm_support;
+		dev->link_state->aspm_capable = link->aspm_support;
 	}
-	list_for_each_entry(link, &link_list, sibling) {
+
+	list_for_each_entry(dev, &rootbus->devices, bus_list) {
 		struct pci_dev *child;
-		struct pci_bus *linkbus = link->pdev->subordinate;
-		dev = pcie_get_root(link->pdev);
-		if (dev->link_state != root)
-			continue;
+		struct pci_bus *linkbus = dev->subordinate;
 
 		list_for_each_entry(child, &linkbus->devices, bus_list) {
 			if ((pci_pcie_type(child) != PCI_EXP_TYPE_ENDPOINT) &&
@@ -1022,7 +1015,6 @@ void pcie_aspm_exit_link_state(struct pci_dev *pdev)
 
 	/* All functions are removed, so just disable ASPM for the link */
 	pcie_config_aspm_link(link, 0);
-	list_del(&link->sibling);
 	/* Clock PM is for endpoint device */
 	free_link_state(link);
 
@@ -1157,11 +1149,33 @@ int pci_disable_link_state(struct pci_dev *pdev, int state)
 }
 EXPORT_SYMBOL(pci_disable_link_state);
 
+static void pcie_config_bus_devices(struct pci_bus *bus)
+{
+	struct pci_dev *pdev;
+	struct pcie_link_state *link;
+
+	list_for_each_entry(pdev, &bus->devices, bus_list) {
+		if (!pci_is_pcie(pdev))
+			break;
+
+		link = pdev->link_state;
+		if (!link)
+			continue;
+
+		pcie_config_aspm_link(link, policy_to_aspm_state(link));
+		pcie_set_clkpm(link, policy_to_clkpm_state(link));
+
+		/* if this is a bridge, cross it */
+		if (pdev->subordinate && pdev->subordinate != bus)
+			pcie_config_bus_devices(pdev->subordinate);
+	}
+}
+
 static int pcie_aspm_set_policy(const char *val,
 				const struct kernel_param *kp)
 {
 	int i;
-	struct pcie_link_state *link;
+	struct pci_bus *bus;
 
 	if (aspm_disabled)
 		return -EPERM;
@@ -1174,10 +1188,9 @@ static int pcie_aspm_set_policy(const char *val,
 	down_read(&pci_bus_sem);
 	mutex_lock(&aspm_lock);
 	aspm_policy = i;
-	list_for_each_entry(link, &link_list, sibling) {
-		pcie_config_aspm_link(link, policy_to_aspm_state(link));
-		pcie_set_clkpm(link, policy_to_clkpm_state(link));
-	}
+	list_for_each_entry(bus, &pci_root_buses, node)
+		pcie_config_bus_devices(bus);
+
 	mutex_unlock(&aspm_lock);
 	up_read(&pci_bus_sem);
 	return 0;
-- 
2.20.1

Re: [RFC PATCH v3 5/5] PCI/ASPM: Remove unncessary linked list from aspm.c

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 3403 bytes --]

Hi "Saheed,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on helgaas-pci/next]
[also build test WARNING on v5.15 next-20211106]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Saheed-O-Bolarinwa/Remove-unncessary-linked-list-from-aspm-c/20211107-015558
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: arm-buildonly-randconfig-r001-20211107 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 627868263cd4d57c230b61904483a3dad9e1a1da)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/0day-ci/linux/commit/1363f09cfe46f62adad834d4ea2eb67fb29ab60a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Saheed-O-Bolarinwa/Remove-unncessary-linked-list-from-aspm-c/20211107-015558
        git checkout 1363f09cfe46f62adad834d4ea2eb67fb29ab60a
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/pci/pcie/aspm.c:978:35: warning: variable 'link' is uninitialized when used here [-Wuninitialized]
                   dev->link_state->aspm_capable = link->aspm_support;
                                                   ^~~~
   drivers/pci/pcie/aspm.c:966:30: note: initialize the variable 'link' to silence this warning
           struct pcie_link_state *link;
                                       ^
                                        = NULL
   1 warning generated.


vim +/link +978 drivers/pci/pcie/aspm.c

   962	
   963	/* Recheck latencies and update aspm_capable for links under the root */
   964	static void pcie_update_aspm_capable(struct pcie_link_state *root)
   965	{
   966		struct pcie_link_state *link;
   967		struct pci_dev *dev, *root_dev;
   968		struct pci_bus *rootbus = root->pdev->bus;
   969	
   970		/* Ensure it is the root device */
   971		root_dev = pcie_get_root(root->pdev);
   972		root = root_dev ? root_dev->link_state : root;
   973	
   974		list_for_each_entry(dev, &rootbus->devices, bus_list) {
   975			if (!dev->link_state)
   976				continue;
   977	
 > 978			dev->link_state->aspm_capable = link->aspm_support;
   979		}
   980	
   981		list_for_each_entry(dev, &rootbus->devices, bus_list) {
   982			struct pci_dev *child;
   983			struct pci_bus *linkbus = dev->subordinate;
   984	
   985			list_for_each_entry(child, &linkbus->devices, bus_list) {
   986				if ((pci_pcie_type(child) != PCI_EXP_TYPE_ENDPOINT) &&
   987				    (pci_pcie_type(child) != PCI_EXP_TYPE_LEG_END))
   988					continue;
   989				pcie_aspm_check_latency(child);
   990			}
   991		}
   992	}
   993	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 52764 bytes --]

Re: [RFC PATCH v3 5/5] PCI/ASPM: Remove unncessary linked list from aspm.c

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 3483 bytes --]

Hi "Saheed,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on helgaas-pci/next]
[also build test WARNING on v5.15 next-20211106]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Saheed-O-Bolarinwa/Remove-unncessary-linked-list-from-aspm-c/20211107-015558
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: arm-buildonly-randconfig-r001-20211107 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 627868263cd4d57c230b61904483a3dad9e1a1da)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/0day-ci/linux/commit/1363f09cfe46f62adad834d4ea2eb67fb29ab60a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Saheed-O-Bolarinwa/Remove-unncessary-linked-list-from-aspm-c/20211107-015558
        git checkout 1363f09cfe46f62adad834d4ea2eb67fb29ab60a
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/pci/pcie/aspm.c:978:35: warning: variable 'link' is uninitialized when used here [-Wuninitialized]
                   dev->link_state->aspm_capable = link->aspm_support;
                                                   ^~~~
   drivers/pci/pcie/aspm.c:966:30: note: initialize the variable 'link' to silence this warning
           struct pcie_link_state *link;
                                       ^
                                        = NULL
   1 warning generated.


vim +/link +978 drivers/pci/pcie/aspm.c

   962	
   963	/* Recheck latencies and update aspm_capable for links under the root */
   964	static void pcie_update_aspm_capable(struct pcie_link_state *root)
   965	{
   966		struct pcie_link_state *link;
   967		struct pci_dev *dev, *root_dev;
   968		struct pci_bus *rootbus = root->pdev->bus;
   969	
   970		/* Ensure it is the root device */
   971		root_dev = pcie_get_root(root->pdev);
   972		root = root_dev ? root_dev->link_state : root;
   973	
   974		list_for_each_entry(dev, &rootbus->devices, bus_list) {
   975			if (!dev->link_state)
   976				continue;
   977	
 > 978			dev->link_state->aspm_capable = link->aspm_support;
   979		}
   980	
   981		list_for_each_entry(dev, &rootbus->devices, bus_list) {
   982			struct pci_dev *child;
   983			struct pci_bus *linkbus = dev->subordinate;
   984	
   985			list_for_each_entry(child, &linkbus->devices, bus_list) {
   986				if ((pci_pcie_type(child) != PCI_EXP_TYPE_ENDPOINT) &&
   987				    (pci_pcie_type(child) != PCI_EXP_TYPE_LEG_END))
   988					continue;
   989				pcie_aspm_check_latency(child);
   990			}
   991		}
   992	}
   993	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 52764 bytes --]

Re: [RFC PATCH v3 2/5] PCI/ASPM: Remove struct pcie_link_state.parent

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 3728 bytes --]

Hi "Saheed,

[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on helgaas-pci/next]
[also build test ERROR on v5.15 next-20211106]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Saheed-O-Bolarinwa/Remove-unncessary-linked-list-from-aspm-c/20211107-015558
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: powerpc-allyesconfig (attached as .config)
compiler: powerpc-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/d60048f94473e70fdbbe82d7e4c3df8ab7431416
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Saheed-O-Bolarinwa/Remove-unncessary-linked-list-from-aspm-c/20211107-015558
        git checkout d60048f94473e70fdbbe82d7e4c3df8ab7431416
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=powerpc SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

Note: the linux-review/Saheed-O-Bolarinwa/Remove-unncessary-linked-list-from-aspm-c/20211107-015558 HEAD 1363f09cfe46f62adad834d4ea2eb67fb29ab60a builds fine.
      It only hurts bisectability.

All errors (new ones prefixed by >>):

   drivers/pci/pcie/aspm.c: In function 'alloc_pcie_link_state':
>> drivers/pci/pcie/aspm.c:886:30: error: 'ulink_bridge' undeclared (first use in this function); did you mean 'uplink_bridge'?
     886 |                 link->root = ulink_bridge->root;
         |                              ^~~~~~~~~~~~
         |                              uplink_bridge
   drivers/pci/pcie/aspm.c:886:30: note: each undeclared identifier is reported only once for each function it appears in


vim +886 drivers/pci/pcie/aspm.c

   853	
   854	static struct pcie_link_state *alloc_pcie_link_state(struct pci_dev *pdev)
   855	{
   856		struct pcie_link_state *link;
   857	
   858		link = kzalloc(sizeof(*link), GFP_KERNEL);
   859		if (!link)
   860			return NULL;
   861	
   862		INIT_LIST_HEAD(&link->sibling);
   863		link->pdev = pdev;
   864		link->downstream = pci_function_0(pdev->subordinate);
   865	
   866		/*
   867		 * Root Ports and PCI/PCI-X to PCIe Bridges are roots of PCIe
   868		 * hierarchies.  Note that some PCIe host implementations omit
   869		 * the root ports entirely, in which case a downstream port on
   870		 * a switch may become the root of the link state chain for all
   871		 * its subordinate endpoints.
   872		 */
   873		if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT ||
   874		    pci_pcie_type(pdev) == PCI_EXP_TYPE_PCIE_BRIDGE ||
   875		    !pdev->bus->parent->self) {
   876			link->root = link;
   877		} else {
   878			struct pcie_link_state *uplink_bridge;
   879	
   880			uplink_bridge = pcie_upstream_link(pdev);
   881			if (!uplink_bridge) {
   882				kfree(link);
   883				return NULL;
   884			}
   885	
 > 886			link->root = ulink_bridge->root;
   887		}
   888	
   889		list_add(&link->sibling, &link_list);
   890		pdev->link_state = link;
   891		return link;
   892	}
   893	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 72181 bytes --]