linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/4] powerpc/eeh: Add restore_bars operation
@ 2013-12-25  8:58 Gavin Shan
  2013-12-25  8:58 ` [PATCH 2/4] powerpc/eeh: Cache AER capability in EEH dev Gavin Shan
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Gavin Shan @ 2013-12-25  8:58 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Gavin Shan

After reset on the specific PE or PHB, we never configure AER
correctly on PowerNV platform. We needn't care it on pSeries
platform. The patch introduces additional EEH operation eeh_ops::
restore_bars() so that we have chance to configure AER correctly
for PowerNV platform.

Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/eeh.h               |    1 +
 arch/powerpc/kernel/eeh_pe.c                 |    3 +++
 arch/powerpc/platforms/pseries/eeh_pseries.c |    4 +++-
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index d3e5e9b..4b709bf 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -157,6 +157,7 @@ struct eeh_ops {
 	int (*read_config)(struct device_node *dn, int where, int size, u32 *val);
 	int (*write_config)(struct device_node *dn, int where, int size, u32 val);
 	int (*next_error)(struct eeh_pe **pe);
+	void (*restore_bars)(struct device_node *dn);
 };
 
 extern struct eeh_ops *eeh_ops;
diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
index f945053..19eb95a 100644
--- a/arch/powerpc/kernel/eeh_pe.c
+++ b/arch/powerpc/kernel/eeh_pe.c
@@ -737,6 +737,9 @@ static void *eeh_restore_one_device_bars(void *data, void *flag)
 	else
 		eeh_restore_device_bars(edev, dn);
 
+	if (eeh_ops->restore_bars)
+		eeh_ops->restore_bars(dn);
+
 	return NULL;
 }
 
diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
index ccb633e..623adaf 100644
--- a/arch/powerpc/platforms/pseries/eeh_pseries.c
+++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
@@ -689,7 +689,9 @@ static struct eeh_ops pseries_eeh_ops = {
 	.get_log		= pseries_eeh_get_log,
 	.configure_bridge       = pseries_eeh_configure_bridge,
 	.read_config		= pseries_eeh_read_config,
-	.write_config		= pseries_eeh_write_config
+	.write_config		= pseries_eeh_write_config,
+	.next_error		= NULL,
+	.restore_bars		= NULL
 };
 
 /**
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/4] powerpc/eeh: Cache AER capability in EEH dev
  2013-12-25  8:58 [PATCH 1/4] powerpc/eeh: Add restore_bars operation Gavin Shan
@ 2013-12-25  8:58 ` Gavin Shan
  2013-12-25  8:58 ` [PATCH 3/4] powerpc/powernv: Detect PHB chip revision Gavin Shan
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Gavin Shan @ 2013-12-25  8:58 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Gavin Shan

When fixing AER registers on PowerNV platform, we need the position
of AER capability for each PCI device. The patch caches that to
EEH device during probe time. Also, the patch figures the EEH device
is associated with the upstream port of PCIe bridge or not, which
is useful while fixing AER registers on PowerNV platform.

Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/eeh.h               |    8 +++++---
 arch/powerpc/platforms/powernv/eeh-powernv.c |    5 ++++-
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 4b709bf..92c2ec6 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -86,9 +86,10 @@ struct eeh_pe {
  */
 #define EEH_DEV_BRIDGE		(1 << 0)	/* PCI bridge		*/
 #define EEH_DEV_ROOT_PORT	(1 << 1)	/* PCIe root port	*/
-#define EEH_DEV_DS_PORT		(1 << 2)	/* Downstream port	*/
-#define EEH_DEV_IRQ_DISABLED	(1 << 3)	/* Interrupt disabled	*/
-#define EEH_DEV_DISCONNECTED	(1 << 4)	/* Removing from PE	*/
+#define EEH_DEV_US_PORT		(1 << 2)	/* Upstream port	*/
+#define EEH_DEV_DS_PORT		(1 << 3)	/* Downstream port	*/
+#define EEH_DEV_IRQ_DISABLED	(1 << 4)	/* Interrupt disabled	*/
+#define EEH_DEV_DISCONNECTED	(1 << 5)	/* Removing from PE	*/
 
 #define EEH_DEV_SYSFS		(1 << 8)	/* Sysfs created        */
 
@@ -99,6 +100,7 @@ struct eeh_dev {
 	int pe_config_addr;		/* PE config address		*/
 	u32 config_space[16];		/* Saved PCI config space	*/
 	u8 pcie_cap;			/* Saved PCIe capability	*/
+	int aer_cap;			/* Saved AER capability		*/
 	struct eeh_pe *pe;		/* Associated PE		*/
 	struct list_head list;		/* Form link list in the PE	*/
 	struct pci_controller *phb;	/* Associated PHB		*/
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index 73b9814..df54b76 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -128,9 +128,12 @@ static int powernv_eeh_dev_probe(struct pci_dev *dev, void *flag)
 		edev->mode |= EEH_DEV_BRIDGE;
 	if (pci_is_pcie(dev)) {
 		edev->pcie_cap = pci_pcie_cap(dev);
-
+		edev->aer_cap = pci_find_ext_capability(dev,
+						PCI_EXT_CAP_ID_ERR);
 		if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT)
 			edev->mode |= EEH_DEV_ROOT_PORT;
+		else if (pci_pcie_type(dev) == PCI_EXP_TYPE_UPSTREAM)
+			edev->mode |= EEH_DEV_US_PORT;
 		else if (pci_pcie_type(dev) == PCI_EXP_TYPE_DOWNSTREAM)
 			edev->mode |= EEH_DEV_DS_PORT;
 	}
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/4] powerpc/powernv: Detect PHB chip revision
  2013-12-25  8:58 [PATCH 1/4] powerpc/eeh: Add restore_bars operation Gavin Shan
  2013-12-25  8:58 ` [PATCH 2/4] powerpc/eeh: Cache AER capability in EEH dev Gavin Shan
@ 2013-12-25  8:58 ` Gavin Shan
  2013-12-25  8:58 ` [PATCH 4/4] powerpc/eeh: Eliminate AER gap Gavin Shan
  2013-12-27 22:20 ` [PATCH 1/4] powerpc/eeh: Add restore_bars operation Benjamin Herrenschmidt
  3 siblings, 0 replies; 6+ messages in thread
From: Gavin Shan @ 2013-12-25  8:58 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Gavin Shan

The patch intends to detect the PHB3 chip revision that was exported
by the underly firmware. In turn, we can have different AER configuration
for switch ports and endpoints accordingly.

Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/powernv/pci-ioda.c |    5 +++++
 arch/powerpc/platforms/powernv/pci.h      |    1 +
 2 files changed, 6 insertions(+)

diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 2c6d173..aea45fa 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1201,6 +1201,11 @@ void __init pnv_pci_init_ioda_phb(struct device_node *np,
 	else
 		phb->model = PNV_PHB_MODEL_UNKNOWN;
 
+	/* Detect chip revision */
+	prop32 = of_get_property(np, "ibm,revision", NULL);
+	if (prop32)
+		phb->rev = be32_to_cpup(prop32);
+
 	/* Parse 32-bit and IO ranges (if any) */
 	pci_process_bridge_OF_ranges(hose, np, !hose->global_number);
 
diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
index 911c24e..c5a0810 100644
--- a/arch/powerpc/platforms/powernv/pci.h
+++ b/arch/powerpc/platforms/powernv/pci.h
@@ -89,6 +89,7 @@ struct pnv_phb {
 	struct pci_controller	*hose;
 	enum pnv_phb_type	type;
 	enum pnv_phb_model	model;
+	u32			rev;
 	u64			hub_id;
 	u64			opal_id;
 	void __iomem		*regs;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 4/4] powerpc/eeh: Eliminate AER gap
  2013-12-25  8:58 [PATCH 1/4] powerpc/eeh: Add restore_bars operation Gavin Shan
  2013-12-25  8:58 ` [PATCH 2/4] powerpc/eeh: Cache AER capability in EEH dev Gavin Shan
  2013-12-25  8:58 ` [PATCH 3/4] powerpc/powernv: Detect PHB chip revision Gavin Shan
@ 2013-12-25  8:58 ` Gavin Shan
  2013-12-27 22:20 ` [PATCH 1/4] powerpc/eeh: Add restore_bars operation Benjamin Herrenschmidt
  3 siblings, 0 replies; 6+ messages in thread
From: Gavin Shan @ 2013-12-25  8:58 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Gavin Shan

The patch intends to implement the backend of eeh_ops::restore_bars
so that we can eliminate the gap of AER and PCI error reporting
between pHyp and sapphire that is introduced by reset on one specific
PE or the whole PHB. It's notable that the PHB3 and P7IOC is sharing
the same code to eliminate the gap. The details on PHB3 is shown as
follows:

 Offset - <pHyp> <original sapphire value> <current> <after reset>

 PHB3 - Root Complex (pcie_cap: 48 aer_cap: 100)
=C2=A0004 - 00100147 00100147 00100147 00100147
=C2=A003C - 00020000 00020000 00020000 00020000
=C2=A0050 - 0000004F 0000000F 0000000F 0001000F
       (Different maximal payload size)
=C2=A0108 - 0008D000 0008D000 0008D000 0008D000
=C2=A010C - 00072030 00072030 00072030 00072030
=C2=A0114 - 00002000 00000000 00002000 00002000
=C2=A0118 - 000001E0 000001E0 000001E0 000001E0
=C2=A012C - 00000007 00000007 00000007 00000007

 Switch upstream port (pcie_cap: 68 aer_cap: fb4)
=C2=A0004 - 00100547 00100007 00100547 00100547
=C2=A003C - 00020100 00000100 00020100 00020100
=C2=A0070 - 00000857 00000810 00090817 00000817
       (Different maximal payload size)
=C2=A0fbc - 00000000 00400000 00000000 00000000
=C2=A0fc0 - 00462030 00462030 00462030 00462030
=C2=A0fc8 - 00002000 0000f1c1 00002000 00002000
=C2=A0fcc - 000000E0 000000A0 000000ff 000000ff
       (same RWS bit#6/8)

 Switch downstream port (pcie_cap: 68 aer_cap: fb4)
=C2=A0004 - 00100547 00100007 00100547 00100547
=C2=A003C - 00020100 00000100 00020100 00020100
=C2=A0070 - 00000857 00000810 00008017 00008017
       (Different maximal payload size)
=C2=A0fbc - 00000000 00400000 00000000 00000000
=C2=A0fc0 - 00402000 00462030 00402000 00402000
=C2=A0fc8 - 00002000 0000f1c1 00002000 00002000
=C2=A0fcc - 000000e0 000000A0 000000ff 000000ff
       (same RWS bit#6/8)

 Endpoint (PCI_COMMAND, pcie_cap+0x8)
 004 - 00100146 00100406 00100546 00100146
       (Same error reporting)
 068 - 0000585e 00002810 0010240e 0010240e
       (Same error reporting)

Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/powernv/eeh-powernv.c |  145 ++++++++++++++++++++=
+++++-
 1 file changed, 144 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/=
platforms/powernv/eeh-powernv.c
index df54b76..1d4e958 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -347,6 +347,148 @@ static int powernv_eeh_next_error(struct eeh_pe **p=
e)
 	return -EEXIST;
 }
=20
+static void powernv_eeh_restore_root_port(struct device_node *dn,
+					  int ecap, int aercap)
+{
+	u32 val;
+
+	/* Enable SERR and parity checking */
+	pnv_pci_cfg_read(dn, PCI_COMMAND, 2, &val);
+	val |=3D (PCI_COMMAND_SERR | PCI_COMMAND_PARITY);
+	pnv_pci_cfg_write(dn, PCI_COMMAND, 2, val);
+
+	/* Enable reporting various errors */
+	pnv_pci_cfg_read(dn, ecap + PCI_EXP_DEVCTL, 2, &val);
+	val |=3D (PCI_EXP_DEVCTL_URRE | PCI_EXP_DEVCTL_FERE |
+		PCI_EXP_DEVCTL_NFERE | PCI_EXP_DEVCTL_CERE);
+	pnv_pci_cfg_write(dn, ecap + PCI_EXP_DEVCTL, 2, val);
+
+	/* Mask various unrecoverable errors */
+	if (!aercap) return;
+	pnv_pci_cfg_read(dn, aercap + 0x8, 4, &val);
+	val |=3D 0x0008d000;
+	pnv_pci_cfg_write(dn, aercap + 0x8, 4, val);
+
+	/* Report various unrecoverable errors as fatal errors */
+	pnv_pci_cfg_write(dn, aercap + 0xc, 4, 0x00072030);
+
+	/* Mask various recoverable errors */
+	pnv_pci_cfg_read(dn, aercap + 0x14, 4, &val);
+	val |=3D 0x00002000;
+	pnv_pci_cfg_write(dn, aercap + 0x14, 4, val);
+
+	/* Enable ECRC check */
+	pnv_pci_cfg_read(dn, aercap + 0x18, 4, &val);
+	val |=3D 0x00000140;
+	pnv_pci_cfg_write(dn, aercap + 0x18, 4, val);
+
+	/* Enable all error reporting */
+	pnv_pci_cfg_read(dn, aercap + 0x2c, 4, &val);
+	val |=3D 0x00000007;
+	pnv_pci_cfg_write(dn, aercap + 0x2c, 4, val);
+}
+
+static void powernv_eeh_restore_sw_port(struct device_node *dn,
+					int ecap, int aercap)
+{
+	struct eeh_dev *edev =3D of_node_to_eeh_dev(dn);
+	struct pnv_phb *phb =3D edev->phb->private_data;
+	u32 val;
+
+	/* Enable SERR and parity checking and disable INTx */
+	pnv_pci_cfg_read(dn, PCI_COMMAND, 2, &val);
+	val |=3D (PCI_COMMAND_INTX_DISABLE |
+		PCI_COMMAND_SERR | PCI_COMMAND_PARITY);
+	pnv_pci_cfg_write(dn, PCI_COMMAND, 2, val);
+
+	/* Disable partity error and enable system error */
+	pnv_pci_cfg_read(dn, PCI_BRIDGE_CONTROL, 2, &val);
+	val &=3D ~PCI_BRIDGE_CTL_PARITY;
+	val |=3D PCI_BRIDGE_CTL_SERR;
+	pnv_pci_cfg_write(dn, PCI_BRIDGE_CONTROL, 2, val);
+
+	/* Enable reporting various errors */
+	pnv_pci_cfg_read(dn, ecap + PCI_EXP_DEVCTL, 2, &val);
+	val |=3D (PCI_EXP_DEVCTL_FERE |
+		PCI_EXP_DEVCTL_NFERE |
+		PCI_EXP_DEVCTL_CERE);
+	pnv_pci_cfg_write(dn, ecap + PCI_EXP_DEVCTL, 2, val);
+
+	/* Unmask all unrecoverable errors */
+	if (!aercap) return;
+	pnv_pci_cfg_write(dn, aercap + 0x8, 4, 0x0);
+
+	/* Severity of unrecoverable errors */
+	if (edev->mode & EEH_DEV_US_PORT)
+		val =3D 0x00462030;
+	else
+		val =3D 0x00402000;
+	pnv_pci_cfg_write(dn, aercap + 0xc, 4, val);
+
+	/* Mask various correctable errors */
+	if (phb->model =3D=3D PNV_PHB_MODEL_PHB3 &&
+	    phb->rev < 0xa30003)
+		val =3D 0xffffffff;
+	else
+		val =3D 0x2000;
+	pnv_pci_cfg_write(dn, aercap + 0x14, 4, val);
+
+	/* Enable ECRC generation and disable ECRC check */
+	pnv_pci_cfg_read(dn, aercap + 0x18, 4, &val);
+	val &=3D ~0x00000100;
+	val |=3D 0x00000040;
+	pnv_pci_cfg_write(dn, aercap + 0x18, 4, val);
+}
+
+static void powernv_eeh_restore_endpoint(struct device_node *dn,
+					 int ecap, int aercap)
+{
+	struct eeh_dev *edev =3D of_node_to_eeh_dev(dn);
+	struct pnv_phb *phb =3D edev->phb->private_data;
+	u32 val;
+
+	/* Enable SERR and parity checking */
+	pnv_pci_cfg_read(dn, PCI_COMMAND, 2, &val);
+	val |=3D (PCI_COMMAND_SERR | PCI_COMMAND_PARITY);
+	pnv_pci_cfg_write(dn, PCI_COMMAND, 2, val);
+
+	/* Enable reporting various errors */
+	if (!ecap) return;
+	pnv_pci_cfg_read(dn, ecap + PCI_EXP_DEVCTL, 2, &val);
+	val &=3D ~PCI_EXP_DEVCTL_CERE;
+	val |=3D (PCI_EXP_DEVCTL_URRE |
+		PCI_EXP_DEVCTL_FERE |
+		PCI_EXP_DEVCTL_NFERE);
+	pnv_pci_cfg_write(dn, ecap + PCI_EXP_DEVCTL, 2, val);
+
+	if (!aercap) return;
+	if (phb->model =3D=3D PNV_PHB_MODEL_PHB3 &&
+	    phb->rev < 0xa30003)
+		pnv_pci_cfg_write(dn, aercap + 0x14, 4, 0xffffffff);
+
+	/* Enable ECRC generation and check */
+	pnv_pci_cfg_read(dn, aercap + 0x18, 4, &val);
+	val |=3D 0x00000140;
+	pnv_pci_cfg_write(dn, aercap + 0x18, 4, val);
+}
+
+static void powernv_eeh_restore_bars(struct device_node *dn)
+{
+	struct eeh_dev *edev =3D of_node_to_eeh_dev(dn);
+
+	if (!edev) return;
+	if (edev->mode & EEH_DEV_ROOT_PORT)
+		powernv_eeh_restore_root_port(dn,
+			edev->pcie_cap, edev->aer_cap);
+	else if ((edev->mode & EEH_DEV_US_PORT) ||
+		 (edev->mode & EEH_DEV_DS_PORT))
+		powernv_eeh_restore_sw_port(dn,
+			edev->pcie_cap, edev->aer_cap);
+	else
+		powernv_eeh_restore_endpoint(dn,
+			edev->pcie_cap, edev->aer_cap);
+}
+
 static struct eeh_ops powernv_eeh_ops =3D {
 	.name                   =3D "powernv",
 	.init                   =3D powernv_eeh_init,
@@ -362,7 +504,8 @@ static struct eeh_ops powernv_eeh_ops =3D {
 	.configure_bridge       =3D powernv_eeh_configure_bridge,
 	.read_config            =3D pnv_pci_cfg_read,
 	.write_config           =3D pnv_pci_cfg_write,
-	.next_error		=3D powernv_eeh_next_error
+	.next_error		=3D powernv_eeh_next_error,
+	.restore_bars		=3D powernv_eeh_restore_bars
 };
=20
 /**
--=20
1.7.10.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/4] powerpc/eeh: Add restore_bars operation
  2013-12-25  8:58 [PATCH 1/4] powerpc/eeh: Add restore_bars operation Gavin Shan
                   ` (2 preceding siblings ...)
  2013-12-25  8:58 ` [PATCH 4/4] powerpc/eeh: Eliminate AER gap Gavin Shan
@ 2013-12-27 22:20 ` Benjamin Herrenschmidt
  2014-01-01  3:29   ` Gavin Shan
  3 siblings, 1 reply; 6+ messages in thread
From: Benjamin Herrenschmidt @ 2013-12-27 22:20 UTC (permalink / raw)
  To: Gavin Shan; +Cc: linuxppc-dev

On Wed, 2013-12-25 at 16:58 +0800, Gavin Shan wrote:
> After reset on the specific PE or PHB, we never configure AER
> correctly on PowerNV platform. We needn't care it on pSeries
> platform. The patch introduces additional EEH operation eeh_ops::
> restore_bars() so that we have chance to configure AER correctly
> for PowerNV platform.

Why call it "restore_bars" if it restores something else (in this case
AER) ?

I would call it "restore_config" instead... Also rather than adding
the knowledge of what AER bit works or not for the device, would it
make sense to instead have a FW call to re-init the device ?

Otherwise, you introduce duplication between Linux and Firmware with
the risk of getting out of sync...

Cheers,
Ben.

> Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
> ---
>  arch/powerpc/include/asm/eeh.h               |    1 +
>  arch/powerpc/kernel/eeh_pe.c                 |    3 +++
>  arch/powerpc/platforms/pseries/eeh_pseries.c |    4 +++-
>  3 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
> index d3e5e9b..4b709bf 100644
> --- a/arch/powerpc/include/asm/eeh.h
> +++ b/arch/powerpc/include/asm/eeh.h
> @@ -157,6 +157,7 @@ struct eeh_ops {
>  	int (*read_config)(struct device_node *dn, int where, int size, u32 *val);
>  	int (*write_config)(struct device_node *dn, int where, int size, u32 val);
>  	int (*next_error)(struct eeh_pe **pe);
> +	void (*restore_bars)(struct device_node *dn);
>  };
>  
>  extern struct eeh_ops *eeh_ops;
> diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
> index f945053..19eb95a 100644
> --- a/arch/powerpc/kernel/eeh_pe.c
> +++ b/arch/powerpc/kernel/eeh_pe.c
> @@ -737,6 +737,9 @@ static void *eeh_restore_one_device_bars(void *data, void *flag)
>  	else
>  		eeh_restore_device_bars(edev, dn);
>  
> +	if (eeh_ops->restore_bars)
> +		eeh_ops->restore_bars(dn);
> +
>  	return NULL;
>  }
>  
> diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
> index ccb633e..623adaf 100644
> --- a/arch/powerpc/platforms/pseries/eeh_pseries.c
> +++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
> @@ -689,7 +689,9 @@ static struct eeh_ops pseries_eeh_ops = {
>  	.get_log		= pseries_eeh_get_log,
>  	.configure_bridge       = pseries_eeh_configure_bridge,
>  	.read_config		= pseries_eeh_read_config,
> -	.write_config		= pseries_eeh_write_config
> +	.write_config		= pseries_eeh_write_config,
> +	.next_error		= NULL,
> +	.restore_bars		= NULL
>  };
>  
>  /**

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/4] powerpc/eeh: Add restore_bars operation
  2013-12-27 22:20 ` [PATCH 1/4] powerpc/eeh: Add restore_bars operation Benjamin Herrenschmidt
@ 2014-01-01  3:29   ` Gavin Shan
  0 siblings, 0 replies; 6+ messages in thread
From: Gavin Shan @ 2014-01-01  3:29 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Gavin Shan

On Sat, Dec 28, 2013 at 09:20:04AM +1100, Benjamin Herrenschmidt wrote:
>On Wed, 2013-12-25 at 16:58 +0800, Gavin Shan wrote:
>> After reset on the specific PE or PHB, we never configure AER
>> correctly on PowerNV platform. We needn't care it on pSeries
>> platform. The patch introduces additional EEH operation eeh_ops::
>> restore_bars() so that we have chance to configure AER correctly
>> for PowerNV platform.
>
>Why call it "restore_bars" if it restores something else (in this case
>AER) ?
>
>I would call it "restore_config" instead... Also rather than adding
>the knowledge of what AER bit works or not for the device, would it
>make sense to instead have a FW call to re-init the device ?
>
>Otherwise, you introduce duplication between Linux and Firmware with
>the risk of getting out of sync...
>

Thanks for your comments, Ben. It's reasonable to have name "restore_config"
and I'll introduce a FW call in next revision as you suggested :-)

Thanks,
Gavin

>> Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
>> ---
>>  arch/powerpc/include/asm/eeh.h               |    1 +
>>  arch/powerpc/kernel/eeh_pe.c                 |    3 +++
>>  arch/powerpc/platforms/pseries/eeh_pseries.c |    4 +++-
>>  3 files changed, 7 insertions(+), 1 deletion(-)
>> 
>> diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
>> index d3e5e9b..4b709bf 100644
>> --- a/arch/powerpc/include/asm/eeh.h
>> +++ b/arch/powerpc/include/asm/eeh.h
>> @@ -157,6 +157,7 @@ struct eeh_ops {
>>  	int (*read_config)(struct device_node *dn, int where, int size, u32 *val);
>>  	int (*write_config)(struct device_node *dn, int where, int size, u32 val);
>>  	int (*next_error)(struct eeh_pe **pe);
>> +	void (*restore_bars)(struct device_node *dn);
>>  };
>>  
>>  extern struct eeh_ops *eeh_ops;
>> diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
>> index f945053..19eb95a 100644
>> --- a/arch/powerpc/kernel/eeh_pe.c
>> +++ b/arch/powerpc/kernel/eeh_pe.c
>> @@ -737,6 +737,9 @@ static void *eeh_restore_one_device_bars(void *data, void *flag)
>>  	else
>>  		eeh_restore_device_bars(edev, dn);
>>  
>> +	if (eeh_ops->restore_bars)
>> +		eeh_ops->restore_bars(dn);
>> +
>>  	return NULL;
>>  }
>>  
>> diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
>> index ccb633e..623adaf 100644
>> --- a/arch/powerpc/platforms/pseries/eeh_pseries.c
>> +++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
>> @@ -689,7 +689,9 @@ static struct eeh_ops pseries_eeh_ops = {
>>  	.get_log		= pseries_eeh_get_log,
>>  	.configure_bridge       = pseries_eeh_configure_bridge,
>>  	.read_config		= pseries_eeh_read_config,
>> -	.write_config		= pseries_eeh_write_config
>> +	.write_config		= pseries_eeh_write_config,
>> +	.next_error		= NULL,
>> +	.restore_bars		= NULL
>>  };
>>  
>>  /**
>
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-01-01  3:29 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-25  8:58 [PATCH 1/4] powerpc/eeh: Add restore_bars operation Gavin Shan
2013-12-25  8:58 ` [PATCH 2/4] powerpc/eeh: Cache AER capability in EEH dev Gavin Shan
2013-12-25  8:58 ` [PATCH 3/4] powerpc/powernv: Detect PHB chip revision Gavin Shan
2013-12-25  8:58 ` [PATCH 4/4] powerpc/eeh: Eliminate AER gap Gavin Shan
2013-12-27 22:20 ` [PATCH 1/4] powerpc/eeh: Add restore_bars operation Benjamin Herrenschmidt
2014-01-01  3:29   ` Gavin Shan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).