Re: [RFC PATCH v4 4/4] cxl/pci: Add support for stand alone CXL Switch mailbox CCI

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Wed, 26 Jul 2023 13:00:31 -0700
Davidlohr Bueso <dave@stgolabs.net> wrote:

> On Wed, 19 Jul 2023, Jonathan Cameron wrote:
> 
> >+static int cxl_swmbcci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> >+{
> >+	struct cxl_register_map map;
> >+	struct cxl_swdev *cxlswd;
> >+	int rc;
> >+
> >+	rc = pcim_enable_device(pdev);
> >+	if (rc)
> >+		return rc;
> >+
> >+	cxlswd = cxl_swdev_alloc(&pdev->dev);
> >+	if (IS_ERR(cxlswd))
> >+		return PTR_ERR(cxlswd);
> >+
> >+	mutex_init(&cxlswd->mbox.mbox_mutex);
> >+	rc = cxl_find_regblock(pdev, CXL_REGLOC_RBI_MEMDEV, &map);
> >+	if (rc)
> >+		return rc;
> >+	rc = cxl_setup_regs(&map);
> >+	if (rc)
> >+		return rc;
> >+
> >+	rc = cxl_map_device_regs(&map, &cxlswd->cxlds.regs.device_regs);
> >+	if (rc)
> >+		return rc;
> >+
> >+	rc = cxl_map_mbox_regs(&map, &cxlswd->mbox.mbox);
> >+	if (rc)
> >+		return rc;
> >+
> >+	cxlswd->mbox.status = cxlswd->cxlds.regs.status;
> >+	cxlswd->mbox.dev = &pdev->dev;
> >+
> >+	rc = cxl_swmbcci_setup_mailbox(&cxlswd->mbox);
> >+	if (rc)
> >+		return rc;
> >+
> >+
> >+	pci_set_drvdata(pdev, cxlswd);
> >+
> >+	rc = cxl_enumerate_cmds(&cxlswd->mbox);  
> 
> Making the whole mbox more generically available makes me wonder if
> we want to start using Get Supported Logs Sub-List instead of the current
> gsl? The spec is pretty explicit about this:
> 
> ""
> It is strongly recommended that the Get Supported Logs Sub-List (see
> Section 8.2.9.5.6) is supported by Components and used by software instead of Get
> Supported Logs so that requestors may control the output payload size, as needed.
> Type 3 Devices that implement support for the Get Supported Logs opcode on an
> MCTP-based CCI shall also support the Get Supported Logs Sub-List opcode.
> ""

Agreed - it would be good to make that move on any devices that support
the command.  Patches welcome :)


Jonathan

> 
> Thanks,
> Davidlohr
> 
> >+	if (rc)
> >+		goto error_put_device;
> >+
> >+	rc = cdev_device_add(&cxlswd->cdev, &cxlswd->dev);
> >+	if (rc)
> >+		goto error_put_device;
> >+
> >+	return 0;
> >+
> >+error_put_device:
> >+	cxl_swdev_shutdown(cxlswd);
> >+	put_device(&cxlswd->dev);
> >+	return rc;
> >+}