* [PATCH V6 1/5] PCI/ASPM: introduce pci_aspm_init() and add to pci_init_capabilities()
[not found] <1490828667-30973-1-git-send-email-okaya@codeaurora.org>
@ 2017-03-29 23:04 ` Sinan Kaya
2017-03-29 23:04 ` [PATCH V6 2/5] PCI/ASPM: split pci_aspm_init() into two Sinan Kaya
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Sinan Kaya @ 2017-03-29 23:04 UTC (permalink / raw)
To: linux-pci, timur
Cc: mayurkumar.patel, linux-arm-msm, linux-arm-kernel, Sinan Kaya,
Bjorn Helgaas, Rajat Jain, Julia Lawall, Yinghai Lu, David Daney,
Shawn Lin, linux-kernel
We need a callback from pci_init_capabilities function for every
single new PCI device that is currently being added.
pci_aspm_init() will be used to save the power on state of the HW.
Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=194895
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/pci/pcie/aspm.c | 10 ++++++++++
drivers/pci/probe.c | 3 +++
include/linux/pci.h | 2 ++
3 files changed, 15 insertions(+)
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 1dfa10c..dc36717 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -827,6 +827,16 @@ static struct pcie_link_state *alloc_pcie_link_state(struct pci_dev *pdev)
}
/*
+ * pci_aspm_init: Initiate PCI express link state.
+ * It is called from device_add for every single pci device.
+ * @pdev: all pci devices
+ */
+int pci_aspm_init(struct pci_dev *pdev)
+{
+ return 0;
+}
+
+/*
* pcie_aspm_init_link_state: Initiate PCI express link state.
* It is called after the pcie and its children devices are scanned.
* @pdev: the root port or switch downstream port
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index dfc9a27..1e19364 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1847,6 +1847,9 @@ static void pci_init_capabilities(struct pci_dev *dev)
/* Advanced Error Reporting */
pci_aer_init(dev);
+
+ /* Active State Power Management */
+ pci_aspm_init(dev);
}
/*
diff --git a/include/linux/pci.h b/include/linux/pci.h
index eb3da1a..8828dd7 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1396,8 +1396,10 @@ static inline int pci_irq_get_node(struct pci_dev *pdev, int vec)
#ifdef CONFIG_PCIEASPM
bool pcie_aspm_support_enabled(void);
+int pci_aspm_init(struct pci_dev *pdev);
#else
static inline bool pcie_aspm_support_enabled(void) { return false; }
+static inline int pci_aspm_init(struct pci_dev *pdev) { return -ENODEV; }
#endif
#ifdef CONFIG_PCIEAER
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH V6 2/5] PCI/ASPM: split pci_aspm_init() into two
[not found] <1490828667-30973-1-git-send-email-okaya@codeaurora.org>
2017-03-29 23:04 ` [PATCH V6 1/5] PCI/ASPM: introduce pci_aspm_init() and add to pci_init_capabilities() Sinan Kaya
@ 2017-03-29 23:04 ` Sinan Kaya
2017-03-29 23:04 ` [PATCH V6 3/5] PCI/ASPM: add init hook to device_add Sinan Kaya
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Sinan Kaya @ 2017-03-29 23:04 UTC (permalink / raw)
To: linux-pci, timur
Cc: mayurkumar.patel, linux-arm-msm, linux-arm-kernel, Sinan Kaya,
Bjorn Helgaas, Rajat Jain, Julia Lawall, David Daney,
linux-kernel
Split pci_aspm_init() body into pci_aspm_init_upstream()
and pci_aspm_init_downstream() for bridge and endpoint
specific code behavior.
Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=194895
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/pci/pcie/aspm.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index dc36717..a80d64b 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -826,6 +826,16 @@ static struct pcie_link_state *alloc_pcie_link_state(struct pci_dev *pdev)
return link;
}
+static int pci_aspm_init_downstream(struct pci_dev *pdev)
+{
+ return 0;
+}
+
+static int pci_aspm_init_upstream(struct pci_dev *pdev)
+{
+ return 0;
+}
+
/*
* pci_aspm_init: Initiate PCI express link state.
* It is called from device_add for every single pci device.
@@ -833,7 +843,10 @@ static struct pcie_link_state *alloc_pcie_link_state(struct pci_dev *pdev)
*/
int pci_aspm_init(struct pci_dev *pdev)
{
- return 0;
+ if (!pdev->has_secondary_link)
+ return pci_aspm_init_downstream(pdev);
+
+ return pci_aspm_init_upstream(pdev);
}
/*
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH V6 3/5] PCI/ASPM: add init hook to device_add
[not found] <1490828667-30973-1-git-send-email-okaya@codeaurora.org>
2017-03-29 23:04 ` [PATCH V6 1/5] PCI/ASPM: introduce pci_aspm_init() and add to pci_init_capabilities() Sinan Kaya
2017-03-29 23:04 ` [PATCH V6 2/5] PCI/ASPM: split pci_aspm_init() into two Sinan Kaya
@ 2017-03-29 23:04 ` Sinan Kaya
2017-03-29 23:04 ` [PATCH V6 4/5] PCI/ASPM: save power on values during bridge init Sinan Kaya
2017-03-29 23:04 ` [PATCH V6 5/5] PCI/ASPM: move link_state cleanup to bridge remove Sinan Kaya
4 siblings, 0 replies; 6+ messages in thread
From: Sinan Kaya @ 2017-03-29 23:04 UTC (permalink / raw)
To: linux-pci, timur
Cc: mayurkumar.patel, linux-arm-msm, linux-arm-kernel, Sinan Kaya,
Bjorn Helgaas, Rajat Jain, Shawn Lin, David Daney, Yinghai Lu,
Julia Lawall, linux-kernel
For bridges, have pcie_aspm_init_link_state() allocate a link_state,
regardless of whether it currently has any children.
pcie_aspm_init_link_state(): Called for bridges (upstream end of
link) after all children have been enumerated. No longer needs to
check aspm_support_enabled or pdev->has_secondary_link or the VIA
quirk: pci_aspm_init() already checked that stuff, so we only need
to check pdev->link_state here.
Now that we are calling alloc_pcie_link_state() from pci_aspm_init()
, get rid of pci_function_0 function and detect downstream link in
pci_aspm_init_upstream() instead when the function number is 0.
Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=194895
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/pci/pcie/aspm.c | 72 ++++++++++++++++++++++++-------------------------
1 file changed, 36 insertions(+), 36 deletions(-)
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index a80d64b..e33f84b 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -422,20 +422,6 @@ static void pcie_aspm_check_latency(struct pci_dev *endpoint)
}
}
-/*
- * The L1 PM substate capability is only implemented in function 0 in a
- * multi function device.
- */
-static struct pci_dev *pci_function_0(struct pci_bus *linkbus)
-{
- struct pci_dev *child;
-
- list_for_each_entry(child, &linkbus->devices, bus_list)
- if (PCI_FUNC(child->devfn) == 0)
- return child;
- return NULL;
-}
-
/* Calculate L1.2 PM substate timing parameters */
static void aspm_calc_l1ss_info(struct pcie_link_state *link,
struct aspm_register_info *upreg,
@@ -798,7 +784,6 @@ static struct pcie_link_state *alloc_pcie_link_state(struct pci_dev *pdev)
INIT_LIST_HEAD(&link->children);
INIT_LIST_HEAD(&link->link);
link->pdev = pdev;
- link->downstream = pci_function_0(pdev->subordinate);
/*
* Root Ports and PCI/PCI-X to PCIe Bridges are roots of PCIe
@@ -828,11 +813,33 @@ static struct pcie_link_state *alloc_pcie_link_state(struct pci_dev *pdev)
static int pci_aspm_init_downstream(struct pci_dev *pdev)
{
+ struct pcie_link_state *link;
+ struct pci_dev *parent;
+
+ /*
+ * The L1 PM substate capability is only implemented in function 0 in a
+ * multi function device.
+ */
+ if (PCI_FUNC(pdev->devfn) != 0)
+ return -EINVAL;
+
+ parent = pdev->bus->self;
+ if (!parent)
+ return -EINVAL;
+
+ link = parent->link_state;
+ link->downstream = pdev;
return 0;
}
static int pci_aspm_init_upstream(struct pci_dev *pdev)
{
+ struct pcie_link_state *link;
+
+ link = alloc_pcie_link_state(pdev);
+ if (!link)
+ return -ENOMEM;
+
return 0;
}
@@ -843,6 +850,17 @@ static int pci_aspm_init_upstream(struct pci_dev *pdev)
*/
int pci_aspm_init(struct pci_dev *pdev)
{
+ if (!aspm_support_enabled)
+ return 0;
+
+ if (!pci_is_pcie(pdev))
+ return -EINVAL;
+
+ /* VIA has a strange chipset, root port is under a bridge */
+ if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT &&
+ pdev->bus->self)
+ return 0;
+
if (!pdev->has_secondary_link)
return pci_aspm_init_downstream(pdev);
@@ -859,33 +877,16 @@ void pcie_aspm_init_link_state(struct pci_dev *pdev)
struct pcie_link_state *link;
int blacklist = !!pcie_aspm_sanity_check(pdev);
- if (!aspm_support_enabled)
- return;
-
- if (pdev->link_state)
- return;
-
- /*
- * We allocate pcie_link_state for the component on the upstream
- * end of a Link, so there's nothing to do unless this device has a
- * Link on its secondary side.
- */
- if (!pdev->has_secondary_link)
- return;
-
- /* VIA has a strange chipset, root port is under a bridge */
- if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT &&
- pdev->bus->self)
+ if (!pdev->link_state)
return;
+ link = pdev->link_state;
down_read(&pci_bus_sem);
if (list_empty(&pdev->subordinate->devices))
goto out;
mutex_lock(&aspm_lock);
- link = alloc_pcie_link_state(pdev);
- if (!link)
- goto unlock;
+
/*
* Setup initial ASPM state. Note that we need to configure
* upstream links also because capable state of them can be
@@ -910,7 +911,6 @@ void pcie_aspm_init_link_state(struct pci_dev *pdev)
pcie_set_clkpm(link, policy_to_clkpm_state(link));
}
-unlock:
mutex_unlock(&aspm_lock);
out:
up_read(&pci_bus_sem);
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH V6 4/5] PCI/ASPM: save power on values during bridge init
[not found] <1490828667-30973-1-git-send-email-okaya@codeaurora.org>
` (2 preceding siblings ...)
2017-03-29 23:04 ` [PATCH V6 3/5] PCI/ASPM: add init hook to device_add Sinan Kaya
@ 2017-03-29 23:04 ` Sinan Kaya
2017-03-29 23:04 ` [PATCH V6 5/5] PCI/ASPM: move link_state cleanup to bridge remove Sinan Kaya
4 siblings, 0 replies; 6+ messages in thread
From: Sinan Kaya @ 2017-03-29 23:04 UTC (permalink / raw)
To: linux-pci, timur
Cc: mayurkumar.patel, linux-arm-msm, linux-arm-kernel, Sinan Kaya,
Bjorn Helgaas, Rajat Jain, David Daney, Yinghai Lu, Julia Lawall,
linux-kernel
Now that we added a hook to be called from device_add, save the
default values from the HW registers early in the boot for further
reuse during hot device add/remove operations.
Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=194895
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/pci/pcie/aspm.c | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index e33f84b..a38602a 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -505,8 +505,10 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
*/
if (dwreg.support & upreg.support & PCIE_LINK_STATE_L0S)
link->aspm_support |= ASPM_STATE_L0S;
- if (dwreg.enabled & PCIE_LINK_STATE_L0S)
+ if (dwreg.enabled & PCIE_LINK_STATE_L0S) {
link->aspm_enabled |= ASPM_STATE_L0S_UP;
+ link->aspm_default |= 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);
@@ -542,9 +544,6 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
if (link->aspm_support & ASPM_STATE_L1SS)
aspm_calc_l1ss_info(link, &upreg, &dwreg);
- /* Save default state */
- link->aspm_default = link->aspm_enabled;
-
/* Setup initial capable state. Will be updated later */
link->aspm_capable = link->aspm_support;
/*
@@ -835,11 +834,26 @@ static int pci_aspm_init_downstream(struct pci_dev *pdev)
static int pci_aspm_init_upstream(struct pci_dev *pdev)
{
struct pcie_link_state *link;
+ struct aspm_register_info upreg;
link = alloc_pcie_link_state(pdev);
if (!link)
return -ENOMEM;
+ pcie_get_aspm_reg(pdev, &upreg);
+ if (upreg.enabled & PCIE_LINK_STATE_L0S)
+ link->aspm_default |= ASPM_STATE_L0S_DW;
+ if (upreg.enabled & PCIE_LINK_STATE_L1)
+ link->aspm_default |= ASPM_STATE_L1;
+ if (upreg.l1ss_ctl1 & PCI_L1SS_CTL1_ASPM_L1_1)
+ link->aspm_default |= ASPM_STATE_L1_1;
+ if (upreg.l1ss_ctl1 & PCI_L1SS_CTL1_ASPM_L1_2)
+ link->aspm_default |= ASPM_STATE_L1_2;
+ if (upreg.l1ss_ctl1 & PCI_L1SS_CTL1_PCIPM_L1_1)
+ link->aspm_default |= ASPM_STATE_L1_1_PCIPM;
+ if (upreg.l1ss_ctl1 & PCI_L1SS_CTL1_PCIPM_L1_2)
+ link->aspm_default |= ASPM_STATE_L1_2_PCIPM;
+
return 0;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH V6 5/5] PCI/ASPM: move link_state cleanup to bridge remove
[not found] <1490828667-30973-1-git-send-email-okaya@codeaurora.org>
` (3 preceding siblings ...)
2017-03-29 23:04 ` [PATCH V6 4/5] PCI/ASPM: save power on values during bridge init Sinan Kaya
@ 2017-03-29 23:04 ` Sinan Kaya
2017-03-30 13:25 ` Sinan Kaya
4 siblings, 1 reply; 6+ messages in thread
From: Sinan Kaya @ 2017-03-29 23:04 UTC (permalink / raw)
To: linux-pci, timur
Cc: mayurkumar.patel, linux-arm-msm, linux-arm-kernel, Sinan Kaya,
Bjorn Helgaas, Rajat Jain, David Daney, Yinghai Lu, linux-kernel
For endpoints, change pcie_aspm_exit_link_state() so it cleans up
the device's own state and disables ASPM if necessary, but doesn't
remove the parent's link_state.
For bridges, change pcie_aspm_exit_link_state() so it frees the
bridge's own link_state.
Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=194895
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/pci/pcie/aspm.c | 20 +++++++++++++++-----
drivers/pci/remove.c | 3 +--
2 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index a38602a..37ca9b3 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -963,6 +963,21 @@ void pcie_aspm_exit_link_state(struct pci_dev *pdev)
if (!parent || !parent->link_state)
return;
+ if (pdev->has_secondary_link) {
+ link = pdev->link_state;
+ down_read(&pci_bus_sem);
+ mutex_lock(&aspm_lock);
+
+ list_del(&link->sibling);
+ list_del(&link->link);
+
+ /* Clock PM is for endpoint device */
+ free_link_state(link);
+ mutex_unlock(&aspm_lock);
+ up_read(&pci_bus_sem);
+ return;
+ }
+
down_read(&pci_bus_sem);
mutex_lock(&aspm_lock);
/*
@@ -978,11 +993,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);
- list_del(&link->link);
- /* Clock PM is for endpoint device */
- free_link_state(link);
-
/* Recheck latencies and configure upstream links */
if (parent_link) {
pcie_update_aspm_capable(root);
diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index 73a03d3..7e14ebd 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -25,8 +25,7 @@ static void pci_stop_dev(struct pci_dev *dev)
dev->is_added = 0;
}
- if (dev->bus->self)
- pcie_aspm_exit_link_state(dev);
+ pcie_aspm_exit_link_state(dev);
}
static void pci_destroy_dev(struct pci_dev *dev)
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH V6 5/5] PCI/ASPM: move link_state cleanup to bridge remove
2017-03-29 23:04 ` [PATCH V6 5/5] PCI/ASPM: move link_state cleanup to bridge remove Sinan Kaya
@ 2017-03-30 13:25 ` Sinan Kaya
0 siblings, 0 replies; 6+ messages in thread
From: Sinan Kaya @ 2017-03-30 13:25 UTC (permalink / raw)
To: linux-pci, timur
Cc: mayurkumar.patel, linux-arm-msm, linux-arm-kernel, Bjorn Helgaas,
Rajat Jain, David Daney, Yinghai Lu, linux-kernel
On 3/29/2017 7:04 PM, Sinan Kaya wrote:
> if (!parent || !parent->link_state)
> return;
>
> + if (pdev->has_secondary_link) {
> + link = pdev->link_state;
I think I accidentally moved the parent check above this code
and broke the case where you can actually remove the root port.
I'll post v7 with reverting this change.
--
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-03-30 13:25 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1490828667-30973-1-git-send-email-okaya@codeaurora.org>
2017-03-29 23:04 ` [PATCH V6 1/5] PCI/ASPM: introduce pci_aspm_init() and add to pci_init_capabilities() Sinan Kaya
2017-03-29 23:04 ` [PATCH V6 2/5] PCI/ASPM: split pci_aspm_init() into two Sinan Kaya
2017-03-29 23:04 ` [PATCH V6 3/5] PCI/ASPM: add init hook to device_add Sinan Kaya
2017-03-29 23:04 ` [PATCH V6 4/5] PCI/ASPM: save power on values during bridge init Sinan Kaya
2017-03-29 23:04 ` [PATCH V6 5/5] PCI/ASPM: move link_state cleanup to bridge remove Sinan Kaya
2017-03-30 13:25 ` Sinan Kaya
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).