| Message ID | 20211120000250.1663391-17-ben.widawsky@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add drivers for CXL ports and mem devices | expand |
On Fri, 19 Nov 2021 16:02:43 -0800 Ben Widawsky <ben.widawsky@intel.com> wrote: > The PCIe device DVSEC, defined in the CXL 2.0 spec, 8.1.3 is required to > be implemented by CXL 2.0 endpoint devices. Since the information > contained within this DVSEC will be critically important for region > configuration, it makes sense to find the value early. > > Since this DVSEC is not strictly required for mailbox functionality, > failure to find this information does not result in the driver failing > to bind. That feels like a path we are going to forget to test sometime in the future. Given it's a specification requirement, I'd treat it as an error and make our lives easier going forwards! Otherwise looks good to me. > > Signed-off-by: Ben Widawsky <ben.widawsky@intel.com> > --- > drivers/cxl/cxlmem.h | 2 ++ > drivers/cxl/pci.c | 7 +++++++ > 2 files changed, 9 insertions(+) > > diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h > index 8d96d009ad90..3ef3c652599e 100644 > --- a/drivers/cxl/cxlmem.h > +++ b/drivers/cxl/cxlmem.h > @@ -98,6 +98,7 @@ struct cxl_mbox_cmd { > * > * @dev: The device associated with this CXL state > * @regs: Parsed register blocks > + * @device_dvsec: Offset to the PCIe device DVSEC > * @payload_size: Size of space for payload > * (CXL 2.0 8.2.8.4.3 Mailbox Capabilities Register) > * @lsa_size: Size of Label Storage Area > @@ -125,6 +126,7 @@ struct cxl_dev_state { > struct device *dev; > > struct cxl_regs regs; > + int device_dvsec; > > size_t payload_size; > size_t lsa_size; > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c > index d2c743a31b0c..f3872cbee7f8 100644 > --- a/drivers/cxl/pci.c > +++ b/drivers/cxl/pci.c > @@ -474,6 +474,13 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) > if (IS_ERR(cxlds)) > return PTR_ERR(cxlds); > > + cxlds->device_dvsec = pci_find_dvsec_capability(pdev, > + PCI_DVSEC_VENDOR_ID_CXL, > + CXL_DVSEC_PCIE_DEVICE); > + if (!cxlds->device_dvsec) > + dev_warn(&pdev->dev, > + "Device DVSEC not present. Expect limited functionality.\n"); > + > rc = cxl_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map); > if (rc) > return rc;
On 21-11-22 16:46:21, Jonathan Cameron wrote: > On Fri, 19 Nov 2021 16:02:43 -0800 > Ben Widawsky <ben.widawsky@intel.com> wrote: > > > The PCIe device DVSEC, defined in the CXL 2.0 spec, 8.1.3 is required to > > be implemented by CXL 2.0 endpoint devices. Since the information > > contained within this DVSEC will be critically important for region > > configuration, it makes sense to find the value early. > > > > Since this DVSEC is not strictly required for mailbox functionality, > > failure to find this information does not result in the driver failing > > to bind. > > That feels like a path we are going to forget to test sometime in the > future. Given it's a specification requirement, I'd treat it as > an error and make our lives easier going forwards! > > Otherwise looks good to me. > Agreed. This is silly. Basically nothing will work if the device dvsec can't be found. I don't remember what I was thinking... > > > > Signed-off-by: Ben Widawsky <ben.widawsky@intel.com> > > --- > > drivers/cxl/cxlmem.h | 2 ++ > > drivers/cxl/pci.c | 7 +++++++ > > 2 files changed, 9 insertions(+) > > > > diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h > > index 8d96d009ad90..3ef3c652599e 100644 > > --- a/drivers/cxl/cxlmem.h > > +++ b/drivers/cxl/cxlmem.h > > @@ -98,6 +98,7 @@ struct cxl_mbox_cmd { > > * > > * @dev: The device associated with this CXL state > > * @regs: Parsed register blocks > > + * @device_dvsec: Offset to the PCIe device DVSEC > > * @payload_size: Size of space for payload > > * (CXL 2.0 8.2.8.4.3 Mailbox Capabilities Register) > > * @lsa_size: Size of Label Storage Area > > @@ -125,6 +126,7 @@ struct cxl_dev_state { > > struct device *dev; > > > > struct cxl_regs regs; > > + int device_dvsec; > > > > size_t payload_size; > > size_t lsa_size; > > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c > > index d2c743a31b0c..f3872cbee7f8 100644 > > --- a/drivers/cxl/pci.c > > +++ b/drivers/cxl/pci.c > > @@ -474,6 +474,13 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) > > if (IS_ERR(cxlds)) > > return PTR_ERR(cxlds); > > > > + cxlds->device_dvsec = pci_find_dvsec_capability(pdev, > > + PCI_DVSEC_VENDOR_ID_CXL, > > + CXL_DVSEC_PCIE_DEVICE); > > + if (!cxlds->device_dvsec) > > + dev_warn(&pdev->dev, > > + "Device DVSEC not present. Expect limited functionality.\n"); > > + > > rc = cxl_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map); > > if (rc) > > return rc; >
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h index 8d96d009ad90..3ef3c652599e 100644 --- a/drivers/cxl/cxlmem.h +++ b/drivers/cxl/cxlmem.h @@ -98,6 +98,7 @@ struct cxl_mbox_cmd { * * @dev: The device associated with this CXL state * @regs: Parsed register blocks + * @device_dvsec: Offset to the PCIe device DVSEC * @payload_size: Size of space for payload * (CXL 2.0 8.2.8.4.3 Mailbox Capabilities Register) * @lsa_size: Size of Label Storage Area @@ -125,6 +126,7 @@ struct cxl_dev_state { struct device *dev; struct cxl_regs regs; + int device_dvsec; size_t payload_size; size_t lsa_size; diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c index d2c743a31b0c..f3872cbee7f8 100644 --- a/drivers/cxl/pci.c +++ b/drivers/cxl/pci.c @@ -474,6 +474,13 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) if (IS_ERR(cxlds)) return PTR_ERR(cxlds); + cxlds->device_dvsec = pci_find_dvsec_capability(pdev, + PCI_DVSEC_VENDOR_ID_CXL, + CXL_DVSEC_PCIE_DEVICE); + if (!cxlds->device_dvsec) + dev_warn(&pdev->dev, + "Device DVSEC not present. Expect limited functionality.\n"); + rc = cxl_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map); if (rc) return rc;
The PCIe device DVSEC, defined in the CXL 2.0 spec, 8.1.3 is required to be implemented by CXL 2.0 endpoint devices. Since the information contained within this DVSEC will be critically important for region configuration, it makes sense to find the value early. Since this DVSEC is not strictly required for mailbox functionality, failure to find this information does not result in the driver failing to bind. Signed-off-by: Ben Widawsky <ben.widawsky@intel.com> --- drivers/cxl/cxlmem.h | 2 ++ drivers/cxl/pci.c | 7 +++++++ 2 files changed, 9 insertions(+)