On Thu, Feb 15, 2024 at 01:40:46PM -0600, Ben Cheatham wrote:
> Add and enable CXL.mem error isolation support (CXL 3.0 12.3.2)
> to the CXL Timeout & Isolation service driver.
> @@ -341,7 +366,8 @@ static int cxl_timeout_probe(struct pcie_device *dev)
> struct pci_dev *port = dev->port;
> struct pcie_cxlt_data *pdata;
> struct cxl_timeout *cxlt;
> - int rc = 0;
> + bool timeout_enabled;
> + int rc;
>
> /* Limit to CXL root ports */
> if (!pci_find_dvsec_capability(port, PCI_DVSEC_VENDOR_ID_CXL,
> @@ -360,6 +386,18 @@ static int cxl_timeout_probe(struct pcie_device *dev)
> pci_dbg(dev->port, "Failed to enable CXL.mem timeout: %d\n",
> rc);
>
> + timeout_enabled = !rc;
This ends up being a little weird: mixing int and bool, negating rc
here and then negating timeout_enabled later, several messages.
Maybe could just keep rc1 and rc2, drop the pci_dbg messages and
enhance the "enabled" message to be something like:
"enabled %s%s with IRQ", rc1 ? "" : "timeout", rc2 ? "" : "isolation"
("&" left for your imagination).
Or something like
#define FLAG(x) ((x) ? '-' : '+')
"CXL.mem timeout%c isolation%c enabled with IRQ %d", FLAG(rc1), FLAG(rc2)
> + rc = cxl_enable_isolation(dev, cxlt);
> + if (rc)
> + pci_dbg(dev->port, "Failed to enable CXL.mem isolation: %d\n",
> + rc);
"(%pe)"
> + if (rc && !timeout_enabled) {
> + pci_info(dev->port,
> + "Failed to enable CXL.mem timeout and isolation.\n");
Most messages don't include a period at end. It just adds the chance
for line wrapping unnecessarily.
