| Message ID | 1421290601-3293-2-git-send-email-grimm@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
Excerpts from Ryan Grimm's message of 2015-01-15 13:56:40 +1100: > Add mode to opal call. SNOOP control turns CAPP unit snooping on/off. This is > needed for the following reset patch, which turns snoops off in the CAPP > recovery path. Looking at patch 3 in this series I think this description needs to be updated, as it doesn't seem to turn off snoops? > +/* CAPI modes for PHB */ > +enum { > + OPAL_PHB_CAPI_MODE_PCIE = 0, > + OPAL_PHB_CAPI_MODE_CAPI = 1, > + OPAL_PHB_CAPI_MODE_SNOOP_OFF = 2, > + OPAL_PHB_CAPI_MODE_SNOOP_ON = 3, > +}; Spaces have been used for indention here > +/* CAPI feature flags (in device-tree) */ > +#define OPAL_PHB_CAPI_FLAG_SNOOP_CONTROL 0x00000001 > +#define OPAL_PHB_CAPI_FLAG_REVERT_TO_PCIE 0x00000002 It doesn't look like these are used? > -int pnv_phb_to_cxl(struct pci_dev *dev) > +int pnv_phb_to_cxl(struct pci_dev *dev, uint64_t mode) Should we rename this function since it no longer just sets the PHB to CXL mode? Maybe something like pnv_phb_set_cxl_mode? > + if ((rc = pnv_phb_to_cxl(dev, OPAL_PHB_CAPI_MODE_SNOOP_ON))) { > + dev_err(&dev->dev, "enable capp snoops: %i\n", rc); > + } Ok, we turn on snooping here, but I don't see where we turned it off - has patch 3 changed so that never happens? Also - why this late in in the init sequence? Not saying it's wrong, just wondering if this has to happen after all the AFUs have been initialised, or if it can happen earlier in the adapter initialisation, like when we set the PHB to capi mode? Cheers, -Ian
On 01/15/2015 12:16 AM, Ian Munsie wrote: > Excerpts from Ryan Grimm's message of 2015-01-15 13:56:40 +1100: >> Add mode to opal call. SNOOP control turns CAPP unit snooping on/off. This is >> needed for the following reset patch, which turns snoops off in the CAPP >> recovery path. > > Looking at patch 3 in this series I think this description needs to be > updated, as it doesn't seem to turn off snoops? > OK, will make commit message more clear. Maybe I'll rename the first line to CXL: Enable CAPP recovery, since that's what it does. > >> +/* CAPI modes for PHB */ >> +enum { >> + OPAL_PHB_CAPI_MODE_PCIE = 0, >> + OPAL_PHB_CAPI_MODE_CAPI = 1, >> + OPAL_PHB_CAPI_MODE_SNOOP_OFF = 2, >> + OPAL_PHB_CAPI_MODE_SNOOP_ON = 3, >> +}; > > Spaces have been used for indention here > Fixed. > >> +/* CAPI feature flags (in device-tree) */ >> +#define OPAL_PHB_CAPI_FLAG_SNOOP_CONTROL 0x00000001 >> +#define OPAL_PHB_CAPI_FLAG_REVERT_TO_PCIE 0x00000002 > > It doesn't look like these are used? Yeah. > > >> -int pnv_phb_to_cxl(struct pci_dev *dev) >> +int pnv_phb_to_cxl(struct pci_dev *dev, uint64_t mode) > > Should we rename this function since it no longer just sets the PHB to > CXL mode? Maybe something like pnv_phb_set_cxl_mode? > Good call. The names are updated in the skiboot code base...missed this one. > >> + if ((rc = pnv_phb_to_cxl(dev, OPAL_PHB_CAPI_MODE_SNOOP_ON))) { >> + dev_err(&dev->dev, "enable capp snoops: %i\n", rc); >> + } > > Ok, we turn on snooping here, but I don't see where we turned it off - > has patch 3 changed so that never happens? > snoops are disabled by Sapphire in the CAPP recovery path. > Also - why this late in in the init sequence? Not saying it's wrong, just > wondering if this has to happen after all the AFUs have been initialised, or if > it can happen earlier in the adapter initialisation, like when we set > the PHB to capi mode? > It has no effect when the driver is bound initially since Sapphire already turns on snoops as part of the phb to cxl procedure. I did it this way to handle CAPP recovery. The driver doesn't know that capp recovery happened...EEH unbinds the driver, Sapphire does the CAPP recovery procedure, then EEH rebinds the driver. The last step in capp recovery is to turn on snoops. I'll put a comment to explain that better...but, sure it could be earlier in the sequence. -Ryan > > Cheers, > -Ian >
diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h index 5d073e5..965e232 100644 --- a/arch/powerpc/include/asm/opal.h +++ b/arch/powerpc/include/asm/opal.h @@ -676,6 +676,18 @@ enum { OPAL_PHB3_NUM_PEST_REGS = 256 }; +/* CAPI modes for PHB */ +enum { + OPAL_PHB_CAPI_MODE_PCIE = 0, + OPAL_PHB_CAPI_MODE_CAPI = 1, + OPAL_PHB_CAPI_MODE_SNOOP_OFF = 2, + OPAL_PHB_CAPI_MODE_SNOOP_ON = 3, +}; + +/* CAPI feature flags (in device-tree) */ +#define OPAL_PHB_CAPI_FLAG_SNOOP_CONTROL 0x00000001 +#define OPAL_PHB_CAPI_FLAG_REVERT_TO_PCIE 0x00000002 + struct OpalIoPhbErrorCommon { __be32 version; __be32 ioType; diff --git a/arch/powerpc/include/asm/pnv-pci.h b/arch/powerpc/include/asm/pnv-pci.h index f09a22f..e18082c 100644 --- a/arch/powerpc/include/asm/pnv-pci.h +++ b/arch/powerpc/include/asm/pnv-pci.h @@ -13,7 +13,7 @@ #include <linux/pci.h> #include <misc/cxl.h> -int pnv_phb_to_cxl(struct pci_dev *dev); +int pnv_phb_to_cxl(struct pci_dev *dev, uint64_t mode); int pnv_cxl_ioda_msi_setup(struct pci_dev *dev, unsigned int hwirq, unsigned int virq); int pnv_cxl_alloc_hwirqs(struct pci_dev *dev, int num); diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c index 468a0f2..acacc1b 100644 --- a/arch/powerpc/platforms/powernv/pci-ioda.c +++ b/arch/powerpc/platforms/powernv/pci-ioda.c @@ -1363,7 +1363,7 @@ struct device_node *pnv_pci_to_phb_node(struct pci_dev *dev) } EXPORT_SYMBOL(pnv_pci_to_phb_node); -int pnv_phb_to_cxl(struct pci_dev *dev) +int pnv_phb_to_cxl(struct pci_dev *dev, uint64_t mode) { struct pci_controller *hose = pci_bus_to_host(dev->bus); struct pnv_phb *phb = hose->private_data; @@ -1376,7 +1376,7 @@ int pnv_phb_to_cxl(struct pci_dev *dev) pe_info(pe, "Switching PHB to CXL\n"); - rc = opal_pci_set_phb_cxl_mode(phb->opal_id, 1, pe->pe_number); + rc = opal_pci_set_phb_cxl_mode(phb->opal_id, mode, pe->pe_number); if (rc) dev_err(&dev->dev, "opal_pci_set_phb_cxl_mode failed: %i\n", rc); diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c index 26cacc1..9aa95f9 100644 --- a/drivers/misc/cxl/pci.c +++ b/drivers/misc/cxl/pci.c @@ -924,7 +924,7 @@ static struct cxl *cxl_init_adapter(struct pci_dev *dev) if ((rc = init_implementation_adapter_regs(adapter, dev))) goto err3; - if ((rc = pnv_phb_to_cxl(dev))) + if ((rc = pnv_phb_to_cxl(dev, OPAL_PHB_CAPI_MODE_CAPI))) goto err3; if ((rc = cxl_register_psl_err_irq(adapter))) @@ -1009,6 +1009,10 @@ static int cxl_probe(struct pci_dev *dev, const struct pci_device_id *id) dev_err(&dev->dev, "AFU %i failed to initialise: %i\n", slice, rc); } + if ((rc = pnv_phb_to_cxl(dev, OPAL_PHB_CAPI_MODE_SNOOP_ON))) { + dev_err(&dev->dev, "enable capp snoops: %i\n", rc); + } + return 0; }
Add mode to opal call. SNOOP control turns CAPP unit snooping on/off. This is needed for the following reset patch, which turns snoops off in the CAPP recovery path. Signed-off-by: Ryan Grimm <grimm@linux.vnet.ibm.com> --- arch/powerpc/include/asm/opal.h | 12 ++++++++++++ arch/powerpc/include/asm/pnv-pci.h | 2 +- arch/powerpc/platforms/powernv/pci-ioda.c | 4 ++-- drivers/misc/cxl/pci.c | 6 +++++- 4 files changed, 20 insertions(+), 4 deletions(-)