Re: [PATCH] cxl: Configure the PSL for dual port CAPI on Naples

[Philippe Bergheaud <felix@linux.vnet.ibm.com>]

Michael Neuling wrote:
> On Tue, 2016-03-15 at 15:26 +0100, Philippe Bergheaud wrote:
> 
>>Naples CPUs have two CAPI ports.  
> 
> 
> Naples is an internal name, don't use that.  Use POWER8NVL is the name
> we use in the kernel.
> 
> alsi, it's a "chip" that has two CAPI ports, not the CPU.
> 
OK, I will rephrase.
> 
>>Configure the PSL to route data to
>>the port corresponding to the PHB index.
> 
> 
> Isn't this capp unit in reality, not phb index?
> 
Yes, I meant capp unit port.
> 
>>Signed-off-by: Philippe Bergheaud <felix@linux.vnet.ibm.com>
>>---
>> drivers/misc/cxl/pci.c | 15 ++++++++++++++-
>> 1 file changed, 14 insertions(+), 1 deletion(-)
>>
>>diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
>>index 0c6c17a1..3db0a0b 100644
>>--- a/drivers/misc/cxl/pci.c
>>+++ b/drivers/misc/cxl/pci.c
>>@@ -340,12 +340,15 @@ static void dump_afu_descriptor(struct cxl_afu
>>*afu)
>> #undef show_reg
>> }
>> 
>>+#define CPU_IS_NAPLES() (cur_cpu_spec->pvr_value == 0x004c0000)
> 
> 
> Use pvr_version_is(PVR_POWER8NVL))
> 
OK.
> 
>>+
>> 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 phb_index;
>> 
>> 	if (!(np = pnv_pci_get_phb_node(dev)))
>> 		return -ENODEV;
>>@@ -355,10 +358,20 @@ static int
>>init_implementation_adapter_regs(struct cxl *adapter, struct pci_dev
>> 	if (!np)
>> 		return -ENODEV;
>> 	chipid = be32_to_cpup(prop);
>>-	of_node_put(np);
>> 
>> 	/* Tell PSL where to route data to */
>> 	psl_dsnctl = 0x02E8900002000000ULL | (chipid << (63-5));
>>+	if (CPU_IS_NAPLES()) {
>>+		prop = of_get_property(np, "ibm,phb-index", NULL);
>>+		if (!prop) {
>>+			of_node_put(np);
>>+			return -ENODEV;
>>+		}
>>+		phb_index = be32_to_cpup(prop);
>>+		psl_dsnctl |= (phb_index << (63-11));
> 
> 
> Looking at the psl docs, cappunitid in the dsndctl is bits 6 to 13.  So
> why 11 here?
> 
Because on POWER8NVL, dsndctl bit 11 == phb_index == cappunitid.
Bits 6-10 and 12-13 do not change between POWER8 and POWER8NVL.

> Can you abstract this better and make it clear what's happening?  Try
> something like this:
> 
> int capp_unit()
> {
> 
> 	if (!pvr_version_is(PVR_POWER8NVL))
> 	/* For chips other than POWER8NVL, we only have CAPP 0
>          * irrespective of which PHB is used */
> 		return 0;
> 
> 	/* 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) {
> 		of_node_put(np);
> 		return -ENODEV;
> 	}
> 	return be32_to_cpup(prop);
> }
> 
> Then you can do something like (although you need to fix the error
> case)
>         psl_dsnctl |= (capp_unit(p) << (63-13));
> 
> Mikey
> 
OK. I will. Thank you.

Philippe
> 
>>+	}
>>+	of_node_put(np);
>>+
>> 	cxl_p1_write(adapter, CXL_PSL_DSNDCTL, psl_dsnctl);
>> 	cxl_p1_write(adapter, CXL_PSL_RESLCKTO, 0x20000000200ULL);
>> 	/* snoop write mask */
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev