On 2017-03-03 04:43, Patel, Mayurkumar wrote:
Hi Kaya
Hi Mayurkumar
On 3/2/2017 11:05 AM, Patel, Mayurkumar wrote:
Hi Bjorn,
On 2/28/2017 1:57 PM, Patel, Mayurkumar wrote:
I was trying to figure out when to use saved values vs. the values
in
registers by looking at the enable_cnt.
enable_cnt is 0 during boot on my system.
enable_cnt for the root port on my system is set to 1 for "root
port" already without saving
any ASPM related settings.
What would be the best way to figure out when to save power-on
values from
the registers?
I can upload the diffs) because enable_cnt in pci_enable_device() can
be triggered
from multiple places at boot time so it might not be safe to use it?
Go ahead and share your diff. It doesn't hurt to look at other
alternatives.
So basically, I introduced a new atomic variable to save the
aspm_policy for the first time.
Below is my diff.
Thanks.
Ok, i will incorporate your change and add your signoff, then repost the
next version after testing next week. I am OoO until next week.
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 3dd8bcb..c8e1e3a 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -338,8 +338,9 @@ static void pcie_aspm_check_latency(struct pci_dev
*endpoint)
}
}
-static void pcie_aspm_cap_init(struct pcie_link_state *link, int
blacklist)
+static void pcie_aspm_cap_init(struct pci_dev *pdev, int blacklist)
{
+ struct pcie_link_state *link = pdev->link_state;
struct pci_dev *child, *parent = link->pdev;
struct pci_bus *linkbus = parent->subordinate;
struct aspm_register_info upreg, dwreg;
@@ -397,8 +398,21 @@ static void pcie_aspm_cap_init(struct
pcie_link_state *link, int blacklist)
link->latency_up.l1 =
calc_l1_latency(upreg.latency_encoding_l1);
link->latency_dw.l1 =
calc_l1_latency(dwreg.latency_encoding_l1);
- /* Save default state */
- link->aspm_default = link->aspm_enabled;
+ /*
+ * Save default state from FW when enabling ASPM for the first
time
+ * during boot by looking at the calculated link->aspm_enabled
bits
+ * above and aspm_enable_cnt will be zero.
+ *
+ * If this path is getting called for the second/third time
+ * (aspm_enable_cnt will be non-zero). Assume that the current
state
+ * of the ASPM registers may not necessarily match what FW
asked us to
+ * do as in the case of hotplug insertion/removal.
+ */
+ if (atomic_inc_return(&pdev->aspm_enable_cnt) == 1)
+ pdev->aspm_default = link->aspm_default =
link->aspm_enabled;
+ else
+ link->aspm_default = pdev->aspm_default;
+
/* Setup initial capable state. Will be updated later */
link->aspm_capable = link->aspm_support;
@@ -606,7 +620,7 @@ void pcie_aspm_init_link_state(struct pci_dev
*pdev)
* upstream links also because capable state of them can be
* update through pcie_aspm_cap_init().
*/
- pcie_aspm_cap_init(link, blacklist);
+ pcie_aspm_cap_init(pdev, blacklist);
/* Setup initial Clock PM state */
pcie_clkpm_cap_init(link, blacklist);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index e2d1a12..aa7bd7e 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -321,6 +321,8 @@ struct pci_dev {
#ifdef CONFIG_PCIEASPM
struct pcie_link_state *link_state; /* ASPM link state */
+ unsigned int aspm_default; /* ASPM policy set by
BIOS */
+ atomic_t aspm_enable_cnt; /* ASPM policy
initialization */
