Re: [RFC PATCH 4/6] pcie/cxl_timeout: Add CXL.mem error isolation support

[Bjorn Helgaas <helgaas@kernel.org>]

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.