LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] powerpc/kerenl: Enable EEH for IO accessors
From: Gavin Shan @ 2014-06-23  0:56 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Gavin Shan

In arch/powerpc/kernel/iomap.c, lots of IO reading accessors missed
to check EEH error as Ben pointed. The patch fixes it.

For the writing accessors, we change the called functions only for
making them look similar to the reading counterparts.

Suggested-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/iomap.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/kernel/iomap.c b/arch/powerpc/kernel/iomap.c
index b82227e..12e48d5 100644
--- a/arch/powerpc/kernel/iomap.c
+++ b/arch/powerpc/kernel/iomap.c
@@ -23,7 +23,7 @@ unsigned int ioread16(void __iomem *addr)
 }
 unsigned int ioread16be(void __iomem *addr)
 {
-	return in_be16(addr);
+	return readw_be(addr);
 }
 unsigned int ioread32(void __iomem *addr)
 {
@@ -31,7 +31,7 @@ unsigned int ioread32(void __iomem *addr)
 }
 unsigned int ioread32be(void __iomem *addr)
 {
-	return in_be32(addr);
+	return readl_be(addr);
 }
 EXPORT_SYMBOL(ioread8);
 EXPORT_SYMBOL(ioread16);
@@ -49,7 +49,7 @@ void iowrite16(u16 val, void __iomem *addr)
 }
 void iowrite16be(u16 val, void __iomem *addr)
 {
-	out_be16(addr, val);
+	writew_be(val, addr);
 }
 void iowrite32(u32 val, void __iomem *addr)
 {
@@ -57,7 +57,7 @@ void iowrite32(u32 val, void __iomem *addr)
 }
 void iowrite32be(u32 val, void __iomem *addr)
 {
-	out_be32(addr, val);
+	writel_be(val, addr);
 }
 EXPORT_SYMBOL(iowrite8);
 EXPORT_SYMBOL(iowrite16);
@@ -75,15 +75,15 @@ EXPORT_SYMBOL(iowrite32be);
  */
 void ioread8_rep(void __iomem *addr, void *dst, unsigned long count)
 {
-	_insb((u8 __iomem *) addr, dst, count);
+	readsb(addr, dst, count);
 }
 void ioread16_rep(void __iomem *addr, void *dst, unsigned long count)
 {
-	_insw_ns((u16 __iomem *) addr, dst, count);
+	readsw(addr, dst, count);
 }
 void ioread32_rep(void __iomem *addr, void *dst, unsigned long count)
 {
-	_insl_ns((u32 __iomem *) addr, dst, count);
+	readsl(addr, dst, count);
 }
 EXPORT_SYMBOL(ioread8_rep);
 EXPORT_SYMBOL(ioread16_rep);
@@ -91,15 +91,15 @@ EXPORT_SYMBOL(ioread32_rep);
 
 void iowrite8_rep(void __iomem *addr, const void *src, unsigned long count)
 {
-	_outsb((u8 __iomem *) addr, src, count);
+	writesb(addr, src, count);
 }
 void iowrite16_rep(void __iomem *addr, const void *src, unsigned long count)
 {
-	_outsw_ns((u16 __iomem *) addr, src, count);
+	writesw(addr, src, count);
 }
 void iowrite32_rep(void __iomem *addr, const void *src, unsigned long count)
 {
-	_outsl_ns((u32 __iomem *) addr, src, count);
+	writesl(addr, src, count);
 }
 EXPORT_SYMBOL(iowrite8_rep);
 EXPORT_SYMBOL(iowrite16_rep);
-- 
1.8.3.2

^ permalink raw reply related

* [PATCH v1 0/3] Support PCI Error Injection
From: Gavin Shan @ 2014-06-23  2:14 UTC (permalink / raw)
  To: kvm-ppc, linuxppc-dev; +Cc: aik, qiudayu, agraf, Gavin Shan

The series of patches supports PCI error injection as defined in PAPR spec.
There are 3 RTAS calls related to it:

"ibm,open-errinjct": Apply for token to do error injection
"ibm,close-errinjct": Free the token assigned previously
"ibm,errinjct": Do error injection

For PCI errors, we need manupulate 3 hardware registers by predetermined
OPAL API. In order to export the capability to userland, one OPAL firmware
related sysfs entries are created: /sys/firmware/opal/errinjct. It accepts
strings to call OPAL API to inject specific errors. So it corresponds to
"ibm,errinjct". So "ibm,open-errinjct" and "ibm,close-errinjct" won't be
handled by host kernel and applications have full freedom to implement logic
of themselves for them.

Gavin Shan (3):
  powerpc/powernv: Sync header with firmware
  powerpc/powernv: Support PCI error injection
  powerpc/powernv: Clear PAPR error injection registers

 arch/powerpc/include/asm/opal.h                |  66 +++++++++
 arch/powerpc/platforms/powernv/Makefile        |   2 +-
 arch/powerpc/platforms/powernv/eeh-ioda.c      |  24 ++++
 arch/powerpc/platforms/powernv/opal-errinjct.c | 184 +++++++++++++++++++++++++
 arch/powerpc/platforms/powernv/opal-wrappers.S |   1 +
 arch/powerpc/platforms/powernv/opal.c          |   2 +
 6 files changed, 278 insertions(+), 1 deletion(-)
 create mode 100644 arch/powerpc/platforms/powernv/opal-errinjct.c

-- 
1.8.3.2

^ permalink raw reply

* [PATCH v1 1/3] powerpc/powernv: Sync header with firmware
From: Gavin Shan @ 2014-06-23  2:14 UTC (permalink / raw)
  To: kvm-ppc, linuxppc-dev; +Cc: aik, qiudayu, agraf, Gavin Shan
In-Reply-To: <1403489682-14841-1-git-send-email-gwshan@linux.vnet.ibm.com>

The patch synchronizes firmware header file (opal.h) for PCI error
injection.

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/opal.h                | 65 ++++++++++++++++++++++++++
 arch/powerpc/platforms/powernv/opal-wrappers.S |  1 +
 2 files changed, 66 insertions(+)

diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index 66ad7a7..d982bb8 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -175,6 +175,7 @@ extern int opal_enter_rtas(struct rtas_args *args,
 #define OPAL_SET_PARAM				90
 #define OPAL_DUMP_RESEND			91
 #define OPAL_DUMP_INFO2				94
+#define OPAL_ERR_INJECT				96
 
 #ifndef __ASSEMBLY__
 
@@ -219,6 +220,69 @@ enum OpalPciErrorSeverity {
 	OPAL_EEH_SEV_INF	= 5
 };
 
+enum OpalErrinjctType {
+	OpalErrinjctTypeFirst			= 0,
+	OpalErrinjctTypeFatal			= 1,
+	OpalErrinjctTypeRecoverRandomEvent	= 2,
+	OpalErrinjctTypeRecoverSpecialEvent	= 3,
+	OpalErrinjctTypeCorruptedPage		= 4,
+	OpalErrinjctTypeCorruptedSlb		= 5,
+	OpalErrinjctTypeTranslatorFailure	= 6,
+	OpalErrinjctTypeIoaBusError		= 7,
+	OpalErrinjctTypeIoaBusError64		= 8,
+	OpalErrinjctTypePlatformSpecific	= 9,
+	OpalErrinjctTypeDcacheStart		= 10,
+	OpalErrinjctTypeDcacheEnd		= 11,
+	OpalErrinjctTypeIcacheStart		= 12,
+	OpalErrinjctTypeIcacheEnd		= 13,
+	OpalErrinjctTypeTlbStart		= 14,
+	OpalErrinjctTypeTlbEnd			= 15,
+	OpalErrinjctTypeUpstreamIoError		= 16,
+	OpalErrinjctTypeLast			= 17,
+
+	/* IoaBusError & IoaBusError64 */
+	OpalEitIoaLoadMemAddr			= 0,
+	OpalEitIoaLoadMemData			= 1,
+	OpalEitIoaLoadIoAddr			= 2,
+	OpalEitIoaLoadIoData			= 3,
+	OpalEitIoaLoadConfigAddr		= 4,
+	OpalEitIoaLoadConfigData		= 5,
+	OpalEitIoaStoreMemAddr			= 6,
+	OpalEitIoaStoreMemData			= 7,
+	OpalEitIoaStoreIoAddr			= 8,
+	OpalEitIoaStoreIoData			= 9,
+	OpalEitIoaStoreConfigAddr		= 10,
+	OpalEitIoaStoreConfigData		= 11,
+	OpalEitIoaDmaReadMemAddr		= 12,
+	OpalEitIoaDmaReadMemData		= 13,
+	OpalEitIoaDmaReadMemMaster		= 14,
+	OpalEitIoaDmaReadMemTarget		= 15,
+	OpalEitIoaDmaWriteMemAddr		= 16,
+	OpalEitIoaDmaWriteMemData		= 17,
+	OpalEitIoaDmaWriteMemMaster		= 18,
+	OpalEitIoaDmaWriteMemTarget		= 19,
+};
+
+struct OpalErrinjct {
+	int32_t type;
+	union {
+		struct {
+			uint32_t addr;
+			uint32_t mask;
+			uint64_t phb_id;
+			uint32_t pe;
+			uint32_t function;
+		} ioa;
+		struct {
+			uint64_t addr;
+			uint64_t mask;
+			uint64_t phb_id;
+			uint32_t pe;
+			uint32_t function;
+		} ioa64;
+	};
+};
+
 enum OpalShpcAction {
 	OPAL_SHPC_GET_LINK_STATE = 0,
 	OPAL_SHPC_GET_SLOT_STATE = 1
@@ -870,6 +934,7 @@ int64_t opal_update_flash(uint64_t blk_list);
 int64_t opal_dump_init(uint8_t dump_type);
 int64_t opal_dump_info(__be32 *dump_id, __be32 *dump_size);
 int64_t opal_dump_info2(__be32 *dump_id, __be32 *dump_size, __be32 *dump_type);
+int64_t opal_err_injct(struct OpalErrinjct *ei);
 int64_t opal_dump_read(uint32_t dump_id, uint64_t buffer);
 int64_t opal_dump_ack(uint32_t dump_id);
 int64_t opal_dump_resend_notification(void);
diff --git a/arch/powerpc/platforms/powernv/opal-wrappers.S b/arch/powerpc/platforms/powernv/opal-wrappers.S
index f531ffe..44b3d81 100644
--- a/arch/powerpc/platforms/powernv/opal-wrappers.S
+++ b/arch/powerpc/platforms/powernv/opal-wrappers.S
@@ -136,6 +136,7 @@ OPAL_CALL(opal_resync_timebase,			OPAL_RESYNC_TIMEBASE);
 OPAL_CALL(opal_dump_init,			OPAL_DUMP_INIT);
 OPAL_CALL(opal_dump_info,			OPAL_DUMP_INFO);
 OPAL_CALL(opal_dump_info2,			OPAL_DUMP_INFO2);
+OPAL_CALL(opal_err_injct,			OPAL_ERR_INJECT);
 OPAL_CALL(opal_dump_read,			OPAL_DUMP_READ);
 OPAL_CALL(opal_dump_ack,			OPAL_DUMP_ACK);
 OPAL_CALL(opal_get_msg,				OPAL_GET_MSG);
-- 
1.8.3.2

^ permalink raw reply related

* [PATCH v1 2/3] powerpc/powernv: Support PCI error injection
From: Gavin Shan @ 2014-06-23  2:14 UTC (permalink / raw)
  To: kvm-ppc, linuxppc-dev; +Cc: aik, qiudayu, agraf, Gavin Shan
In-Reply-To: <1403489682-14841-1-git-send-email-gwshan@linux.vnet.ibm.com>

The patch implements one OPAL firmware sysfs file to support PCI error
injection: "/sys/firmware/opal/errinjct", which will be used like the
way described as follows.

According to PAPR spec, there are 3 RTAS calls related to error injection:
"ibm,open-errinjct": allocate token prior to doing error injection.
"ibm,close-errinjct": release the token allocated from "ibm,open-errinjct".
"ibm,errinjct": do error injection.

Sysfs file /sys/firmware/opal/errinjct accepts strings that have fixed
format "ei_token ...". For now, we only support 32-bits and 64-bits
PCI error injection and they should have following strings written to
/sys/firmware/opal/errinjct as follows. We don't have corresponding
sysfs files for "ibm,open-errinjct" and "ibm,close-errinjct", which
means that we rely on userland to maintain the token by itself.

32-bits PCI error: "7:addr:mask:iommu_group_id:function".
64-bits PCI error: "8:addr:mask:iommu_group_id:function".

The above "7" and "8" represent 32-bits and 64-bits PCI error seperately
and "function" is one of the specific PCI errors (e.g. MMIO access address
parity error), which are defined by PAPR spec.

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/opal.h                |   1 +
 arch/powerpc/platforms/powernv/Makefile        |   2 +-
 arch/powerpc/platforms/powernv/opal-errinjct.c | 184 +++++++++++++++++++++++++
 arch/powerpc/platforms/powernv/opal.c          |   2 +
 4 files changed, 188 insertions(+), 1 deletion(-)
 create mode 100644 arch/powerpc/platforms/powernv/opal-errinjct.c

diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index d982bb8..bf280d9 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -985,6 +985,7 @@ extern int opal_elog_init(void);
 extern void opal_platform_dump_init(void);
 extern void opal_sys_param_init(void);
 extern void opal_msglog_init(void);
+extern void opal_errinjct_init(void);
 
 extern int opal_machine_check(struct pt_regs *regs);
 extern bool opal_mce_check_early_recovery(struct pt_regs *regs);
diff --git a/arch/powerpc/platforms/powernv/Makefile b/arch/powerpc/platforms/powernv/Makefile
index 63cebb9..4711de8 100644
--- a/arch/powerpc/platforms/powernv/Makefile
+++ b/arch/powerpc/platforms/powernv/Makefile
@@ -1,7 +1,7 @@
 obj-y			+= setup.o opal-takeover.o opal-wrappers.o opal.o opal-async.o
 obj-y			+= opal-rtc.o opal-nvram.o opal-lpc.o opal-flash.o
 obj-y			+= rng.o opal-elog.o opal-dump.o opal-sysparam.o opal-sensor.o
-obj-y			+= opal-msglog.o
+obj-y			+= opal-msglog.o opal-errinjct.o
 
 obj-$(CONFIG_SMP)	+= smp.o
 obj-$(CONFIG_PCI)	+= pci.o pci-p5ioc2.o pci-ioda.o
diff --git a/arch/powerpc/platforms/powernv/opal-errinjct.c b/arch/powerpc/platforms/powernv/opal-errinjct.c
new file mode 100644
index 0000000..29c9e83
--- /dev/null
+++ b/arch/powerpc/platforms/powernv/opal-errinjct.c
@@ -0,0 +1,184 @@
+/*
+ * The file supports error injection, which works based on OPAL API.
+ * For now, we only support PCI error injection. We need support
+ * injecting other types of errors in future.
+ *
+ * Copyright Gavin Shan, IBM Corporation 2014.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/msi.h>
+#include <linux/pci.h>
+#include <linux/iommu.h>
+#include <linux/random.h>
+#include <linux/slab.h>
+#include <linux/sysfs.h>
+#include <linux/fs.h>
+#include <linux/fcntl.h>
+#include <linux/kobject.h>
+
+#include <asm/msi_bitmap.h>
+#include <asm/iommu.h>
+#include <asm/pci-bridge.h>
+#include <asm/ppc-pci.h>
+#include <asm/opal.h>
+
+#include "powernv.h"
+#include "pci.h"
+
+static DEFINE_MUTEX(errinjct_mutex);
+
+static int errinjct_iommu_group_to_phb_and_pe(uint32_t iommu_grp_id,
+					      uint64_t *phb_id,
+					      uint32_t *pe_num)
+{
+#ifdef CONFIG_IOMMU_API
+	struct iommu_group *iommu_grp;
+	struct iommu_table *tbl;
+	struct pnv_ioda_pe *pe;
+
+	iommu_grp = iommu_group_get_by_id(iommu_grp_id);
+	if (!iommu_grp)
+		return -ENODEV;
+
+	tbl = iommu_group_get_iommudata(iommu_grp);
+	if (!tbl)
+		return -ENODEV;
+
+	pe = container_of(tbl, struct pnv_ioda_pe, tce32_table);
+	if (!pe->phb)
+		return -ENODEV;
+
+	*phb_id = pe->phb->opal_id;
+	*pe_num = pe->pe_number;
+
+	return 0;
+#endif
+
+	return -ENXIO;
+}
+
+static int errinjct_ioa_bus_error(const char *buf, struct OpalErrinjct *ei)
+{
+	uint32_t iommu_grp_id;
+	int ret;
+
+	/* Extract parameters */
+	ret = sscanf(buf, "%x:%x:%x:%x:%x",
+		     &ei->type, &ei->ioa.addr,
+		     &ei->ioa.mask, &iommu_grp_id, ei->ioa.function);
+	if (ret != 5)
+		return -EINVAL;
+
+	/* Invalid function ? */
+	if (ei->ioa.function < OpalEitIoaLoadMemAddr ||
+	    ei->ioa.function > OpalEitIoaDmaWriteMemTarget)
+		return -ERANGE;
+
+	/* Retrieve PHB ID and PE number */
+	ret = errinjct_iommu_group_to_phb_and_pe(iommu_grp_id,
+						 &ei->ioa.phb_id,
+						 &ei->ioa.pe);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int errinjct_ioa_bus_error64(const char *buf, struct OpalErrinjct *ei)
+{
+	uint32_t iommu_grp_id;
+	int ret;
+
+	/* Extract parameter */
+	ret = sscanf(buf, "%x:%llx:%llx:%x:%x",
+		     &ei->type, &ei->ioa64.addr,
+		     &ei->ioa64.mask, &iommu_grp_id, &ei->ioa64.function);
+	if (ret != 5)
+		return -EINVAL;
+
+	/* Invalid function ? */
+	if (ei->ioa64.function < OpalEitIoaLoadMemAddr ||
+	    ei->ioa64.function > OpalEitIoaDmaWriteMemTarget)
+		return -ERANGE;
+
+	/* Retrieve PHB ID and PE number */
+	ret = errinjct_iommu_group_to_phb_and_pe(iommu_grp_id,
+						 &ei->ioa64.phb_id,
+						 &ei->ioa64.pe);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static ssize_t errinjct_store(struct kobject *kobj,
+			      struct kobj_attribute *attr,
+			      const char *buf, size_t count)
+{
+	struct OpalErrinjct ei;
+	int ret;
+	long rc;
+
+	/* Extract common parameters */
+	ret = sscanf(buf, "%x", &ei.type);
+	if (ret != 1)
+		return -EINVAL;
+
+	/* Error injection might be in progress */
+	if (!mutex_trylock(&errinjct_mutex))
+		return -EAGAIN;
+
+	switch (ei.type) {
+	case OpalErrinjctTypeIoaBusError:
+		ret = errinjct_ioa_bus_error(buf, &ei);
+		break;
+	case OpalErrinjctTypeIoaBusError64:
+		ret = errinjct_ioa_bus_error64(buf, &ei);
+		break;
+	default:
+		ret = -ERANGE;
+	}
+
+	/* Invalid parameters ? */
+	if (ret)
+		goto mutex_unlock_exit;
+
+	/* OPAL call */
+	rc = opal_err_injct(&ei);
+	if (rc == OPAL_SUCCESS)
+		ret = count;
+	else
+		ret = -EIO;
+
+mutex_unlock_exit:
+	mutex_unlock(&errinjct_mutex);
+	return ret;
+}
+
+static struct kobj_attribute errinjct_attr =
+	__ATTR(errinjct, 0600, NULL, errinjct_store);
+
+void __init opal_errinjct_init(void)
+{
+	int ret;
+
+	/* Make sure /sys/firmware/opal directory is created */
+	if (!opal_kobj) {
+		pr_warn("%s: opal kobject is not available\n",
+			__func__);
+		return;
+	}
+
+	/* Create the sysfs files */
+	ret = sysfs_create_file(opal_kobj, &errinjct_attr.attr);
+	if (ret)
+		pr_warn("%s: Cannot create sysfs file (%d)\n",
+			__func__, ret);
+}
diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index 360ad80c..cb29bb5 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -604,6 +604,8 @@ static int __init opal_init(void)
 		opal_sys_param_init();
 		/* Setup message log interface. */
 		opal_msglog_init();
+		/* Setup error injection interface */
+		opal_errinjct_init();
 	}
 
 	return 0;
-- 
1.8.3.2

^ permalink raw reply related

* [PATCH v1 3/3] powerpc/powernv: Clear PAPR error injection registers
From: Gavin Shan @ 2014-06-23  2:14 UTC (permalink / raw)
  To: kvm-ppc, linuxppc-dev; +Cc: aik, qiudayu, agraf, Gavin Shan
In-Reply-To: <1403489682-14841-1-git-send-email-gwshan@linux.vnet.ibm.com>

The frozen state on one specific PE is probably caused by error
injection, which is done with help of PAPR error injection registers.
According to the hardware spec, those registers should be cleared
automatically after one-shot frozen PE. However, that's not always
true, at least on P7IOC of Firebird-L. So we have to clear them
before doing PE reset to avoid recursive EEH errors at recovery
stage.

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/powernv/eeh-ioda.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
index 79193eb..cbc7c98 100644
--- a/arch/powerpc/platforms/powernv/eeh-ioda.c
+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
@@ -657,6 +657,30 @@ static int ioda_eeh_reset(struct eeh_pe *pe, int option)
 	if (pe->type & EEH_PE_PHB) {
 		ret = ioda_eeh_phb_reset(hose, option);
 	} else {
+		struct pnv_phb *phb;
+		s64 rc;
+
+		/*
+		 * The frozen PE might be caused by PAPR error injection
+		 * registers, which are expected to be cleared after hitting
+		 * frozen PE as stated in the hardware spec. Unfortunately,
+		 * that's not true on P7IOC. So we have to clear it manually
+		 * to avoid recursive EEH errors during recovery.
+		 */
+		phb = hose->private_data;
+		if (phb->model == PNV_PHB_MODEL_P7IOC &&
+		    (option == EEH_RESET_HOT ||
+		    option == EEH_RESET_FUNDAMENTAL)) {
+			rc = opal_pci_reset(phb->opal_id,
+					    OPAL_PHB_ERROR,
+					    OPAL_ASSERT_RESET);
+			if (rc != OPAL_SUCCESS) {
+				pr_warn("%s: Can't clear errinjct reg (%lld)\n",
+					__func__, rc);
+				return -EIO;
+			}
+		}
+
 		bus = eeh_pe_bus_get(pe);
 		if (pci_is_root_bus(bus) ||
 		    pci_is_root_bus(bus->parent))
-- 
1.8.3.2

^ permalink raw reply related

* [PATCH] powerpc/kprobes: Fix jprobes on ABI v2 (LE)
From: Michael Ellerman @ 2014-06-23  3:23 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Anton Blanchard

In commit 721aeaa9 "Build little endian ppc64 kernel with ABIv2", we
missed some updates required in the kprobes code to make jprobes work
when the kernel is built with ABI v2.

Firstly update arch_deref_entry_point() to do the right thing. Now that
we have added ppc_global_function_entry() we can just always use that, it
will do the right thing for 32 & 64 bit and ABI v1 & v2.

Secondly we need to update the code that sets up the register state before
calling the jprobe handler. On ABI v1 we setup r2 to hold the TOC, on ABI
v2 we need to populate r12 with the function entry point address.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
Obviously depends on my previous patch to add ppc_global_function_entry().

 arch/powerpc/kernel/kprobes.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index 90fab64..2f72af8 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -32,6 +32,7 @@
 #include <linux/module.h>
 #include <linux/kdebug.h>
 #include <linux/slab.h>
+#include <asm/code-patching.h>
 #include <asm/cacheflush.h>
 #include <asm/sstep.h>
 #include <asm/uaccess.h>
@@ -491,12 +492,10 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self,
 	return ret;
 }
 
-#ifdef CONFIG_PPC64
 unsigned long arch_deref_entry_point(void *entry)
 {
-	return ((func_descr_t *)entry)->entry;
+	return ppc_global_function_entry(entry);
 }
-#endif
 
 int __kprobes setjmp_pre_handler(struct kprobe *p, struct pt_regs *regs)
 {
@@ -508,8 +507,12 @@ int __kprobes setjmp_pre_handler(struct kprobe *p, struct pt_regs *regs)
 	/* setup return addr to the jprobe handler routine */
 	regs->nip = arch_deref_entry_point(jp->entry);
 #ifdef CONFIG_PPC64
+#if defined(_CALL_ELF) && _CALL_ELF == 2
+	regs->gpr[12] = (unsigned long)jp->entry;
+#else
 	regs->gpr[2] = (unsigned long)(((func_descr_t *)jp->entry)->toc);
 #endif
+#endif
 
 	return 1;
 }
-- 
1.9.1

^ permalink raw reply related

* [PATCH] powerpc: Remove ancient DEBUG_SIG code
From: Michael Ellerman @ 2014-06-23  4:17 UTC (permalink / raw)
  To: linuxppc-dev

We have some compile-time disabled debug code in signal_xx.c. It's from
some ancient time BG, almost certainly part of the original port, given
the very similar code on other arches.

The show_unhandled_signal logic, added in d0c3d534a438 (2.6.24) is
cleaner and prints more useful information, so drop the debug code.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/kernel/signal_32.c | 9 ---------
 arch/powerpc/kernel/signal_64.c | 9 ---------
 2 files changed, 18 deletions(-)

diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 4e47db6..1bc5a17 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -54,7 +54,6 @@
 
 #include "signal.h"
 
-#undef DEBUG_SIG
 
 #ifdef CONFIG_PPC64
 #define sys_rt_sigreturn	compat_sys_rt_sigreturn
@@ -1063,10 +1062,6 @@ int handle_rt_signal32(unsigned long sig, struct k_sigaction *ka,
 	return 1;
 
 badframe:
-#ifdef DEBUG_SIG
-	printk("badframe in handle_rt_signal, regs=%p frame=%p newsp=%lx\n",
-	       regs, frame, newsp);
-#endif
 	if (show_unhandled_signals)
 		printk_ratelimited(KERN_INFO
 				   "%s[%d]: bad frame in handle_rt_signal32: "
@@ -1484,10 +1479,6 @@ int handle_signal32(unsigned long sig, struct k_sigaction *ka,
 	return 1;
 
 badframe:
-#ifdef DEBUG_SIG
-	printk("badframe in handle_signal, regs=%p frame=%p newsp=%lx\n",
-	       regs, frame, newsp);
-#endif
 	if (show_unhandled_signals)
 		printk_ratelimited(KERN_INFO
 				   "%s[%d]: bad frame in handle_signal32: "
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index d501dc4..97c1e4b 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -38,7 +38,6 @@
 
 #include "signal.h"
 
-#define DEBUG_SIG 0
 
 #define GP_REGS_SIZE	min(sizeof(elf_gregset_t), sizeof(struct pt_regs))
 #define FP_REGS_SIZE	sizeof(elf_fpregset_t)
@@ -700,10 +699,6 @@ int sys_rt_sigreturn(unsigned long r3, unsigned long r4, unsigned long r5,
 	return 0;
 
 badframe:
-#if DEBUG_SIG
-	printk("badframe in sys_rt_sigreturn, regs=%p uc=%p &uc->uc_mcontext=%p\n",
-	       regs, uc, &uc->uc_mcontext);
-#endif
 	if (show_unhandled_signals)
 		printk_ratelimited(regs->msr & MSR_64BIT ? fmt64 : fmt32,
 				   current->comm, current->pid, "rt_sigreturn",
@@ -809,10 +804,6 @@ int handle_rt_signal64(int signr, struct k_sigaction *ka, siginfo_t *info,
 	return 1;
 
 badframe:
-#if DEBUG_SIG
-	printk("badframe in setup_rt_frame, regs=%p frame=%p newsp=%lx\n",
-	       regs, frame, newsp);
-#endif
 	if (show_unhandled_signals)
 		printk_ratelimited(regs->msr & MSR_64BIT ? fmt64 : fmt32,
 				   current->comm, current->pid, "setup_rt_frame",
-- 
1.9.1

^ permalink raw reply related

* Re: [RFC PATCH V3 04/17] PCI: SRIOV: add VF enable/disable hook
From: Gavin Shan @ 2014-06-23  5:03 UTC (permalink / raw)
  To: Wei Yang; +Cc: benh, linux-pci, gwshan, yan, bhelgaas, qiudayu, linuxppc-dev
In-Reply-To: <1402365399-5121-5-git-send-email-weiyang@linux.vnet.ibm.com>

On Tue, Jun 10, 2014 at 09:56:26AM +0800, Wei Yang wrote:
>VFs are dynamically created/released when driver enable them. On some
>platforms, like PowerNV, special resources are necessary to enable VFs.
>
>This patch adds two hooks for platform initialization before creating the VFs.
>
>Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>---
> drivers/pci/iov.c |   19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
>diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
>index 1d21f43..cc87773 100644
>--- a/drivers/pci/iov.c
>+++ b/drivers/pci/iov.c
>@@ -250,6 +250,11 @@ static void sriov_disable_migration(struct pci_dev *dev)
> 	iounmap(iov->mstate);
> }
>
>+int __weak pcibios_sriov_enable(struct pci_dev *pdev, u16 vf_num)
>+{
>+       return 0;
>+}
>+
> static int sriov_enable(struct pci_dev *dev, int nr_virtfn)
> {
> 	int rc;
>@@ -260,6 +265,7 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn)
> 	struct pci_dev *pdev;
> 	struct pci_sriov *iov = dev->sriov;
> 	int bars = 0;
>+	int retval;
>
> 	if (!nr_virtfn)
> 		return 0;
>@@ -334,6 +340,12 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn)
> 	if (nr_virtfn < initial)
> 		initial = nr_virtfn;
>
>+	if ((retval = pcibios_sriov_enable(dev, initial))) {
>+		dev_err(&dev->dev, "Failure %d from pcibios_sriov_setup()\n",
>+				retval);

		dev_err(&dev->dev, "Failure %d from pcibios_sriov_enable()\n",
			retval);

>+		return retval;
>+	}
>+
> 	for (i = 0; i < initial; i++) {
> 		rc = virtfn_add(dev, i, 0);
> 		if (rc)
>@@ -368,6 +380,11 @@ failed:
> 	return rc;
> }
>
>+int __weak pcibios_sriov_disable(struct pci_dev *pdev)
>+{
>+       return 0;
>+}
>+
> static void sriov_disable(struct pci_dev *dev)
> {
> 	int i;
>@@ -382,6 +399,8 @@ static void sriov_disable(struct pci_dev *dev)
> 	for (i = 0; i < iov->num_VFs; i++)
> 		virtfn_remove(dev, i, 0);
>
>+	pcibios_sriov_disable(dev);
>+
> 	iov->ctrl &= ~(PCI_SRIOV_CTRL_VFE | PCI_SRIOV_CTRL_MSE);
> 	pci_cfg_access_lock(dev);
> 	pci_write_config_word(dev, iov->pos + PCI_SRIOV_CTRL, iov->ctrl);

Thanks,
Gavin

^ permalink raw reply

* Re: [RFC PATCH V3 02/17] pci/of: Match PCI VFs to dev-tree nodes dynamically
From: Gavin Shan @ 2014-06-23  5:07 UTC (permalink / raw)
  To: Wei Yang; +Cc: benh, linux-pci, gwshan, yan, bhelgaas, qiudayu, linuxppc-dev
In-Reply-To: <1402365399-5121-3-git-send-email-weiyang@linux.vnet.ibm.com>

On Tue, Jun 10, 2014 at 09:56:24AM +0800, Wei Yang wrote:
>As introduced by commit 98d9f30c82 ("pci/of: Match PCI devices to dev-tree nodes
>dynamically"), we need to match PCI devices to their corresponding dev-tree
>nodes. While for VFs, this step was missed.
>
>This patch matches VFs' PCI devices to dev-tree nodes dynamically.
>
>Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>---
> drivers/pci/iov.c |    1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
>index 589ef7d..1d21f43 100644
>--- a/drivers/pci/iov.c
>+++ b/drivers/pci/iov.c
>@@ -67,6 +67,7 @@ static int virtfn_add(struct pci_dev *dev, int id, int reset)
>
> 	virtfn->devfn = pci_iov_virtfn_devfn(dev, id);
> 	virtfn->vendor = dev->vendor;
>+	pci_set_of_node(virtfn);

If the VF and PF seats on different PCI buses, I guess pci_set_of_node() always
binds nothing with the VF. It might be one of the problem your code missed and
I didn't catch this in the code review done previously. However, it shouldn't
be a real problem if we're not going to rely on dynamic device_node.

> 	pci_read_config_word(dev, iov->pos + PCI_SRIOV_VF_DID, &virtfn->device);
> 	pci_setup_device(virtfn);
> 	virtfn->dev.parent = dev->dev.parent;

Thanks,
Gavin

^ permalink raw reply

* Re: [RFC PATCH V3 05/17] ppc/pnv: user macro to define the TCE size
From: Gavin Shan @ 2014-06-23  5:12 UTC (permalink / raw)
  To: Wei Yang; +Cc: benh, linux-pci, gwshan, yan, bhelgaas, qiudayu, linuxppc-dev
In-Reply-To: <1402365399-5121-6-git-send-email-weiyang@linux.vnet.ibm.com>

On Tue, Jun 10, 2014 at 09:56:27AM +0800, Wei Yang wrote:
>During the initialization of the TVT/TCE, it uses digits to specify the TCE IO
>Page Size, TCE Table Size, TCE Entry Size, etc.
>
>This patch replaces those digits with macros, which will be more meaningful and
>easy to read.
>
>Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>

It looks conflicting with "dynamic page size support" posted by Alexey:

http://patchwork.ozlabs.org/patch/356718/
 
>---
> arch/powerpc/include/asm/tce.h            |    3 ++-
> arch/powerpc/platforms/powernv/pci-ioda.c |   25 +++++++++++--------------
> arch/powerpc/platforms/powernv/pci.c      |    2 +-
> arch/powerpc/platforms/powernv/pci.h      |    5 +++++
> 4 files changed, 19 insertions(+), 16 deletions(-)
>
>diff --git a/arch/powerpc/include/asm/tce.h b/arch/powerpc/include/asm/tce.h
>index 743f36b..28a1d06 100644
>--- a/arch/powerpc/include/asm/tce.h
>+++ b/arch/powerpc/include/asm/tce.h
>@@ -40,7 +40,8 @@
> #define TCE_SHIFT	12
> #define TCE_PAGE_SIZE	(1 << TCE_SHIFT)
>
>-#define TCE_ENTRY_SIZE		8		/* each TCE is 64 bits */
>+#define TCE_ENTRY_SHIFT		3
>+#define TCE_ENTRY_SIZE		(1 << TCE_ENTRY_SHIFT)	/* each TCE is 64 bits */
>
> #define TCE_RPN_MASK		0xfffffffffful  /* 40-bit RPN (4K pages) */
> #define TCE_RPN_SHIFT		12
>diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
>index 8ae09cf..9715351 100644
>--- a/arch/powerpc/platforms/powernv/pci-ioda.c
>+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
>@@ -820,9 +820,6 @@ static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,
> 	int64_t rc;
> 	void *addr;
>
>-	/* 256M DMA window, 4K TCE pages, 8 bytes TCE */
>-#define TCE32_TABLE_SIZE	((0x10000000 / 0x1000) * 8)
>-
> 	/* XXX FIXME: Handle 64-bit only DMA devices */
> 	/* XXX FIXME: Provide 64-bit DMA facilities & non-4K TCE tables etc.. */
> 	/* XXX FIXME: Allocate multi-level tables on PHB3 */
>@@ -834,7 +831,7 @@ static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,
> 	/* Grab a 32-bit TCE table */
> 	pe->tce32_seg = base;
> 	pe_info(pe, " Setting up 32-bit TCE table at %08x..%08x\n",
>-		(base << 28), ((base + segs) << 28) - 1);
>+		(base << PNV_TCE32_SEG_SHIFT), ((base + segs) << PNV_TCE32_SEG_SHIFT) - 1);
>
> 	/* XXX Currently, we allocate one big contiguous table for the
> 	 * TCEs. We only really need one chunk per 256M of TCE space
>@@ -842,21 +839,21 @@ static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,
> 	 * requires some added smarts with our get/put_tce implementation
> 	 */
> 	tce_mem = alloc_pages_node(phb->hose->node, GFP_KERNEL,
>-				   get_order(TCE32_TABLE_SIZE * segs));
>+				   get_order(PNV_TCE32_TAB_SIZE * segs));
> 	if (!tce_mem) {
> 		pe_err(pe, " Failed to allocate a 32-bit TCE memory\n");
> 		goto fail;
> 	}
> 	addr = page_address(tce_mem);
>-	memset(addr, 0, TCE32_TABLE_SIZE * segs);
>+	memset(addr, 0, PNV_TCE32_TAB_SIZE * segs);
>
> 	/* Configure HW */
> 	for (i = 0; i < segs; i++) {
> 		rc = opal_pci_map_pe_dma_window(phb->opal_id,
> 					      pe->pe_number,
> 					      base + i, 1,
>-					      __pa(addr) + TCE32_TABLE_SIZE * i,
>-					      TCE32_TABLE_SIZE, 0x1000);
>+					      __pa(addr) + PNV_TCE32_TAB_SIZE * i,
>+					      PNV_TCE32_TAB_SIZE, TCE_PAGE_SIZE);
> 		if (rc) {
> 			pe_err(pe, " Failed to configure 32-bit TCE table,"
> 			       " err %ld\n", rc);
>@@ -866,8 +863,8 @@ static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,
>
> 	/* Setup linux iommu table */
> 	tbl = &pe->tce32_table;
>-	pnv_pci_setup_iommu_table(tbl, addr, TCE32_TABLE_SIZE * segs,
>-				  base << 28);
>+	pnv_pci_setup_iommu_table(tbl, addr, PNV_TCE32_TAB_SIZE * segs,
>+				  base << PNV_TCE32_SEG_SHIFT);
>
> 	/* OPAL variant of P7IOC SW invalidated TCEs */
> 	swinvp = of_get_property(phb->hose->dn, "ibm,opal-tce-kill", NULL);
>@@ -898,7 +895,7 @@ static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,
> 	if (pe->tce32_seg >= 0)
> 		pe->tce32_seg = -1;
> 	if (tce_mem)
>-		__free_pages(tce_mem, get_order(TCE32_TABLE_SIZE * segs));
>+		__free_pages(tce_mem, get_order(PNV_TCE32_TAB_SIZE * segs));
> }
>
> static void pnv_pci_ioda2_set_bypass(struct iommu_table *tbl, bool enable)
>@@ -968,7 +965,7 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb,
> 	/* The PE will reserve all possible 32-bits space */
> 	pe->tce32_seg = 0;
> 	end = (1 << ilog2(phb->ioda.m32_pci_base));
>-	tce_table_size = (end / 0x1000) * 8;
>+	tce_table_size = (end / TCE_PAGE_SIZE) * TCE_ENTRY_SIZE;
> 	pe_info(pe, "Setting up 32-bit TCE table at 0..%08x\n",
> 		end);
>
>@@ -988,7 +985,7 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb,
> 	 */
> 	rc = opal_pci_map_pe_dma_window(phb->opal_id, pe->pe_number,
> 					pe->pe_number << 1, 1, __pa(addr),
>-					tce_table_size, 0x1000);
>+					tce_table_size, TCE_PAGE_SIZE);
> 	if (rc) {
> 		pe_err(pe, "Failed to configure 32-bit TCE table,"
> 		       " err %ld\n", rc);
>@@ -1573,7 +1570,7 @@ void __init pnv_pci_init_ioda_phb(struct device_node *np,
> 	INIT_LIST_HEAD(&phb->ioda.pe_list);
>
> 	/* Calculate how many 32-bit TCE segments we have */
>-	phb->ioda.tce32_count = phb->ioda.m32_pci_base >> 28;
>+	phb->ioda.tce32_count = phb->ioda.m32_pci_base >> PNV_TCE32_SEG_SHIFT;
>
> #if 0 /* We should really do that ... */
> 	rc = opal_pci_set_phb_mem_window(opal->phb_id,
>diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
>index 8518817..687a068 100644
>--- a/arch/powerpc/platforms/powernv/pci.c
>+++ b/arch/powerpc/platforms/powernv/pci.c
>@@ -597,7 +597,7 @@ void pnv_pci_setup_iommu_table(struct iommu_table *tbl,
> 	tbl->it_page_shift = IOMMU_PAGE_SHIFT_4K;
> 	tbl->it_offset = dma_offset >> tbl->it_page_shift;
> 	tbl->it_index = 0;
>-	tbl->it_size = tce_size >> 3;
>+	tbl->it_size = tce_size >> TCE_ENTRY_SHIFT;
> 	tbl->it_busno = 0;
> 	tbl->it_type = TCE_PCI;
> }
>diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
>index 3e5f5a1..90f6da4 100644
>--- a/arch/powerpc/platforms/powernv/pci.h
>+++ b/arch/powerpc/platforms/powernv/pci.h
>@@ -227,4 +227,9 @@ extern void pnv_pci_init_ioda2_phb(struct device_node *np);
> extern void pnv_pci_ioda_tce_invalidate(struct iommu_table *tbl,
> 					__be64 *startp, __be64 *endp, bool rm);
>
>+#define PNV_TCE32_SEG_SHIFT     28
>+#define PNV_TCE32_SEG_SIZE      (1UL << PNV_TCE32_SEG_SHIFT)
>+/* 256M DMA window, 4K TCE pages, 8 bytes TCE */
>+#define PNV_TCE32_TAB_SIZE	((PNV_TCE32_SEG_SIZE / TCE_PAGE_SIZE) * TCE_ENTRY_SIZE)
>+
> #endif /* __POWERNV_PCI_H */

Thanks,
Gavin

^ permalink raw reply

* Re: [RFC PATCH V3 07/17] ppc/pnv: Add function to deconfig a PE
From: Gavin Shan @ 2014-06-23  5:27 UTC (permalink / raw)
  To: Wei Yang; +Cc: benh, linux-pci, gwshan, yan, bhelgaas, qiudayu, linuxppc-dev
In-Reply-To: <1402365399-5121-8-git-send-email-weiyang@linux.vnet.ibm.com>

On Tue, Jun 10, 2014 at 09:56:29AM +0800, Wei Yang wrote:
>On PowerNV platform, it will support dynamic PE allocation and deallocation.
>
>This patch adds a function to release those resources related to a PE.
>
>Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>---
> arch/powerpc/platforms/powernv/pci-ioda.c |   77 +++++++++++++++++++++++++++++
> 1 file changed, 77 insertions(+)
>
>diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
>index 8ca3926..87cb3089 100644
>--- a/arch/powerpc/platforms/powernv/pci-ioda.c
>+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
>@@ -330,6 +330,83 @@ static struct pnv_ioda_pe *pnv_ioda_get_pe(struct pci_dev *dev)
> }
> #endif /* CONFIG_PCI_MSI */
>
>+static int pnv_ioda_deconfigure_pe(struct pnv_phb *phb, struct pnv_ioda_pe *pe)
>+{

Richard, it seems that the deconfiguration is incomplete. Something seems
missed: DMA, IO and MMIO, MSI. If I understand correctly, pnv_ioda_deconfigure_pe()
won't tear down DMA, IO and MMIO, MSI properly. For MSI/MSIx, it wouldn't
be a problem as the VF driver should disable them before calling this function.

>+	struct pci_dev *parent;
>+	uint8_t bcomp, dcomp, fcomp;
>+	int64_t rc;
>+	long rid_end, rid;

Blank line needed here to separate variable declaration and logic. And I think
we won't run into case "if (pe->pbus)" for now. So it's worthy to have some
comments to explain it for a bit :-)

>+	if (pe->pbus) {
>+		int count;
>+
>+		dcomp = OPAL_IGNORE_RID_DEVICE_NUMBER;
>+		fcomp = OPAL_IGNORE_RID_FUNCTION_NUMBER;
>+		parent = pe->pbus->self;
>+		if (pe->flags & PNV_IODA_PE_BUS_ALL)
>+			count = pe->pbus->busn_res.end - pe->pbus->busn_res.start + 1;
>+		else
>+			count = 1;
>+
>+		switch(count) {
>+		case  1: bcomp = OpalPciBusAll;         break;
>+		case  2: bcomp = OpalPciBus7Bits;       break;
>+		case  4: bcomp = OpalPciBus6Bits;       break;
>+		case  8: bcomp = OpalPciBus5Bits;       break;
>+		case 16: bcomp = OpalPciBus4Bits;       break;
>+		case 32: bcomp = OpalPciBus3Bits;       break;
>+		default:
>+			pr_err("%s: Number of subordinate busses %d"
>+			       " unsupported\n",
>+			       pci_name(pe->pbus->self), count);

I guess it's not safe to do "pci_name(pe->pbus->self)" root root bus.

>+			/* Do an exact match only */
>+			bcomp = OpalPciBusAll;
>+		}
>+		rid_end = pe->rid + (count << 8);
>+	}else {

	} else {

>+		parent = pe->pdev->bus->self;
>+		bcomp = OpalPciBusAll;
>+		dcomp = OPAL_COMPARE_RID_DEVICE_NUMBER;
>+		fcomp = OPAL_COMPARE_RID_FUNCTION_NUMBER;
>+		rid_end = pe->rid + 1;
>+	}
>+
>+	/* Disable MVT on IODA1 */
>+	if (phb->type == PNV_PHB_IODA1) {
>+		rc = opal_pci_set_mve_enable(phb->opal_id,
>+					     pe->mve_number, OPAL_DISABLE_MVE);
>+		if (rc) {
>+			pe_err(pe, "OPAL error %ld enabling MVE %d\n",
>+			       rc, pe->mve_number);
>+			pe->mve_number = -1;
>+		}
>+	}
>+	/* Clear the reverse map */
>+	for (rid = pe->rid; rid < rid_end; rid++)
>+		phb->ioda.pe_rmap[rid] = 0;
>+
>+	/* Release from all parents PELT-V */
>+	while (parent) {
>+		struct pci_dn *pdn = pci_get_pdn(parent);
>+		if (pdn && pdn->pe_number != IODA_INVALID_PE) {
>+			rc = opal_pci_set_peltv(phb->opal_id, pdn->pe_number,
>+						pe->pe_number, OPAL_REMOVE_PE_FROM_DOMAIN);
>+			/* XXX What to do in case of error ? */
>+		}
>+		parent = parent->bus->self;
>+	}

It seems that you missed removing the PE from its own PELTV, which was
introduced by commit 631ad69 ("powerpc/powernv: Add PE to its own PELTV").

>+
>+	/* Dissociate PE in PELT */
>+	rc = opal_pci_set_pe(phb->opal_id, pe->pe_number, pe->rid,
>+			     bcomp, dcomp, fcomp, OPAL_UNMAP_PE);
>+	if (rc)
>+		pe_err(pe, "OPAL error %ld trying to setup PELT table\n", rc);
>+
>+	pe->pbus = NULL;
>+	pe->pdev = NULL;
>+
>+	return 0;
>+}
>+
> static int pnv_ioda_configure_pe(struct pnv_phb *phb, struct pnv_ioda_pe *pe)
> {
> 	struct pci_dev *parent;

Thanks,
Gavin

^ permalink raw reply

* Re: [RFC PATCH V3 08/17] PCI: Add weak pcibios_sriov_resource_size() interface
From: Gavin Shan @ 2014-06-23  5:41 UTC (permalink / raw)
  To: Wei Yang; +Cc: benh, linux-pci, gwshan, yan, bhelgaas, qiudayu, linuxppc-dev
In-Reply-To: <1402365399-5121-9-git-send-email-weiyang@linux.vnet.ibm.com>

On Tue, Jun 10, 2014 at 09:56:30AM +0800, Wei Yang wrote:
>When retrieving sriov resource size in pci_sriov_resource_size(), it will
>divide the total IOV resource size with the totalVF number. This is true for
>most cases, while may not be correct on some specific platform.
>
>For example on powernv platform, in order to fix the IOV BAR into a hardware
>alignment, the IOV resource size would be expended. This means the original
>method couldn't work.
>
>This patch introduces a weak pcibios_sriov_resource_size() interface, which
>gives platform a chance to implement specific method to calculate the sriov
>resource size.
>
>Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>---
> drivers/pci/iov.c   |   27 +++++++++++++++++++++++++--
> include/linux/pci.h |    3 +++
> 2 files changed, 28 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
>index cc87773..9fd4648 100644
>--- a/drivers/pci/iov.c
>+++ b/drivers/pci/iov.c
>@@ -45,6 +45,30 @@ static void virtfn_remove_bus(struct pci_bus *physbus, struct pci_bus *virtbus)
> 		pci_remove_bus(virtbus);
> }
>
>+resource_size_t __weak pcibios_sriov_resource_size(struct pci_dev *dev, int resno)
>+{
>+	return 0;
>+}
>+

Please define the prototype of weak function in header files (e.g.
linux/include/pci.h) :-)

If you missed doing same thing for the weak functions added in the
previous patches, you need fix it as well.

>+resource_size_t pci_sriov_resource_size(struct pci_dev *dev, int resno)
>+{
>+	u64 size;

I guess it'd better to be "resource_size_t". 

>+	struct pci_sriov *iov;
>+
>+	if (!dev->is_physfn)
>+		return 0;
>+
>+	size = pcibios_sriov_resource_size(dev, resno);
>+	if (size != 0)
>+		return size;
>+
>+	iov = dev->sriov;
>+	size = resource_size(dev->resource + resno);
>+	do_div(size, iov->total_VFs);
>+
>+	return size;
>+}
>+
> static int virtfn_add(struct pci_dev *dev, int id, int reset)
> {
> 	int i;
>@@ -81,8 +105,7 @@ static int virtfn_add(struct pci_dev *dev, int id, int reset)
> 			continue;
> 		virtfn->resource[i].name = pci_name(virtfn);
> 		virtfn->resource[i].flags = res->flags;
>-		size = resource_size(res);
>-		do_div(size, iov->total_VFs);
>+		size = pci_sriov_resource_size(dev, i + PCI_IOV_RESOURCES);
> 		virtfn->resource[i].start = res->start + size * id;
> 		virtfn->resource[i].end = virtfn->resource[i].start + size - 1;
> 		rc = request_resource(res, &virtfn->resource[i]);
>diff --git a/include/linux/pci.h b/include/linux/pci.h
>index ddb1ca0..315c150 100644
>--- a/include/linux/pci.h
>+++ b/include/linux/pci.h
>@@ -1637,6 +1637,7 @@ int pci_num_vf(struct pci_dev *dev);
> int pci_vfs_assigned(struct pci_dev *dev);
> int pci_sriov_set_totalvfs(struct pci_dev *dev, u16 numvfs);
> int pci_sriov_get_totalvfs(struct pci_dev *dev);
>+resource_size_t pci_sriov_resource_size(struct pci_dev *dev, int resno);
> #else
> static inline int pci_iov_virtfn_bus(struct pci_dev *dev, int id)
> {
>@@ -1658,6 +1659,8 @@ static inline int pci_sriov_set_totalvfs(struct pci_dev *dev, u16 numvfs)
> { return 0; }
> static inline int pci_sriov_get_totalvfs(struct pci_dev *dev)
> { return 0; }
>+static inline resource_size_t pci_sriov_resource_size(struct pci_dev *dev, int resno)
>+{ return -1; }
> #endif
>
> #if defined(CONFIG_HOTPLUG_PCI) || defined(CONFIG_HOTPLUG_PCI_MODULE)

Thanks,
Gavin

^ permalink raw reply

* Re: [RFC PATCH V3 11/17] ppc/pnv: Expand VF resources according to the number of total_pe
From: Gavin Shan @ 2014-06-23  6:07 UTC (permalink / raw)
  To: Wei Yang; +Cc: benh, linux-pci, gwshan, yan, bhelgaas, qiudayu, linuxppc-dev
In-Reply-To: <1402365399-5121-12-git-send-email-weiyang@linux.vnet.ibm.com>

On Tue, Jun 10, 2014 at 09:56:33AM +0800, Wei Yang wrote:
>On PHB3, VF resources will be covered by M64 BAR to have better PE isolation.
>Mostly the total_pe number is different from the total_VFs, which will lead to
>a conflict between MMIO space and the PE number.
>
>This patch expands the VF resource size to reserve total_pe number of VFs'
>resource, which prevents the conflict.
>
>Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>---
> arch/powerpc/include/asm/machdep.h        |    6 +++
> arch/powerpc/include/asm/pci-bridge.h     |    3 ++
> arch/powerpc/kernel/pci-common.c          |   15 ++++++
> arch/powerpc/platforms/powernv/pci-ioda.c |   83 +++++++++++++++++++++++++++++
> 4 files changed, 107 insertions(+)
>
>diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h
>index ad3025d..2f2e770 100644
>--- a/arch/powerpc/include/asm/machdep.h
>+++ b/arch/powerpc/include/asm/machdep.h
>@@ -234,9 +234,15 @@ struct machdep_calls {
>
> 	/* Called after scan and before resource survey */
> 	void (*pcibios_fixup_phb)(struct pci_controller *hose);
>+#ifdef CONFIG_PCI_IOV
>+	void (*pcibios_fixup_sriov)(struct pci_bus *bus);
>+#endif /* CONFIG_PCI_IOV */
>
> 	/* Called during PCI resource reassignment */
> 	resource_size_t (*pcibios_window_alignment)(struct pci_bus *, unsigned long type);
>+#ifdef CONFIG_PCI_IOV
>+	resource_size_t (*__pci_sriov_resource_size)(struct pci_dev *, int resno);

	resource_size_t (*pcibios_sriov_resource_size)(struct pci_dev *, int resno);

You probably can put all SRIOV related functions together:

#ifdef CONFIG_PCI_IOV
	func_a;
	func_b;
	 :
#endif

>+#endif /* CONFIG_PCI_IOV */
>
> 	/* Called to shutdown machine specific hardware not already controlled
> 	 * by other drivers.
>diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
>index 4ca90a3..8c849d8 100644
>--- a/arch/powerpc/include/asm/pci-bridge.h
>+++ b/arch/powerpc/include/asm/pci-bridge.h
>@@ -168,6 +168,9 @@ struct pci_dn {
> #define IODA_INVALID_PE		(-1)
> #ifdef CONFIG_PPC_POWERNV
> 	int	pe_number;
>+#ifdef CONFIG_PCI_IOV
>+	u16     vfs;
>+#endif /* CONFIG_PCI_IOV */
> #endif
> };
>
>diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
>index c449a26..c4e2e92 100644
>--- a/arch/powerpc/kernel/pci-common.c
>+++ b/arch/powerpc/kernel/pci-common.c
>@@ -120,6 +120,16 @@ resource_size_t pcibios_window_alignment(struct pci_bus *bus,
> 	return 1;
> }
>
>+#ifdef CONFIG_PCI_IOV
>+resource_size_t pcibios_sriov_resource_size(struct pci_dev *pdev, int resno)
>+{
>+	if (ppc_md.__pci_sriov_resource_size)
>+		return ppc_md.__pci_sriov_resource_size(pdev, resno);
>+
>+	return 0;
>+}
>+#endif /* CONFIG_PCI_IOV */
>+
> static resource_size_t pcibios_io_size(const struct pci_controller *hose)
> {
> #ifdef CONFIG_PPC64
>@@ -1675,6 +1685,11 @@ void pcibios_scan_phb(struct pci_controller *hose)
> 	if (ppc_md.pcibios_fixup_phb)
> 		ppc_md.pcibios_fixup_phb(hose);
>
>+#ifdef CONFIG_PCI_IOV
>+	if (ppc_md.pcibios_fixup_sriov)
>+		ppc_md.pcibios_fixup_sriov(bus);

One question I probably asked before: why we can't put the logic
of ppc_md.pcibios_fixup_sriov() to ppc_md.pcibios_fixup_phb()?

>+#endif /* CONFIG_PCI_IOV */
>+
> 	/* Configure PCI Express settings */
> 	if (bus && !pci_has_flag(PCI_PROBE_ONLY)) {
> 		struct pci_bus *child;
>diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
>index 87cb3089..7dfad6a 100644
>--- a/arch/powerpc/platforms/powernv/pci-ioda.c
>+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
>@@ -1298,6 +1298,67 @@ static void pnv_pci_init_ioda_msis(struct pnv_phb *phb)
> static void pnv_pci_init_ioda_msis(struct pnv_phb *phb) { }
> #endif /* CONFIG_PCI_MSI */
>
>+#ifdef CONFIG_PCI_IOV
>+static void pnv_pci_ioda_fixup_iov_resources(struct pci_dev *pdev)
>+{
>+	struct pci_controller *hose;
>+	struct pnv_phb *phb;
>+	struct resource *res;
>+	int i;
>+	resource_size_t size;
>+	struct pci_dn *pdn;
>+
>+	if (!pdev->is_physfn || pdev->is_added)
>+		return;
>+
>+	hose = pci_bus_to_host(pdev->bus);
>+	if (!hose) {
>+		dev_err(&pdev->dev, "%s: NULL pci_controller\n", __func__);
>+		return;
>+	}
>+
>+	phb = hose->private_data;
>+	if (!phb) {
>+		dev_err(&pdev->dev, "%s: NULL PHB\n", __func__);
>+		return;
>+	}
>+
>+	pdn = pci_get_pdn(pdev);
>+	pdn->vfs = 0;
>+
>+	for (i = PCI_IOV_RESOURCES; i <= PCI_IOV_RESOURCE_END; i++) {
>+		res = &pdev->resource[i];
>+		if (!res->flags || res->parent)
>+			continue;
>+
>+		if (!is_mem_pref_64_type(res->flags))
>+			continue;
>+
>+		dev_info(&pdev->dev, "PowerNV: Fixing VF BAR[%d] %pR to\n",
>+				i, res);
>+		size = pci_sriov_resource_size(pdev, i);
>+		res->end = res->start + size * phb->ioda.total_pe - 1;
>+		dev_info(&pdev->dev, "                       %pR\n", res);
>+	}
>+	pdn->vfs = phb->ioda.total_pe;
>+}
>+
>+static void pnv_pci_ioda_fixup_sriov(struct pci_bus *bus)
>+{
>+	struct pci_dev *pdev;
>+	struct pci_bus *b;
>+
>+	list_for_each_entry(pdev, &bus->devices, bus_list) {
>+		b = pdev->subordinate;
>+
>+		if (b)
>+			pnv_pci_ioda_fixup_sriov(b);
>+
>+		pnv_pci_ioda_fixup_iov_resources(pdev);
>+	}
>+}
>+#endif /* CONFIG_PCI_IOV */
>+
> /*
>  * This function is supposed to be called on basis of PE from top
>  * to bottom style. So the the I/O or MMIO segment assigned to
>@@ -1498,6 +1559,22 @@ static resource_size_t pnv_pci_window_alignment(struct pci_bus *bus,
> 	return phb->ioda.io_segsize;
> }
>
>+#ifdef CONFIG_PCI_IOV
>+static resource_size_t __pnv_pci_sriov_resource_size(struct pci_dev *pdev, int resno)
>+{
>+	struct pci_dn *pdn = pci_get_pdn(pdev);
>+	u64 size = 0;
>+
>+	if (!pdn->vfs)
>+		return size;
>+
>+	size = resource_size(pdev->resource + resno);
>+	do_div(size, pdn->vfs);
>+
>+	return size;
>+}
>+#endif /* CONFIG_PCI_IOV */
>+
> /* Prevent enabling devices for which we couldn't properly
>  * assign a PE
>  */
>@@ -1692,9 +1769,15 @@ void __init pnv_pci_init_ioda_phb(struct device_node *np,
> 	 * for the P2P bridge bars so that each PCI bus (excluding
> 	 * the child P2P bridges) can form individual PE.
> 	 */
>+#ifdef CONFIG_PCI_IOV
>+	ppc_md.pcibios_fixup_sriov = pnv_pci_ioda_fixup_sriov;
>+#endif /* CONFIG_PCI_IOV */
> 	ppc_md.pcibios_fixup = pnv_pci_ioda_fixup;
> 	ppc_md.pcibios_enable_device_hook = pnv_pci_enable_device_hook;
> 	ppc_md.pcibios_window_alignment = pnv_pci_window_alignment;
>+#ifdef CONFIG_PCI_IOV
>+	ppc_md.__pci_sriov_resource_size = __pnv_pci_sriov_resource_size;
>+#endif /* CONFIG_PCI_IOV */
> 	pci_add_flags(PCI_REASSIGN_ALL_RSRC);
>
> 	/* Reset IODA tables to a clean state */

Thanks,
Gavin

^ permalink raw reply

* Re: [RFC PATCH V3 12/17] powerpc/powernv: implement pcibios_sriov_resource_alignment on powernv
From: Gavin Shan @ 2014-06-23  6:09 UTC (permalink / raw)
  To: Wei Yang; +Cc: benh, linux-pci, gwshan, yan, bhelgaas, qiudayu, linuxppc-dev
In-Reply-To: <1402365399-5121-13-git-send-email-weiyang@linux.vnet.ibm.com>

On Tue, Jun 10, 2014 at 09:56:34AM +0800, Wei Yang wrote:
>This patch implements the pcibios_sriov_resource_alignment() on powernv
>platform.
>
>Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>---
> arch/powerpc/include/asm/machdep.h        |    1 +
> arch/powerpc/kernel/pci-common.c          |    8 ++++++++
> arch/powerpc/platforms/powernv/pci-ioda.c |   17 +++++++++++++++++
> 3 files changed, 26 insertions(+)
>
>diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h
>index 2f2e770..3bbc55f 100644
>--- a/arch/powerpc/include/asm/machdep.h
>+++ b/arch/powerpc/include/asm/machdep.h
>@@ -242,6 +242,7 @@ struct machdep_calls {
> 	resource_size_t (*pcibios_window_alignment)(struct pci_bus *, unsigned long type);
> #ifdef CONFIG_PCI_IOV
> 	resource_size_t (*__pci_sriov_resource_size)(struct pci_dev *, int resno);
>+	resource_size_t (*__pci_sriov_resource_alignment)(struct pci_dev *, int resno, resource_size_t align);
> #endif /* CONFIG_PCI_IOV */
>
> 	/* Called to shutdown machine specific hardware not already controlled
>diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
>index c4e2e92..35345ac 100644
>--- a/arch/powerpc/kernel/pci-common.c
>+++ b/arch/powerpc/kernel/pci-common.c
>@@ -128,6 +128,14 @@ resource_size_t pcibios_sriov_resource_size(struct pci_dev *pdev, int resno)
>
> 	return 0;
> }
>+
>+resource_size_t pcibios_sriov_resource_alignment(struct pci_dev *pdev, int resno, resource_size_t align)
>+{
>+	if (ppc_md.__pci_sriov_resource_alignment)
>+		return ppc_md.__pci_sriov_resource_alignment(pdev, resno, align);
>+
>+	return 0;
>+}
> #endif /* CONFIG_PCI_IOV */
>
> static resource_size_t pcibios_io_size(const struct pci_controller *hose)
>diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
>index 7dfad6a..b0ac851 100644
>--- a/arch/powerpc/platforms/powernv/pci-ioda.c
>+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
>@@ -1573,6 +1573,22 @@ static resource_size_t __pnv_pci_sriov_resource_size(struct pci_dev *pdev, int r
>
> 	return size;
> }
>+
>+static resource_size_t __pnv_pci_sriov_resource_alignment(struct pci_dev *pdev, int resno,
>+		resource_size_t align)

The function could be "pcibios_sriov_resource_alignment()", but it's not a big deal.
If you prefer the original one, then keep it :)

>+{
>+	struct pci_dn *pdn = pci_get_pdn(pdev);
>+	resource_size_t iov_align;
>+
>+	iov_align = resource_size(&pdev->resource[resno]);
>+	if (iov_align)
>+		return iov_align;
>+
>+	if (pdn->vfs)
>+		return pdn->vfs * align;
>+
>+	return align;
>+}
> #endif /* CONFIG_PCI_IOV */
>
> /* Prevent enabling devices for which we couldn't properly
>@@ -1777,6 +1793,7 @@ void __init pnv_pci_init_ioda_phb(struct device_node *np,
> 	ppc_md.pcibios_window_alignment = pnv_pci_window_alignment;
> #ifdef CONFIG_PCI_IOV
> 	ppc_md.__pci_sriov_resource_size = __pnv_pci_sriov_resource_size;
>+	ppc_md.__pci_sriov_resource_alignment = __pnv_pci_sriov_resource_alignment;
> #endif /* CONFIG_PCI_IOV */
> 	pci_add_flags(PCI_REASSIGN_ALL_RSRC);
>

Thanks,
Gavin

^ permalink raw reply

* RE: [PATCH v2 1/2] flexcan: add err_irq handler for flexcan
From: qiang.zhao @ 2014-06-23  6:20 UTC (permalink / raw)
  To: Scott Wood
  Cc: mkl@pengutronix.de, linuxppc-dev@lists.ozlabs.org,
	wg@grandegger.com, linux-can@vger.kernel.org
In-Reply-To: <1403281165.12851.186.camel@snotra.buserror.net>

T24gU2F0LCAyMDE0LTA2LTIxIGF0IDEyOjE5LCBXb29kIFNjb3R0IHdyb3RlOg0KDQo+IC0tLS0t
T3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+IEZyb206IFdvb2QgU2NvdHQtQjA3NDIxDQo+IFNlbnQ6
IFNhdHVyZGF5LCBKdW5lIDIxLCAyMDE0IDEyOjE5IEFNDQo+IFRvOiBaaGFvIFFpYW5nLUI0NTQ3
NQ0KPiBDYzogbGludXhwcGMtZGV2QGxpc3RzLm96bGFicy5vcmc7IGxpbnV4LWNhbkB2Z2VyLmtl
cm5lbC5vcmc7DQo+IHdnQGdyYW5kZWdnZXIuY29tOyBta2xAcGVuZ3V0cm9uaXguZGU7IFdvb2Qg
U2NvdHQtQjA3NDIxDQo+IFN1YmplY3Q6IFJlOiBbUEFUQ0ggdjIgMS8yXSBmbGV4Y2FuOiBhZGQg
ZXJyX2lycSBoYW5kbGVyIGZvciBmbGV4Y2FuDQo+IA0KPiBPbiBGcmksIDIwMTQtMDYtMjAgYXQg
MTA6MDEgKzA4MDAsIFpoYW8gUWlhbmcgd3JvdGU6DQo+ID4gd2hlbiBmbGV4Y2FuIGlzIG5vdCBw
aHlzaWNhbGx5IGxpbmtlZCwgY29tbWFuZCAnY2FudGVzdCcgd2lsbCB0cmlnZ2VyDQo+ID4gYW4g
ZXJyX2lycSwgYWRkIGVycl9pcnEgaGFuZGxlciBmb3IgaXQuDQo+ID4NCj4gPiBTaWduZWQtb2Zm
LWJ5OiBaaGFvIFFpYW5nIDxCNDU0NzVAZnJlZXNjYWxlLmNvbT4NCj4gPiAtLS0NCj4gPiBDaGFu
Z2VzIGZvciB2MjoNCj4gPiAJLSB1c2UgYSBzcGFjZSBpbnN0ZWFkIG9mIHRhYg0KPiA+IAktIHVz
ZSBmbGV4Y2FuX3BvbGxfc3RhdGUgaW5zdGVhZCBvZiBwcmludA0KPiA+DQo+ID4gIGRyaXZlcnMv
bmV0L2Nhbi9mbGV4Y2FuLmMgfCAzMSArKysrKysrKysrKysrKysrKysrKysrKysrKysrKystDQo+
ID4gIDEgZmlsZSBjaGFuZ2VkLCAzMCBpbnNlcnRpb25zKCspLCAxIGRlbGV0aW9uKC0pDQo+ID4N
Cj4gPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9uZXQvY2FuL2ZsZXhjYW4uYyBiL2RyaXZlcnMvbmV0
L2Nhbi9mbGV4Y2FuLmMNCj4gPiBpbmRleCBmNDI1ZWMyLi43NDMyYmE0IDEwMDY0NA0KPiA+IC0t
LSBhL2RyaXZlcnMvbmV0L2Nhbi9mbGV4Y2FuLmMNCj4gPiArKysgYi9kcml2ZXJzL25ldC9jYW4v
ZmxleGNhbi5jDQo+ID4gQEAgLTIwOCw2ICsyMDgsNyBAQCBzdHJ1Y3QgZmxleGNhbl9wcml2IHsN
Cj4gPiAgCXZvaWQgX19pb21lbSAqYmFzZTsNCj4gPiAgCXUzMiByZWdfZXNyOw0KPiA+ICAJdTMy
IHJlZ19jdHJsX2RlZmF1bHQ7DQo+ID4gKwl1bnNpZ25lZCBpbnQgZXJyX2lycTsNCj4gDQo+IFdo
eSB1bnNpZ25lZD8NCkVycl9pcnEgaXMgZnJvbSAwLg0KPiANCj4gPiArc3RhdGljIGlycXJldHVy
bl90IGZsZXhjYW5fZXJyX2lycShpbnQgaXJxLCB2b2lkICpkZXZfaWQpIHsNCj4gPiArCXN0cnVj
dCBuZXRfZGV2aWNlICpkZXYgPSBkZXZfaWQ7DQo+ID4gKwlzdHJ1Y3QgZmxleGNhbl9wcml2ICpw
cml2ID0gbmV0ZGV2X3ByaXYoZGV2KTsNCj4gPiArCXN0cnVjdCBmbGV4Y2FuX3JlZ3MgX19pb21l
bSAqcmVncyA9IHByaXYtPmJhc2U7DQo+ID4gKwl1MzIgcmVnX2N0cmwsIHJlZ19lc3I7DQo+ID4g
Kw0KPiA+ICsJcmVnX2VzciA9IGZsZXhjYW5fcmVhZCgmcmVncy0+ZXNyKTsNCj4gPiArCXJlZ19j
dHJsID0gZmxleGNhbl9yZWFkKCZyZWdzLT5jdHJsKTsNCj4gPiArCWlmIChyZWdfZXNyICYgRkxF
WENBTl9FU1JfVFhfV1JOKSB7DQo+ID4gKwkJZmxleGNhbl93cml0ZShyZWdfZXNyICYgfkZMRVhD
QU5fRVNSX1RYX1dSTiwgJnJlZ3MtPmVzcik7DQo+ID4gKwkJZmxleGNhbl93cml0ZShyZWdfY3Ry
bCAmIH5GTEVYQ0FOX0NUUkxfRVJSX01TSywgJnJlZ3MtPmN0cmwpOw0KPiA+ICsJCWZsZXhjYW5f
cG9sbF9zdGF0ZShkZXYsIHJlZ19lc3IpOw0KPiA+ICsJfQ0KPiA+ICsJcmV0dXJuIElSUV9IQU5E
TEVEOw0KPiA+ICt9DQo+IA0KPiBZb3Ugc2hvdWxkIG9ubHkgcmV0dXJuIElSUV9IQU5ETEVEIGlm
IHRoZXJlIHdhcyBzb21ldGhpbmcgdG8gaGFuZGxlLg0KPiANCj4gPiBAQCAtOTQ0LDYgKzk2Miwx
MiBAQCBzdGF0aWMgaW50IGZsZXhjYW5fb3BlbihzdHJ1Y3QgbmV0X2RldmljZSAqZGV2KQ0KPiA+
ICAJaWYgKGVycikNCj4gPiAgCQlnb3RvIG91dF9jbG9zZTsNCj4gPg0KPiA+ICsJaWYgKHByaXYt
PmVycl9pcnEpDQo+ID4gKwkJZXJyID0gcmVxdWVzdF9pcnEocHJpdi0+ZXJyX2lycSwgZmxleGNh
bl9lcnJfaXJxLCBJUlFGX1NIQVJFRCwNCj4gPiArCQkJCSAgZGV2LT5uYW1lLCBkZXYpOw0KPiA+
ICsJaWYgKGVycikNCj4gPiArCQlnb3RvIG91dF9jbG9zZTsNCj4gDQo+IElzIHRoaXMgcmVhbGx5
IGEgZmF0YWwgZXJyb3I/ICBBbmQgd2h5IGRvIHlvdSBjaGVjayBlcnIgb3V0c2lkZSB0aGUgImlm
DQo+IChwcml2LT5lcnJfaXJxKSIgYmxvY2s/ICBXaGF0IGlmIHNvbWUgcHJldmlvdXMgY29kZSBs
ZWZ0IGVyciBub24temVybw0KPiAoZWl0aGVyIG5vdyBvciBhZnRlciBzb21lIGZ1dHVyZSBjb2Rl
IGNoYW5nZSk/DQo+IA0KPiA+IEBAIC0xMTI2LDYgKzExNTAsMTAgQEAgc3RhdGljIGludCBmbGV4
Y2FuX3Byb2JlKHN0cnVjdCBwbGF0Zm9ybV9kZXZpY2UNCj4gKnBkZXYpDQo+ID4gIAlpZiAoaXJx
IDw9IDApDQo+ID4gIAkJcmV0dXJuIC1FTk9ERVY7DQo+ID4NCj4gPiArCWVycl9pcnEgPSBwbGF0
Zm9ybV9nZXRfaXJxKHBkZXYsIDEpOw0KPiA+ICsJaWYgKGVycl9pcnEgPD0gMCkNCj4gPiArCQll
cnJfaXJxID0gMDsNCj4gPiArDQo+IA0KPiBXaHkgaXMgdGhpcyA8PSAwIGNoZWNrIG5lZWRlZD8N
Cg0KSW50ZXJydXB0WzFdIGlzIG9wdGlvbmFsLiBJZiB0aGVyZSBpcyBub3QgaW50ZXJydXB0WzFd
IGluIGR0YiwgZXJyX2lycSB3aWxsIGJlIDw9MC4NCg0KPiANCj4gLVNjb3R0DQo+IA0KDQo=

^ permalink raw reply

* Re: [RFC PATCH V3 02/17] pci/of: Match PCI VFs to dev-tree nodes dynamically
From: Wei Yang @ 2014-06-23  6:29 UTC (permalink / raw)
  To: Gavin Shan
  Cc: Wei Yang, benh, linux-pci, yan, bhelgaas, qiudayu, linuxppc-dev
In-Reply-To: <20140623050747.GB5069@shangw>

On Mon, Jun 23, 2014 at 03:07:47PM +1000, Gavin Shan wrote:
>On Tue, Jun 10, 2014 at 09:56:24AM +0800, Wei Yang wrote:
>>As introduced by commit 98d9f30c82 ("pci/of: Match PCI devices to dev-tree nodes
>>dynamically"), we need to match PCI devices to their corresponding dev-tree
>>nodes. While for VFs, this step was missed.
>>
>>This patch matches VFs' PCI devices to dev-tree nodes dynamically.
>>
>>Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>>---
>> drivers/pci/iov.c |    1 +
>> 1 file changed, 1 insertion(+)
>>
>>diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
>>index 589ef7d..1d21f43 100644
>>--- a/drivers/pci/iov.c
>>+++ b/drivers/pci/iov.c
>>@@ -67,6 +67,7 @@ static int virtfn_add(struct pci_dev *dev, int id, int reset)
>>
>> 	virtfn->devfn = pci_iov_virtfn_devfn(dev, id);
>> 	virtfn->vendor = dev->vendor;
>>+	pci_set_of_node(virtfn);
>
>If the VF and PF seats on different PCI buses, I guess pci_set_of_node() always
>binds nothing with the VF. It might be one of the problem your code missed and
>I didn't catch this in the code review done previously. However, it shouldn't
>be a real problem if we're not going to rely on dynamic device_node.
>

Thanks for the comment.

This case is not took into consideration yet, so it is not supported now.
While I think it is time to think about the solution now.

Hmm... after reading the code a while, this seems some change in current code.

1. The hierarchy of VF's device node
   
                    +---------+
                    |P2P      |parent
                    +----+----+
                         |       pbus
            +------------+------------+              vbus
            |                         |           ---------------+
       +---------+              +-----+---+          |
       |DEV      |child1        |DEV      |child2    |
       +---------+              +---------+          |
                                                  +----+------+
                                                  |VF         | vchild
                                                  +-----------+

   From the chart above, the left side is the device node hierarchy without
   VFs. Each pci device is the direct child of the P2P bridge. When match pci
   device and its device node, the code go through the parent bus node's child
   list and find the one with same devfn.(in pci_set_of_node()). And we can
   tell the parent bus node is the P2P bridge's device node.

   This works fine, untill VFs need to be added. vbus is a child of the pbus,
   and vbus->self is NULL. So first thing is to set the correct device node
   for this virtual bus. From the chart above, looks both P2P bridge and the
   DEV could be the device node. While the later one seems more reasonable.

2. Reserve virtual bus number in firmware
   This is not a big issue, just reserve enough bus number in firmware.
   Otherwise, pci device and device node may not match.

>> 	pci_|ead_config_word(dev, iov->pos + PCI_SRIOV_VF_DID, &virtfn->device);
>> 	pci_setup_device(virtfn);
>> 	virtfn->dev.parent = dev->dev.parent;
>
>Thanks,
>Gavin

-- 
Richard Yang
Help you, Help me

^ permalink raw reply

* Re: [RFC PATCH V3 04/17] PCI: SRIOV: add VF enable/disable hook
From: Wei Yang @ 2014-06-23  6:29 UTC (permalink / raw)
  To: Gavin Shan
  Cc: Wei Yang, benh, linux-pci, yan, bhelgaas, qiudayu, linuxppc-dev
In-Reply-To: <20140623050310.GA5069@shangw>

On Mon, Jun 23, 2014 at 03:03:10PM +1000, Gavin Shan wrote:
>On Tue, Jun 10, 2014 at 09:56:26AM +0800, Wei Yang wrote:
>>VFs are dynamically created/released when driver enable them. On some
>>platforms, like PowerNV, special resources are necessary to enable VFs.
>>
>>This patch adds two hooks for platform initialization before creating the VFs.
>>
>>Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>>---
>> drivers/pci/iov.c |   19 +++++++++++++++++++
>> 1 file changed, 19 insertions(+)
>>
>>diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
>>index 1d21f43..cc87773 100644
>>--- a/drivers/pci/iov.c
>>+++ b/drivers/pci/iov.c
>>@@ -250,6 +250,11 @@ static void sriov_disable_migration(struct pci_dev *dev)
>> 	iounmap(iov->mstate);
>> }
>>
>>+int __weak pcibios_sriov_enable(struct pci_dev *pdev, u16 vf_num)
>>+{
>>+       return 0;
>>+}
>>+
>> static int sriov_enable(struct pci_dev *dev, int nr_virtfn)
>> {
>> 	int rc;
>>@@ -260,6 +265,7 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn)
>> 	struct pci_dev *pdev;
>> 	struct pci_sriov *iov = dev->sriov;
>> 	int bars = 0;
>>+	int retval;
>>
>> 	if (!nr_virtfn)
>> 		return 0;
>>@@ -334,6 +340,12 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn)
>> 	if (nr_virtfn < initial)
>> 		initial = nr_virtfn;
>>
>>+	if ((retval = pcibios_sriov_enable(dev, initial))) {
>>+		dev_err(&dev->dev, "Failure %d from pcibios_sriov_setup()\n",
>>+				retval);
>
>		dev_err(&dev->dev, "Failure %d from pcibios_sriov_enable()\n",
>			retval);

Thanks

>
>>+		return retval;
>>+	}
>>+
>> 	for (i = 0; i < initial; i++) {
>> 		rc = virtfn_add(dev, i, 0);
>> 		if (rc)
>>@@ -368,6 +380,11 @@ failed:
>> 	return rc;
>> }
>>
>>+int __weak pcibios_sriov_disable(struct pci_dev *pdev)
>>+{
>>+       return 0;
>>+}
>>+
>> static void sriov_disable(struct pci_dev *dev)
>> {
>> 	int i;
>>@@ -382,6 +399,8 @@ static void sriov_disable(struct pci_dev *dev)
>> 	for (i = 0; i < iov->num_VFs; i++)
>> 		virtfn_remove(dev, i, 0);
>>
>>+	pcibios_sriov_disable(dev);
>>+
>> 	iov->ctrl &= ~(PCI_SRIOV_CTRL_VFE | PCI_SRIOV_CTRL_MSE);
>> 	pci_cfg_access_lock(dev);
>> 	pci_write_config_word(dev, iov->pos + PCI_SRIOV_CTRL, iov->ctrl);
>
>Thanks,
>Gavin

-- 
Richard Yang
Help you, Help me

^ permalink raw reply

* Re: [RFC PATCH V3 05/17] ppc/pnv: user macro to define the TCE size
From: Wei Yang @ 2014-06-23  6:31 UTC (permalink / raw)
  To: Gavin Shan
  Cc: Wei Yang, benh, linux-pci, yan, bhelgaas, qiudayu, linuxppc-dev
In-Reply-To: <20140623051233.GA7223@shangw>

On Mon, Jun 23, 2014 at 03:12:33PM +1000, Gavin Shan wrote:
>On Tue, Jun 10, 2014 at 09:56:27AM +0800, Wei Yang wrote:
>>During the initialization of the TVT/TCE, it uses digits to specify the TCE IO
>>Page Size, TCE Table Size, TCE Entry Size, etc.
>>
>>This patch replaces those digits with macros, which will be more meaningful and
>>easy to read.
>>
>>Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>
>It looks conflicting with "dynamic page size support" posted by Alexey:
>
>http://patchwork.ozlabs.org/patch/356718/

Ok, will did some change in next version.

>
>>---
>> arch/powerpc/include/asm/tce.h            |    3 ++-
>> arch/powerpc/platforms/powernv/pci-ioda.c |   25 +++++++++++--------------
>> arch/powerpc/platforms/powernv/pci.c      |    2 +-
>> arch/powerpc/platforms/powernv/pci.h      |    5 +++++
>> 4 files changed, 19 insertions(+), 16 deletions(-)
>>
>>diff --git a/arch/powerpc/include/asm/tce.h b/arch/powerpc/include/asm/tce.h
>>index 743f36b..28a1d06 100644
>>--- a/arch/powerpc/include/asm/tce.h
>>+++ b/arch/powerpc/include/asm/tce.h
>>@@ -40,7 +40,8 @@
>> #define TCE_SHIFT	12
>> #define TCE_PAGE_SIZE	(1 << TCE_SHIFT)
>>
>>-#define TCE_ENTRY_SIZE		8		/* each TCE is 64 bits */
>>+#define TCE_ENTRY_SHIFT		3
>>+#define TCE_ENTRY_SIZE		(1 << TCE_ENTRY_SHIFT)	/* each TCE is 64 bits */
>>
>> #define TCE_RPN_MASK		0xfffffffffful  /* 40-bit RPN (4K pages) */
>> #define TCE_RPN_SHIFT		12
>>diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
>>index 8ae09cf..9715351 100644
>>--- a/arch/powerpc/platforms/powernv/pci-ioda.c
>>+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
>>@@ -820,9 +820,6 @@ static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,
>> 	int64_t rc;
>> 	void *addr;
>>
>>-	/* 256M DMA window, 4K TCE pages, 8 bytes TCE */
>>-#define TCE32_TABLE_SIZE	((0x10000000 / 0x1000) * 8)
>>-
>> 	/* XXX FIXME: Handle 64-bit only DMA devices */
>> 	/* XXX FIXME: Provide 64-bit DMA facilities & non-4K TCE tables etc.. */
>> 	/* XXX FIXME: Allocate multi-level tables on PHB3 */
>>@@ -834,7 +831,7 @@ static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,
>> 	/* Grab a 32-bit TCE table */
>> 	pe->tce32_seg = base;
>> 	pe_info(pe, " Setting up 32-bit TCE table at %08x..%08x\n",
>>-		(base << 28), ((base + segs) << 28) - 1);
>>+		(base << PNV_TCE32_SEG_SHIFT), ((base + segs) << PNV_TCE32_SEG_SHIFT) - 1);
>>
>> 	/* XXX Currently, we allocate one big contiguous table for the
>> 	 * TCEs. We only really need one chunk per 256M of TCE space
>>@@ -842,21 +839,21 @@ static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,
>> 	 * requires some added smarts with our get/put_tce implementation
>> 	 */
>> 	tce_mem = alloc_pages_node(phb->hose->node, GFP_KERNEL,
>>-				   get_order(TCE32_TABLE_SIZE * segs));
>>+				   get_order(PNV_TCE32_TAB_SIZE * segs));
>> 	if (!tce_mem) {
>> 		pe_err(pe, " Failed to allocate a 32-bit TCE memory\n");
>> 		goto fail;
>> 	}
>> 	addr = page_address(tce_mem);
>>-	memset(addr, 0, TCE32_TABLE_SIZE * segs);
>>+	memset(addr, 0, PNV_TCE32_TAB_SIZE * segs);
>>
>> 	/* Configure HW */
>> 	for (i = 0; i < segs; i++) {
>> 		rc = opal_pci_map_pe_dma_window(phb->opal_id,
>> 					      pe->pe_number,
>> 					      base + i, 1,
>>-					      __pa(addr) + TCE32_TABLE_SIZE * i,
>>-					      TCE32_TABLE_SIZE, 0x1000);
>>+					      __pa(addr) + PNV_TCE32_TAB_SIZE * i,
>>+					      PNV_TCE32_TAB_SIZE, TCE_PAGE_SIZE);
>> 		if (rc) {
>> 			pe_err(pe, " Failed to configure 32-bit TCE table,"
>> 			       " err %ld\n", rc);
>>@@ -866,8 +863,8 @@ static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,
>>
>> 	/* Setup linux iommu table */
>> 	tbl = &pe->tce32_table;
>>-	pnv_pci_setup_iommu_table(tbl, addr, TCE32_TABLE_SIZE * segs,
>>-				  base << 28);
>>+	pnv_pci_setup_iommu_table(tbl, addr, PNV_TCE32_TAB_SIZE * segs,
>>+				  base << PNV_TCE32_SEG_SHIFT);
>>
>> 	/* OPAL variant of P7IOC SW invalidated TCEs */
>> 	swinvp = of_get_property(phb->hose->dn, "ibm,opal-tce-kill", NULL);
>>@@ -898,7 +895,7 @@ static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,
>> 	if (pe->tce32_seg >= 0)
>> 		pe->tce32_seg = -1;
>> 	if (tce_mem)
>>-		__free_pages(tce_mem, get_order(TCE32_TABLE_SIZE * segs));
>>+		__free_pages(tce_mem, get_order(PNV_TCE32_TAB_SIZE * segs));
>> }
>>
>> static void pnv_pci_ioda2_set_bypass(struct iommu_table *tbl, bool enable)
>>@@ -968,7 +965,7 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb,
>> 	/* The PE will reserve all possible 32-bits space */
>> 	pe->tce32_seg = 0;
>> 	end = (1 << ilog2(phb->ioda.m32_pci_base));
>>-	tce_table_size = (end / 0x1000) * 8;
>>+	tce_table_size = (end / TCE_PAGE_SIZE) * TCE_ENTRY_SIZE;
>> 	pe_info(pe, "Setting up 32-bit TCE table at 0..%08x\n",
>> 		end);
>>
>>@@ -988,7 +985,7 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb,
>> 	 */
>> 	rc = opal_pci_map_pe_dma_window(phb->opal_id, pe->pe_number,
>> 					pe->pe_number << 1, 1, __pa(addr),
>>-					tce_table_size, 0x1000);
>>+					tce_table_size, TCE_PAGE_SIZE);
>> 	if (rc) {
>> 		pe_err(pe, "Failed to configure 32-bit TCE table,"
>> 		       " err %ld\n", rc);
>>@@ -1573,7 +1570,7 @@ void __init pnv_pci_init_ioda_phb(struct device_node *np,
>> 	INIT_LIST_HEAD(&phb->ioda.pe_list);
>>
>> 	/* Calculate how many 32-bit TCE segments we have */
>>-	phb->ioda.tce32_count = phb->ioda.m32_pci_base >> 28;
>>+	phb->ioda.tce32_count = phb->ioda.m32_pci_base >> PNV_TCE32_SEG_SHIFT;
>>
>> #if 0 /* We should really do that ... */
>> 	rc = opal_pci_set_phb_mem_window(opal->phb_id,
>>diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
>>index 8518817..687a068 100644
>>--- a/arch/powerpc/platforms/powernv/pci.c
>>+++ b/arch/powerpc/platforms/powernv/pci.c
>>@@ -597,7 +597,7 @@ void pnv_pci_setup_iommu_table(struct iommu_table *tbl,
>> 	tbl->it_page_shift = IOMMU_PAGE_SHIFT_4K;
>> 	tbl->it_offset = dma_offset >> tbl->it_page_shift;
>> 	tbl->it_index = 0;
>>-	tbl->it_size = tce_size >> 3;
>>+	tbl->it_size = tce_size >> TCE_ENTRY_SHIFT;
>> 	tbl->it_busno = 0;
>> 	tbl->it_type = TCE_PCI;
>> }
>>diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
>>index 3e5f5a1..90f6da4 100644
>>--- a/arch/powerpc/platforms/powernv/pci.h
>>+++ b/arch/powerpc/platforms/powernv/pci.h
>>@@ -227,4 +227,9 @@ extern void pnv_pci_init_ioda2_phb(struct device_node *np);
>> extern void pnv_pci_ioda_tce_invalidate(struct iommu_table *tbl,
>> 					__be64 *startp, __be64 *endp, bool rm);
>>
>>+#define PNV_TCE32_SEG_SHIFT     28
>>+#define PNV_TCE32_SEG_SIZE      (1UL << PNV_TCE32_SEG_SHIFT)
>>+/* 256M DMA window, 4K TCE pages, 8 bytes TCE */
>>+#define PNV_TCE32_TAB_SIZE	((PNV_TCE32_SEG_SIZE / TCE_PAGE_SIZE) * TCE_ENTRY_SIZE)
>>+
>> #endif /* __POWERNV_PCI_H */
>
>Thanks,
>Gavin

-- 
Richard Yang
Help you, Help me

^ permalink raw reply

* Re: [RFC PATCH V3 11/17] ppc/pnv: Expand VF resources according to the number of total_pe
From: Wei Yang @ 2014-06-23  6:56 UTC (permalink / raw)
  To: Gavin Shan
  Cc: Wei Yang, benh, linux-pci, yan, bhelgaas, qiudayu, linuxppc-dev
In-Reply-To: <20140623060707.GA12055@shangw>

On Mon, Jun 23, 2014 at 04:07:07PM +1000, Gavin Shan wrote:
>On Tue, Jun 10, 2014 at 09:56:33AM +0800, Wei Yang wrote:
>>On PHB3, VF resources will be covered by M64 BAR to have better PE isolation.
>>Mostly the total_pe number is different from the total_VFs, which will lead to
>>a conflict between MMIO space and the PE number.
>>
>>This patch expands the VF resource size to reserve total_pe number of VFs'
>>resource, which prevents the conflict.
>>
>>Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>>---
>> arch/powerpc/include/asm/machdep.h        |    6 +++
>> arch/powerpc/include/asm/pci-bridge.h     |    3 ++
>> arch/powerpc/kernel/pci-common.c          |   15 ++++++
>> arch/powerpc/platforms/powernv/pci-ioda.c |   83 +++++++++++++++++++++++++++++
>> 4 files changed, 107 insertions(+)
>>
>>diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h
>>index ad3025d..2f2e770 100644
>>--- a/arch/powerpc/include/asm/machdep.h
>>+++ b/arch/powerpc/include/asm/machdep.h
>>@@ -234,9 +234,15 @@ struct machdep_calls {
>>
>> 	/* Called after scan and before resource survey */
>> 	void (*pcibios_fixup_phb)(struct pci_controller *hose);
>>+#ifdef CONFIG_PCI_IOV
>>+	void (*pcibios_fixup_sriov)(struct pci_bus *bus);
>>+#endif /* CONFIG_PCI_IOV */
>>
>> 	/* Called during PCI resource reassignment */
>> 	resource_size_t (*pcibios_window_alignment)(struct pci_bus *, unsigned long type);
>>+#ifdef CONFIG_PCI_IOV
>>+	resource_size_t (*__pci_sriov_resource_size)(struct pci_dev *, int resno);
>
>	resource_size_t (*pcibios_sriov_resource_size)(struct pci_dev *, int resno);
>
>You probably can put all SRIOV related functions together:
>
>#ifdef CONFIG_PCI_IOV
>	func_a;
>	func_b;
>	 :
>#endif
>
>>+#endif /* CONFIG_PCI_IOV */
>>
>> 	/* Called to shutdown machine specific hardware not already controlled
>> 	 * by other drivers.
>>diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
>>index 4ca90a3..8c849d8 100644
>>--- a/arch/powerpc/include/asm/pci-bridge.h
>>+++ b/arch/powerpc/include/asm/pci-bridge.h
>>@@ -168,6 +168,9 @@ struct pci_dn {
>> #define IODA_INVALID_PE		(-1)
>> #ifdef CONFIG_PPC_POWERNV
>> 	int	pe_number;
>>+#ifdef CONFIG_PCI_IOV
>>+	u16     vfs;
>>+#endif /* CONFIG_PCI_IOV */
>> #endif
>> };
>>
>>diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
>>index c449a26..c4e2e92 100644
>>--- a/arch/powerpc/kernel/pci-common.c
>>+++ b/arch/powerpc/kernel/pci-common.c
>>@@ -120,6 +120,16 @@ resource_size_t pcibios_window_alignment(struct pci_bus *bus,
>> 	return 1;
>> }
>>
>>+#ifdef CONFIG_PCI_IOV
>>+resource_size_t pcibios_sriov_resource_size(struct pci_dev *pdev, int resno)
>>+{
>>+	if (ppc_md.__pci_sriov_resource_size)
>>+		return ppc_md.__pci_sriov_resource_size(pdev, resno);
>>+
>>+	return 0;
>>+}
>>+#endif /* CONFIG_PCI_IOV */
>>+
>> static resource_size_t pcibios_io_size(const struct pci_controller *hose)
>> {
>> #ifdef CONFIG_PPC64
>>@@ -1675,6 +1685,11 @@ void pcibios_scan_phb(struct pci_controller *hose)
>> 	if (ppc_md.pcibios_fixup_phb)
>> 		ppc_md.pcibios_fixup_phb(hose);
>>
>>+#ifdef CONFIG_PCI_IOV
>>+	if (ppc_md.pcibios_fixup_sriov)
>>+		ppc_md.pcibios_fixup_sriov(bus);
>
>One question I probably asked before: why we can't put the logic
>of ppc_md.pcibios_fixup_sriov() to ppc_md.pcibios_fixup_phb()?
>

Yep, you have asked before and I replied before too :-)

During EEH hotplug, if the PF are removed, the IOV BAR will be retrieved from
the device itself again. If I merge this fixup into
ppc_md.pcibios_fixup_phb(), this is not proper to be invoked at hotplug event.

Or fixup the phb during EEH hotplug is reasonable?


-- 
Richard Yang
Help you, Help me

^ permalink raw reply

* Re: [RFC PATCH V3 11/17] ppc/pnv: Expand VF resources according to the number of total_pe
From: Gavin Shan @ 2014-06-23  7:08 UTC (permalink / raw)
  To: Wei Yang
  Cc: benh, linux-pci, Gavin Shan, yan, bhelgaas, qiudayu, linuxppc-dev
In-Reply-To: <20140623065651.GD5172@richard>

On Mon, Jun 23, 2014 at 02:56:52PM +0800, Wei Yang wrote:
>On Mon, Jun 23, 2014 at 04:07:07PM +1000, Gavin Shan wrote:
>>On Tue, Jun 10, 2014 at 09:56:33AM +0800, Wei Yang wrote:
>>>On PHB3, VF resources will be covered by M64 BAR to have better PE isolation.
>>>Mostly the total_pe number is different from the total_VFs, which will lead to
>>>a conflict between MMIO space and the PE number.
>>>
>>>This patch expands the VF resource size to reserve total_pe number of VFs'
>>>resource, which prevents the conflict.
>>>
>>>Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>>>---
>>> arch/powerpc/include/asm/machdep.h        |    6 +++
>>> arch/powerpc/include/asm/pci-bridge.h     |    3 ++
>>> arch/powerpc/kernel/pci-common.c          |   15 ++++++
>>> arch/powerpc/platforms/powernv/pci-ioda.c |   83 +++++++++++++++++++++++++++++
>>> 4 files changed, 107 insertions(+)
>>>
>>>diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h
>>>index ad3025d..2f2e770 100644
>>>--- a/arch/powerpc/include/asm/machdep.h
>>>+++ b/arch/powerpc/include/asm/machdep.h
>>>@@ -234,9 +234,15 @@ struct machdep_calls {
>>>
>>> 	/* Called after scan and before resource survey */
>>> 	void (*pcibios_fixup_phb)(struct pci_controller *hose);
>>>+#ifdef CONFIG_PCI_IOV
>>>+	void (*pcibios_fixup_sriov)(struct pci_bus *bus);
>>>+#endif /* CONFIG_PCI_IOV */
>>>
>>> 	/* Called during PCI resource reassignment */
>>> 	resource_size_t (*pcibios_window_alignment)(struct pci_bus *, unsigned long type);
>>>+#ifdef CONFIG_PCI_IOV
>>>+	resource_size_t (*__pci_sriov_resource_size)(struct pci_dev *, int resno);
>>
>>	resource_size_t (*pcibios_sriov_resource_size)(struct pci_dev *, int resno);
>>
>>You probably can put all SRIOV related functions together:
>>
>>#ifdef CONFIG_PCI_IOV
>>	func_a;
>>	func_b;
>>	 :
>>#endif
>>
>>>+#endif /* CONFIG_PCI_IOV */
>>>
>>> 	/* Called to shutdown machine specific hardware not already controlled
>>> 	 * by other drivers.
>>>diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
>>>index 4ca90a3..8c849d8 100644
>>>--- a/arch/powerpc/include/asm/pci-bridge.h
>>>+++ b/arch/powerpc/include/asm/pci-bridge.h
>>>@@ -168,6 +168,9 @@ struct pci_dn {
>>> #define IODA_INVALID_PE		(-1)
>>> #ifdef CONFIG_PPC_POWERNV
>>> 	int	pe_number;
>>>+#ifdef CONFIG_PCI_IOV
>>>+	u16     vfs;
>>>+#endif /* CONFIG_PCI_IOV */
>>> #endif
>>> };
>>>
>>>diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
>>>index c449a26..c4e2e92 100644
>>>--- a/arch/powerpc/kernel/pci-common.c
>>>+++ b/arch/powerpc/kernel/pci-common.c
>>>@@ -120,6 +120,16 @@ resource_size_t pcibios_window_alignment(struct pci_bus *bus,
>>> 	return 1;
>>> }
>>>
>>>+#ifdef CONFIG_PCI_IOV
>>>+resource_size_t pcibios_sriov_resource_size(struct pci_dev *pdev, int resno)
>>>+{
>>>+	if (ppc_md.__pci_sriov_resource_size)
>>>+		return ppc_md.__pci_sriov_resource_size(pdev, resno);
>>>+
>>>+	return 0;
>>>+}
>>>+#endif /* CONFIG_PCI_IOV */
>>>+
>>> static resource_size_t pcibios_io_size(const struct pci_controller *hose)
>>> {
>>> #ifdef CONFIG_PPC64
>>>@@ -1675,6 +1685,11 @@ void pcibios_scan_phb(struct pci_controller *hose)
>>> 	if (ppc_md.pcibios_fixup_phb)
>>> 		ppc_md.pcibios_fixup_phb(hose);
>>>
>>>+#ifdef CONFIG_PCI_IOV
>>>+	if (ppc_md.pcibios_fixup_sriov)
>>>+		ppc_md.pcibios_fixup_sriov(bus);
>>
>>One question I probably asked before: why we can't put the logic
>>of ppc_md.pcibios_fixup_sriov() to ppc_md.pcibios_fixup_phb()?
>>
>
>Yep, you have asked before and I replied before too :-)
>
>During EEH hotplug, if the PF are removed, the IOV BAR will be retrieved from
>the device itself again. If I merge this fixup into
>ppc_md.pcibios_fixup_phb(), this is not proper to be invoked at hotplug event.
>
>Or fixup the phb during EEH hotplug is reasonable?
>

Yeah. It's not reasonable to apply fixup to PHB when doing hotplug on PF.

Thanks,
Gavin

^ permalink raw reply

* [PATCH v3 1/2] flexcan: add err_irq handler for flexcan
From: Zhao Qiang @ 2014-06-23  7:11 UTC (permalink / raw)
  To: linuxppc-dev, linux-can, wg, mkl, B07421; +Cc: Zhao Qiang

when flexcan is not physically linked, command 'cantest' will
trigger an err_irq, add err_irq handler for it.

Signed-off-by: Zhao Qiang <B45475@freescale.com>
---
Changes for v2:
	- use a space instead of tab
	- use flexcan_poll_state instead of print
Changes for v3:
	- return IRQ_HANDLED if err is triggered
	- stop transmitted packets when there is an err_interrupt 

 drivers/net/can/flexcan.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index f425ec2..6802a25 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -208,6 +208,7 @@ struct flexcan_priv {
 	void __iomem *base;
 	u32 reg_esr;
 	u32 reg_ctrl_default;
+	unsigned int err_irq;
 
 	struct clk *clk_ipg;
 	struct clk *clk_per;
@@ -744,6 +745,24 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static irqreturn_t flexcan_err_irq(int irq, void *dev_id)
+{
+	struct net_device *dev = dev_id;
+	struct flexcan_priv *priv = netdev_priv(dev);
+	struct flexcan_regs __iomem *regs = priv->base;
+	u32 reg_ctrl, reg_esr;
+
+	reg_esr = flexcan_read(&regs->esr);
+	reg_ctrl = flexcan_read(&regs->ctrl);
+	if (reg_esr & FLEXCAN_ESR_TX_WRN) {
+		flexcan_write(reg_esr & ~FLEXCAN_ESR_TX_WRN, &regs->esr);
+		flexcan_write(reg_ctrl & ~FLEXCAN_CTRL_ERR_MSK, &regs->ctrl);
+		netif_stop_queue(dev);
+		return IRQ_HANDLED;
+	}
+	return IRQ_NONE;
+}
+
 static void flexcan_set_bittiming(struct net_device *dev)
 {
 	const struct flexcan_priv *priv = netdev_priv(dev);
@@ -944,6 +963,15 @@ static int flexcan_open(struct net_device *dev)
 	if (err)
 		goto out_close;
 
+	if (priv->err_irq) {
+		err = request_irq(priv->err_irq, flexcan_err_irq, IRQF_SHARED,
+				  dev->name, dev);
+		if (err) {
+			free_irq(priv->err_irq, dev);
+			goto out_free_irq;
+		}
+	}
+
 	/* start chip and queuing */
 	err = flexcan_chip_start(dev);
 	if (err)
@@ -1099,7 +1127,7 @@ static int flexcan_probe(struct platform_device *pdev)
 	struct resource *mem;
 	struct clk *clk_ipg = NULL, *clk_per = NULL;
 	void __iomem *base;
-	int err, irq;
+	int err, irq, err_irq;
 	u32 clock_freq = 0;
 
 	if (pdev->dev.of_node)
@@ -1126,6 +1154,10 @@ static int flexcan_probe(struct platform_device *pdev)
 	if (irq <= 0)
 		return -ENODEV;
 
+	err_irq = platform_get_irq(pdev, 1);
+	if (err_irq <= 0)
+		err_irq = 0;
+
 	base = devm_ioremap_resource(&pdev->dev, mem);
 	if (IS_ERR(base))
 		return PTR_ERR(base);
@@ -1149,6 +1181,7 @@ static int flexcan_probe(struct platform_device *pdev)
 	dev->flags |= IFF_ECHO;
 
 	priv = netdev_priv(dev);
+	priv->err_irq = err_irq;
 	priv->can.clock.freq = clock_freq;
 	priv->can.bittiming_const = &flexcan_bittiming_const;
 	priv->can.do_set_mode = flexcan_set_mode;
-- 
1.8.5

^ permalink raw reply related

* [PATCH v3 2/2] flexcan: add err interrupt for p1010rdb
From: Zhao Qiang @ 2014-06-23  7:11 UTC (permalink / raw)
  To: linuxppc-dev, linux-can, wg, mkl, B07421; +Cc: Zhao Qiang
In-Reply-To: <1403507484-46547-1-git-send-email-B45475@freescale.com>

add err interrupt for p1010rdb into dts.

Signed-off-by: Zhao Qiang <B45475@freescale.com>
---
Changes for v2:
	- add binding documentation update
Changes for v3:
	- update binding documentation

 Documentation/devicetree/bindings/net/can/fsl-flexcan.txt | 15 +++++++++++++--
 arch/powerpc/boot/dts/fsl/p1010si-post.dtsi               |  6 ++++--
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
index 56d6cc3..7bf377c 100644
--- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
+++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
@@ -10,7 +10,9 @@ Required properties:
   - fsl,p1010-flexcan
 
 - reg : Offset and length of the register set for this device
-- interrupts : Interrupt tuple for this device
+- interrupts : Interrupt tuple for this device.
+	The first interrupt is for FlexCAN(Message Buffer and Wake Up)
+	The second(optional) is for error
 
 Optional properties:
 
@@ -23,7 +25,16 @@ Example:
 	can@1c000 {
 		compatible = "fsl,p1010-flexcan";
 		reg = <0x1c000 0x1000>;
-		interrupts = <48 0x2>;
+		interrupts = <48 0x2 0 0>;
+		interrupt-parent = <&mpic>;
+		clock-frequency = <200000000>; // filled in by bootloader
+	};
+
+	can@1c000 {
+		compatible = "fsl,p1010-flexcan";
+		reg = <0x1c000 0x1000>;
+		interrupts = <48 0x2 0 0
+			      16 0x2 0 0>;
 		interrupt-parent = <&mpic>;
 		clock-frequency = <200000000>; // filled in by bootloader
 	};
diff --git a/arch/powerpc/boot/dts/fsl/p1010si-post.dtsi b/arch/powerpc/boot/dts/fsl/p1010si-post.dtsi
index af12ead..47125a6 100644
--- a/arch/powerpc/boot/dts/fsl/p1010si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/p1010si-post.dtsi
@@ -136,13 +136,15 @@
 	can0: can@1c000 {
 		compatible = "fsl,p1010-flexcan";
 		reg = <0x1c000 0x1000>;
-		interrupts = <48 0x2 0 0>;
+		interrupts = <48 0x2 0 0
+			      16 0x2 0 0>;
 	};
 
 	can1: can@1d000 {
 		compatible = "fsl,p1010-flexcan";
 		reg = <0x1d000 0x1000>;
-		interrupts = <61 0x2 0 0>;
+		interrupts = <61 0x2 0 0
+			      16 0x2 0 0>;
 	};
 
 	L2: l2-cache-controller@20000 {
-- 
1.8.5

^ permalink raw reply related

* Re: [PATCH v3 1/2] flexcan: add err_irq handler for flexcan
From: Marc Kleine-Budde @ 2014-06-23  7:17 UTC (permalink / raw)
  To: Zhao Qiang, linuxppc-dev, linux-can, wg, B07421
In-Reply-To: <1403507484-46547-1-git-send-email-B45475@freescale.com>

[-- Attachment #1: Type: text/plain, Size: 2034 bytes --]

On 06/23/2014 09:11 AM, Zhao Qiang wrote:
> when flexcan is not physically linked, command 'cantest' will
> trigger an err_irq, add err_irq handler for it.
> 
> Signed-off-by: Zhao Qiang <B45475@freescale.com>
> ---
> Changes for v2:
> 	- use a space instead of tab
> 	- use flexcan_poll_state instead of print
> Changes for v3:
> 	- return IRQ_HANDLED if err is triggered
> 	- stop transmitted packets when there is an err_interrupt 
> 
>  drivers/net/can/flexcan.c | 35 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index f425ec2..6802a25 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -208,6 +208,7 @@ struct flexcan_priv {
>  	void __iomem *base;
>  	u32 reg_esr;
>  	u32 reg_ctrl_default;
> +	unsigned int err_irq;
>  
>  	struct clk *clk_ipg;
>  	struct clk *clk_per;
> @@ -744,6 +745,24 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
>  	return IRQ_HANDLED;
>  }
>  
> +static irqreturn_t flexcan_err_irq(int irq, void *dev_id)
> +{
> +	struct net_device *dev = dev_id;
> +	struct flexcan_priv *priv = netdev_priv(dev);
> +	struct flexcan_regs __iomem *regs = priv->base;
> +	u32 reg_ctrl, reg_esr;
> +
> +	reg_esr = flexcan_read(&regs->esr);
> +	reg_ctrl = flexcan_read(&regs->ctrl);
> +	if (reg_esr & FLEXCAN_ESR_TX_WRN) {

When does the hardware trigger the interrupt?

> +		flexcan_write(reg_esr & ~FLEXCAN_ESR_TX_WRN, &regs->esr);
> +		flexcan_write(reg_ctrl & ~FLEXCAN_CTRL_ERR_MSK, &regs->ctrl);
> +		netif_stop_queue(dev);

Why are you stopping the txqueue?

> +		return IRQ_HANDLED;
> +	}
> +	return IRQ_NONE;
> +}
> +

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 242 bytes --]

^ permalink raw reply

* RE: [PATCH v3 1/2] flexcan: add err_irq handler for flexcan
From: qiang.zhao @ 2014-06-23  7:26 UTC (permalink / raw)
  To: Marc Kleine-Budde, linuxppc-dev@lists.ozlabs.org,
	linux-can@vger.kernel.org, wg@grandegger.com, Scott Wood
In-Reply-To: <53A7D497.7080806@pengutronix.de>

DQpPbiAwNi8yMy8yMDE0IDAzOjE4IFBNLCBNYXJjIEtsZWluZS1CdWRkZSB3cm90ZToNCg0KPiAN
Cj4gT24gMDYvMjMvMjAxNCAwOToxMSBBTSwgWmhhbyBRaWFuZyB3cm90ZToNCj4gPiB3aGVuIGZs
ZXhjYW4gaXMgbm90IHBoeXNpY2FsbHkgbGlua2VkLCBjb21tYW5kICdjYW50ZXN0JyB3aWxsIHRy
aWdnZXINCj4gPiBhbiBlcnJfaXJxLCBhZGQgZXJyX2lycSBoYW5kbGVyIGZvciBpdC4NCj4gPg0K
PiA+IFNpZ25lZC1vZmYtYnk6IFpoYW8gUWlhbmcgPEI0NTQ3NUBmcmVlc2NhbGUuY29tPg0KPiA+
IC0tLQ0KPiA+IENoYW5nZXMgZm9yIHYyOg0KPiA+IAktIHVzZSBhIHNwYWNlIGluc3RlYWQgb2Yg
dGFiDQo+ID4gCS0gdXNlIGZsZXhjYW5fcG9sbF9zdGF0ZSBpbnN0ZWFkIG9mIHByaW50IENoYW5n
ZXMgZm9yIHYzOg0KPiA+IAktIHJldHVybiBJUlFfSEFORExFRCBpZiBlcnIgaXMgdHJpZ2dlcmVk
DQo+ID4gCS0gc3RvcCB0cmFuc21pdHRlZCBwYWNrZXRzIHdoZW4gdGhlcmUgaXMgYW4gZXJyX2lu
dGVycnVwdA0KPiA+DQo+ID4gIGRyaXZlcnMvbmV0L2Nhbi9mbGV4Y2FuLmMgfCAzNSArKysrKysr
KysrKysrKysrKysrKysrKysrKysrKysrKysrLQ0KPiA+ICAxIGZpbGUgY2hhbmdlZCwgMzQgaW5z
ZXJ0aW9ucygrKSwgMSBkZWxldGlvbigtKQ0KPiA+DQo+ID4gZGlmZiAtLWdpdCBhL2RyaXZlcnMv
bmV0L2Nhbi9mbGV4Y2FuLmMgYi9kcml2ZXJzL25ldC9jYW4vZmxleGNhbi5jDQo+ID4gaW5kZXgg
ZjQyNWVjMi4uNjgwMmEyNSAxMDA2NDQNCj4gPiAtLS0gYS9kcml2ZXJzL25ldC9jYW4vZmxleGNh
bi5jDQo+ID4gKysrIGIvZHJpdmVycy9uZXQvY2FuL2ZsZXhjYW4uYw0KPiA+IEBAIC0yMDgsNiAr
MjA4LDcgQEAgc3RydWN0IGZsZXhjYW5fcHJpdiB7DQo+ID4gIAl2b2lkIF9faW9tZW0gKmJhc2U7
DQo+ID4gIAl1MzIgcmVnX2VzcjsNCj4gPiAgCXUzMiByZWdfY3RybF9kZWZhdWx0Ow0KPiA+ICsJ
dW5zaWduZWQgaW50IGVycl9pcnE7DQo+ID4NCj4gPiAgCXN0cnVjdCBjbGsgKmNsa19pcGc7DQo+
ID4gIAlzdHJ1Y3QgY2xrICpjbGtfcGVyOw0KPiA+IEBAIC03NDQsNiArNzQ1LDI0IEBAIHN0YXRp
YyBpcnFyZXR1cm5fdCBmbGV4Y2FuX2lycShpbnQgaXJxLCB2b2lkDQo+ICpkZXZfaWQpDQo+ID4g
IAlyZXR1cm4gSVJRX0hBTkRMRUQ7DQo+ID4gIH0NCj4gPg0KPiA+ICtzdGF0aWMgaXJxcmV0dXJu
X3QgZmxleGNhbl9lcnJfaXJxKGludCBpcnEsIHZvaWQgKmRldl9pZCkgew0KPiA+ICsJc3RydWN0
IG5ldF9kZXZpY2UgKmRldiA9IGRldl9pZDsNCj4gPiArCXN0cnVjdCBmbGV4Y2FuX3ByaXYgKnBy
aXYgPSBuZXRkZXZfcHJpdihkZXYpOw0KPiA+ICsJc3RydWN0IGZsZXhjYW5fcmVncyBfX2lvbWVt
ICpyZWdzID0gcHJpdi0+YmFzZTsNCj4gPiArCXUzMiByZWdfY3RybCwgcmVnX2VzcjsNCj4gPiAr
DQo+ID4gKwlyZWdfZXNyID0gZmxleGNhbl9yZWFkKCZyZWdzLT5lc3IpOw0KPiA+ICsJcmVnX2N0
cmwgPSBmbGV4Y2FuX3JlYWQoJnJlZ3MtPmN0cmwpOw0KPiA+ICsJaWYgKHJlZ19lc3IgJiBGTEVY
Q0FOX0VTUl9UWF9XUk4pIHsNCj4gDQo+IFdoZW4gZG9lcyB0aGUgaGFyZHdhcmUgdHJpZ2dlciB0
aGUgaW50ZXJydXB0Pw0KDQpXaGVuIHRoZXJlIGlzIG5vIHdpcmUgbGluayBiZXR3ZWVuIHR4IGFu
ZCByeCwgdHggc3RhcnQgdHJhbnNmZXIgYW5kIGRvZXNu4oCZdCBnZXQgdGhlIGFjay4NCg0KPiAN
Cj4gPiArCQlmbGV4Y2FuX3dyaXRlKHJlZ19lc3IgJiB+RkxFWENBTl9FU1JfVFhfV1JOLCAmcmVn
cy0+ZXNyKTsNCj4gPiArCQlmbGV4Y2FuX3dyaXRlKHJlZ19jdHJsICYgfkZMRVhDQU5fQ1RSTF9F
UlJfTVNLLCAmcmVncy0+Y3RybCk7DQo+ID4gKwkJbmV0aWZfc3RvcF9xdWV1ZShkZXYpOw0KPiAN
Cj4gV2h5IGFyZSB5b3Ugc3RvcHBpbmcgdGhlIHR4cXVldWU/DQoNClRoZXJlIGlzIG5vIHdpcmUg
bGluaywgdHggY2FuJ3QgdHJhbnNmZXIgc3VjY2Vzc2Z1bGx5LiANCg0KPiANCj4gPiArCQlyZXR1
cm4gSVJRX0hBTkRMRUQ7DQo+ID4gKwl9DQo+ID4gKwlyZXR1cm4gSVJRX05PTkU7DQo+ID4gK30N
Cj4gPiArDQo+IA0KPiBNYXJjDQo+IA0KPiAtLQ0KPiBQZW5ndXRyb25peCBlLksuICAgICAgICAg
ICAgICAgICAgfCBNYXJjIEtsZWluZS1CdWRkZSAgICAgICAgICAgfA0KPiBJbmR1c3RyaWFsIExp
bnV4IFNvbHV0aW9ucyAgICAgICAgfCBQaG9uZTogKzQ5LTIzMS0yODI2LTkyNCAgICAgfA0KPiBW
ZXJ0cmV0dW5nIFdlc3QvRG9ydG11bmQgICAgICAgICAgfCBGYXg6ICAgKzQ5LTUxMjEtMjA2OTE3
LTU1NTUgfA0KPiBBbXRzZ2VyaWNodCBIaWxkZXNoZWltLCBIUkEgMjY4NiAgfCBodHRwOi8vd3d3
LnBlbmd1dHJvbml4LmRlICAgfA0KDQo=

^ permalink raw reply

* Re: [PATCH v3 1/2] flexcan: add err_irq handler for flexcan
From: Marc Kleine-Budde @ 2014-06-23  7:37 UTC (permalink / raw)
  To: qiang.zhao@freescale.com, linuxppc-dev@lists.ozlabs.org,
	linux-can@vger.kernel.org, wg@grandegger.com, Scott Wood
In-Reply-To: <d7e8cae0bc584530bedc8febf5476fcf@BLUPR03MB341.namprd03.prod.outlook.com>

[-- Attachment #1: Type: text/plain, Size: 2901 bytes --]

On 06/23/2014 09:26 AM, qiang.zhao@freescale.com wrote:
> 
> On 06/23/2014 03:18 PM, Marc Kleine-Budde wrote:
> 
>>
>> On 06/23/2014 09:11 AM, Zhao Qiang wrote:
>>> when flexcan is not physically linked, command 'cantest' will trigger
>>> an err_irq, add err_irq handler for it.
>>>
>>> Signed-off-by: Zhao Qiang <B45475@freescale.com>
>>> ---
>>> Changes for v2:
>>> 	- use a space instead of tab
>>> 	- use flexcan_poll_state instead of print Changes for v3:
>>> 	- return IRQ_HANDLED if err is triggered
>>> 	- stop transmitted packets when there is an err_interrupt
>>>
>>>  drivers/net/can/flexcan.c | 35 ++++++++++++++++++++++++++++++++++-
>>>  1 file changed, 34 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
>>> index f425ec2..6802a25 100644
>>> --- a/drivers/net/can/flexcan.c
>>> +++ b/drivers/net/can/flexcan.c
>>> @@ -208,6 +208,7 @@ struct flexcan_priv {
>>>  	void __iomem *base;
>>>  	u32 reg_esr;
>>>  	u32 reg_ctrl_default;
>>> +	unsigned int err_irq;
>>>
>>>  	struct clk *clk_ipg;
>>>  	struct clk *clk_per;
>>> @@ -744,6 +745,24 @@ static irqreturn_t flexcan_irq(int irq, void
>> *dev_id)
>>>  	return IRQ_HANDLED;
>>>  }
>>>
>>> +static irqreturn_t flexcan_err_irq(int irq, void *dev_id) {
>>> +	struct net_device *dev = dev_id;
>>> +	struct flexcan_priv *priv = netdev_priv(dev);
>>> +	struct flexcan_regs __iomem *regs = priv->base;
>>> +	u32 reg_ctrl, reg_esr;
>>> +
>>> +	reg_esr = flexcan_read(&regs->esr);
>>> +	reg_ctrl = flexcan_read(&regs->ctrl);
>>> +	if (reg_esr & FLEXCAN_ESR_TX_WRN) {
>>
>> When does the hardware trigger the interrupt?
> 
> When there is no wire link between tx and rx, tx start transfer and doesn’t get the ack.

You are testing for the warning interrupt, not for the
FLEXCAN_ESR_ACK_ERR (which is triggered there isn't any ACK).

>>> +		flexcan_write(reg_esr & ~FLEXCAN_ESR_TX_WRN, &regs->esr);
>>> +		flexcan_write(reg_ctrl & ~FLEXCAN_CTRL_ERR_MSK, &regs->ctrl);
>>> +		netif_stop_queue(dev);
>>
>> Why are you stopping the txqueue?
> 
> There is no wire link, tx can't transfer successfully. 

You are testing for the warning interrupt, which is triggered if the
error counter increases from 95 to 96. And the error counter can
increase due to several reasons. No link is only one of them. If the CAN
core cannot transmit new packages any more the flow control in the
driver will take care.

What about calling the normal interrupt if er err_irq occurs, as this
function will take care of both normal and error interrupts anyway?

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 242 bytes --]

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox