[PATCH 0/8] PCI: Move some ASPM info to struct pci_dev

[PATCH 0/8] PCI: Move some ASPM info to struct pci_dev

[Saheed O. Bolarinwa]

These series migrates some ASPM information into struct pci_dev.
struct pcie_link_state.l1ss and struct aspm_register_info were
removed.


Saheed O. Bolarinwa (8):
  PCI/ASPM: Cache device's ASPM link capability in struct pci_dev
  PCI/ASPM: Rework calc_l*_latency() to take a struct pci_dev
  PCI/ASPM: Compute the value of aspm_register_info.support directly
  PCI/ASPM: Replace aspm_register_info.l1ss_cap* with pci_dev.l1ss_cap*
  PCI/ASPM: Remove aspm_register_info.l1ss_ctl*
  PCI/ASPM: Remove aspm_register_info.enable
  PCI/ASPM: Remove pcie_get_aspm_reg() and struct aspm_register_info
  PCI/ASPM: Remove struct pcie_link_state.l1ss

 drivers/pci/pcie/aspm.c | 203 +++++++++++++++-------------------------
 drivers/pci/probe.c     |   7 ++
 include/linux/pci.h     |   3 +
 3 files changed, 84 insertions(+), 129 deletions(-)

-- 
2.18.4

[PATCH 1/8] PCI/ASPM: Cache device's ASPM link capability in struct pci_dev

[Saheed O. Bolarinwa]

pcie_get_aspm_reg() reads LNKCAP to learn whether the device supports
ASPM L0s and/or L1 and L1 substates.

If we cache the entire LNKCAP word early enough, we may be able to
use it in other places that read LNKCAP, e.g. pcie_get_speed_cap(),
pcie_get_width_cap(), pcie_init(), etc.

 - Add struct pci_dev.lnkcap (u32)
 - Read PCI_EXP_LNKCAP in set_pcie_port_type() and save it
   in pci_dev.lnkcap
 - Use pdev->lnkcap instead of reading PCI_EXP_LNKCAP

Signed-off-by: Saheed O. Bolarinwa <refactormyself@gmail.com>
---
 drivers/pci/pcie/aspm.c | 7 ++-----
 drivers/pci/probe.c     | 1 +
 include/linux/pci.h     | 1 +
 3 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 253c30cc1967..d7e69b3595a0 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -177,15 +177,13 @@ static void pcie_set_clkpm(struct pcie_link_state *link, int enable)
 static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
 {
 	int capable = 1, enabled = 1;
-	u32 reg32;
 	u16 reg16;
 	struct pci_dev *child;
 	struct pci_bus *linkbus = link->pdev->subordinate;
 
 	/* All functions should have the same cap and state, take the worst */
 	list_for_each_entry(child, &linkbus->devices, bus_list) {
-		pcie_capability_read_dword(child, PCI_EXP_LNKCAP, &reg32);
-		if (!(reg32 & PCI_EXP_LNKCAP_CLKPM)) {
+		if (!(child->lnkcap & PCI_EXP_LNKCAP_CLKPM)) {
 			capable = 0;
 			enabled = 0;
 			break;
@@ -397,9 +395,8 @@ static void pcie_get_aspm_reg(struct pci_dev *pdev,
 			      struct aspm_register_info *info)
 {
 	u16 reg16;
-	u32 reg32;
+	u32 reg32 = pdev->lnkcap;
 
-	pcie_capability_read_dword(pdev, PCI_EXP_LNKCAP, &reg32);
 	info->support = (reg32 & PCI_EXP_LNKCAP_ASPMS) >> 10;
 	info->latency_encoding_l0s = (reg32 & PCI_EXP_LNKCAP_L0SEL) >> 12;
 	info->latency_encoding_l1  = (reg32 & PCI_EXP_LNKCAP_L1EL) >> 15;
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 03d37128a24f..2d5898f05f89 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1486,6 +1486,7 @@ void set_pcie_port_type(struct pci_dev *pdev)
 	pdev->pcie_flags_reg = reg16;
 	pci_read_config_word(pdev, pos + PCI_EXP_DEVCAP, &reg16);
 	pdev->pcie_mpss = reg16 & PCI_EXP_DEVCAP_PAYLOAD;
+	pcie_capability_read_dword(pdev, PCI_EXP_LNKCAP, &pdev->lnkcap);
 
 	parent = pci_upstream_bridge(pdev);
 	if (!parent)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 835530605c0d..5b305cfeb1dc 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -375,6 +375,7 @@ struct pci_dev {
 						   bit manually */
 	unsigned int	d3_delay;	/* D3->D0 transition time in ms */
 	unsigned int	d3cold_delay;	/* D3cold->D0 transition time in ms */
+	u32		lnkcap;		/* Link Capabilities */
 
 #ifdef CONFIG_PCIEASPM
 	struct pcie_link_state	*link_state;	/* ASPM link state */
-- 
2.18.4

[PATCH 2/8] PCI/ASPM: Rework calc_l*_latency() to take a struct pci_dev

[Saheed O. Bolarinwa]

 - Change the argument of calc_l0s_latency() to  pci_dev *,
 - Compute latency_encoding_l0s encoding inside calc_l0s_latency()
 - Compute latency_encoding_l1 encoding inside calc_l1_latency()
 - Make calc_l*_latency() take only pci_dev *,
 - Make callers to calc_l0s_latency() and calc_l1_latency() pass
   in struct pci_dev
 - In pcie_get_aspm_reg() remove assignments to the latency encodings
 - Remove aspm_register_info.latency_encoding_l1
 - Remove aspm_register_info.latency_encoding_l0s

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

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index d7e69b3595a0..5f7cf47b6a40 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -306,8 +306,10 @@ static void pcie_aspm_configure_common_clock(struct pcie_link_state *link)
 }
 
 /* Convert L0s latency encoding to ns */
-static u32 calc_l0s_latency(u32 encoding)
+static u32 calc_l0s_latency(struct pci_dev *pdev)
 {
+	u32 encoding = (pdev->lnkcap & PCI_EXP_LNKCAP_L0SEL) >> 12;
+
 	if (encoding == 0x7)
 		return (5 * 1000);	/* > 4us */
 	return (64 << encoding);
@@ -322,8 +324,10 @@ static u32 calc_l0s_acceptable(u32 encoding)
 }
 
 /* Convert L1 latency encoding to ns */
-static u32 calc_l1_latency(u32 encoding)
+static u32 calc_l1_latency(struct pci_dev *pdev)
 {
+	u32 encoding = (pdev->lnkcap & PCI_EXP_LNKCAP_L1EL) >> 15;
+
 	if (encoding == 0x7)
 		return (65 * 1000);	/* > 64us */
 	return (1000 << encoding);
@@ -381,8 +385,6 @@ static void encode_l12_threshold(u32 threshold_us, u32 *scale, u32 *value)
 struct aspm_register_info {
 	u32 support:2;
 	u32 enabled:2;
-	u32 latency_encoding_l0s;
-	u32 latency_encoding_l1;
 
 	/* L1 substates */
 	u32 l1ss_cap_ptr;
@@ -398,8 +400,6 @@ static void pcie_get_aspm_reg(struct pci_dev *pdev,
 	u32 reg32 = pdev->lnkcap;
 
 	info->support = (reg32 & PCI_EXP_LNKCAP_ASPMS) >> 10;
-	info->latency_encoding_l0s = (reg32 & PCI_EXP_LNKCAP_L0SEL) >> 12;
-	info->latency_encoding_l1  = (reg32 & PCI_EXP_LNKCAP_L1EL) >> 15;
 	pcie_capability_read_word(pdev, PCI_EXP_LNKCTL, &reg16);
 	info->enabled = reg16 & PCI_EXP_LNKCTL_ASPMC;
 
@@ -587,16 +587,16 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 		link->aspm_enabled |= ASPM_STATE_L0S_UP;
 	if (upreg.enabled & PCIE_LINK_STATE_L0S)
 		link->aspm_enabled |= ASPM_STATE_L0S_DW;
-	link->latency_up.l0s = calc_l0s_latency(upreg.latency_encoding_l0s);
-	link->latency_dw.l0s = calc_l0s_latency(dwreg.latency_encoding_l0s);
+	link->latency_up.l0s = calc_l0s_latency(parent);
+	link->latency_dw.l0s = calc_l0s_latency(child);
 
 	/* Setup L1 state */
 	if (upreg.support & dwreg.support & PCIE_LINK_STATE_L1)
 		link->aspm_support |= ASPM_STATE_L1;
 	if (upreg.enabled & dwreg.enabled & PCIE_LINK_STATE_L1)
 		link->aspm_enabled |= ASPM_STATE_L1;
-	link->latency_up.l1 = calc_l1_latency(upreg.latency_encoding_l1);
-	link->latency_dw.l1 = calc_l1_latency(dwreg.latency_encoding_l1);
+	link->latency_up.l1 = calc_l1_latency(parent);
+	link->latency_dw.l1 = calc_l1_latency(child);
 
 	/* Setup L1 substate */
 	if (upreg.l1ss_cap & dwreg.l1ss_cap & PCI_L1SS_CAP_ASPM_L1_1)
-- 
2.18.4

[PATCH 3/8] PCI/ASPM: Compute the value of aspm_register_info.support directly

[Saheed O. Bolarinwa]

 - Calculate aspm_register_info.support inside aspm_support()
 - Replace references to aspm_register_info.support with aspm_support().
 - In pcie_get_aspm_reg() remove assignment to aspm_register_info.support
 - Remove aspm_register_info.support

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

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 5f7cf47b6a40..321b328347c1 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -383,7 +383,6 @@ static void encode_l12_threshold(u32 threshold_us, u32 *scale, u32 *value)
 }
 
 struct aspm_register_info {
-	u32 support:2;
 	u32 enabled:2;
 
 	/* L1 substates */
@@ -396,12 +395,10 @@ struct aspm_register_info {
 static void pcie_get_aspm_reg(struct pci_dev *pdev,
 			      struct aspm_register_info *info)
 {
-	u16 reg16;
-	u32 reg32 = pdev->lnkcap;
+	u16 ctl;
 
-	info->support = (reg32 & PCI_EXP_LNKCAP_ASPMS) >> 10;
-	pcie_capability_read_word(pdev, PCI_EXP_LNKCTL, &reg16);
-	info->enabled = reg16 & PCI_EXP_LNKCTL_ASPMC;
+	pcie_capability_read_word(pdev, PCI_EXP_LNKCTL, &ctl);
+	info->enabled = ctl & PCI_EXP_LNKCTL_ASPMC;
 
 	/* Read L1 PM substate capabilities */
 	info->l1ss_cap = info->l1ss_ctl1 = info->l1ss_ctl2 = 0;
@@ -540,6 +537,11 @@ static void aspm_calc_l1ss_info(struct pcie_link_state *link,
 	link->l1ss.ctl1 |= t_common_mode << 8 | scale << 29 | value << 16;
 }
 
+static void aspm_support(struct pci_dev *pdev)
+{
+	return (pdev->lnkcap & PCI_EXP_LNKCAP_ASPMS) >> 10;
+}
+
 static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 {
 	struct pci_dev *child = link->downstream, *parent = link->pdev;
@@ -561,7 +563,7 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 	 * If ASPM not supported, don't mess with the clocks and link,
 	 * bail out now.
 	 */
-	if (!(upreg.support & dwreg.support))
+	if (!(aspm_support(parent) & aspm_support(child)))
 		return;
 
 	/* Configure common clock before checking latencies */
@@ -581,8 +583,9 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 	 * given link unless components on both sides of the link each
 	 * support L0s.
 	 */
-	if (dwreg.support & upreg.support & PCIE_LINK_STATE_L0S)
+	if (aspm_support(parent) & aspm_support(child) & PCIE_LINK_STATE_L0S)
 		link->aspm_support |= ASPM_STATE_L0S;
+
 	if (dwreg.enabled & PCIE_LINK_STATE_L0S)
 		link->aspm_enabled |= ASPM_STATE_L0S_UP;
 	if (upreg.enabled & PCIE_LINK_STATE_L0S)
@@ -591,8 +594,9 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 	link->latency_dw.l0s = calc_l0s_latency(child);
 
 	/* Setup L1 state */
-	if (upreg.support & dwreg.support & PCIE_LINK_STATE_L1)
+	if (aspm_support(parent) & aspm_support(child) & PCIE_LINK_STATE_L1)
 		link->aspm_support |= ASPM_STATE_L1;
+
 	if (upreg.enabled & dwreg.enabled & PCIE_LINK_STATE_L1)
 		link->aspm_enabled |= ASPM_STATE_L1;
 	link->latency_up.l1 = calc_l1_latency(parent);
-- 
2.18.4

[PATCH 4/8] PCI/ASPM: Replace aspm_register_info.l1ss_cap* with pci_dev.l1ss_cap*

[Saheed O. Bolarinwa]

 - Add l1ss_cap and l1ss_cap_ptr to struct pci_dev
 - In pci_configure_ltr(), compute the value of pci_dev.l1ss_cap
   and pci_dev.l1ss_cap_ptr
 - Replace all references to aspm_register_info.(l1ss_cap && l1ss_cap_ptr)
   with pci_dev.(l1ss_cap && l1ss_cap_ptr)
 - In pcie_get_aspm_reg() remove reference to aspm_register_info.l1ss_cap*
 - In pcie_get_aspm_reg() remove reading of l1ss_cap_ptr and l1ss_cap
 - Remove aspm_register_info.(l1ss_cap && l1ss_cap_ptr)

Signed-off-by: Saheed O. Bolarinwa <refactormyself@gmail.com>
---
 drivers/pci/pcie/aspm.c | 56 +++++++++++++----------------------------
 drivers/pci/probe.c     |  6 +++++
 include/linux/pci.h     |  2 ++
 3 files changed, 26 insertions(+), 38 deletions(-)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 321b328347c1..d3ad31a230b5 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -386,8 +386,6 @@ struct aspm_register_info {
 	u32 enabled:2;
 
 	/* L1 substates */
-	u32 l1ss_cap_ptr;
-	u32 l1ss_cap;
 	u32 l1ss_ctl1;
 	u32 l1ss_ctl2;
 };
@@ -400,26 +398,6 @@ static void pcie_get_aspm_reg(struct pci_dev *pdev,
 	pcie_capability_read_word(pdev, PCI_EXP_LNKCTL, &ctl);
 	info->enabled = ctl & PCI_EXP_LNKCTL_ASPMC;
 
-	/* Read L1 PM substate capabilities */
-	info->l1ss_cap = info->l1ss_ctl1 = info->l1ss_ctl2 = 0;
-	info->l1ss_cap_ptr = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_L1SS);
-	if (!info->l1ss_cap_ptr)
-		return;
-	pci_read_config_dword(pdev, info->l1ss_cap_ptr + PCI_L1SS_CAP,
-			      &info->l1ss_cap);
-	if (!(info->l1ss_cap & PCI_L1SS_CAP_L1_PM_SS)) {
-		info->l1ss_cap = 0;
-		return;
-	}
-
-	/*
-	 * 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 (!pdev->ltr_path)
-		info->l1ss_cap &= ~PCI_L1SS_CAP_ASPM_L1_2;
-
 	pci_read_config_dword(pdev, info->l1ss_cap_ptr + PCI_L1SS_CTL1,
 			      &info->l1ss_ctl1);
 	pci_read_config_dword(pdev, info->l1ss_cap_ptr + PCI_L1SS_CTL2,
@@ -494,32 +472,34 @@ static void aspm_calc_l1ss_info(struct pcie_link_state *link,
 {
 	u32 val1, val2, scale1, scale2;
 	u32 t_common_mode, t_power_on, l1_2_threshold, scale, value;
+	struct pci_dev *dw_pdev = link->downstream;
+	struct pci_dev *up_pdev = link->pdev;
 
-	link->l1ss.up_cap_ptr = upreg->l1ss_cap_ptr;
-	link->l1ss.dw_cap_ptr = dwreg->l1ss_cap_ptr;
+	link->l1ss.up_cap_ptr = up_pdev->l1ss_cap_ptr;
+	link->l1ss.dw_cap_ptr = dw_pdev->l1ss_cap_ptr;
 	link->l1ss.ctl1 = link->l1ss.ctl2 = 0;
 
 	if (!(link->aspm_support & ASPM_STATE_L1_2_MASK))
 		return;
 
 	/* Choose the greater of the two Port Common_Mode_Restore_Times */
-	val1 = (upreg->l1ss_cap & PCI_L1SS_CAP_CM_RESTORE_TIME) >> 8;
-	val2 = (dwreg->l1ss_cap & PCI_L1SS_CAP_CM_RESTORE_TIME) >> 8;
+	val1 = (up_pdev->l1ss_cap & PCI_L1SS_CAP_CM_RESTORE_TIME) >> 8;
+	val2 = (dw_pdev->l1ss_cap & PCI_L1SS_CAP_CM_RESTORE_TIME) >> 8;
 	t_common_mode = max(val1, val2);
 
 	/* Choose the greater of the two Port T_POWER_ON times */
-	val1   = (upreg->l1ss_cap & PCI_L1SS_CAP_P_PWR_ON_VALUE) >> 19;
-	scale1 = (upreg->l1ss_cap & PCI_L1SS_CAP_P_PWR_ON_SCALE) >> 16;
-	val2   = (dwreg->l1ss_cap & PCI_L1SS_CAP_P_PWR_ON_VALUE) >> 19;
-	scale2 = (dwreg->l1ss_cap & PCI_L1SS_CAP_P_PWR_ON_SCALE) >> 16;
+	val1   = (up_pdev->l1ss_cap & PCI_L1SS_CAP_P_PWR_ON_VALUE) >> 19;
+	scale1 = (up_pdev->l1ss_cap & PCI_L1SS_CAP_P_PWR_ON_SCALE) >> 16;
+	val2   = (dw_pdev->l1ss_cap & PCI_L1SS_CAP_P_PWR_ON_VALUE) >> 19;
+	scale2 = (dw_pdev->l1ss_cap & PCI_L1SS_CAP_P_PWR_ON_SCALE) >> 16;
 
-	if (calc_l1ss_pwron(link->pdev, scale1, val1) >
-	    calc_l1ss_pwron(link->downstream, scale2, val2)) {
+	if (calc_l1ss_pwron(up_pdev, scale1, val1) >
+	    calc_l1ss_pwron(dw_pdev, scale2, val2)) {
 		link->l1ss.ctl2 |= scale1 | (val1 << 3);
-		t_power_on = calc_l1ss_pwron(link->pdev, scale1, val1);
+		t_power_on = calc_l1ss_pwron(up_pdev, scale1, val1);
 	} else {
 		link->l1ss.ctl2 |= scale2 | (val2 << 3);
-		t_power_on = calc_l1ss_pwron(link->downstream, scale2, val2);
+		t_power_on = calc_l1ss_pwron(dw_pdev, scale2, val2);
 	}
 
 	/*
@@ -603,13 +583,13 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 	link->latency_dw.l1 = calc_l1_latency(child);
 
 	/* Setup L1 substate */
-	if (upreg.l1ss_cap & dwreg.l1ss_cap & PCI_L1SS_CAP_ASPM_L1_1)
+	if (parent->l1ss_cap & child->l1ss_cap & PCI_L1SS_CAP_ASPM_L1_1)
 		link->aspm_support |= ASPM_STATE_L1_1;
-	if (upreg.l1ss_cap & dwreg.l1ss_cap & PCI_L1SS_CAP_ASPM_L1_2)
+	if (parent->l1ss_cap & child->l1ss_cap & PCI_L1SS_CAP_ASPM_L1_2)
 		link->aspm_support |= ASPM_STATE_L1_2;
-	if (upreg.l1ss_cap & dwreg.l1ss_cap & PCI_L1SS_CAP_PCIPM_L1_1)
+	if (parent->l1ss_cap & child->l1ss_cap & PCI_L1SS_CAP_PCIPM_L1_1)
 		link->aspm_support |= ASPM_STATE_L1_1_PCIPM;
-	if (upreg.l1ss_cap & dwreg.l1ss_cap & PCI_L1SS_CAP_PCIPM_L1_2)
+	if (parent->l1ss_cap & child->l1ss_cap & PCI_L1SS_CAP_PCIPM_L1_2)
 		link->aspm_support |= ASPM_STATE_L1_2_PCIPM;
 
 	if (upreg.l1ss_ctl1 & dwreg.l1ss_ctl1 & PCI_L1SS_CTL1_ASPM_L1_1)
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 2d5898f05f89..71a714065e14 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2107,6 +2107,12 @@ static void pci_configure_ltr(struct pci_dev *dev)
 	if (!pci_is_pcie(dev))
 		return;
 
+	/* Read L1 PM substate capabilities */
+	dev->l1ss_cap_ptr = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_L1SS);
+	if (dev->l1ss_cap_ptr)
+		pci_read_config_dword(dev, dev->l1ss_cap_ptr + PCI_L1SS_CAP,
+								&dev->l1ss_cap);
+
 	pcie_capability_read_dword(dev, PCI_EXP_DEVCAP2, &cap);
 	if (!(cap & PCI_EXP_DEVCAP2_LTR))
 		return;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 5b305cfeb1dc..60b82e255738 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -381,6 +381,8 @@ struct pci_dev {
 	struct pcie_link_state	*link_state;	/* ASPM link state */
 	unsigned int	ltr_path:1;	/* Latency Tolerance Reporting
 					   supported from root to here */
+	int		l1ss_cap_ptr;	/* L1SS cap ptr, 0 if not supported */
+	u32		l1ss_cap;	/* L1 PM substate Capabilities */
 #endif
 	unsigned int	eetlp_prefix_path:1;	/* End-to-End TLP Prefix */
 
-- 
2.18.4

[PATCH 5/8] PCI/ASPM: Remove aspm_register_info.l1ss_ctl*

[Saheed O. Bolarinwa]

 - Read the value of PCI_L1SS_CTL1 directly and cache in local variables.
 - Replace references to aspm_register_info.l1ss_ctl1 with the variables.
 - In pcie_get_aspm_reg() remove reference to aspm_register_info.l1ss_ctl*
 - In pcie_get_aspm_reg() remove reading PCI_L1SS_CTL1 and PCI_L1SS_CTL2
 - Remove aspm_register_info.(l1ss_ctl1 && l1ss_ctl2)

Note that aspm_register_info.l1ss_ctl2 is eliminated totally since it is
not used.

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

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index d3ad31a230b5..f89d3b2be1c7 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -384,10 +384,6 @@ static void encode_l12_threshold(u32 threshold_us, u32 *scale, u32 *value)
 
 struct aspm_register_info {
 	u32 enabled:2;
-
-	/* L1 substates */
-	u32 l1ss_ctl1;
-	u32 l1ss_ctl2;
 };
 
 static void pcie_get_aspm_reg(struct pci_dev *pdev,
@@ -397,11 +393,6 @@ static void pcie_get_aspm_reg(struct pci_dev *pdev,
 
 	pcie_capability_read_word(pdev, PCI_EXP_LNKCTL, &ctl);
 	info->enabled = ctl & PCI_EXP_LNKCTL_ASPMC;
-
-	pci_read_config_dword(pdev, info->l1ss_cap_ptr + PCI_L1SS_CTL1,
-			      &info->l1ss_ctl1);
-	pci_read_config_dword(pdev, info->l1ss_cap_ptr + PCI_L1SS_CTL2,
-			      &info->l1ss_ctl2);
 }
 
 static void pcie_aspm_check_latency(struct pci_dev *endpoint)
@@ -527,6 +518,7 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 	struct pci_dev *child = link->downstream, *parent = link->pdev;
 	struct pci_bus *linkbus = parent->subordinate;
 	struct aspm_register_info upreg, dwreg;
+	u32 up_l1ss_ctl1, dw_l1ss_ctl1;
 
 	if (blacklist) {
 		/* Set enabled/disable so that we will disable ASPM later */
@@ -549,6 +541,11 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 	/* Configure common clock before checking latencies */
 	pcie_aspm_configure_common_clock(link);
 
+	pci_read_config_dword(parent, parent->l1ss_cap_ptr + PCI_L1SS_CTL1,
+				&up_l1ss_ctl1);
+	pci_read_config_dword(child, child->l1ss_cap_ptr + PCI_L1SS_CTL1,
+				&dw_l1ss_ctl1);
+
 	/*
 	 * Re-read upstream/downstream components' register state
 	 * after clock configuration
@@ -592,13 +589,13 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 	if (parent->l1ss_cap & child->l1ss_cap & PCI_L1SS_CAP_PCIPM_L1_2)
 		link->aspm_support |= ASPM_STATE_L1_2_PCIPM;
 
-	if (upreg.l1ss_ctl1 & dwreg.l1ss_ctl1 & PCI_L1SS_CTL1_ASPM_L1_1)
+	if (up_l1ss_ctl1 & dw_l1ss_ctl1 & PCI_L1SS_CTL1_ASPM_L1_1)
 		link->aspm_enabled |= ASPM_STATE_L1_1;
-	if (upreg.l1ss_ctl1 & dwreg.l1ss_ctl1 & PCI_L1SS_CTL1_ASPM_L1_2)
+	if (up_l1ss_ctl1 & dw_l1ss_ctl1 & PCI_L1SS_CTL1_ASPM_L1_2)
 		link->aspm_enabled |= ASPM_STATE_L1_2;
-	if (upreg.l1ss_ctl1 & dwreg.l1ss_ctl1 & PCI_L1SS_CTL1_PCIPM_L1_1)
+	if (up_l1ss_ctl1 & dw_l1ss_ctl1 & PCI_L1SS_CTL1_PCIPM_L1_1)
 		link->aspm_enabled |= ASPM_STATE_L1_1_PCIPM;
-	if (upreg.l1ss_ctl1 & dwreg.l1ss_ctl1 & PCI_L1SS_CTL1_PCIPM_L1_2)
+	if (up_l1ss_ctl1 & dw_l1ss_ctl1 & PCI_L1SS_CTL1_PCIPM_L1_2)
 		link->aspm_enabled |= ASPM_STATE_L1_2_PCIPM;
 
 	if (link->aspm_support & ASPM_STATE_L1SS)
-- 
2.18.4

[PATCH 6/8] PCI/ASPM: Remove aspm_register_info.enable

[Saheed O. Bolarinwa]

 - Create get_aspm_enable() to compute aspm_register_info.enable directly
 - Replace all aspm_register_info.enable references with get_aspm_enable()
 - In pcie_get_aspm_reg() remove reference to aspm_register_info.l1ss_cap*
 - Remove aspm_register_info.enabled
 - In pcie_aspm_cap_init() remove all calls to pcie_get_aspm_reg(), since
   it now does nothing. All the values are calculated elsewhere.

Signed-off-by: Saheed O. Bolarinwa <refactormyself@gmail.com>
---
NOTE: To avoid messing up this patch, the struct aspm_register_info is 
removed in the next patch. I am not sure if it is better to merge them.

 drivers/pci/pcie/aspm.c | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index f89d3b2be1c7..e43fdf0cd08c 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -383,7 +383,6 @@ static void encode_l12_threshold(u32 threshold_us, u32 *scale, u32 *value)
 }
 
 struct aspm_register_info {
-	u32 enabled:2;
 };
 
 static void pcie_get_aspm_reg(struct pci_dev *pdev,
@@ -392,7 +391,6 @@ static void pcie_get_aspm_reg(struct pci_dev *pdev,
 	u16 ctl;
 
 	pcie_capability_read_word(pdev, PCI_EXP_LNKCTL, &ctl);
-	info->enabled = ctl & PCI_EXP_LNKCTL_ASPMC;
 }
 
 static void pcie_aspm_check_latency(struct pci_dev *endpoint)
@@ -513,6 +511,14 @@ static void aspm_support(struct pci_dev *pdev)
 	return (pdev->lnkcap & PCI_EXP_LNKCAP_ASPMS) >> 10;
 }
 
+static u32 get_aspm_enable(struct pci_dev *pdev)
+{
+	u16 ctl;
+
+	pcie_capability_read_word(pdev, PCI_EXP_LNKCTL, &ctl);
+	return (ctl & PCI_EXP_LNKCTL_ASPMC);
+}
+
 static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 {
 	struct pci_dev *child = link->downstream, *parent = link->pdev;
@@ -527,10 +533,6 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 		return;
 	}
 
-	/* Get upstream/downstream components' register state */
-	pcie_get_aspm_reg(parent, &upreg);
-	pcie_get_aspm_reg(child, &dwreg);
-
 	/*
 	 * If ASPM not supported, don't mess with the clocks and link,
 	 * bail out now.
@@ -546,13 +548,6 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 	pci_read_config_dword(child, child->l1ss_cap_ptr + PCI_L1SS_CTL1,
 				&dw_l1ss_ctl1);
 
-	/*
-	 * Re-read upstream/downstream components' register state
-	 * after clock configuration
-	 */
-	pcie_get_aspm_reg(parent, &upreg);
-	pcie_get_aspm_reg(child, &dwreg);
-
 	/*
 	 * Setup L0s state
 	 *
@@ -563,9 +558,9 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 	if (aspm_support(parent) & aspm_support(child) & PCIE_LINK_STATE_L0S)
 		link->aspm_support |= ASPM_STATE_L0S;
 
-	if (dwreg.enabled & PCIE_LINK_STATE_L0S)
+	if (get_aspm_enable(child) & PCIE_LINK_STATE_L0S)
 		link->aspm_enabled |= ASPM_STATE_L0S_UP;
-	if (upreg.enabled & PCIE_LINK_STATE_L0S)
+	if (get_aspm_enable(parent) & PCIE_LINK_STATE_L0S)
 		link->aspm_enabled |= ASPM_STATE_L0S_DW;
 	link->latency_up.l0s = calc_l0s_latency(parent);
 	link->latency_dw.l0s = calc_l0s_latency(child);
@@ -574,7 +569,8 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 	if (aspm_support(parent) & aspm_support(child) & PCIE_LINK_STATE_L1)
 		link->aspm_support |= ASPM_STATE_L1;
 
-	if (upreg.enabled & dwreg.enabled & PCIE_LINK_STATE_L1)
+	if (get_aspm_enable(parent) & get_aspm_enable(child)
+				    & PCIE_LINK_STATE_L1)
 		link->aspm_enabled |= ASPM_STATE_L1;
 	link->latency_up.l1 = calc_l1_latency(parent);
 	link->latency_dw.l1 = calc_l1_latency(child);
-- 
2.18.4

[PATCH 7/8] PCI/ASPM: Remove pcie_get_aspm_reg() and struct aspm_register_info

[Saheed O. Bolarinwa]

Two of aspm_calc_l1ss_info()'s arguments are now redundant. All members
of struct aspm_register_info are now calculated directly, so both the
struct and pcie_get_aspm_reg() are now useless.

 - Remove redundant arguments in aspm_calc_l1ss_info().
 - Refactor callers to also remove redundant parameters
 - Remove any remaining call to pcie_get_aspm_reg()
 - Remove pcie_get_aspm_reg()
 - Remove remaining reference to struct aspm_register_info.
 - Remove struct aspm_register_info

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

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index e43fdf0cd08c..f4fc2d65240c 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -382,17 +382,6 @@ static void encode_l12_threshold(u32 threshold_us, u32 *scale, u32 *value)
 	}
 }
 
-struct aspm_register_info {
-};
-
-static void pcie_get_aspm_reg(struct pci_dev *pdev,
-			      struct aspm_register_info *info)
-{
-	u16 ctl;
-
-	pcie_capability_read_word(pdev, PCI_EXP_LNKCTL, &ctl);
-}
-
 static void pcie_aspm_check_latency(struct pci_dev *endpoint)
 {
 	u32 latency, l1_switch_latency = 0;
@@ -455,9 +444,7 @@ static struct pci_dev *pci_function_0(struct pci_bus *linkbus)
 }
 
 /* Calculate L1.2 PM substate timing parameters */
-static void aspm_calc_l1ss_info(struct pcie_link_state *link,
-				struct aspm_register_info *upreg,
-				struct aspm_register_info *dwreg)
+static void aspm_calc_l1ss_info(struct pcie_link_state *link)
 {
 	u32 val1, val2, scale1, scale2;
 	u32 t_common_mode, t_power_on, l1_2_threshold, scale, value;
@@ -523,7 +510,6 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 {
 	struct pci_dev *child = link->downstream, *parent = link->pdev;
 	struct pci_bus *linkbus = parent->subordinate;
-	struct aspm_register_info upreg, dwreg;
 	u32 up_l1ss_ctl1, dw_l1ss_ctl1;
 
 	if (blacklist) {
@@ -595,7 +581,7 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 		link->aspm_enabled |= ASPM_STATE_L1_2_PCIPM;
 
 	if (link->aspm_support & ASPM_STATE_L1SS)
-		aspm_calc_l1ss_info(link, &upreg, &dwreg);
+		aspm_calc_l1ss_info(link);
 
 	/* Save default state */
 	link->aspm_default = link->aspm_enabled;
-- 
2.18.4

[PATCH 8/8] PCI/ASPM: Remove struct pcie_link_state.l1ss

[Saheed O. Bolarinwa]

pcie_link_state.l1ss.{up_cap_ptr, dw_cap_ptr} are used to cache the value
of l1ss_cap_ptr for upstream and downstream respectively. This value can
now be obtained directly from struct pci_dev, it is no longer useful to
cache it. So, aspm_calc_l1ss_info() will only be computing the values for
ctl1 and ctl2. The addresses of these can also be passed in.

Then if aspm_calc_l1ss_info() calculates pcie_link_state.l1ss.{ctl1, ctl2}
which are only used inside pcie_config_aspm_l1ss(). Calling the function
where it is needed will remove the need to cache the values in the struct.

 - Move call to aspm_calc_l1ss_info() from pcie_aspm_cap_init() to
   pcie_config_aspm_l1ss().
 - Rename aspm_calc_l1ss_info() to aspm_calc_l1ss_ctl_values().
 - Rework the function to take a pci_dev and pointers to ctl1 and ctl2.
 - Change calls to aspm_calc_l1ss_info() into new function.
 - Replace l1ss.{up,dw}_cap_ptr with pci_dev->l1ss_cap_ptr
 - Replace pcie_link_state.l1ss.{ctl1, ctl2} with local variables.
 - No more reference to struct pcie_link_state.l1ss, so remove it.
 - Remove pcie_link_state.l1ss

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

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index f4fc2d65240c..b9bacdef8c80 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -74,14 +74,6 @@ struct pcie_link_state {
 	 * has one slot under it, so at most there are 8 functions.
 	 */
 	struct aspm_latency acceptable[8];
-
-	/* L1 PM Substate info */
-	struct {
-		u32 up_cap_ptr;		/* L1SS cap ptr in upstream dev */
-		u32 dw_cap_ptr;		/* L1SS cap ptr in downstream dev */
-		u32 ctl1;		/* value to be programmed in ctl1 */
-		u32 ctl2;		/* value to be programmed in ctl2 */
-	} l1ss;
 };
 
 static int aspm_disabled, aspm_force;
@@ -444,17 +436,15 @@ static struct pci_dev *pci_function_0(struct pci_bus *linkbus)
 }
 
 /* Calculate L1.2 PM substate timing parameters */
-static void aspm_calc_l1ss_info(struct pcie_link_state *link)
+static void aspm_calc_l1ss_ctl_values(struct pci_dev *pdev,
+					u32 *ctl1, u32 *ctl2)
 {
+	struct pcie_link_state *link = pdev->link_state;
 	u32 val1, val2, scale1, scale2;
 	u32 t_common_mode, t_power_on, l1_2_threshold, scale, value;
 	struct pci_dev *dw_pdev = link->downstream;
 	struct pci_dev *up_pdev = link->pdev;
 
-	link->l1ss.up_cap_ptr = up_pdev->l1ss_cap_ptr;
-	link->l1ss.dw_cap_ptr = dw_pdev->l1ss_cap_ptr;
-	link->l1ss.ctl1 = link->l1ss.ctl2 = 0;
-
 	if (!(link->aspm_support & ASPM_STATE_L1_2_MASK))
 		return;
 
@@ -471,10 +461,10 @@ static void aspm_calc_l1ss_info(struct pcie_link_state *link)
 
 	if (calc_l1ss_pwron(up_pdev, scale1, val1) >
 	    calc_l1ss_pwron(dw_pdev, scale2, val2)) {
-		link->l1ss.ctl2 |= scale1 | (val1 << 3);
+		*ctl2 |= scale1 | (val1 << 3);
 		t_power_on = calc_l1ss_pwron(up_pdev, scale1, val1);
 	} else {
-		link->l1ss.ctl2 |= scale2 | (val2 << 3);
+		*ctl2 |= scale2 | (val2 << 3);
 		t_power_on = calc_l1ss_pwron(dw_pdev, scale2, val2);
 	}
 
@@ -490,7 +480,7 @@ static void aspm_calc_l1ss_info(struct pcie_link_state *link)
 	 */
 	l1_2_threshold = 2 + 4 + t_common_mode + t_power_on;
 	encode_l12_threshold(l1_2_threshold, &scale, &value);
-	link->l1ss.ctl1 |= t_common_mode << 8 | scale << 29 | value << 16;
+	*ctl1 |= t_common_mode << 8 | scale << 29 | value << 16;
 }
 
 static void aspm_support(struct pci_dev *pdev)
@@ -580,9 +570,6 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 	if (up_l1ss_ctl1 & dw_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);
-
 	/* Save default state */
 	link->aspm_default = link->aspm_enabled;
 
@@ -625,12 +612,13 @@ static void pci_clear_and_set_dword(struct pci_dev *pdev, int pos,
 /* Configure the ASPM L1 substates */
 static void pcie_config_aspm_l1ss(struct pcie_link_state *link, u32 state)
 {
-	u32 val, enable_req;
+	u32 val, enable_req, ctl1, ctl2;
 	struct pci_dev *child = link->downstream, *parent = link->pdev;
-	u32 up_cap_ptr = link->l1ss.up_cap_ptr;
-	u32 dw_cap_ptr = link->l1ss.dw_cap_ptr;
+	int up_cap_ptr = parent->l1ss_cap_ptr;
+	int dw_cap_ptr = child->l1ss_cap_ptr;
 
 	enable_req = (link->aspm_enabled ^ state) & state;
+	aspm_calc_l1ss_ctl_values(parent, &ctl1, &ctl2);
 
 	/*
 	 * Here are the rules specified in the PCIe spec for enabling L1SS:
@@ -665,24 +653,21 @@ static void pcie_config_aspm_l1ss(struct pcie_link_state *link, u32 state)
 
 		/* Program T_POWER_ON times in both ports */
 		pci_write_config_dword(parent, up_cap_ptr + PCI_L1SS_CTL2,
-				       link->l1ss.ctl2);
+								ctl2);
 		pci_write_config_dword(child, dw_cap_ptr + PCI_L1SS_CTL2,
-				       link->l1ss.ctl2);
+								ctl2);
 
 		/* Program Common_Mode_Restore_Time in upstream device */
 		pci_clear_and_set_dword(parent, up_cap_ptr + PCI_L1SS_CTL1,
-					PCI_L1SS_CTL1_CM_RESTORE_TIME,
-					link->l1ss.ctl1);
+					PCI_L1SS_CTL1_CM_RESTORE_TIME, ctl1);
 
 		/* Program LTR_L1.2_THRESHOLD time in both ports */
 		pci_clear_and_set_dword(parent,	up_cap_ptr + PCI_L1SS_CTL1,
 					PCI_L1SS_CTL1_LTR_L12_TH_VALUE |
-					PCI_L1SS_CTL1_LTR_L12_TH_SCALE,
-					link->l1ss.ctl1);
+					PCI_L1SS_CTL1_LTR_L12_TH_SCALE, ctl1);
 		pci_clear_and_set_dword(child, dw_cap_ptr + PCI_L1SS_CTL1,
 					PCI_L1SS_CTL1_LTR_L12_TH_VALUE |
-					PCI_L1SS_CTL1_LTR_L12_TH_SCALE,
-					link->l1ss.ctl1);
+					PCI_L1SS_CTL1_LTR_L12_TH_SCALE, ctl1);
 	}
 
 	val = 0;
-- 
2.18.4

Re: [PATCH 3/8] PCI/ASPM: Compute the value of aspm_register_info.support directly

[kernel test robot]

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

Hi "Saheed,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on pci/next]
[also build test ERROR on v5.9-rc6 next-20200924]
[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/PCI-Move-some-ASPM-info-to-struct-pci_dev/20200924-081626
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: x86_64-randconfig-r024-20200924 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project d6ac649ccda289ecc2d2c0cb51892d57e8ec328c)
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 x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

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

All errors (new ones prefixed by >>):

>> drivers/pci/pcie/aspm.c:542:2: error: void function 'aspm_support' should not return a value [-Wreturn-type]
           return (pdev->lnkcap & PCI_EXP_LNKCAP_ASPMS) >> 10;
           ^      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/pci/pcie/aspm.c:566:29: error: invalid operands to binary expression ('void' and 'void')
           if (!(aspm_support(parent) & aspm_support(child)))
                 ~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~
   drivers/pci/pcie/aspm.c:586:27: error: invalid operands to binary expression ('void' and 'void')
           if (aspm_support(parent) & aspm_support(child) & PCIE_LINK_STATE_L0S)
               ~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~
   drivers/pci/pcie/aspm.c:597:27: error: invalid operands to binary expression ('void' and 'void')
           if (aspm_support(parent) & aspm_support(child) & PCIE_LINK_STATE_L1)
               ~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~
   4 errors generated.

# https://github.com/0day-ci/linux/commit/835c34b3f165061415e22e546de958901adca123
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Saheed-O-Bolarinwa/PCI-Move-some-ASPM-info-to-struct-pci_dev/20200924-081626
git checkout 835c34b3f165061415e22e546de958901adca123
vim +/aspm_support +542 drivers/pci/pcie/aspm.c

   539	
   540	static void aspm_support(struct pci_dev *pdev)
   541	{
 > 542		return (pdev->lnkcap & PCI_EXP_LNKCAP_ASPMS) >> 10;
   543	}
   544	
   545	static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
   546	{
   547		struct pci_dev *child = link->downstream, *parent = link->pdev;
   548		struct pci_bus *linkbus = parent->subordinate;
   549		struct aspm_register_info upreg, dwreg;
   550	
   551		if (blacklist) {
   552			/* Set enabled/disable so that we will disable ASPM later */
   553			link->aspm_enabled = ASPM_STATE_ALL;
   554			link->aspm_disable = ASPM_STATE_ALL;
   555			return;
   556		}
   557	
   558		/* Get upstream/downstream components' register state */
   559		pcie_get_aspm_reg(parent, &upreg);
   560		pcie_get_aspm_reg(child, &dwreg);
   561	
   562		/*
   563		 * If ASPM not supported, don't mess with the clocks and link,
   564		 * bail out now.
   565		 */
 > 566		if (!(aspm_support(parent) & aspm_support(child)))
   567			return;
   568	
   569		/* Configure common clock before checking latencies */
   570		pcie_aspm_configure_common_clock(link);
   571	
   572		/*
   573		 * Re-read upstream/downstream components' register state
   574		 * after clock configuration
   575		 */
   576		pcie_get_aspm_reg(parent, &upreg);
   577		pcie_get_aspm_reg(child, &dwreg);
   578	
   579		/*
   580		 * Setup L0s state
   581		 *
   582		 * Note that we must not enable L0s in either direction on a
   583		 * given link unless components on both sides of the link each
   584		 * support L0s.
   585		 */
   586		if (aspm_support(parent) & aspm_support(child) & PCIE_LINK_STATE_L0S)
   587			link->aspm_support |= ASPM_STATE_L0S;
   588	
   589		if (dwreg.enabled & PCIE_LINK_STATE_L0S)
   590			link->aspm_enabled |= ASPM_STATE_L0S_UP;
   591		if (upreg.enabled & PCIE_LINK_STATE_L0S)
   592			link->aspm_enabled |= ASPM_STATE_L0S_DW;
   593		link->latency_up.l0s = calc_l0s_latency(parent);
   594		link->latency_dw.l0s = calc_l0s_latency(child);
   595	
   596		/* Setup L1 state */
   597		if (aspm_support(parent) & aspm_support(child) & PCIE_LINK_STATE_L1)
   598			link->aspm_support |= ASPM_STATE_L1;
   599	
   600		if (upreg.enabled & dwreg.enabled & PCIE_LINK_STATE_L1)
   601			link->aspm_enabled |= ASPM_STATE_L1;
   602		link->latency_up.l1 = calc_l1_latency(parent);
   603		link->latency_dw.l1 = calc_l1_latency(child);
   604	
   605		/* Setup L1 substate */
   606		if (upreg.l1ss_cap & dwreg.l1ss_cap & PCI_L1SS_CAP_ASPM_L1_1)
   607			link->aspm_support |= ASPM_STATE_L1_1;
   608		if (upreg.l1ss_cap & dwreg.l1ss_cap & PCI_L1SS_CAP_ASPM_L1_2)
   609			link->aspm_support |= ASPM_STATE_L1_2;
   610		if (upreg.l1ss_cap & dwreg.l1ss_cap & PCI_L1SS_CAP_PCIPM_L1_1)
   611			link->aspm_support |= ASPM_STATE_L1_1_PCIPM;
   612		if (upreg.l1ss_cap & dwreg.l1ss_cap & PCI_L1SS_CAP_PCIPM_L1_2)
   613			link->aspm_support |= ASPM_STATE_L1_2_PCIPM;
   614	
   615		if (upreg.l1ss_ctl1 & dwreg.l1ss_ctl1 & PCI_L1SS_CTL1_ASPM_L1_1)
   616			link->aspm_enabled |= ASPM_STATE_L1_1;
   617		if (upreg.l1ss_ctl1 & dwreg.l1ss_ctl1 & PCI_L1SS_CTL1_ASPM_L1_2)
   618			link->aspm_enabled |= ASPM_STATE_L1_2;
   619		if (upreg.l1ss_ctl1 & dwreg.l1ss_ctl1 & PCI_L1SS_CTL1_PCIPM_L1_1)
   620			link->aspm_enabled |= ASPM_STATE_L1_1_PCIPM;
   621		if (upreg.l1ss_ctl1 & dwreg.l1ss_ctl1 & PCI_L1SS_CTL1_PCIPM_L1_2)
   622			link->aspm_enabled |= ASPM_STATE_L1_2_PCIPM;
   623	
   624		if (link->aspm_support & ASPM_STATE_L1SS)
   625			aspm_calc_l1ss_info(link, &upreg, &dwreg);
   626	
   627		/* Save default state */
   628		link->aspm_default = link->aspm_enabled;
   629	
   630		/* Setup initial capable state. Will be updated later */
   631		link->aspm_capable = link->aspm_support;
   632	
   633		/* Get and check endpoint acceptable latencies */
   634		list_for_each_entry(child, &linkbus->devices, bus_list) {
   635			u32 reg32, encoding;
   636			struct aspm_latency *acceptable =
   637				&link->acceptable[PCI_FUNC(child->devfn)];
   638	
   639			if (pci_pcie_type(child) != PCI_EXP_TYPE_ENDPOINT &&
   640			    pci_pcie_type(child) != PCI_EXP_TYPE_LEG_END)
   641				continue;
   642	
   643			pcie_capability_read_dword(child, PCI_EXP_DEVCAP, &reg32);
   644			/* Calculate endpoint L0s acceptable latency */
   645			encoding = (reg32 & PCI_EXP_DEVCAP_L0S) >> 6;
   646			acceptable->l0s = calc_l0s_acceptable(encoding);
   647			/* Calculate endpoint L1 acceptable latency */
   648			encoding = (reg32 & PCI_EXP_DEVCAP_L1) >> 9;
   649			acceptable->l1 = calc_l1_acceptable(encoding);
   650	
   651			pcie_aspm_check_latency(child);
   652		}
   653	}
   654	

---
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: 35415 bytes --]

Re: [PATCH 3/8] PCI/ASPM: Compute the value of aspm_register_info.support directly

[kernel test robot]

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

Hi "Saheed,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on pci/next]
[also build test ERROR on v5.9-rc6 next-20200924]
[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/PCI-Move-some-ASPM-info-to-struct-pci_dev/20200924-081626
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: x86_64-randconfig-r024-20200924 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project d6ac649ccda289ecc2d2c0cb51892d57e8ec328c)
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 x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

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

All errors (new ones prefixed by >>):

>> drivers/pci/pcie/aspm.c:542:2: error: void function 'aspm_support' should not return a value [-Wreturn-type]
           return (pdev->lnkcap & PCI_EXP_LNKCAP_ASPMS) >> 10;
           ^      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/pci/pcie/aspm.c:566:29: error: invalid operands to binary expression ('void' and 'void')
           if (!(aspm_support(parent) & aspm_support(child)))
                 ~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~
   drivers/pci/pcie/aspm.c:586:27: error: invalid operands to binary expression ('void' and 'void')
           if (aspm_support(parent) & aspm_support(child) & PCIE_LINK_STATE_L0S)
               ~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~
   drivers/pci/pcie/aspm.c:597:27: error: invalid operands to binary expression ('void' and 'void')
           if (aspm_support(parent) & aspm_support(child) & PCIE_LINK_STATE_L1)
               ~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~
   4 errors generated.

# https://github.com/0day-ci/linux/commit/835c34b3f165061415e22e546de958901adca123
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Saheed-O-Bolarinwa/PCI-Move-some-ASPM-info-to-struct-pci_dev/20200924-081626
git checkout 835c34b3f165061415e22e546de958901adca123
vim +/aspm_support +542 drivers/pci/pcie/aspm.c

   539	
   540	static void aspm_support(struct pci_dev *pdev)
   541	{
 > 542		return (pdev->lnkcap & PCI_EXP_LNKCAP_ASPMS) >> 10;
   543	}
   544	
   545	static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
   546	{
   547		struct pci_dev *child = link->downstream, *parent = link->pdev;
   548		struct pci_bus *linkbus = parent->subordinate;
   549		struct aspm_register_info upreg, dwreg;
   550	
   551		if (blacklist) {
   552			/* Set enabled/disable so that we will disable ASPM later */
   553			link->aspm_enabled = ASPM_STATE_ALL;
   554			link->aspm_disable = ASPM_STATE_ALL;
   555			return;
   556		}
   557	
   558		/* Get upstream/downstream components' register state */
   559		pcie_get_aspm_reg(parent, &upreg);
   560		pcie_get_aspm_reg(child, &dwreg);
   561	
   562		/*
   563		 * If ASPM not supported, don't mess with the clocks and link,
   564		 * bail out now.
   565		 */
 > 566		if (!(aspm_support(parent) & aspm_support(child)))
   567			return;
   568	
   569		/* Configure common clock before checking latencies */
   570		pcie_aspm_configure_common_clock(link);
   571	
   572		/*
   573		 * Re-read upstream/downstream components' register state
   574		 * after clock configuration
   575		 */
   576		pcie_get_aspm_reg(parent, &upreg);
   577		pcie_get_aspm_reg(child, &dwreg);
   578	
   579		/*
   580		 * Setup L0s state
   581		 *
   582		 * Note that we must not enable L0s in either direction on a
   583		 * given link unless components on both sides of the link each
   584		 * support L0s.
   585		 */
   586		if (aspm_support(parent) & aspm_support(child) & PCIE_LINK_STATE_L0S)
   587			link->aspm_support |= ASPM_STATE_L0S;
   588	
   589		if (dwreg.enabled & PCIE_LINK_STATE_L0S)
   590			link->aspm_enabled |= ASPM_STATE_L0S_UP;
   591		if (upreg.enabled & PCIE_LINK_STATE_L0S)
   592			link->aspm_enabled |= ASPM_STATE_L0S_DW;
   593		link->latency_up.l0s = calc_l0s_latency(parent);
   594		link->latency_dw.l0s = calc_l0s_latency(child);
   595	
   596		/* Setup L1 state */
   597		if (aspm_support(parent) & aspm_support(child) & PCIE_LINK_STATE_L1)
   598			link->aspm_support |= ASPM_STATE_L1;
   599	
   600		if (upreg.enabled & dwreg.enabled & PCIE_LINK_STATE_L1)
   601			link->aspm_enabled |= ASPM_STATE_L1;
   602		link->latency_up.l1 = calc_l1_latency(parent);
   603		link->latency_dw.l1 = calc_l1_latency(child);
   604	
   605		/* Setup L1 substate */
   606		if (upreg.l1ss_cap & dwreg.l1ss_cap & PCI_L1SS_CAP_ASPM_L1_1)
   607			link->aspm_support |= ASPM_STATE_L1_1;
   608		if (upreg.l1ss_cap & dwreg.l1ss_cap & PCI_L1SS_CAP_ASPM_L1_2)
   609			link->aspm_support |= ASPM_STATE_L1_2;
   610		if (upreg.l1ss_cap & dwreg.l1ss_cap & PCI_L1SS_CAP_PCIPM_L1_1)
   611			link->aspm_support |= ASPM_STATE_L1_1_PCIPM;
   612		if (upreg.l1ss_cap & dwreg.l1ss_cap & PCI_L1SS_CAP_PCIPM_L1_2)
   613			link->aspm_support |= ASPM_STATE_L1_2_PCIPM;
   614	
   615		if (upreg.l1ss_ctl1 & dwreg.l1ss_ctl1 & PCI_L1SS_CTL1_ASPM_L1_1)
   616			link->aspm_enabled |= ASPM_STATE_L1_1;
   617		if (upreg.l1ss_ctl1 & dwreg.l1ss_ctl1 & PCI_L1SS_CTL1_ASPM_L1_2)
   618			link->aspm_enabled |= ASPM_STATE_L1_2;
   619		if (upreg.l1ss_ctl1 & dwreg.l1ss_ctl1 & PCI_L1SS_CTL1_PCIPM_L1_1)
   620			link->aspm_enabled |= ASPM_STATE_L1_1_PCIPM;
   621		if (upreg.l1ss_ctl1 & dwreg.l1ss_ctl1 & PCI_L1SS_CTL1_PCIPM_L1_2)
   622			link->aspm_enabled |= ASPM_STATE_L1_2_PCIPM;
   623	
   624		if (link->aspm_support & ASPM_STATE_L1SS)
   625			aspm_calc_l1ss_info(link, &upreg, &dwreg);
   626	
   627		/* Save default state */
   628		link->aspm_default = link->aspm_enabled;
   629	
   630		/* Setup initial capable state. Will be updated later */
   631		link->aspm_capable = link->aspm_support;
   632	
   633		/* Get and check endpoint acceptable latencies */
   634		list_for_each_entry(child, &linkbus->devices, bus_list) {
   635			u32 reg32, encoding;
   636			struct aspm_latency *acceptable =
   637				&link->acceptable[PCI_FUNC(child->devfn)];
   638	
   639			if (pci_pcie_type(child) != PCI_EXP_TYPE_ENDPOINT &&
   640			    pci_pcie_type(child) != PCI_EXP_TYPE_LEG_END)
   641				continue;
   642	
   643			pcie_capability_read_dword(child, PCI_EXP_DEVCAP, &reg32);
   644			/* Calculate endpoint L0s acceptable latency */
   645			encoding = (reg32 & PCI_EXP_DEVCAP_L0S) >> 6;
   646			acceptable->l0s = calc_l0s_acceptable(encoding);
   647			/* Calculate endpoint L1 acceptable latency */
   648			encoding = (reg32 & PCI_EXP_DEVCAP_L1) >> 9;
   649			acceptable->l1 = calc_l1_acceptable(encoding);
   650	
   651			pcie_aspm_check_latency(child);
   652		}
   653	}
   654	

---
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: 35415 bytes --]