| Message ID | 164730734812.3806189.2726330688692684104.stgit@dwillia2-desk3.amr.corp.intel.com |
|---|---|
| State | Accepted |
| Commit | e39f9be08d9dfe685c8a325ac1755c04f383effc |
| Headers | show |
| Series | cxl: Handle DVSEC range init failures | expand |
On 22-03-14 18:22:28, Dan Williams wrote: > In preparation for not treating DVSEC range initialization failures as > fatal to cxl_pci_probe() add individual dev_dbg() statements for each of > the major failure reasons in cxl_dvsec_ranges(). > > The rationale for cxl_dvsec_ranges() failure not being fatal is that > there is still value for cxl_pci to enable mailbox operations even if > CXL.mem operation is disabled. > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > drivers/cxl/pci.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c > index 8a7267d116b7..257cf735505d 100644 > --- a/drivers/cxl/pci.c > +++ b/drivers/cxl/pci.c > @@ -467,12 +467,15 @@ static int cxl_dvsec_ranges(struct cxl_dev_state *cxlds) > { > struct cxl_endpoint_dvsec_info *info = &cxlds->info; > struct pci_dev *pdev = to_pci_dev(cxlds->dev); > + struct device *dev = &pdev->dev; > int d = cxlds->cxl_dvsec; > int hdm_count, rc, i; > u16 cap, ctrl; > > - if (!d) > + if (!d) { > + dev_dbg(dev, "No DVSEC Capability\n"); > return -ENXIO; > + } > > rc = pci_read_config_word(pdev, d + CXL_DVSEC_CAP_OFFSET, &cap); > if (rc) > @@ -482,8 +485,10 @@ static int cxl_dvsec_ranges(struct cxl_dev_state *cxlds) > if (rc) > return rc; > > - if (!(cap & CXL_DVSEC_MEM_CAPABLE)) > + if (!(cap & CXL_DVSEC_MEM_CAPABLE)) { > + dev_dbg(dev, "Not MEM Capable\n"); > return -ENXIO; > + } > > /* > * It is not allowed by spec for MEM.capable to be set and have 0 legacy > @@ -496,8 +501,10 @@ static int cxl_dvsec_ranges(struct cxl_dev_state *cxlds) > return -EINVAL; > > rc = wait_for_valid(cxlds); > - if (rc) > + if (rc) { > + dev_dbg(dev, "Failure awaiting MEM_INFO_VALID (%d)\n", rc); > return rc; > + } > > info->mem_enabled = FIELD_GET(CXL_DVSEC_MEM_ENABLE, ctrl); > > It kind of stinks now that this function has some pdev->dev usages and plain dev usages now. Either way, it's correct and an improvement. Reviewed-by: Ben Widawsky <ben.widawsky@intel.com>
On Mon, 14 Mar 2022 18:22:28 -0700 Dan Williams <dan.j.williams@intel.com> wrote: > In preparation for not treating DVSEC range initialization failures as > fatal to cxl_pci_probe() add individual dev_dbg() statements for each of > the major failure reasons in cxl_dvsec_ranges(). > > The rationale for cxl_dvsec_ranges() failure not being fatal is that > there is still value for cxl_pci to enable mailbox operations even if > CXL.mem operation is disabled. > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> LGTM Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > drivers/cxl/pci.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c > index 8a7267d116b7..257cf735505d 100644 > --- a/drivers/cxl/pci.c > +++ b/drivers/cxl/pci.c > @@ -467,12 +467,15 @@ static int cxl_dvsec_ranges(struct cxl_dev_state *cxlds) > { > struct cxl_endpoint_dvsec_info *info = &cxlds->info; > struct pci_dev *pdev = to_pci_dev(cxlds->dev); > + struct device *dev = &pdev->dev; > int d = cxlds->cxl_dvsec; > int hdm_count, rc, i; > u16 cap, ctrl; > > - if (!d) > + if (!d) { > + dev_dbg(dev, "No DVSEC Capability\n"); > return -ENXIO; > + } > > rc = pci_read_config_word(pdev, d + CXL_DVSEC_CAP_OFFSET, &cap); > if (rc) > @@ -482,8 +485,10 @@ static int cxl_dvsec_ranges(struct cxl_dev_state *cxlds) > if (rc) > return rc; > > - if (!(cap & CXL_DVSEC_MEM_CAPABLE)) > + if (!(cap & CXL_DVSEC_MEM_CAPABLE)) { > + dev_dbg(dev, "Not MEM Capable\n"); > return -ENXIO; > + } > > /* > * It is not allowed by spec for MEM.capable to be set and have 0 legacy > @@ -496,8 +501,10 @@ static int cxl_dvsec_ranges(struct cxl_dev_state *cxlds) > return -EINVAL; > > rc = wait_for_valid(cxlds); > - if (rc) > + if (rc) { > + dev_dbg(dev, "Failure awaiting MEM_INFO_VALID (%d)\n", rc); > return rc; > + } > > info->mem_enabled = FIELD_GET(CXL_DVSEC_MEM_ENABLE, ctrl); > >
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c index 8a7267d116b7..257cf735505d 100644 --- a/drivers/cxl/pci.c +++ b/drivers/cxl/pci.c @@ -467,12 +467,15 @@ static int cxl_dvsec_ranges(struct cxl_dev_state *cxlds) { struct cxl_endpoint_dvsec_info *info = &cxlds->info; struct pci_dev *pdev = to_pci_dev(cxlds->dev); + struct device *dev = &pdev->dev; int d = cxlds->cxl_dvsec; int hdm_count, rc, i; u16 cap, ctrl; - if (!d) + if (!d) { + dev_dbg(dev, "No DVSEC Capability\n"); return -ENXIO; + } rc = pci_read_config_word(pdev, d + CXL_DVSEC_CAP_OFFSET, &cap); if (rc) @@ -482,8 +485,10 @@ static int cxl_dvsec_ranges(struct cxl_dev_state *cxlds) if (rc) return rc; - if (!(cap & CXL_DVSEC_MEM_CAPABLE)) + if (!(cap & CXL_DVSEC_MEM_CAPABLE)) { + dev_dbg(dev, "Not MEM Capable\n"); return -ENXIO; + } /* * It is not allowed by spec for MEM.capable to be set and have 0 legacy @@ -496,8 +501,10 @@ static int cxl_dvsec_ranges(struct cxl_dev_state *cxlds) return -EINVAL; rc = wait_for_valid(cxlds); - if (rc) + if (rc) { + dev_dbg(dev, "Failure awaiting MEM_INFO_VALID (%d)\n", rc); return rc; + } info->mem_enabled = FIELD_GET(CXL_DVSEC_MEM_ENABLE, ctrl);
In preparation for not treating DVSEC range initialization failures as fatal to cxl_pci_probe() add individual dev_dbg() statements for each of the major failure reasons in cxl_dvsec_ranges(). The rationale for cxl_dvsec_ranges() failure not being fatal is that there is still value for cxl_pci to enable mailbox operations even if CXL.mem operation is disabled. Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- drivers/cxl/pci.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)