Re: [PATCH v2 2/6] cxl/pci: Add debug for DVSEC range init failures

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
To: Dan Williams <dan.j.williams@intel.com>
Cc: <linux-cxl@vger.kernel.org>, <ben.widawsky@intel.com>,
	<ira.weiny@intel.com>, <vishal.l.verma@intel.com>,
	<alison.schofield@intel.com>
Subject: Re: [PATCH v2 2/6] cxl/pci: Add debug for DVSEC range init failures
Date: Fri, 25 Mar 2022 11:38:26 +0000	[thread overview]
Message-ID: <20220325113826.00007549@huawei.com> (raw)
In-Reply-To: <164730734812.3806189.2726330688692684104.stgit@dwillia2-desk3.amr.corp.intel.com>

On Mon, 14 Mar 2022 18:22:28 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> In preparation for not treating DVSEC range initialization failures as
> fatal to cxl_pci_probe() add individual dev_dbg() statements for each of
> the major failure reasons in cxl_dvsec_ranges().
> 
> The rationale for cxl_dvsec_ranges() failure not being fatal is that
> there is still value for cxl_pci to enable mailbox operations even if
> CXL.mem operation is disabled.
> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
LGTM
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  drivers/cxl/pci.c |   13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index 8a7267d116b7..257cf735505d 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -467,12 +467,15 @@ static int cxl_dvsec_ranges(struct cxl_dev_state *cxlds)
>  {
>  	struct cxl_endpoint_dvsec_info *info = &cxlds->info;
>  	struct pci_dev *pdev = to_pci_dev(cxlds->dev);
> +	struct device *dev = &pdev->dev;
>  	int d = cxlds->cxl_dvsec;
>  	int hdm_count, rc, i;
>  	u16 cap, ctrl;
>  
> -	if (!d)
> +	if (!d) {
> +		dev_dbg(dev, "No DVSEC Capability\n");
>  		return -ENXIO;
> +	}
>  
>  	rc = pci_read_config_word(pdev, d + CXL_DVSEC_CAP_OFFSET, &cap);
>  	if (rc)
> @@ -482,8 +485,10 @@ static int cxl_dvsec_ranges(struct cxl_dev_state *cxlds)
>  	if (rc)
>  		return rc;
>  
> -	if (!(cap & CXL_DVSEC_MEM_CAPABLE))
> +	if (!(cap & CXL_DVSEC_MEM_CAPABLE)) {
> +		dev_dbg(dev, "Not MEM Capable\n");
>  		return -ENXIO;
> +	}
>  
>  	/*
>  	 * It is not allowed by spec for MEM.capable to be set and have 0 legacy
> @@ -496,8 +501,10 @@ static int cxl_dvsec_ranges(struct cxl_dev_state *cxlds)
>  		return -EINVAL;
>  
>  	rc = wait_for_valid(cxlds);
> -	if (rc)
> +	if (rc) {
> +		dev_dbg(dev, "Failure awaiting MEM_INFO_VALID (%d)\n", rc);
>  		return rc;
> +	}
>  
>  	info->mem_enabled = FIELD_GET(CXL_DVSEC_MEM_ENABLE, ctrl);
>  
>