[PATCH v2 1/2] powerpc: Define PVR value for POWER8NVL processor

Signed-off-by: Philippe Bergheaud <redacted>
---
V2:
  - New patch, added to patch set

 arch/powerpc/include/asm/reg.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index c4cb2ff..6a6de4a 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h

@@ -1174,6 +1174,7 @@
 #define PVR_970GX	0x0045
 #define PVR_POWER7p	0x004A
 #define PVR_POWER8E	0x004B
+#define PVR_POWER8NVL	0x004C
 #define PVR_POWER8	0x004D
 #define PVR_BE		0x0070
 #define PVR_PA6T	0x0090

-- 
2.1.0

The POWER8NVL chip has two CAPI ports.  Configure the PSL to route
data to the port corresponding to the CAPP unit.

Signed-off-by: Philippe Bergheaud <redacted>
---
V2:
  - Complete rewrite after Mikey's review

 drivers/misc/cxl/pci.c | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
index 0c6c17a1..924ba63 100644
--- a/drivers/misc/cxl/pci.c
+++ b/drivers/misc/cxl/pci.c

@@ -22,6 +22,7 @@
 #include <asm/pci-bridge.h> /* for struct pci_controller */
 #include <asm/pnv-pci.h>
 #include <asm/io.h>
+#include <asm/reg.h>
 
 #include "cxl.h"
 #include <misc/cxl.h>

@@ -340,12 +341,35 @@ static void dump_afu_descriptor(struct cxl_afu *afu)
 #undef show_reg
 }
 
+#define CAPP_UNIT0_ID 0xBA
+#define CAPP_UNIT1_ID 0XBE
+
+static u64 capp_unit_id(struct device_node *np)
+{
+	const __be32 *prop;
+	u64 phb_index;
+
+	/* For chips other than POWER8NVL, we only have CAPP 0,
+	 * irrespective of which PHB is used */
+	if (!pvr_version_is(PVR_POWER8NVL))
+		return CAPP_UNIT0_ID ;
+
+	/* For POWER8NVL, assume CAPP 0 is attached to PHB0 and
+	 * CAPP 1 is attached to PHB1 */
+	prop = of_get_property(np, "ibm,phb-index", NULL);
+	if (!prop)
+		return 0;
+	phb_index = be32_to_cpup(prop);
+	return phb_index ? CAPP_UNIT1_ID : CAPP_UNIT0_ID;
+}
+
 static int init_implementation_adapter_regs(struct cxl *adapter, struct pci_dev *dev)
 {
 	struct device_node *np;
 	const __be32 *prop;
 	u64 psl_dsnctl;
 	u64 chipid;
+	u64 cappunitid;
 
 	if (!(np = pnv_pci_get_phb_node(dev)))
 		return -ENODEV;

@@ -355,10 +379,15 @@ static int init_implementation_adapter_regs(struct cxl *adapter, struct pci_dev
 	if (!np)
 		return -ENODEV;
 	chipid = be32_to_cpup(prop);
+	cappunitid = capp_unit_id(np);
 	of_node_put(np);
+	if (!cappunitid)
+		return -ENODEV;
 
 	/* Tell PSL where to route data to */
-	psl_dsnctl = 0x02E8900002000000ULL | (chipid << (63-5));
+	psl_dsnctl = 0x0000900002000000ULL | (chipid << (63-5));
+	psl_dsnctl |= (cappunitid << (63-13));
+
 	cxl_p1_write(adapter, CXL_PSL_DSNDCTL, psl_dsnctl);
 	cxl_p1_write(adapter, CXL_PSL_RESLCKTO, 0x20000000200ULL);
 	/* snoop write mask */

-- 
2.1.0

Acked-by: Ian Munsie <redacted>

Acked-by: Ian Munsie <redacted>

On Wed, 2016-16-03 at 16:28:55 UTC, Philippe Bergheaud wrote:

Oh wow, we have those defined separately to the cputable. That is "special" :/

Not your fault though.

cheers

On Wed, 2016-16-03 at 16:28:56 UTC, Philippe Bergheaud wrote:

It's always nice if function names contain a verb, eg. get_capp_unit_id().

+{
+	const __be32 *prop;
+	u64 phb_index;
+
+	/* For chips other than POWER8NVL, we only have CAPP 0,
+	 * irrespective of which PHB is used */

Can you use the standard block commenting style please, eg:

/*
 * For chips ..
 */

+	if (!pvr_version_is(PVR_POWER8NVL))
+		return CAPP_UNIT0_ID ;

                                    ^
				    don't need a space here

+	/* For POWER8NVL, assume CAPP 0 is attached to PHB0 and
+	 * CAPP 1 is attached to PHB1 */
+	prop = of_get_property(np, "ibm,phb-index", NULL);

Please use of_property_read_u32().

+	if (!prop)
+		return 0;
+	phb_index = be32_to_cpup(prop);

Some blank lines here would help readability.

+	return phb_index ? CAPP_UNIT1_ID : CAPP_UNIT0_ID;

This logic doesn't quite match your comment above. This says that any non-zero
PHB index is CAPP1, whereas the comment says CAPP1 == PHB1.

+}
+
 static int init_implementation_adapter_regs(struct cxl *adapter, struct pci_dev *dev)
 {
 	struct device_node *np;
 	const __be32 *prop;
 	u64 psl_dsnctl;
 	u64 chipid;
+	u64 cappunitid;

capp_unit_id ?

You probably want a pr_xxx() there don't you?

 	/* Tell PSL where to route data to */
-	psl_dsnctl = 0x02E8900002000000ULL | (chipid << (63-5));
+	psl_dsnctl = 0x0000900002000000ULL | (chipid << (63-5));
+	psl_dsnctl |= (cappunitid << (63-13));
+
 	cxl_p1_write(adapter, CXL_PSL_DSNDCTL, psl_dsnctl);
 	cxl_p1_write(adapter, CXL_PSL_RESLCKTO, 0x20000000200ULL);
 	/* snoop write mask */

cheers