[PATCH kernel 0/5] powerpc/eeh: Some cleanups

[PATCH kernel 0/5] powerpc/eeh: Some cleanups

[Alexey Kardashevskiy]

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.


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(-)

-- 
2.11.0

[PATCH kernel 1/5] powerpc/pci: Remove unused parameter from add_one_dev_pci_data()

[Alexey Kardashevskiy]

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

[PATCH kernel 2/5] powerpc/eeh: Reduce to one the number of places where edev is allocated

[Alexey Kardashevskiy]

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

[PATCH kernel 3/5] powerpc/eeh: Remove unnecessary pointer to phb from eeh_dev

[Alexey Kardashevskiy]

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

[PATCH kernel 4/5] powerpc/eeh: Remove unnecessary config_addr from eeh_dev

[Alexey Kardashevskiy]

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

[PATCH kernel 5/5] powerpc/eeh: Remove unnecessary node from pci_dn

[Alexey Kardashevskiy]

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

Re: [PATCH kernel 0/5] powerpc/eeh: Some cleanups

[Russell Currey]

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(-)
> 

Re: [PATCH kernel 0/5] powerpc/eeh: Some cleanups

[Alexey Kardashevskiy]

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

Re: [PATCH kernel 1/5] powerpc/pci: Remove unused parameter from add_one_dev_pci_data()

[Andrew Donnellan]

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

Re: [PATCH kernel 2/5] powerpc/eeh: Reduce to one the number of places where edev is allocated

[Andrew Donnellan]

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

Re: [PATCH kernel 3/5] powerpc/eeh: Remove unnecessary pointer to phb from eeh_dev

[Andrew Donnellan]

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

Re: [PATCH kernel 4/5] powerpc/eeh: Remove unnecessary config_addr from eeh_dev

[Andrew Donnellan]

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

Re: [PATCH kernel 5/5] powerpc/eeh: Remove unnecessary node from pci_dn

[Andrew Donnellan]

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

Re: [PATCH kernel 0/5] powerpc/eeh: Some cleanups

[Andrew Donnellan]

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

Re: [PATCH kernel 3/5] powerpc/eeh: Remove unnecessary pointer to phb from eeh_dev

[Michael Ellerman]

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

Re: [PATCH kernel 4/5] powerpc/eeh: Remove unnecessary config_addr from eeh_dev

[Michael Ellerman]

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

Re: [PATCH kernel 5/5] powerpc/eeh: Remove unnecessary node from pci_dn

[Michael Ellerman]

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