* [PATCH kernel 1/5] powerpc/pci: Remove unused parameter from add_one_dev_pci_data()
2017-08-23 10:18 [PATCH kernel 0/5] powerpc/eeh: Some cleanups Alexey Kardashevskiy
@ 2017-08-23 10:18 ` Alexey Kardashevskiy
2017-08-23 10:18 ` [PATCH kernel 2/5] powerpc/eeh: Reduce to one the number of places where edev is allocated Alexey Kardashevskiy
` (8 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Alexey Kardashevskiy @ 2017-08-23 10:18 UTC (permalink / raw)
To: linuxppc-dev
Cc: Alexey Kardashevskiy, David Gibson, Benjamin Herrenschmidt,
Alistair Popple, Russell Currey
pdev is always NULL, remove it.
To make checkpatch.pl happy, this also removes the "out of memory"
message.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
arch/powerpc/kernel/pci_dn.c | 14 ++------------
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
index 592693437070..0256372b72de 100644
--- a/arch/powerpc/kernel/pci_dn.c
+++ b/arch/powerpc/kernel/pci_dn.c
@@ -139,7 +139,6 @@ struct pci_dn *pci_get_pdn(struct pci_dev *pdev)
#ifdef CONFIG_PCI_IOV
static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
- struct pci_dev *pdev,
int vf_index,
int busno, int devfn)
{
@@ -150,10 +149,8 @@ static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
return NULL;
pdn = kzalloc(sizeof(*pdn), GFP_KERNEL);
- if (!pdn) {
- dev_warn(&pdev->dev, "%s: Out of memory!\n", __func__);
+ if (!pdn)
return NULL;
- }
pdn->phb = parent->phb;
pdn->parent = parent;
@@ -167,13 +164,6 @@ static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
INIT_LIST_HEAD(&pdn->list);
list_add_tail(&pdn->list, &parent->child_list);
- /*
- * If we already have PCI device instance, lets
- * bind them.
- */
- if (pdev)
- pdev->dev.archdata.pci_data = pdn;
-
return pdn;
}
#endif
@@ -201,7 +191,7 @@ struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
for (i = 0; i < pci_sriov_get_totalvfs(pdev); i++) {
struct eeh_dev *edev __maybe_unused;
- pdn = add_one_dev_pci_data(parent, NULL, i,
+ pdn = add_one_dev_pci_data(parent, i,
pci_iov_virtfn_bus(pdev, i),
pci_iov_virtfn_devfn(pdev, i));
if (!pdn) {
--
2.11.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH kernel 2/5] powerpc/eeh: Reduce to one the number of places where edev is allocated
2017-08-23 10:18 [PATCH kernel 0/5] powerpc/eeh: Some cleanups Alexey Kardashevskiy
2017-08-23 10:18 ` [PATCH kernel 1/5] powerpc/pci: Remove unused parameter from add_one_dev_pci_data() Alexey Kardashevskiy
@ 2017-08-23 10:18 ` Alexey Kardashevskiy
2017-08-23 10:18 ` [PATCH kernel 3/5] powerpc/eeh: Remove unnecessary pointer to phb from eeh_dev Alexey Kardashevskiy
` (7 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Alexey Kardashevskiy @ 2017-08-23 10:18 UTC (permalink / raw)
To: linuxppc-dev
Cc: Alexey Kardashevskiy, David Gibson, Benjamin Herrenschmidt,
Alistair Popple, Russell Currey
arch/powerpc/kernel/eeh_dev.c:57 is the only legit place where edev
is allocated; other 2 places allocate it on stack and in the heap for
a very short period of time to use eeh_pe_get() as takes edev.
This changes eeh_pe_get() to receive required parameters explicitly.
This removes unnecessary temporary allocation of edev.
This uses the "pe_no" name instead of the "pe_config_addr" name as
it actually is a PE number and not a config space address as it seemed.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
arch/powerpc/include/asm/eeh.h | 3 ++-
arch/powerpc/kernel/eeh_pe.c | 32 ++++++++++++++++++----------
arch/powerpc/platforms/powernv/eeh-powernv.c | 15 ++-----------
3 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 8e37b71674f4..26a6a43f8799 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -262,7 +262,8 @@ typedef void *(*eeh_traverse_func)(void *data, void *flag);
void eeh_set_pe_aux_size(int size);
int eeh_phb_pe_create(struct pci_controller *phb);
struct eeh_pe *eeh_phb_pe_get(struct pci_controller *phb);
-struct eeh_pe *eeh_pe_get(struct eeh_dev *edev);
+struct eeh_pe *eeh_pe_get(struct pci_controller *phb,
+ int pe_no, int config_addr);
int eeh_add_to_parent_pe(struct eeh_dev *edev);
int eeh_rmv_from_parent_pe(struct eeh_dev *edev);
void eeh_pe_update_time_stamp(struct eeh_pe *pe);
diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
index cc4b206f77e4..84d79f3da7d6 100644
--- a/arch/powerpc/kernel/eeh_pe.c
+++ b/arch/powerpc/kernel/eeh_pe.c
@@ -230,10 +230,15 @@ void *eeh_pe_dev_traverse(struct eeh_pe *root,
* Bus/Device/Function number. The extra data referred by flag
* indicates which type of address should be used.
*/
+struct eeh_pe_get_flag {
+ int pe_no;
+ int config_addr;
+};
+
static void *__eeh_pe_get(void *data, void *flag)
{
struct eeh_pe *pe = (struct eeh_pe *)data;
- struct eeh_dev *edev = (struct eeh_dev *)flag;
+ struct eeh_pe_get_flag *tmp = (struct eeh_pe_get_flag *) flag;
/* Unexpected PHB PE */
if (pe->type & EEH_PE_PHB)
@@ -244,17 +249,17 @@ static void *__eeh_pe_get(void *data, void *flag)
* have non-zero PE address
*/
if (eeh_has_flag(EEH_VALID_PE_ZERO)) {
- if (edev->pe_config_addr == pe->addr)
+ if (tmp->pe_no == pe->addr)
return pe;
} else {
- if (edev->pe_config_addr &&
- (edev->pe_config_addr == pe->addr))
+ if (tmp->pe_no &&
+ (tmp->pe_no == pe->addr))
return pe;
}
/* Try BDF address */
- if (edev->config_addr &&
- (edev->config_addr == pe->config_addr))
+ if (tmp->config_addr &&
+ (tmp->config_addr == pe->config_addr))
return pe;
return NULL;
@@ -262,7 +267,9 @@ static void *__eeh_pe_get(void *data, void *flag)
/**
* eeh_pe_get - Search PE based on the given address
- * @edev: EEH device
+ * @phb: PCI controller
+ * @pe_no: PE number
+ * @config_addr: Config address
*
* Search the corresponding PE based on the specified address which
* is included in the eeh device. The function is used to check if
@@ -271,12 +278,14 @@ static void *__eeh_pe_get(void *data, void *flag)
* which is composed of PCI bus/device/function number, or unified
* PE address.
*/
-struct eeh_pe *eeh_pe_get(struct eeh_dev *edev)
+struct eeh_pe *eeh_pe_get(struct pci_controller *phb,
+ int pe_no, int config_addr)
{
- struct eeh_pe *root = eeh_phb_pe_get(edev->phb);
+ struct eeh_pe *root = eeh_phb_pe_get(phb);
+ struct eeh_pe_get_flag tmp = { pe_no, config_addr };
struct eeh_pe *pe;
- pe = eeh_pe_traverse(root, __eeh_pe_get, edev);
+ pe = eeh_pe_traverse(root, __eeh_pe_get, &tmp);
return pe;
}
@@ -344,7 +353,8 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
* PE should be composed of PCI bus and its subordinate
* components.
*/
- pe = eeh_pe_get(edev);
+ pe = eeh_pe_get(edev->pdn->phb, edev->pe_config_addr,
+ edev->config_addr);
if (pe && !(pe->type & EEH_PE_INVALID)) {
/* Mark the PE as type of PCI bus */
pe->type = EEH_PE_BUS;
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index 3f48f6df1cf3..ac8c01cd251c 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -113,7 +113,6 @@ static ssize_t pnv_eeh_ei_write(struct file *filp,
size_t count, loff_t *ppos)
{
struct pci_controller *hose = filp->private_data;
- struct eeh_dev *edev;
struct eeh_pe *pe;
int pe_no, type, func;
unsigned long addr, mask;
@@ -135,13 +134,7 @@ static ssize_t pnv_eeh_ei_write(struct file *filp,
return -EINVAL;
/* Retrieve PE */
- edev = kzalloc(sizeof(*edev), GFP_KERNEL);
- if (!edev)
- return -ENOMEM;
- edev->phb = hose;
- edev->pe_config_addr = pe_no;
- pe = eeh_pe_get(edev);
- kfree(edev);
+ pe = eeh_pe_get(hose, pe_no, 0);
if (!pe)
return -ENODEV;
@@ -1381,7 +1374,6 @@ static int pnv_eeh_get_pe(struct pci_controller *hose,
struct pnv_phb *phb = hose->private_data;
struct pnv_ioda_pe *pnv_pe;
struct eeh_pe *dev_pe;
- struct eeh_dev edev;
/*
* If PHB supports compound PE, to fetch
@@ -1397,10 +1389,7 @@ static int pnv_eeh_get_pe(struct pci_controller *hose,
}
/* Find the PE according to PE# */
- memset(&edev, 0, sizeof(struct eeh_dev));
- edev.phb = hose;
- edev.pe_config_addr = pe_no;
- dev_pe = eeh_pe_get(&edev);
+ dev_pe = eeh_pe_get(hose, pe_no, 0);
if (!dev_pe)
return -EEXIST;
--
2.11.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH kernel 3/5] powerpc/eeh: Remove unnecessary pointer to phb from eeh_dev
2017-08-23 10:18 [PATCH kernel 0/5] powerpc/eeh: Some cleanups Alexey Kardashevskiy
2017-08-23 10:18 ` [PATCH kernel 1/5] powerpc/pci: Remove unused parameter from add_one_dev_pci_data() Alexey Kardashevskiy
2017-08-23 10:18 ` [PATCH kernel 2/5] powerpc/eeh: Reduce to one the number of places where edev is allocated Alexey Kardashevskiy
@ 2017-08-23 10:18 ` Alexey Kardashevskiy
2017-08-24 4:45 ` Andrew Donnellan
2017-08-28 6:25 ` Michael Ellerman
2017-08-23 10:19 ` [PATCH kernel 4/5] powerpc/eeh: Remove unnecessary config_addr " Alexey Kardashevskiy
` (6 subsequent siblings)
9 siblings, 2 replies; 17+ messages in thread
From: Alexey Kardashevskiy @ 2017-08-23 10:18 UTC (permalink / raw)
To: linuxppc-dev
Cc: Alexey Kardashevskiy, David Gibson, Benjamin Herrenschmidt,
Alistair Popple, Russell Currey
The eeh_dev struct already holds a pointer to pci_dn which it does not
exist without and pci_dn itself holds the very same pointer so just
use it.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
arch/powerpc/include/asm/eeh.h | 1 -
arch/powerpc/kernel/eeh.c | 7 +++----
arch/powerpc/kernel/eeh_dev.c | 2 --
arch/powerpc/kernel/eeh_driver.c | 2 +-
arch/powerpc/kernel/eeh_pe.c | 24 +++++++++++++-----------
arch/powerpc/platforms/powernv/eeh-powernv.c | 5 ++---
6 files changed, 19 insertions(+), 22 deletions(-)
diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 26a6a43f8799..777d37aa0a7f 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -141,7 +141,6 @@ struct eeh_dev {
struct eeh_pe *pe; /* Associated PE */
struct list_head list; /* Form link list in the PE */
struct list_head rmv_list; /* Record the removed edevs */
- struct pci_controller *phb; /* Associated PHB */
struct pci_dn *pdn; /* Associated PCI device node */
struct pci_dev *pdev; /* Associated PCI device */
bool in_error; /* Error flag for edev */
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 63992b2d8e15..c877014d11ce 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -169,10 +169,10 @@ static size_t eeh_dump_dev_log(struct eeh_dev *edev, char *buf, size_t len)
char buffer[128];
n += scnprintf(buf+n, len-n, "%04x:%02x:%02x.%01x\n",
- edev->phb->global_number, pdn->busno,
+ pdn->phb->global_number, pdn->busno,
PCI_SLOT(pdn->devfn), PCI_FUNC(pdn->devfn));
pr_warn("EEH: of node=%04x:%02x:%02x.%01x\n",
- edev->phb->global_number, pdn->busno,
+ pdn->phb->global_number, pdn->busno,
PCI_SLOT(pdn->devfn), PCI_FUNC(pdn->devfn));
eeh_ops->read_config(pdn, PCI_VENDOR_ID, 4, &cfg);
@@ -1064,7 +1064,7 @@ core_initcall_sync(eeh_init);
*/
void eeh_add_device_early(struct pci_dn *pdn)
{
- struct pci_controller *phb;
+ struct pci_controller *phb = pdn ? pdn->phb : NULL;
struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
if (!edev)
@@ -1074,7 +1074,6 @@ void eeh_add_device_early(struct pci_dn *pdn)
return;
/* USB Bus children of PCI devices will not have BUID's */
- phb = edev->phb;
if (NULL == phb ||
(eeh_has_flag(EEH_PROBE_MODE_DEVTREE) && 0 == phb->buid))
return;
diff --git a/arch/powerpc/kernel/eeh_dev.c b/arch/powerpc/kernel/eeh_dev.c
index d6b2ca70d14d..bdf4a3698a35 100644
--- a/arch/powerpc/kernel/eeh_dev.c
+++ b/arch/powerpc/kernel/eeh_dev.c
@@ -50,7 +50,6 @@
*/
struct eeh_dev *eeh_dev_init(struct pci_dn *pdn)
{
- struct pci_controller *phb = pdn->phb;
struct eeh_dev *edev;
/* Allocate EEH device */
@@ -64,7 +63,6 @@ struct eeh_dev *eeh_dev_init(struct pci_dn *pdn)
/* Associate EEH device with OF node */
pdn->edev = edev;
edev->pdn = pdn;
- edev->phb = phb;
INIT_LIST_HEAD(&edev->list);
INIT_LIST_HEAD(&edev->rmv_list);
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index c405c79e50cd..8b840191df59 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -428,7 +428,7 @@ static void *eeh_add_virt_device(void *data, void *userdata)
if (!(edev->physfn)) {
pr_warn("%s: EEH dev %04x:%02x:%02x.%01x not for VF\n",
- __func__, edev->phb->global_number, pdn->busno,
+ __func__, pdn->phb->global_number, pdn->busno,
PCI_SLOT(pdn->devfn), PCI_FUNC(pdn->devfn));
return NULL;
}
diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
index 84d79f3da7d6..419c3f07afd5 100644
--- a/arch/powerpc/kernel/eeh_pe.c
+++ b/arch/powerpc/kernel/eeh_pe.c
@@ -339,11 +339,12 @@ static struct eeh_pe *eeh_pe_get_parent(struct eeh_dev *edev)
int eeh_add_to_parent_pe(struct eeh_dev *edev)
{
struct eeh_pe *pe, *parent;
+ struct pci_dn *pdn = eeh_dev_to_pdn(edev);
/* Check if the PE number is valid */
if (!eeh_has_flag(EEH_VALID_PE_ZERO) && !edev->pe_config_addr) {
pr_err("%s: Invalid PE#0 for edev 0x%x on PHB#%x\n",
- __func__, edev->config_addr, edev->phb->global_number);
+ __func__, edev->config_addr, pdn->phb->global_number);
return -EINVAL;
}
@@ -353,7 +354,7 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
* PE should be composed of PCI bus and its subordinate
* components.
*/
- pe = eeh_pe_get(edev->pdn->phb, edev->pe_config_addr,
+ pe = eeh_pe_get(pdn->phb, edev->pe_config_addr,
edev->config_addr);
if (pe && !(pe->type & EEH_PE_INVALID)) {
/* Mark the PE as type of PCI bus */
@@ -363,7 +364,7 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
/* Put the edev to PE */
list_add_tail(&edev->list, &pe->edevs);
pr_debug("EEH: Add %04x:%02x:%02x.%01x to Bus PE#%x\n",
- edev->phb->global_number,
+ pdn->phb->global_number,
edev->config_addr >> 8,
PCI_SLOT(edev->config_addr & 0xFF),
PCI_FUNC(edev->config_addr & 0xFF),
@@ -386,7 +387,7 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
pr_debug("EEH: Add %04x:%02x:%02x.%01x to Device "
"PE#%x, Parent PE#%x\n",
- edev->phb->global_number,
+ pdn->phb->global_number,
edev->config_addr >> 8,
PCI_SLOT(edev->config_addr & 0xFF),
PCI_FUNC(edev->config_addr & 0xFF),
@@ -396,9 +397,9 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
/* Create a new EEH PE */
if (edev->physfn)
- pe = eeh_pe_alloc(edev->phb, EEH_PE_VF);
+ pe = eeh_pe_alloc(pdn->phb, EEH_PE_VF);
else
- pe = eeh_pe_alloc(edev->phb, EEH_PE_DEVICE);
+ pe = eeh_pe_alloc(pdn->phb, EEH_PE_DEVICE);
if (!pe) {
pr_err("%s: out of memory!\n", __func__);
return -ENOMEM;
@@ -414,10 +415,10 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
*/
parent = eeh_pe_get_parent(edev);
if (!parent) {
- parent = eeh_phb_pe_get(edev->phb);
+ parent = eeh_phb_pe_get(pdn->phb);
if (!parent) {
pr_err("%s: No PHB PE is found (PHB Domain=%d)\n",
- __func__, edev->phb->global_number);
+ __func__, pdn->phb->global_number);
edev->pe = NULL;
kfree(pe);
return -EEXIST;
@@ -434,7 +435,7 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
edev->pe = pe;
pr_debug("EEH: Add %04x:%02x:%02x.%01x to "
"Device PE#%x, Parent PE#%x\n",
- edev->phb->global_number,
+ pdn->phb->global_number,
edev->config_addr >> 8,
PCI_SLOT(edev->config_addr & 0xFF),
PCI_FUNC(edev->config_addr & 0xFF),
@@ -456,10 +457,11 @@ int eeh_rmv_from_parent_pe(struct eeh_dev *edev)
{
struct eeh_pe *pe, *parent, *child;
int cnt;
+ struct pci_dn *pdn = eeh_dev_to_pdn(edev);
if (!edev->pe) {
pr_debug("%s: No PE found for device %04x:%02x:%02x.%01x\n",
- __func__, edev->phb->global_number,
+ __func__, pdn->phb->global_number,
edev->config_addr >> 8,
PCI_SLOT(edev->config_addr & 0xFF),
PCI_FUNC(edev->config_addr & 0xFF));
@@ -722,7 +724,7 @@ static void eeh_bridge_check_link(struct eeh_dev *edev)
return;
pr_debug("%s: Check PCIe link for %04x:%02x:%02x.%01x ...\n",
- __func__, edev->phb->global_number,
+ __func__, pdn->phb->global_number,
edev->config_addr >> 8,
PCI_SLOT(edev->config_addr & 0xFF),
PCI_FUNC(edev->config_addr & 0xFF));
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index ac8c01cd251c..552b0cd4e8ba 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -926,7 +926,6 @@ void pnv_pci_reset_secondary_bus(struct pci_dev *dev)
static void pnv_eeh_wait_for_pending(struct pci_dn *pdn, const char *type,
int pos, u16 mask)
{
- struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
int i, status = 0;
/* Wait for Transaction Pending bit to be cleared */
@@ -940,7 +939,7 @@ static void pnv_eeh_wait_for_pending(struct pci_dn *pdn, const char *type,
pr_warn("%s: Pending transaction while issuing %sFLR to %04x:%02x:%02x.%01x\n",
__func__, type,
- edev->phb->global_number, pdn->busno,
+ pdn->phb->global_number, pdn->busno,
PCI_SLOT(pdn->devfn), PCI_FUNC(pdn->devfn));
}
@@ -1714,7 +1713,7 @@ static int pnv_eeh_restore_config(struct pci_dn *pdn)
if (edev->physfn) {
ret = pnv_eeh_restore_vf_config(pdn);
} else {
- phb = edev->phb->private_data;
+ phb = pdn->phb->private_data;
ret = opal_pci_reinit(phb->opal_id,
OPAL_REINIT_PCI_DEV, edev->config_addr);
}
--
2.11.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH kernel 3/5] powerpc/eeh: Remove unnecessary pointer to phb from eeh_dev
2017-08-23 10:18 ` [PATCH kernel 3/5] powerpc/eeh: Remove unnecessary pointer to phb from eeh_dev Alexey Kardashevskiy
@ 2017-08-24 4:45 ` Andrew Donnellan
2017-08-28 6:25 ` Michael Ellerman
1 sibling, 0 replies; 17+ messages in thread
From: Andrew Donnellan @ 2017-08-24 4:45 UTC (permalink / raw)
To: Alexey Kardashevskiy, linuxppc-dev; +Cc: Alistair Popple, David Gibson
On 23/08/17 20:18, Alexey Kardashevskiy wrote:
> The eeh_dev struct already holds a pointer to pci_dn which it does not
> exist without and pci_dn itself holds the very same pointer so just
> use it.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> ---
> arch/powerpc/include/asm/eeh.h | 1 -
> arch/powerpc/kernel/eeh.c | 7 +++----
> arch/powerpc/kernel/eeh_dev.c | 2 --
> arch/powerpc/kernel/eeh_driver.c | 2 +-
> arch/powerpc/kernel/eeh_pe.c | 24 +++++++++++++-----------
> arch/powerpc/platforms/powernv/eeh-powernv.c | 5 ++---
> 6 files changed, 19 insertions(+), 22 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
> index 26a6a43f8799..777d37aa0a7f 100644
> --- a/arch/powerpc/include/asm/eeh.h
> +++ b/arch/powerpc/include/asm/eeh.h
> @@ -141,7 +141,6 @@ struct eeh_dev {
> struct eeh_pe *pe; /* Associated PE */
> struct list_head list; /* Form link list in the PE */
> struct list_head rmv_list; /* Record the removed edevs */
> - struct pci_controller *phb; /* Associated PHB */
> struct pci_dn *pdn; /* Associated PCI device node */
> struct pci_dev *pdev; /* Associated PCI device */
> bool in_error; /* Error flag for edev */
> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
> index 63992b2d8e15..c877014d11ce 100644
> --- a/arch/powerpc/kernel/eeh.c
> +++ b/arch/powerpc/kernel/eeh.c
> @@ -169,10 +169,10 @@ static size_t eeh_dump_dev_log(struct eeh_dev *edev, char *buf, size_t len)
> char buffer[128];
>
> n += scnprintf(buf+n, len-n, "%04x:%02x:%02x.%01x\n",
> - edev->phb->global_number, pdn->busno,
> + pdn->phb->global_number, pdn->busno,
> PCI_SLOT(pdn->devfn), PCI_FUNC(pdn->devfn));
> pr_warn("EEH: of node=%04x:%02x:%02x.%01x\n",
> - edev->phb->global_number, pdn->busno,
> + pdn->phb->global_number, pdn->busno,
> PCI_SLOT(pdn->devfn), PCI_FUNC(pdn->devfn));
>
> eeh_ops->read_config(pdn, PCI_VENDOR_ID, 4, &cfg);
> @@ -1064,7 +1064,7 @@ core_initcall_sync(eeh_init);
> */
> void eeh_add_device_early(struct pci_dn *pdn)
> {
> - struct pci_controller *phb;
> + struct pci_controller *phb = pdn ? pdn->phb : NULL;
> struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
>
> if (!edev)
> @@ -1074,7 +1074,6 @@ void eeh_add_device_early(struct pci_dn *pdn)
> return;
>
> /* USB Bus children of PCI devices will not have BUID's */
> - phb = edev->phb;
> if (NULL == phb ||
> (eeh_has_flag(EEH_PROBE_MODE_DEVTREE) && 0 == phb->buid))
> return;
> diff --git a/arch/powerpc/kernel/eeh_dev.c b/arch/powerpc/kernel/eeh_dev.c
> index d6b2ca70d14d..bdf4a3698a35 100644
> --- a/arch/powerpc/kernel/eeh_dev.c
> +++ b/arch/powerpc/kernel/eeh_dev.c
> @@ -50,7 +50,6 @@
> */
> struct eeh_dev *eeh_dev_init(struct pci_dn *pdn)
> {
> - struct pci_controller *phb = pdn->phb;
> struct eeh_dev *edev;
>
> /* Allocate EEH device */
> @@ -64,7 +63,6 @@ struct eeh_dev *eeh_dev_init(struct pci_dn *pdn)
> /* Associate EEH device with OF node */
> pdn->edev = edev;
> edev->pdn = pdn;
> - edev->phb = phb;
> INIT_LIST_HEAD(&edev->list);
> INIT_LIST_HEAD(&edev->rmv_list);
>
> diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
> index c405c79e50cd..8b840191df59 100644
> --- a/arch/powerpc/kernel/eeh_driver.c
> +++ b/arch/powerpc/kernel/eeh_driver.c
> @@ -428,7 +428,7 @@ static void *eeh_add_virt_device(void *data, void *userdata)
>
> if (!(edev->physfn)) {
> pr_warn("%s: EEH dev %04x:%02x:%02x.%01x not for VF\n",
> - __func__, edev->phb->global_number, pdn->busno,
> + __func__, pdn->phb->global_number, pdn->busno,
> PCI_SLOT(pdn->devfn), PCI_FUNC(pdn->devfn));
> return NULL;
> }
> diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
> index 84d79f3da7d6..419c3f07afd5 100644
> --- a/arch/powerpc/kernel/eeh_pe.c
> +++ b/arch/powerpc/kernel/eeh_pe.c
> @@ -339,11 +339,12 @@ static struct eeh_pe *eeh_pe_get_parent(struct eeh_dev *edev)
> int eeh_add_to_parent_pe(struct eeh_dev *edev)
> {
> struct eeh_pe *pe, *parent;
> + struct pci_dn *pdn = eeh_dev_to_pdn(edev);
>
> /* Check if the PE number is valid */
> if (!eeh_has_flag(EEH_VALID_PE_ZERO) && !edev->pe_config_addr) {
> pr_err("%s: Invalid PE#0 for edev 0x%x on PHB#%x\n",
> - __func__, edev->config_addr, edev->phb->global_number);
> + __func__, edev->config_addr, pdn->phb->global_number);
> return -EINVAL;
> }
>
> @@ -353,7 +354,7 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
> * PE should be composed of PCI bus and its subordinate
> * components.
> */
> - pe = eeh_pe_get(edev->pdn->phb, edev->pe_config_addr,
> + pe = eeh_pe_get(pdn->phb, edev->pe_config_addr,
> edev->config_addr);
> if (pe && !(pe->type & EEH_PE_INVALID)) {
> /* Mark the PE as type of PCI bus */
> @@ -363,7 +364,7 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
> /* Put the edev to PE */
> list_add_tail(&edev->list, &pe->edevs);
> pr_debug("EEH: Add %04x:%02x:%02x.%01x to Bus PE#%x\n",
> - edev->phb->global_number,
> + pdn->phb->global_number,
> edev->config_addr >> 8,
> PCI_SLOT(edev->config_addr & 0xFF),
> PCI_FUNC(edev->config_addr & 0xFF),
> @@ -386,7 +387,7 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
>
> pr_debug("EEH: Add %04x:%02x:%02x.%01x to Device "
> "PE#%x, Parent PE#%x\n",
> - edev->phb->global_number,
> + pdn->phb->global_number,
> edev->config_addr >> 8,
> PCI_SLOT(edev->config_addr & 0xFF),
> PCI_FUNC(edev->config_addr & 0xFF),
> @@ -396,9 +397,9 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
>
> /* Create a new EEH PE */
> if (edev->physfn)
> - pe = eeh_pe_alloc(edev->phb, EEH_PE_VF);
> + pe = eeh_pe_alloc(pdn->phb, EEH_PE_VF);
> else
> - pe = eeh_pe_alloc(edev->phb, EEH_PE_DEVICE);
> + pe = eeh_pe_alloc(pdn->phb, EEH_PE_DEVICE);
> if (!pe) {
> pr_err("%s: out of memory!\n", __func__);
> return -ENOMEM;
> @@ -414,10 +415,10 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
> */
> parent = eeh_pe_get_parent(edev);
> if (!parent) {
> - parent = eeh_phb_pe_get(edev->phb);
> + parent = eeh_phb_pe_get(pdn->phb);
> if (!parent) {
> pr_err("%s: No PHB PE is found (PHB Domain=%d)\n",
> - __func__, edev->phb->global_number);
> + __func__, pdn->phb->global_number);
> edev->pe = NULL;
> kfree(pe);
> return -EEXIST;
> @@ -434,7 +435,7 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
> edev->pe = pe;
> pr_debug("EEH: Add %04x:%02x:%02x.%01x to "
> "Device PE#%x, Parent PE#%x\n",
> - edev->phb->global_number,
> + pdn->phb->global_number,
> edev->config_addr >> 8,
> PCI_SLOT(edev->config_addr & 0xFF),
> PCI_FUNC(edev->config_addr & 0xFF),
> @@ -456,10 +457,11 @@ int eeh_rmv_from_parent_pe(struct eeh_dev *edev)
> {
> struct eeh_pe *pe, *parent, *child;
> int cnt;
> + struct pci_dn *pdn = eeh_dev_to_pdn(edev);
>
> if (!edev->pe) {
> pr_debug("%s: No PE found for device %04x:%02x:%02x.%01x\n",
> - __func__, edev->phb->global_number,
> + __func__, pdn->phb->global_number,
> edev->config_addr >> 8,
> PCI_SLOT(edev->config_addr & 0xFF),
> PCI_FUNC(edev->config_addr & 0xFF));
> @@ -722,7 +724,7 @@ static void eeh_bridge_check_link(struct eeh_dev *edev)
> return;
>
> pr_debug("%s: Check PCIe link for %04x:%02x:%02x.%01x ...\n",
> - __func__, edev->phb->global_number,
> + __func__, pdn->phb->global_number,
> edev->config_addr >> 8,
> PCI_SLOT(edev->config_addr & 0xFF),
> PCI_FUNC(edev->config_addr & 0xFF));
> diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
> index ac8c01cd251c..552b0cd4e8ba 100644
> --- a/arch/powerpc/platforms/powernv/eeh-powernv.c
> +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
> @@ -926,7 +926,6 @@ void pnv_pci_reset_secondary_bus(struct pci_dev *dev)
> static void pnv_eeh_wait_for_pending(struct pci_dn *pdn, const char *type,
> int pos, u16 mask)
> {
> - struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
> int i, status = 0;
>
> /* Wait for Transaction Pending bit to be cleared */
> @@ -940,7 +939,7 @@ static void pnv_eeh_wait_for_pending(struct pci_dn *pdn, const char *type,
>
> pr_warn("%s: Pending transaction while issuing %sFLR to %04x:%02x:%02x.%01x\n",
> __func__, type,
> - edev->phb->global_number, pdn->busno,
> + pdn->phb->global_number, pdn->busno,
> PCI_SLOT(pdn->devfn), PCI_FUNC(pdn->devfn));
> }
>
> @@ -1714,7 +1713,7 @@ static int pnv_eeh_restore_config(struct pci_dn *pdn)
> if (edev->physfn) {
> ret = pnv_eeh_restore_vf_config(pdn);
> } else {
> - phb = edev->phb->private_data;
> + phb = pdn->phb->private_data;
> ret = opal_pci_reinit(phb->opal_id,
> OPAL_REINIT_PCI_DEV, edev->config_addr);
> }
>
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com IBM Australia Limited
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH kernel 3/5] powerpc/eeh: Remove unnecessary pointer to phb from eeh_dev
2017-08-23 10:18 ` [PATCH kernel 3/5] powerpc/eeh: Remove unnecessary pointer to phb from eeh_dev Alexey Kardashevskiy
2017-08-24 4:45 ` Andrew Donnellan
@ 2017-08-28 6:25 ` Michael Ellerman
1 sibling, 0 replies; 17+ messages in thread
From: Michael Ellerman @ 2017-08-28 6:25 UTC (permalink / raw)
To: Alexey Kardashevskiy, linuxppc-dev
Cc: Alexey Kardashevskiy, Alistair Popple, David Gibson
Alexey Kardashevskiy <aik@ozlabs.ru> writes:
> The eeh_dev struct already holds a pointer to pci_dn which it does not
> exist without and pci_dn itself holds the very same pointer so just
> use it.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> arch/powerpc/include/asm/eeh.h | 1 -
> arch/powerpc/kernel/eeh.c | 7 +++----
> arch/powerpc/kernel/eeh_dev.c | 2 --
> arch/powerpc/kernel/eeh_driver.c | 2 +-
> arch/powerpc/kernel/eeh_pe.c | 24 +++++++++++++---------=
--
> arch/powerpc/platforms/powernv/eeh-powernv.c | 5 ++---
Doesn't build ?
arch/powerpc/platforms/pseries/eeh_pseries.c: In function =E2=80=98pserie=
s_eeh_probe=E2=80=99:
arch/powerpc/platforms/pseries/eeh_pseries.c:250:15: error: =E2=80=98stru=
ct eeh_dev=E2=80=99 has no member named =E2=80=98phb=E2=80=99
pe.phb =3D edev->phb;
^~
cheers
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH kernel 4/5] powerpc/eeh: Remove unnecessary config_addr from eeh_dev
2017-08-23 10:18 [PATCH kernel 0/5] powerpc/eeh: Some cleanups Alexey Kardashevskiy
` (2 preceding siblings ...)
2017-08-23 10:18 ` [PATCH kernel 3/5] powerpc/eeh: Remove unnecessary pointer to phb from eeh_dev Alexey Kardashevskiy
@ 2017-08-23 10:19 ` Alexey Kardashevskiy
2017-08-28 6:27 ` Michael Ellerman
2017-08-23 10:19 ` [PATCH kernel 5/5] powerpc/eeh: Remove unnecessary node from pci_dn Alexey Kardashevskiy
` (5 subsequent siblings)
9 siblings, 1 reply; 17+ messages in thread
From: Alexey Kardashevskiy @ 2017-08-23 10:19 UTC (permalink / raw)
To: linuxppc-dev
Cc: Alexey Kardashevskiy, David Gibson, Benjamin Herrenschmidt,
Alistair Popple, Russell Currey
The eeh_dev struct hold a config space address of an associated node
and the very same address is also stored in the pci_dn struct which
is always present during the eeh_dev lifetime.
This uses bus:devfn directly from pci_dn instead of cached and packed
config_addr.
Since config_addr is made from device's bus:dev.fn, there is no point
in keeping it in the debugfs either so remove that too.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
arch/powerpc/include/asm/eeh.h | 1 -
arch/powerpc/kernel/eeh_pe.c | 42 ++++++++++++++--------------
arch/powerpc/kernel/eeh_sysfs.c | 3 --
arch/powerpc/platforms/powernv/eeh-powernv.c | 9 +++---
4 files changed, 26 insertions(+), 29 deletions(-)
diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 777d37aa0a7f..9847ae3a12d1 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -131,7 +131,6 @@ static inline bool eeh_pe_passed(struct eeh_pe *pe)
struct eeh_dev {
int mode; /* EEH mode */
int class_code; /* Class code of the device */
- int config_addr; /* Config address */
int pe_config_addr; /* PE config address */
u32 config_space[16]; /* Saved PCI config space */
int pcix_cap; /* Saved PCIx capability */
diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
index 419c3f07afd5..2e8d1b2b5af4 100644
--- a/arch/powerpc/kernel/eeh_pe.c
+++ b/arch/powerpc/kernel/eeh_pe.c
@@ -340,11 +340,12 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
{
struct eeh_pe *pe, *parent;
struct pci_dn *pdn = eeh_dev_to_pdn(edev);
+ int config_addr = (pdn->busno << 8) | (pdn->devfn);
/* Check if the PE number is valid */
if (!eeh_has_flag(EEH_VALID_PE_ZERO) && !edev->pe_config_addr) {
pr_err("%s: Invalid PE#0 for edev 0x%x on PHB#%x\n",
- __func__, edev->config_addr, pdn->phb->global_number);
+ __func__, config_addr, pdn->phb->global_number);
return -EINVAL;
}
@@ -354,8 +355,7 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
* PE should be composed of PCI bus and its subordinate
* components.
*/
- pe = eeh_pe_get(pdn->phb, edev->pe_config_addr,
- edev->config_addr);
+ pe = eeh_pe_get(pdn->phb, edev->pe_config_addr, config_addr);
if (pe && !(pe->type & EEH_PE_INVALID)) {
/* Mark the PE as type of PCI bus */
pe->type = EEH_PE_BUS;
@@ -365,10 +365,10 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
list_add_tail(&edev->list, &pe->edevs);
pr_debug("EEH: Add %04x:%02x:%02x.%01x to Bus PE#%x\n",
pdn->phb->global_number,
- edev->config_addr >> 8,
- PCI_SLOT(edev->config_addr & 0xFF),
- PCI_FUNC(edev->config_addr & 0xFF),
- pe->addr);
+ pdn->busno,
+ PCI_SLOT(pdn->devfn),
+ PCI_FUNC(pdn->devfn),
+ pe->addr);
return 0;
} else if (pe && (pe->type & EEH_PE_INVALID)) {
list_add_tail(&edev->list, &pe->edevs);
@@ -388,10 +388,10 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
pr_debug("EEH: Add %04x:%02x:%02x.%01x to Device "
"PE#%x, Parent PE#%x\n",
pdn->phb->global_number,
- edev->config_addr >> 8,
- PCI_SLOT(edev->config_addr & 0xFF),
- PCI_FUNC(edev->config_addr & 0xFF),
- pe->addr, pe->parent->addr);
+ pdn->busno,
+ PCI_SLOT(pdn->devfn),
+ PCI_FUNC(pdn->devfn),
+ pe->addr, pe->parent->addr);
return 0;
}
@@ -405,7 +405,7 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
return -ENOMEM;
}
pe->addr = edev->pe_config_addr;
- pe->config_addr = edev->config_addr;
+ pe->config_addr = config_addr;
/*
* Put the new EEH PE into hierarchy tree. If the parent
@@ -436,9 +436,9 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
pr_debug("EEH: Add %04x:%02x:%02x.%01x to "
"Device PE#%x, Parent PE#%x\n",
pdn->phb->global_number,
- edev->config_addr >> 8,
- PCI_SLOT(edev->config_addr & 0xFF),
- PCI_FUNC(edev->config_addr & 0xFF),
+ pdn->busno,
+ PCI_SLOT(pdn->devfn),
+ PCI_FUNC(pdn->devfn),
pe->addr, pe->parent->addr);
return 0;
@@ -462,9 +462,9 @@ int eeh_rmv_from_parent_pe(struct eeh_dev *edev)
if (!edev->pe) {
pr_debug("%s: No PE found for device %04x:%02x:%02x.%01x\n",
__func__, pdn->phb->global_number,
- edev->config_addr >> 8,
- PCI_SLOT(edev->config_addr & 0xFF),
- PCI_FUNC(edev->config_addr & 0xFF));
+ pdn->busno,
+ PCI_SLOT(pdn->devfn),
+ PCI_FUNC(pdn->devfn));
return -EEXIST;
}
@@ -725,9 +725,9 @@ static void eeh_bridge_check_link(struct eeh_dev *edev)
pr_debug("%s: Check PCIe link for %04x:%02x:%02x.%01x ...\n",
__func__, pdn->phb->global_number,
- edev->config_addr >> 8,
- PCI_SLOT(edev->config_addr & 0xFF),
- PCI_FUNC(edev->config_addr & 0xFF));
+ pdn->busno,
+ PCI_SLOT(pdn->devfn),
+ PCI_FUNC(pdn->devfn));
/* Check slot status */
cap = edev->pcie_cap;
diff --git a/arch/powerpc/kernel/eeh_sysfs.c b/arch/powerpc/kernel/eeh_sysfs.c
index 1ceecdda810b..797549289798 100644
--- a/arch/powerpc/kernel/eeh_sysfs.c
+++ b/arch/powerpc/kernel/eeh_sysfs.c
@@ -51,7 +51,6 @@ static ssize_t eeh_show_##_name(struct device *dev, \
static DEVICE_ATTR(_name, S_IRUGO, eeh_show_##_name, NULL);
EEH_SHOW_ATTR(eeh_mode, mode, "0x%x");
-EEH_SHOW_ATTR(eeh_config_addr, config_addr, "0x%x");
EEH_SHOW_ATTR(eeh_pe_config_addr, pe_config_addr, "0x%x");
static ssize_t eeh_pe_state_show(struct device *dev,
@@ -103,7 +102,6 @@ void eeh_sysfs_add_device(struct pci_dev *pdev)
return;
rc += device_create_file(&pdev->dev, &dev_attr_eeh_mode);
- rc += device_create_file(&pdev->dev, &dev_attr_eeh_config_addr);
rc += device_create_file(&pdev->dev, &dev_attr_eeh_pe_config_addr);
rc += device_create_file(&pdev->dev, &dev_attr_eeh_pe_state);
@@ -128,7 +126,6 @@ void eeh_sysfs_remove_device(struct pci_dev *pdev)
}
device_remove_file(&pdev->dev, &dev_attr_eeh_mode);
- device_remove_file(&pdev->dev, &dev_attr_eeh_config_addr);
device_remove_file(&pdev->dev, &dev_attr_eeh_pe_config_addr);
device_remove_file(&pdev->dev, &dev_attr_eeh_pe_state);
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index 552b0cd4e8ba..8864065eba22 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -352,6 +352,7 @@ static void *pnv_eeh_probe(struct pci_dn *pdn, void *data)
struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
uint32_t pcie_flags;
int ret;
+ int config_addr = (pdn->busno << 8) | (pdn->devfn);
/*
* When probing the root bridge, which doesn't have any
@@ -386,8 +387,7 @@ static void *pnv_eeh_probe(struct pci_dn *pdn, void *data)
}
}
- edev->config_addr = (pdn->busno << 8) | (pdn->devfn);
- edev->pe_config_addr = phb->ioda.pe_rmap[edev->config_addr];
+ edev->pe_config_addr = phb->ioda.pe_rmap[config_addr];
/* Create PE */
ret = eeh_add_to_parent_pe(edev);
@@ -1699,6 +1699,7 @@ static int pnv_eeh_restore_config(struct pci_dn *pdn)
struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
struct pnv_phb *phb;
s64 ret;
+ int config_addr = (pdn->busno << 8) | (pdn->devfn);
if (!edev)
return -EEXIST;
@@ -1715,12 +1716,12 @@ static int pnv_eeh_restore_config(struct pci_dn *pdn)
} else {
phb = pdn->phb->private_data;
ret = opal_pci_reinit(phb->opal_id,
- OPAL_REINIT_PCI_DEV, edev->config_addr);
+ OPAL_REINIT_PCI_DEV, config_addr);
}
if (ret) {
pr_warn("%s: Can't reinit PCI dev 0x%x (%lld)\n",
- __func__, edev->config_addr, ret);
+ __func__, config_addr, ret);
return -EIO;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH kernel 4/5] powerpc/eeh: Remove unnecessary config_addr from eeh_dev
2017-08-23 10:19 ` [PATCH kernel 4/5] powerpc/eeh: Remove unnecessary config_addr " Alexey Kardashevskiy
@ 2017-08-28 6:27 ` Michael Ellerman
0 siblings, 0 replies; 17+ messages in thread
From: Michael Ellerman @ 2017-08-28 6:27 UTC (permalink / raw)
To: Alexey Kardashevskiy, linuxppc-dev
Cc: Alexey Kardashevskiy, Alistair Popple, David Gibson
Alexey Kardashevskiy <aik@ozlabs.ru> writes:
> The eeh_dev struct hold a config space address of an associated node
> and the very same address is also stored in the pci_dn struct which
> is always present during the eeh_dev lifetime.
>
> This uses bus:devfn directly from pci_dn instead of cached and packed
> config_addr.
>
> Since config_addr is made from device's bus:dev.fn, there is no point
> in keeping it in the debugfs either so remove that too.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> arch/powerpc/include/asm/eeh.h | 1 -
> arch/powerpc/kernel/eeh_pe.c | 42 ++++++++++++++--------=
------
> arch/powerpc/kernel/eeh_sysfs.c | 3 --
> arch/powerpc/platforms/powernv/eeh-powernv.c | 9 +++---
arch/powerpc/platforms/pseries/eeh_pseries.c:257:7: error: =E2=80=98struc=
t eeh_dev=E2=80=99 has no member named =E2=80=98config_addr=E2=80=99; did y=
ou mean =E2=80=98pe_config_addr=E2=80=99?
edev->config_addr =3D (pdn->busno << 16) | (pdn->devfn << 8);
^~
arch/powerpc/platforms/pseries/eeh_pseries.c:282:8: error: =E2=80=98struc=
t eeh_dev=E2=80=99 has no member named =E2=80=98config_addr=E2=80=99; did y=
ou mean =E2=80=98pe_config_addr=E2=80=99?
edev->config_addr =3D pdn_to_eeh_dev(pdn->parent)->config_addr;
^~
arch/powerpc/platforms/pseries/eeh_pseries.c:282:51: error: =E2=80=98stru=
ct eeh_dev=E2=80=99 has no member named =E2=80=98config_addr=E2=80=99; did =
you mean =E2=80=98pe_config_addr=E2=80=99?
edev->config_addr =3D pdn_to_eeh_dev(pdn->parent)->config_addr;
^~
cheers
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH kernel 5/5] powerpc/eeh: Remove unnecessary node from pci_dn
2017-08-23 10:18 [PATCH kernel 0/5] powerpc/eeh: Some cleanups Alexey Kardashevskiy
` (3 preceding siblings ...)
2017-08-23 10:19 ` [PATCH kernel 4/5] powerpc/eeh: Remove unnecessary config_addr " Alexey Kardashevskiy
@ 2017-08-23 10:19 ` Alexey Kardashevskiy
2017-08-28 6:27 ` Michael Ellerman
2017-08-24 1:13 ` [PATCH kernel 0/5] powerpc/eeh: Some cleanups Russell Currey
` (4 subsequent siblings)
9 siblings, 1 reply; 17+ messages in thread
From: Alexey Kardashevskiy @ 2017-08-23 10:19 UTC (permalink / raw)
To: linuxppc-dev
Cc: Alexey Kardashevskiy, David Gibson, Benjamin Herrenschmidt,
Alistair Popple, Russell Currey
The pci_dn struct caches a OF device node pointer in order to access
the "ibm,loc-code" property when EEH is recovering.
However, when this happens in eeh_dev_check_failure(), we also have
a pci_dev pointer which should have a valid pointer to the device node
when pci_dn has one (both pointers are not NULL for physical functions
and are NULL for virtual functions).
This removes the node pointer from the pci_dn struct and used pdev
in eeh_dev_check_failure() instead.
This changes pci_remove_device_node_info() to look for a parent of
the node being removed, just like pci_add_device_node_info() does when it
references the parent node.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
arch/powerpc/include/asm/pci-bridge.h | 1 -
arch/powerpc/kernel/eeh.c | 9 +++++----
arch/powerpc/kernel/pci_dn.c | 8 +++++---
3 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
index 56c67d3f0108..0b8aa1fe2d5f 100644
--- a/arch/powerpc/include/asm/pci-bridge.h
+++ b/arch/powerpc/include/asm/pci-bridge.h
@@ -195,7 +195,6 @@ struct pci_dn {
struct pci_dn *parent;
struct pci_controller *phb; /* for pci devices */
struct iommu_table_group *table_group; /* for phb's or bridges */
- struct device_node *node; /* back-pointer to the device_node */
int pci_ext_config_space; /* for pci devices */
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index c877014d11ce..2c926029adbe 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -435,7 +435,7 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
int ret;
int active_flags = (EEH_STATE_MMIO_ACTIVE | EEH_STATE_DMA_ACTIVE);
unsigned long flags;
- struct pci_dn *pdn;
+ struct device_node *dn;
struct pci_dev *dev;
struct eeh_pe *pe, *parent_pe, *phb_pe;
int rc = 0;
@@ -493,9 +493,10 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
if (pe->state & EEH_PE_ISOLATED) {
pe->check_count++;
if (pe->check_count % EEH_MAX_FAILS == 0) {
- pdn = eeh_dev_to_pdn(edev);
- if (pdn->node)
- location = of_get_property(pdn->node, "ibm,loc-code", NULL);
+ dn = pci_device_to_OF_node(dev);
+ if (dn)
+ location = of_get_property(dn, "ibm,loc-code",
+ NULL);
printk(KERN_ERR "EEH: %d reads ignored for recovering device at "
"location=%s driver=%s pci addr=%s\n",
pe->check_count,
diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
index 0256372b72de..0e395afbf0f4 100644
--- a/arch/powerpc/kernel/pci_dn.c
+++ b/arch/powerpc/kernel/pci_dn.c
@@ -293,7 +293,6 @@ struct pci_dn *pci_add_device_node_info(struct pci_controller *hose,
if (pdn == NULL)
return NULL;
dn->data = pdn;
- pdn->node = dn;
pdn->phb = hose;
#ifdef CONFIG_PPC_POWERNV
pdn->pe_number = IODA_INVALID_PE;
@@ -342,6 +341,7 @@ EXPORT_SYMBOL_GPL(pci_add_device_node_info);
void pci_remove_device_node_info(struct device_node *dn)
{
struct pci_dn *pdn = dn ? PCI_DN(dn) : NULL;
+ struct device_node *parent;
#ifdef CONFIG_EEH
struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
@@ -354,8 +354,10 @@ void pci_remove_device_node_info(struct device_node *dn)
WARN_ON(!list_empty(&pdn->child_list));
list_del(&pdn->list);
- if (pdn->parent)
- of_node_put(pdn->parent->node);
+
+ parent = of_get_parent(dn);
+ if (parent)
+ of_node_put(parent);
dn->data = NULL;
kfree(pdn);
--
2.11.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH kernel 5/5] powerpc/eeh: Remove unnecessary node from pci_dn
2017-08-23 10:19 ` [PATCH kernel 5/5] powerpc/eeh: Remove unnecessary node from pci_dn Alexey Kardashevskiy
@ 2017-08-28 6:27 ` Michael Ellerman
0 siblings, 0 replies; 17+ messages in thread
From: Michael Ellerman @ 2017-08-28 6:27 UTC (permalink / raw)
To: Alexey Kardashevskiy, linuxppc-dev
Cc: Alexey Kardashevskiy, Alistair Popple, David Gibson
Alexey Kardashevskiy <aik@ozlabs.ru> writes:
> The pci_dn struct caches a OF device node pointer in order to access
> the "ibm,loc-code" property when EEH is recovering.
>
> However, when this happens in eeh_dev_check_failure(), we also have
> a pci_dev pointer which should have a valid pointer to the device node
> when pci_dn has one (both pointers are not NULL for physical functions
> and are NULL for virtual functions).
>
> This removes the node pointer from the pci_dn struct and used pdev
> in eeh_dev_check_failure() instead.
>
> This changes pci_remove_device_node_info() to look for a parent of
> the node being removed, just like pci_add_device_node_info() does when it
> references the parent node.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> arch/powerpc/include/asm/pci-bridge.h | 1 -
> arch/powerpc/kernel/eeh.c | 9 +++++----
> arch/powerpc/kernel/pci_dn.c | 8 +++++---
> 3 files changed, 10 insertions(+), 8 deletions(-)
arch/powerpc/platforms/pseries/msi.c: In function =E2=80=98check_req=E2=
=80=99:
arch/powerpc/platforms/pseries/msi.c:143:10: error: =E2=80=98struct pci_d=
n=E2=80=99 has no member named =E2=80=98node=E2=80=99
dn =3D pdn->node;
^~
arch/powerpc/platforms/pseries/msi.c: In function =E2=80=98find_pe_dn=E2=
=80=99:
arch/powerpc/platforms/pseries/msi.c:214:16: error: =E2=80=98struct pci_d=
n=E2=80=99 has no member named =E2=80=98node=E2=80=99
dn =3D pdn ? pdn->node : NULL;
^~
cheers
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH kernel 0/5] powerpc/eeh: Some cleanups
2017-08-23 10:18 [PATCH kernel 0/5] powerpc/eeh: Some cleanups Alexey Kardashevskiy
` (4 preceding siblings ...)
2017-08-23 10:19 ` [PATCH kernel 5/5] powerpc/eeh: Remove unnecessary node from pci_dn Alexey Kardashevskiy
@ 2017-08-24 1:13 ` Russell Currey
2017-08-24 2:19 ` Alexey Kardashevskiy
[not found] ` <20170823102823.7F12CC603E@b03ledav006.gho.boulder.ibm.com>
` (3 subsequent siblings)
9 siblings, 1 reply; 17+ messages in thread
From: Russell Currey @ 2017-08-24 1:13 UTC (permalink / raw)
To: Alexey Kardashevskiy, linuxppc-dev
Cc: David Gibson, Benjamin Herrenschmidt, Alistair Popple
On Wed, 2017-08-23 at 20:18 +1000, Alexey Kardashevskiy wrote:
> Here are few patches to get rid of some cached pointers across EEH and
> powernv code as I was struggling to figure out about lifetime of
> structures and so on.
Thanks for the patches. For the whole series:
Acked-by: Russell Currey <ruscur@russell.cc>
>
>
> This is based on sha1
> 98b9f8a45499 Linus Torvalds Merge tag 'ext4_for_linus_stable' of
> git://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4
>
> Please comment. Thanks.
>
>
>
> Alexey Kardashevskiy (5):
> powerpc/pci: Remove unused parameter from add_one_dev_pci_data()
> powerpc/eeh: Reduce to one the number of places where edev is
> allocated
> powerpc/eeh: Remove unnecessary pointer to phb from eeh_dev
> powerpc/eeh: Remove unnecessary config_addr from eeh_dev
> powerpc/eeh: Remove unnecessary node from pci_dn
>
> arch/powerpc/include/asm/eeh.h | 5 +-
> arch/powerpc/include/asm/pci-bridge.h | 1 -
> arch/powerpc/kernel/eeh.c | 16 ++---
> arch/powerpc/kernel/eeh_dev.c | 2 -
> arch/powerpc/kernel/eeh_driver.c | 2 +-
> arch/powerpc/kernel/eeh_pe.c | 90 ++++++++++++++++-----------
> -
> arch/powerpc/kernel/eeh_sysfs.c | 3 -
> arch/powerpc/kernel/pci_dn.c | 22 +++----
> arch/powerpc/platforms/powernv/eeh-powernv.c | 29 +++------
> 9 files changed, 78 insertions(+), 92 deletions(-)
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH kernel 0/5] powerpc/eeh: Some cleanups
2017-08-24 1:13 ` [PATCH kernel 0/5] powerpc/eeh: Some cleanups Russell Currey
@ 2017-08-24 2:19 ` Alexey Kardashevskiy
2017-08-24 5:29 ` Andrew Donnellan
0 siblings, 1 reply; 17+ messages in thread
From: Alexey Kardashevskiy @ 2017-08-24 2:19 UTC (permalink / raw)
To: Russell Currey, linuxppc-dev
Cc: David Gibson, Benjamin Herrenschmidt, Alistair Popple
On 24/08/17 11:13, Russell Currey wrote:
> On Wed, 2017-08-23 at 20:18 +1000, Alexey Kardashevskiy wrote:
>> Here are few patches to get rid of some cached pointers across EEH and
>> powernv code as I was struggling to figure out about lifetime of
>> structures and so on.
>
> Thanks for the patches. For the whole series:
>
> Acked-by: Russell Currey <ruscur@russell.cc>
The patches are labelled as "Needs Review / ACK" in the patchworks, and
until you reply to each individual patch, the "A" part of the "A/R/T"
column will remain "- - -".
>
>>
>>
>> This is based on sha1
>> 98b9f8a45499 Linus Torvalds Merge tag 'ext4_for_linus_stable' of
>> git://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4
>>
>> Please comment. Thanks.
>>
>>
>>
>> Alexey Kardashevskiy (5):
>> powerpc/pci: Remove unused parameter from add_one_dev_pci_data()
>> powerpc/eeh: Reduce to one the number of places where edev is
>> allocated
>> powerpc/eeh: Remove unnecessary pointer to phb from eeh_dev
>> powerpc/eeh: Remove unnecessary config_addr from eeh_dev
>> powerpc/eeh: Remove unnecessary node from pci_dn
>>
>> arch/powerpc/include/asm/eeh.h | 5 +-
>> arch/powerpc/include/asm/pci-bridge.h | 1 -
>> arch/powerpc/kernel/eeh.c | 16 ++---
>> arch/powerpc/kernel/eeh_dev.c | 2 -
>> arch/powerpc/kernel/eeh_driver.c | 2 +-
>> arch/powerpc/kernel/eeh_pe.c | 90 ++++++++++++++++-----------
>> -
>> arch/powerpc/kernel/eeh_sysfs.c | 3 -
>> arch/powerpc/kernel/pci_dn.c | 22 +++----
>> arch/powerpc/platforms/powernv/eeh-powernv.c | 29 +++------
>> 9 files changed, 78 insertions(+), 92 deletions(-)
>>
>
--
Alexey
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH kernel 0/5] powerpc/eeh: Some cleanups
2017-08-24 2:19 ` Alexey Kardashevskiy
@ 2017-08-24 5:29 ` Andrew Donnellan
0 siblings, 0 replies; 17+ messages in thread
From: Andrew Donnellan @ 2017-08-24 5:29 UTC (permalink / raw)
To: Alexey Kardashevskiy, Russell Currey, linuxppc-dev
Cc: Alistair Popple, David Gibson
On 24/08/17 12:19, Alexey Kardashevskiy wrote:
> On 24/08/17 11:13, Russell Currey wrote:
>> On Wed, 2017-08-23 at 20:18 +1000, Alexey Kardashevskiy wrote:
>>> Here are few patches to get rid of some cached pointers across EEH and
>>> powernv code as I was struggling to figure out about lifetime of
>>> structures and so on.
>>
>> Thanks for the patches. For the whole series:
>>
>> Acked-by: Russell Currey <ruscur@russell.cc>
>
>
> The patches are labelled as "Needs Review / ACK" in the patchworks, and
> until you reply to each individual patch, the "A" part of the "A/R/T"
> column will remain "- - -".
https://github.com/getpatchwork/patchwork/issues/113
Too late for the just-released patchwork 2.0, but maybe 2.1 won't be
that far off. :)
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com IBM Australia Limited
^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <20170823102823.7F12CC603E@b03ledav006.gho.boulder.ibm.com>]
* Re: [PATCH kernel 1/5] powerpc/pci: Remove unused parameter from add_one_dev_pci_data()
[not found] ` <20170823102823.7F12CC603E@b03ledav006.gho.boulder.ibm.com>
@ 2017-08-24 4:21 ` Andrew Donnellan
0 siblings, 0 replies; 17+ messages in thread
From: Andrew Donnellan @ 2017-08-24 4:21 UTC (permalink / raw)
To: Alexey Kardashevskiy, linuxppc-dev; +Cc: Alistair Popple, David Gibson
On 23/08/17 20:18, Alexey Kardashevskiy wrote:
> pdev is always NULL, remove it.
>
> To make checkpatch.pl happy, this also removes the "out of memory"
> message.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> ---
> arch/powerpc/kernel/pci_dn.c | 14 ++------------
> 1 file changed, 2 insertions(+), 12 deletions(-)
>
> diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
> index 592693437070..0256372b72de 100644
> --- a/arch/powerpc/kernel/pci_dn.c
> +++ b/arch/powerpc/kernel/pci_dn.c
> @@ -139,7 +139,6 @@ struct pci_dn *pci_get_pdn(struct pci_dev *pdev)
>
> #ifdef CONFIG_PCI_IOV
> static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
> - struct pci_dev *pdev,
> int vf_index,
> int busno, int devfn)
> {
> @@ -150,10 +149,8 @@ static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
> return NULL;
>
> pdn = kzalloc(sizeof(*pdn), GFP_KERNEL);
> - if (!pdn) {
> - dev_warn(&pdev->dev, "%s: Out of memory!\n", __func__);
> + if (!pdn)
> return NULL;
> - }
>
> pdn->phb = parent->phb;
> pdn->parent = parent;
> @@ -167,13 +164,6 @@ static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
> INIT_LIST_HEAD(&pdn->list);
> list_add_tail(&pdn->list, &parent->child_list);
>
> - /*
> - * If we already have PCI device instance, lets
> - * bind them.
> - */
> - if (pdev)
> - pdev->dev.archdata.pci_data = pdn;
> -
> return pdn;
> }
> #endif
> @@ -201,7 +191,7 @@ struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
> for (i = 0; i < pci_sriov_get_totalvfs(pdev); i++) {
> struct eeh_dev *edev __maybe_unused;
>
> - pdn = add_one_dev_pci_data(parent, NULL, i,
> + pdn = add_one_dev_pci_data(parent, i,
> pci_iov_virtfn_bus(pdev, i),
> pci_iov_virtfn_devfn(pdev, i));
> if (!pdn) {
>
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com IBM Australia Limited
^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <20170823101926.03970124035@b01ledav002.gho.pok.ibm.com>]
* Re: [PATCH kernel 2/5] powerpc/eeh: Reduce to one the number of places where edev is allocated
[not found] ` <20170823101926.03970124035@b01ledav002.gho.pok.ibm.com>
@ 2017-08-24 4:34 ` Andrew Donnellan
0 siblings, 0 replies; 17+ messages in thread
From: Andrew Donnellan @ 2017-08-24 4:34 UTC (permalink / raw)
To: Alexey Kardashevskiy, linuxppc-dev; +Cc: Alistair Popple, David Gibson
On 23/08/17 20:18, Alexey Kardashevskiy wrote:
> arch/powerpc/kernel/eeh_dev.c:57 is the only legit place where edev
> is allocated; other 2 places allocate it on stack and in the heap for
> a very short period of time to use eeh_pe_get() as takes edev.
>
> This changes eeh_pe_get() to receive required parameters explicitly.
>
> This removes unnecessary temporary allocation of edev.
>
> This uses the "pe_no" name instead of the "pe_config_addr" name as
> it actually is a PE number and not a config space address as it seemed.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> ---
> arch/powerpc/include/asm/eeh.h | 3 ++-
> arch/powerpc/kernel/eeh_pe.c | 32 ++++++++++++++++++----------
> arch/powerpc/platforms/powernv/eeh-powernv.c | 15 ++-----------
> 3 files changed, 25 insertions(+), 25 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
> index 8e37b71674f4..26a6a43f8799 100644
> --- a/arch/powerpc/include/asm/eeh.h
> +++ b/arch/powerpc/include/asm/eeh.h
> @@ -262,7 +262,8 @@ typedef void *(*eeh_traverse_func)(void *data, void *flag);
> void eeh_set_pe_aux_size(int size);
> int eeh_phb_pe_create(struct pci_controller *phb);
> struct eeh_pe *eeh_phb_pe_get(struct pci_controller *phb);
> -struct eeh_pe *eeh_pe_get(struct eeh_dev *edev);
> +struct eeh_pe *eeh_pe_get(struct pci_controller *phb,
> + int pe_no, int config_addr);
> int eeh_add_to_parent_pe(struct eeh_dev *edev);
> int eeh_rmv_from_parent_pe(struct eeh_dev *edev);
> void eeh_pe_update_time_stamp(struct eeh_pe *pe);
> diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
> index cc4b206f77e4..84d79f3da7d6 100644
> --- a/arch/powerpc/kernel/eeh_pe.c
> +++ b/arch/powerpc/kernel/eeh_pe.c
> @@ -230,10 +230,15 @@ void *eeh_pe_dev_traverse(struct eeh_pe *root,
> * Bus/Device/Function number. The extra data referred by flag
> * indicates which type of address should be used.
> */
> +struct eeh_pe_get_flag {
> + int pe_no;
> + int config_addr;
> +};
> +
> static void *__eeh_pe_get(void *data, void *flag)
> {
> struct eeh_pe *pe = (struct eeh_pe *)data;
> - struct eeh_dev *edev = (struct eeh_dev *)flag;
> + struct eeh_pe_get_flag *tmp = (struct eeh_pe_get_flag *) flag;
>
> /* Unexpected PHB PE */
> if (pe->type & EEH_PE_PHB)
> @@ -244,17 +249,17 @@ static void *__eeh_pe_get(void *data, void *flag)
> * have non-zero PE address
> */
> if (eeh_has_flag(EEH_VALID_PE_ZERO)) {
> - if (edev->pe_config_addr == pe->addr)
> + if (tmp->pe_no == pe->addr)
> return pe;
> } else {
> - if (edev->pe_config_addr &&
> - (edev->pe_config_addr == pe->addr))
> + if (tmp->pe_no &&
> + (tmp->pe_no == pe->addr))
> return pe;
> }
>
> /* Try BDF address */
> - if (edev->config_addr &&
> - (edev->config_addr == pe->config_addr))
> + if (tmp->config_addr &&
> + (tmp->config_addr == pe->config_addr))
> return pe;
>
> return NULL;
> @@ -262,7 +267,9 @@ static void *__eeh_pe_get(void *data, void *flag)
>
> /**
> * eeh_pe_get - Search PE based on the given address
> - * @edev: EEH device
> + * @phb: PCI controller
> + * @pe_no: PE number
> + * @config_addr: Config address
> *
> * Search the corresponding PE based on the specified address which
> * is included in the eeh device. The function is used to check if
> @@ -271,12 +278,14 @@ static void *__eeh_pe_get(void *data, void *flag)
> * which is composed of PCI bus/device/function number, or unified
> * PE address.
> */
> -struct eeh_pe *eeh_pe_get(struct eeh_dev *edev)
> +struct eeh_pe *eeh_pe_get(struct pci_controller *phb,
> + int pe_no, int config_addr)
> {
> - struct eeh_pe *root = eeh_phb_pe_get(edev->phb);
> + struct eeh_pe *root = eeh_phb_pe_get(phb);
> + struct eeh_pe_get_flag tmp = { pe_no, config_addr };
> struct eeh_pe *pe;
>
> - pe = eeh_pe_traverse(root, __eeh_pe_get, edev);
> + pe = eeh_pe_traverse(root, __eeh_pe_get, &tmp);
>
> return pe;
> }
> @@ -344,7 +353,8 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
> * PE should be composed of PCI bus and its subordinate
> * components.
> */
> - pe = eeh_pe_get(edev);
> + pe = eeh_pe_get(edev->pdn->phb, edev->pe_config_addr,
> + edev->config_addr);
> if (pe && !(pe->type & EEH_PE_INVALID)) {
> /* Mark the PE as type of PCI bus */
> pe->type = EEH_PE_BUS;
> diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
> index 3f48f6df1cf3..ac8c01cd251c 100644
> --- a/arch/powerpc/platforms/powernv/eeh-powernv.c
> +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
> @@ -113,7 +113,6 @@ static ssize_t pnv_eeh_ei_write(struct file *filp,
> size_t count, loff_t *ppos)
> {
> struct pci_controller *hose = filp->private_data;
> - struct eeh_dev *edev;
> struct eeh_pe *pe;
> int pe_no, type, func;
> unsigned long addr, mask;
> @@ -135,13 +134,7 @@ static ssize_t pnv_eeh_ei_write(struct file *filp,
> return -EINVAL;
>
> /* Retrieve PE */
> - edev = kzalloc(sizeof(*edev), GFP_KERNEL);
> - if (!edev)
> - return -ENOMEM;
> - edev->phb = hose;
> - edev->pe_config_addr = pe_no;
> - pe = eeh_pe_get(edev);
> - kfree(edev);
> + pe = eeh_pe_get(hose, pe_no, 0);
> if (!pe)
> return -ENODEV;
>
> @@ -1381,7 +1374,6 @@ static int pnv_eeh_get_pe(struct pci_controller *hose,
> struct pnv_phb *phb = hose->private_data;
> struct pnv_ioda_pe *pnv_pe;
> struct eeh_pe *dev_pe;
> - struct eeh_dev edev;
>
> /*
> * If PHB supports compound PE, to fetch
> @@ -1397,10 +1389,7 @@ static int pnv_eeh_get_pe(struct pci_controller *hose,
> }
>
> /* Find the PE according to PE# */
> - memset(&edev, 0, sizeof(struct eeh_dev));
> - edev.phb = hose;
> - edev.pe_config_addr = pe_no;
> - dev_pe = eeh_pe_get(&edev);
> + dev_pe = eeh_pe_get(hose, pe_no, 0);
> if (!dev_pe)
> return -EEXIST;
>
>
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com IBM Australia Limited
^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <20170823102546.29A77AE034@b01ledav005.gho.pok.ibm.com>]
* Re: [PATCH kernel 4/5] powerpc/eeh: Remove unnecessary config_addr from eeh_dev
[not found] ` <20170823102546.29A77AE034@b01ledav005.gho.pok.ibm.com>
@ 2017-08-24 4:55 ` Andrew Donnellan
0 siblings, 0 replies; 17+ messages in thread
From: Andrew Donnellan @ 2017-08-24 4:55 UTC (permalink / raw)
To: Alexey Kardashevskiy, linuxppc-dev; +Cc: Alistair Popple, David Gibson
On 23/08/17 20:19, Alexey Kardashevskiy wrote:
> The eeh_dev struct hold a config space address of an associated node
> and the very same address is also stored in the pci_dn struct which
> is always present during the eeh_dev lifetime.
>
> This uses bus:devfn directly from pci_dn instead of cached and packed
> config_addr.
>
> Since config_addr is made from device's bus:dev.fn, there is no point
> in keeping it in the debugfs either so remove that too.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> ---
> arch/powerpc/include/asm/eeh.h | 1 -
> arch/powerpc/kernel/eeh_pe.c | 42 ++++++++++++++--------------
> arch/powerpc/kernel/eeh_sysfs.c | 3 --
> arch/powerpc/platforms/powernv/eeh-powernv.c | 9 +++---
> 4 files changed, 26 insertions(+), 29 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
> index 777d37aa0a7f..9847ae3a12d1 100644
> --- a/arch/powerpc/include/asm/eeh.h
> +++ b/arch/powerpc/include/asm/eeh.h
> @@ -131,7 +131,6 @@ static inline bool eeh_pe_passed(struct eeh_pe *pe)
> struct eeh_dev {
> int mode; /* EEH mode */
> int class_code; /* Class code of the device */
> - int config_addr; /* Config address */
> int pe_config_addr; /* PE config address */
> u32 config_space[16]; /* Saved PCI config space */
> int pcix_cap; /* Saved PCIx capability */
> diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
> index 419c3f07afd5..2e8d1b2b5af4 100644
> --- a/arch/powerpc/kernel/eeh_pe.c
> +++ b/arch/powerpc/kernel/eeh_pe.c
> @@ -340,11 +340,12 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
> {
> struct eeh_pe *pe, *parent;
> struct pci_dn *pdn = eeh_dev_to_pdn(edev);
> + int config_addr = (pdn->busno << 8) | (pdn->devfn);
>
> /* Check if the PE number is valid */
> if (!eeh_has_flag(EEH_VALID_PE_ZERO) && !edev->pe_config_addr) {
> pr_err("%s: Invalid PE#0 for edev 0x%x on PHB#%x\n",
> - __func__, edev->config_addr, pdn->phb->global_number);
> + __func__, config_addr, pdn->phb->global_number);
> return -EINVAL;
> }
>
> @@ -354,8 +355,7 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
> * PE should be composed of PCI bus and its subordinate
> * components.
> */
> - pe = eeh_pe_get(pdn->phb, edev->pe_config_addr,
> - edev->config_addr);
> + pe = eeh_pe_get(pdn->phb, edev->pe_config_addr, config_addr);
> if (pe && !(pe->type & EEH_PE_INVALID)) {
> /* Mark the PE as type of PCI bus */
> pe->type = EEH_PE_BUS;
> @@ -365,10 +365,10 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
> list_add_tail(&edev->list, &pe->edevs);
> pr_debug("EEH: Add %04x:%02x:%02x.%01x to Bus PE#%x\n",
> pdn->phb->global_number,
> - edev->config_addr >> 8,
> - PCI_SLOT(edev->config_addr & 0xFF),
> - PCI_FUNC(edev->config_addr & 0xFF),
> - pe->addr);
> + pdn->busno,
> + PCI_SLOT(pdn->devfn),
> + PCI_FUNC(pdn->devfn),
> + pe->addr);
> return 0;
> } else if (pe && (pe->type & EEH_PE_INVALID)) {
> list_add_tail(&edev->list, &pe->edevs);
> @@ -388,10 +388,10 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
> pr_debug("EEH: Add %04x:%02x:%02x.%01x to Device "
> "PE#%x, Parent PE#%x\n",
> pdn->phb->global_number,
> - edev->config_addr >> 8,
> - PCI_SLOT(edev->config_addr & 0xFF),
> - PCI_FUNC(edev->config_addr & 0xFF),
> - pe->addr, pe->parent->addr);
> + pdn->busno,
> + PCI_SLOT(pdn->devfn),
> + PCI_FUNC(pdn->devfn),
> + pe->addr, pe->parent->addr);
> return 0;
> }
>
> @@ -405,7 +405,7 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
> return -ENOMEM;
> }
> pe->addr = edev->pe_config_addr;
> - pe->config_addr = edev->config_addr;
> + pe->config_addr = config_addr;
>
> /*
> * Put the new EEH PE into hierarchy tree. If the parent
> @@ -436,9 +436,9 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
> pr_debug("EEH: Add %04x:%02x:%02x.%01x to "
> "Device PE#%x, Parent PE#%x\n",
> pdn->phb->global_number,
> - edev->config_addr >> 8,
> - PCI_SLOT(edev->config_addr & 0xFF),
> - PCI_FUNC(edev->config_addr & 0xFF),
> + pdn->busno,
> + PCI_SLOT(pdn->devfn),
> + PCI_FUNC(pdn->devfn),
> pe->addr, pe->parent->addr);
>
> return 0;
> @@ -462,9 +462,9 @@ int eeh_rmv_from_parent_pe(struct eeh_dev *edev)
> if (!edev->pe) {
> pr_debug("%s: No PE found for device %04x:%02x:%02x.%01x\n",
> __func__, pdn->phb->global_number,
> - edev->config_addr >> 8,
> - PCI_SLOT(edev->config_addr & 0xFF),
> - PCI_FUNC(edev->config_addr & 0xFF));
> + pdn->busno,
> + PCI_SLOT(pdn->devfn),
> + PCI_FUNC(pdn->devfn));
> return -EEXIST;
> }
>
> @@ -725,9 +725,9 @@ static void eeh_bridge_check_link(struct eeh_dev *edev)
>
> pr_debug("%s: Check PCIe link for %04x:%02x:%02x.%01x ...\n",
> __func__, pdn->phb->global_number,
> - edev->config_addr >> 8,
> - PCI_SLOT(edev->config_addr & 0xFF),
> - PCI_FUNC(edev->config_addr & 0xFF));
> + pdn->busno,
> + PCI_SLOT(pdn->devfn),
> + PCI_FUNC(pdn->devfn));
>
> /* Check slot status */
> cap = edev->pcie_cap;
> diff --git a/arch/powerpc/kernel/eeh_sysfs.c b/arch/powerpc/kernel/eeh_sysfs.c
> index 1ceecdda810b..797549289798 100644
> --- a/arch/powerpc/kernel/eeh_sysfs.c
> +++ b/arch/powerpc/kernel/eeh_sysfs.c
> @@ -51,7 +51,6 @@ static ssize_t eeh_show_##_name(struct device *dev, \
> static DEVICE_ATTR(_name, S_IRUGO, eeh_show_##_name, NULL);
>
> EEH_SHOW_ATTR(eeh_mode, mode, "0x%x");
> -EEH_SHOW_ATTR(eeh_config_addr, config_addr, "0x%x");
> EEH_SHOW_ATTR(eeh_pe_config_addr, pe_config_addr, "0x%x");
>
> static ssize_t eeh_pe_state_show(struct device *dev,
> @@ -103,7 +102,6 @@ void eeh_sysfs_add_device(struct pci_dev *pdev)
> return;
>
> rc += device_create_file(&pdev->dev, &dev_attr_eeh_mode);
> - rc += device_create_file(&pdev->dev, &dev_attr_eeh_config_addr);
> rc += device_create_file(&pdev->dev, &dev_attr_eeh_pe_config_addr);
> rc += device_create_file(&pdev->dev, &dev_attr_eeh_pe_state);
>
> @@ -128,7 +126,6 @@ void eeh_sysfs_remove_device(struct pci_dev *pdev)
> }
>
> device_remove_file(&pdev->dev, &dev_attr_eeh_mode);
> - device_remove_file(&pdev->dev, &dev_attr_eeh_config_addr);
> device_remove_file(&pdev->dev, &dev_attr_eeh_pe_config_addr);
> device_remove_file(&pdev->dev, &dev_attr_eeh_pe_state);
>
> diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
> index 552b0cd4e8ba..8864065eba22 100644
> --- a/arch/powerpc/platforms/powernv/eeh-powernv.c
> +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
> @@ -352,6 +352,7 @@ static void *pnv_eeh_probe(struct pci_dn *pdn, void *data)
> struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
> uint32_t pcie_flags;
> int ret;
> + int config_addr = (pdn->busno << 8) | (pdn->devfn);
>
> /*
> * When probing the root bridge, which doesn't have any
> @@ -386,8 +387,7 @@ static void *pnv_eeh_probe(struct pci_dn *pdn, void *data)
> }
> }
>
> - edev->config_addr = (pdn->busno << 8) | (pdn->devfn);
> - edev->pe_config_addr = phb->ioda.pe_rmap[edev->config_addr];
> + edev->pe_config_addr = phb->ioda.pe_rmap[config_addr];
>
> /* Create PE */
> ret = eeh_add_to_parent_pe(edev);
> @@ -1699,6 +1699,7 @@ static int pnv_eeh_restore_config(struct pci_dn *pdn)
> struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
> struct pnv_phb *phb;
> s64 ret;
> + int config_addr = (pdn->busno << 8) | (pdn->devfn);
>
> if (!edev)
> return -EEXIST;
> @@ -1715,12 +1716,12 @@ static int pnv_eeh_restore_config(struct pci_dn *pdn)
> } else {
> phb = pdn->phb->private_data;
> ret = opal_pci_reinit(phb->opal_id,
> - OPAL_REINIT_PCI_DEV, edev->config_addr);
> + OPAL_REINIT_PCI_DEV, config_addr);
> }
>
> if (ret) {
> pr_warn("%s: Can't reinit PCI dev 0x%x (%lld)\n",
> - __func__, edev->config_addr, ret);
> + __func__, config_addr, ret);
> return -EIO;
> }
>
>
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com IBM Australia Limited
^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <20170823102629.539BD2803A@b01ledav001.gho.pok.ibm.com>]
* Re: [PATCH kernel 5/5] powerpc/eeh: Remove unnecessary node from pci_dn
[not found] ` <20170823102629.539BD2803A@b01ledav001.gho.pok.ibm.com>
@ 2017-08-24 5:19 ` Andrew Donnellan
0 siblings, 0 replies; 17+ messages in thread
From: Andrew Donnellan @ 2017-08-24 5:19 UTC (permalink / raw)
To: Alexey Kardashevskiy, linuxppc-dev; +Cc: Alistair Popple, David Gibson
On 23/08/17 20:19, Alexey Kardashevskiy wrote:
> The pci_dn struct caches a OF device node pointer in order to access
> the "ibm,loc-code" property when EEH is recovering.
>
> However, when this happens in eeh_dev_check_failure(), we also have
> a pci_dev pointer which should have a valid pointer to the device node
> when pci_dn has one (both pointers are not NULL for physical functions
> and are NULL for virtual functions).
>
> This removes the node pointer from the pci_dn struct and used pdev
> in eeh_dev_check_failure() instead.
>
> This changes pci_remove_device_node_info() to look for a parent of
> the node being removed, just like pci_add_device_node_info() does when it
> references the parent node.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> ---
> arch/powerpc/include/asm/pci-bridge.h | 1 -
> arch/powerpc/kernel/eeh.c | 9 +++++----
> arch/powerpc/kernel/pci_dn.c | 8 +++++---
> 3 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
> index 56c67d3f0108..0b8aa1fe2d5f 100644
> --- a/arch/powerpc/include/asm/pci-bridge.h
> +++ b/arch/powerpc/include/asm/pci-bridge.h
> @@ -195,7 +195,6 @@ struct pci_dn {
> struct pci_dn *parent;
> struct pci_controller *phb; /* for pci devices */
> struct iommu_table_group *table_group; /* for phb's or bridges */
> - struct device_node *node; /* back-pointer to the device_node */
>
> int pci_ext_config_space; /* for pci devices */
>
> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
> index c877014d11ce..2c926029adbe 100644
> --- a/arch/powerpc/kernel/eeh.c
> +++ b/arch/powerpc/kernel/eeh.c
> @@ -435,7 +435,7 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
> int ret;
> int active_flags = (EEH_STATE_MMIO_ACTIVE | EEH_STATE_DMA_ACTIVE);
> unsigned long flags;
> - struct pci_dn *pdn;
> + struct device_node *dn;
> struct pci_dev *dev;
> struct eeh_pe *pe, *parent_pe, *phb_pe;
> int rc = 0;
> @@ -493,9 +493,10 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
> if (pe->state & EEH_PE_ISOLATED) {
> pe->check_count++;
> if (pe->check_count % EEH_MAX_FAILS == 0) {
> - pdn = eeh_dev_to_pdn(edev);
> - if (pdn->node)
> - location = of_get_property(pdn->node, "ibm,loc-code", NULL);
> + dn = pci_device_to_OF_node(dev);
> + if (dn)
> + location = of_get_property(dn, "ibm,loc-code",
> + NULL);
> printk(KERN_ERR "EEH: %d reads ignored for recovering device at "
> "location=%s driver=%s pci addr=%s\n",
> pe->check_count,
> diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
> index 0256372b72de..0e395afbf0f4 100644
> --- a/arch/powerpc/kernel/pci_dn.c
> +++ b/arch/powerpc/kernel/pci_dn.c
> @@ -293,7 +293,6 @@ struct pci_dn *pci_add_device_node_info(struct pci_controller *hose,
> if (pdn == NULL)
> return NULL;
> dn->data = pdn;
> - pdn->node = dn;
> pdn->phb = hose;
> #ifdef CONFIG_PPC_POWERNV
> pdn->pe_number = IODA_INVALID_PE;
> @@ -342,6 +341,7 @@ EXPORT_SYMBOL_GPL(pci_add_device_node_info);
> void pci_remove_device_node_info(struct device_node *dn)
> {
> struct pci_dn *pdn = dn ? PCI_DN(dn) : NULL;
> + struct device_node *parent;
> #ifdef CONFIG_EEH
> struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
>
> @@ -354,8 +354,10 @@ void pci_remove_device_node_info(struct device_node *dn)
>
> WARN_ON(!list_empty(&pdn->child_list));
> list_del(&pdn->list);
> - if (pdn->parent)
> - of_node_put(pdn->parent->node);
> +
> + parent = of_get_parent(dn);
> + if (parent)
> + of_node_put(parent);
>
> dn->data = NULL;
> kfree(pdn);
>
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com IBM Australia Limited
^ permalink raw reply [flat|nested] 17+ messages in thread