| Message ID | 20240813110532.870869-3-yanfei.xu@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fixes for hdm docoder initialization from DVSEC ranges | expand |
On Tue, 13 Aug 2024 19:05:30 +0800 Yanfei Xu <yanfei.xu@intel.com> wrote: > commit ce17ad0d5498 ("cxl: Wait Memory_Info_Valid before access memory > related info") added another implementation of waiting for > memory_info_valid without realizing it duplicated wait_for_valid() Mention here the name of the other duplicate function Also good to call out why you picked this duplicate to remove over the other one. Otherwise looks good to me. So with that info added. Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > Suggested-by: Dan Williams <dan.j.williams@intel.com> > Signed-off-by: Yanfei Xu <yanfei.xu@intel.com> > --- > drivers/cxl/core/pci.c | 41 +++++------------------------------ > drivers/cxl/cxl.h | 2 +- > drivers/cxl/port.c | 2 +- > tools/testing/cxl/test/mock.c | 4 ++-- > 4 files changed, 9 insertions(+), 40 deletions(-) > > diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c > index 2d69340134da..38c567727dbb 100644 > --- a/drivers/cxl/core/pci.c > +++ b/drivers/cxl/core/pci.c > @@ -211,37 +211,6 @@ int cxl_await_media_ready(struct cxl_dev_state *cxlds) > } > EXPORT_SYMBOL_NS_GPL(cxl_await_media_ready, CXL); > > -static int wait_for_valid(struct pci_dev *pdev, int d) > -{ > - u32 val; > - int rc; > - > - /* > - * Memory_Info_Valid: When set, indicates that the CXL Range 1 Size high > - * and Size Low registers are valid. Must be set within 1 second of > - * deassertion of reset to CXL device. Likely it is already set by the > - * time this runs, but otherwise give a 1.5 second timeout in case of > - * clock skew. > - */ > - rc = pci_read_config_dword(pdev, d + CXL_DVSEC_RANGE_SIZE_LOW(0), &val); > - if (rc) > - return rc; > - > - if (val & CXL_DVSEC_MEM_INFO_VALID) > - return 0; > - > - msleep(1500); > - > - rc = pci_read_config_dword(pdev, d + CXL_DVSEC_RANGE_SIZE_LOW(0), &val); > - if (rc) > - return rc; > - > - if (val & CXL_DVSEC_MEM_INFO_VALID) > - return 0; > - > - return -ETIMEDOUT; > -} > -
On 8/28/2024 12:16 AM, Jonathan Cameron wrote: > On Tue, 13 Aug 2024 19:05:30 +0800 > Yanfei Xu <yanfei.xu@intel.com> wrote: > >> commit ce17ad0d5498 ("cxl: Wait Memory_Info_Valid before access memory >> related info") added another implementation of waiting for >> memory_info_valid without realizing it duplicated wait_for_valid() > Mention here the name of the other duplicate function > > Also good to call out why you picked this duplicate to remove > over the other one. > > Otherwise looks good to me. > So with that info added. > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> Thanks! will improve it in next version. > >> >> Suggested-by: Dan Williams <dan.j.williams@intel.com> >> Signed-off-by: Yanfei Xu <yanfei.xu@intel.com> >> --- >> drivers/cxl/core/pci.c | 41 +++++------------------------------ >> drivers/cxl/cxl.h | 2 +- >> drivers/cxl/port.c | 2 +- >> tools/testing/cxl/test/mock.c | 4 ++-- >> 4 files changed, 9 insertions(+), 40 deletions(-) >> >> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c >> index 2d69340134da..38c567727dbb 100644 >> --- a/drivers/cxl/core/pci.c >> +++ b/drivers/cxl/core/pci.c >> @@ -211,37 +211,6 @@ int cxl_await_media_ready(struct cxl_dev_state *cxlds) >> } >> EXPORT_SYMBOL_NS_GPL(cxl_await_media_ready, CXL); >> >> -static int wait_for_valid(struct pci_dev *pdev, int d) >> -{ >> - u32 val; >> - int rc; >> - >> - /* >> - * Memory_Info_Valid: When set, indicates that the CXL Range 1 Size high >> - * and Size Low registers are valid. Must be set within 1 second of >> - * deassertion of reset to CXL device. Likely it is already set by the >> - * time this runs, but otherwise give a 1.5 second timeout in case of >> - * clock skew. >> - */ >> - rc = pci_read_config_dword(pdev, d + CXL_DVSEC_RANGE_SIZE_LOW(0), &val); >> - if (rc) >> - return rc; >> - >> - if (val & CXL_DVSEC_MEM_INFO_VALID) >> - return 0; >> - >> - msleep(1500); >> - >> - rc = pci_read_config_dword(pdev, d + CXL_DVSEC_RANGE_SIZE_LOW(0), &val); >> - if (rc) >> - return rc; >> - >> - if (val & CXL_DVSEC_MEM_INFO_VALID) >> - return 0; >> - >> - return -ETIMEDOUT; >> -} >> - > >
diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c index 2d69340134da..38c567727dbb 100644 --- a/drivers/cxl/core/pci.c +++ b/drivers/cxl/core/pci.c @@ -211,37 +211,6 @@ int cxl_await_media_ready(struct cxl_dev_state *cxlds) } EXPORT_SYMBOL_NS_GPL(cxl_await_media_ready, CXL); -static int wait_for_valid(struct pci_dev *pdev, int d) -{ - u32 val; - int rc; - - /* - * Memory_Info_Valid: When set, indicates that the CXL Range 1 Size high - * and Size Low registers are valid. Must be set within 1 second of - * deassertion of reset to CXL device. Likely it is already set by the - * time this runs, but otherwise give a 1.5 second timeout in case of - * clock skew. - */ - rc = pci_read_config_dword(pdev, d + CXL_DVSEC_RANGE_SIZE_LOW(0), &val); - if (rc) - return rc; - - if (val & CXL_DVSEC_MEM_INFO_VALID) - return 0; - - msleep(1500); - - rc = pci_read_config_dword(pdev, d + CXL_DVSEC_RANGE_SIZE_LOW(0), &val); - if (rc) - return rc; - - if (val & CXL_DVSEC_MEM_INFO_VALID) - return 0; - - return -ETIMEDOUT; -} - static int cxl_set_mem_enable(struct cxl_dev_state *cxlds, u16 val) { struct pci_dev *pdev = to_pci_dev(cxlds->dev); @@ -322,11 +291,13 @@ static int devm_cxl_enable_hdm(struct device *host, struct cxl_hdm *cxlhdm) return devm_add_action_or_reset(host, disable_hdm, cxlhdm); } -int cxl_dvsec_rr_decode(struct device *dev, int d, +int cxl_dvsec_rr_decode(struct device *dev, struct cxl_port *port, struct cxl_endpoint_dvsec_info *info) { struct pci_dev *pdev = to_pci_dev(dev); + struct cxl_dev_state *cxlds = pci_get_drvdata(pdev); int hdm_count, rc, i, ranges = 0; + int d = cxlds->cxl_dvsec; u16 cap, ctrl; if (!d) { @@ -353,11 +324,9 @@ int cxl_dvsec_rr_decode(struct device *dev, int d, if (!hdm_count || hdm_count > 2) return -EINVAL; - rc = wait_for_valid(pdev, d); - if (rc) { - dev_dbg(dev, "Failure awaiting MEM_INFO_VALID (%d)\n", rc); + rc = cxl_dvsec_mem_range_valid(cxlds, 0); + if (rc) return rc; - } /* * The current DVSEC values are moot if the memory capability is diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h index 9afb407d438f..e2e277463794 100644 --- a/drivers/cxl/cxl.h +++ b/drivers/cxl/cxl.h @@ -809,7 +809,7 @@ struct cxl_hdm *devm_cxl_setup_hdm(struct cxl_port *port, int devm_cxl_enumerate_decoders(struct cxl_hdm *cxlhdm, struct cxl_endpoint_dvsec_info *info); int devm_cxl_add_passthrough_decoder(struct cxl_port *port); -int cxl_dvsec_rr_decode(struct device *dev, int dvsec, +int cxl_dvsec_rr_decode(struct device *dev, struct cxl_port *port, struct cxl_endpoint_dvsec_info *info); bool is_cxl_region(struct device *dev); diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c index d7d5d982ce69..861dde65768f 100644 --- a/drivers/cxl/port.c +++ b/drivers/cxl/port.c @@ -98,7 +98,7 @@ static int cxl_endpoint_port_probe(struct cxl_port *port) struct cxl_port *root; int rc; - rc = cxl_dvsec_rr_decode(cxlds->dev, cxlds->cxl_dvsec, &info); + rc = cxl_dvsec_rr_decode(cxlds->dev, port, &info); if (rc < 0) return rc; diff --git a/tools/testing/cxl/test/mock.c b/tools/testing/cxl/test/mock.c index 6f737941dc0e..79fdfaad49e8 100644 --- a/tools/testing/cxl/test/mock.c +++ b/tools/testing/cxl/test/mock.c @@ -228,7 +228,7 @@ int __wrap_cxl_hdm_decode_init(struct cxl_dev_state *cxlds, } EXPORT_SYMBOL_NS_GPL(__wrap_cxl_hdm_decode_init, CXL); -int __wrap_cxl_dvsec_rr_decode(struct device *dev, int dvsec, +int __wrap_cxl_dvsec_rr_decode(struct device *dev, struct cxl_port *port, struct cxl_endpoint_dvsec_info *info) { int rc = 0, index; @@ -237,7 +237,7 @@ int __wrap_cxl_dvsec_rr_decode(struct device *dev, int dvsec, if (ops && ops->is_mock_dev(dev)) rc = 0; else - rc = cxl_dvsec_rr_decode(dev, dvsec, info); + rc = cxl_dvsec_rr_decode(dev, port, info); put_cxl_mock_ops(index); return rc;
commit ce17ad0d5498 ("cxl: Wait Memory_Info_Valid before access memory related info") added another implementation of waiting for memory_info_valid without realizing it duplicated wait_for_valid() Suggested-by: Dan Williams <dan.j.williams@intel.com> Signed-off-by: Yanfei Xu <yanfei.xu@intel.com> --- drivers/cxl/core/pci.c | 41 +++++------------------------------ drivers/cxl/cxl.h | 2 +- drivers/cxl/port.c | 2 +- tools/testing/cxl/test/mock.c | 4 ++-- 4 files changed, 9 insertions(+), 40 deletions(-)