| Message ID | 1458052008-29254-1-git-send-email-felix@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
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. > Configure the PSL to route data to > the port corresponding to the PHB index. Isn't this capp unit in reality, not phb index? > > 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)) > + > 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? 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 > + } > + of_node_put(np); > + > cxl_p1_write(adapter, CXL_PSL_DSNDCTL, psl_dsnctl); > cxl_p1_write(adapter, CXL_PSL_RESLCKTO, 0x20000000200ULL); > /* snoop write mask */
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
> > > + 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. OK, can you document that better as what you're doing now is not clear even when reading the associated docs. Mikey
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) + 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)); + } + of_node_put(np); + cxl_p1_write(adapter, CXL_PSL_DSNDCTL, psl_dsnctl); cxl_p1_write(adapter, CXL_PSL_RESLCKTO, 0x20000000200ULL); /* snoop write mask */
Naples CPUs have two CAPI ports. Configure the PSL to route data to the port corresponding to the PHB index. Signed-off-by: Philippe Bergheaud <felix@linux.vnet.ibm.com> --- drivers/misc/cxl/pci.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)