Re: [PATCH v2 4/6] cxl/pci: Make cxl_dvsec_ranges() failure not fatal to cxl_pci

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

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

> cxl_dvsec_ranges(), the helper for enumerating the presence of an active
> legacy CXL.mem configuration on a CXL 2.0 Memory Expander, is not fatal
> for cxl_pci because there is still value to enable mailbox operations
> even if CXL.mem operation is disabled. Recall that the reason cxl_pci
> does this initialization and not cxl_mem is to preserve the useful
> property (for unit testing) that cxl_mem is cxl_memdev + mmio generic,
> and does not require access to a 'struct pci_dev' to issue config
> cycles.
> 
> Update 'struct cxl_endpoint_dvsec_info' to carry either a positive
> number of non-zero size legacy CXL DVSEC ranges, or the negative error
> code from __cxl_dvsec_ranges() in its @ranges member.
> 
> Reported-by: Krzysztof Zach <krzysztof.zach@intel.com>
> Fixes: 560f78559006 ("cxl/pci: Retrieve CXL DVSEC memory info")
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
with comment Ben requested

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  drivers/cxl/pci.c |   27 ++++++++++++++++++---------
>  1 file changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index 257cf735505d..994c79bf6afd 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -463,13 +463,18 @@ static int wait_for_media_ready(struct cxl_dev_state *cxlds)
>  	return 0;
>  }
>  
> -static int cxl_dvsec_ranges(struct cxl_dev_state *cxlds)
> +/*
> + * Return positive number of non-zero ranges on success and a negative
> + * error code on failure. The cxl_mem driver depends on ranges == 0 to
> + * init HDM operation.
> + */
> +static int __cxl_dvsec_ranges(struct cxl_dev_state *cxlds,
> +			      struct cxl_endpoint_dvsec_info *info)
>  {
> -	struct cxl_endpoint_dvsec_info *info = &cxlds->info;
>  	struct pci_dev *pdev = to_pci_dev(cxlds->dev);
> +	int hdm_count, rc, i, ranges = 0;
>  	struct device *dev = &pdev->dev;
>  	int d = cxlds->cxl_dvsec;
> -	int hdm_count, rc, i;
>  	u16 cap, ctrl;
>  
>  	if (!d) {
> @@ -546,10 +551,17 @@ static int cxl_dvsec_ranges(struct cxl_dev_state *cxlds)
>  		};
>  
>  		if (size)
> -			info->ranges++;
> +			ranges++;
>  	}
>  
> -	return 0;
> +	return ranges;
> +}
> +
> +static void cxl_dvsec_ranges(struct cxl_dev_state *cxlds)
> +{
> +	struct cxl_endpoint_dvsec_info *info = &cxlds->info;
> +
> +	info->ranges = __cxl_dvsec_ranges(cxlds, info);
>  }
>  
>  static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> @@ -618,10 +630,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	if (rc)
>  		return rc;
>  
> -	rc = cxl_dvsec_ranges(cxlds);
> -	if (rc)
> -		dev_warn(&pdev->dev,
> -			 "Failed to get DVSEC range information (%d)\n", rc);
> +	cxl_dvsec_ranges(cxlds);
>  
>  	cxlmd = devm_cxl_add_memdev(cxlds);
>  	if (IS_ERR(cxlmd))
>