On Wed, Jul 13, 2022 at 08:30:18PM -0700, Dan Williams wrote:
> ira.weiny@ wrote:
> > From: Ira Weiny <ira.weiny@xxxxxxxxx>
> >
[snip]
> > +
> > +static void devm_cxl_pci_create_doe(struct cxl_dev_state *cxlds)
> > +{
> > + struct device *dev = cxlds->dev;
> > + struct pci_dev *pdev = to_pci_dev(dev);
> > + u16 off = 0;
> > +
> > + /*
> > + * Mailbox creation is best effort. Higher layers must determine if
> > + * the lack of a mailbox for their protocol is a device failure or not.
> > + */
> > + pci_doe_for_each_off(pdev, off) {
> > + struct pci_doe_mb *doe_mb;
> > +
> > + doe_mb = pcim_doe_create_mb(pdev, off);
> > + if (IS_ERR(doe_mb)) {
> > + pci_err(pdev,
> > + "Failed to create MB object for MB @ %x\n",
> > + off);
>
> oh, the rest of cxl_pci driver is using dev_err() and dev_dbg(), not a
> big deal, can be a follow-on cleanup to make it consistent.
Changed since I'm reving anyway.
>
> > + continue;
> > + }
> > +
> > + if (xa_insert(&cxlds->doe_mbs, off, doe_mb, GFP_KERNEL)) {
>
> xarray all the things! Nice.
>
> > + pci_err(pdev,
> > + "xa_insert failed to insert MB @ %x\n",
> > + off);
> > + continue;
>
> Lets make these error messages more actionable:
>
> "Failed to setup DOE, CDAT data may not be available"
I really wanted this to be generic rather than mention something like CDAT
here.
But I see in the other patch you mention skipping the creation of all the
mailboxes and just grab the CDAT one.
Let me resolve that first.
>
> > + }
> > +
> > + pci_dbg(pdev, "Created DOE mailbox @%x\n", off);
> > + }
> > +}
> > +
> > static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> > {
> > struct cxl_register_map map;
> > @@ -408,6 +446,10 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> > if (IS_ERR(cxlds))
> > return PTR_ERR(cxlds);
> >
> > + xa_init(&cxlds->doe_mbs);
> > + if (devm_add_action(&pdev->dev, cxl_pci_destroy_doe, &cxlds->doe_mbs))
> > + return -ENOMEM;
> > +
>
> This belongs inside devm_cxl_pci_create_doe().
Done.
Ira
>
> > cxlds->serial = pci_get_dsn(pdev);
> > cxlds->cxl_dvsec = pci_find_dvsec_capability(
> > pdev, PCI_DVSEC_VENDOR_ID_CXL, CXL_DVSEC_PCIE_DEVICE);
> > @@ -434,6 +476,8 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> >
> > cxlds->component_reg_phys = cxl_regmap_to_base(pdev, &map);
> >
> > + devm_cxl_pci_create_doe(cxlds);
> > +
> > rc = cxl_pci_setup_mailbox(cxlds);
> > if (rc)
> > return rc;
> > --
> > 2.35.3
> >
>
>
