Re: [PATCH 05/14] cxl/mem: Validate port connectivity before dvsec ranges

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

On Thu, 12 May 2022 11:14:43 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> In preparation for validating DVSEC ranges against the platform declared
> CXL memory ranges (ACPI CFMWS) move port enumeration before the
> endpoint's decoder validation. Ultimately this logic will move to the
> port driver, but create a bisect point before that larger move.
> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  drivers/cxl/mem.c |   36 ++++++++++++++++++------------------
>  1 file changed, 18 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> index fed7f10ef9b2..80e75a410499 100644
> --- a/drivers/cxl/mem.c
> +++ b/drivers/cxl/mem.c
> @@ -140,22 +140,6 @@ static int cxl_mem_probe(struct device *dev)
>  	if (work_pending(&cxlmd->detach_work))
>  		return -EBUSY;
>  
> -	rc = cxlds->wait_media_ready(cxlds);
> -	if (rc) {
> -		dev_err(dev, "Media not active (%d)\n", rc);
> -		return rc;
> -	}
> -
> -	/*
> -	 * If DVSEC ranges are being used instead of HDM decoder registers there
> -	 * is no use in trying to manage those.
> -	 */
> -	if (!cxl_hdm_decode_init(cxlds)) {
> -		dev_err(dev,
> -			"Legacy range registers configuration prevents HDM operation.\n");
> -		return -EBUSY;
> -	}
> -
>  	rc = devm_cxl_enumerate_ports(cxlmd);
>  	if (rc)
>  		return rc;
> @@ -171,16 +155,32 @@ static int cxl_mem_probe(struct device *dev)
>  		dev_err(dev, "CXL port topology %s not enabled\n",
>  			dev_name(&parent_port->dev));
>  		rc = -ENXIO;
> -		goto out;
> +		goto unlock;
>  	}
>  
>  	rc = create_endpoint(cxlmd, parent_port);
> -out:
> +unlock:

Trivial but I think this rename of the label would be better in the earlier
patch where you first added the if (rc) below.
I suppose it is arguable that the previous patch is a fix so should be minimal
though so I'm not that bothered if you leave it here.


>  	device_unlock(&parent_port->dev);
>  	put_device(&parent_port->dev);
>  	if (rc)
>  		return rc;
>  
> +	rc = cxlds->wait_media_ready(cxlds);
> +	if (rc) {
> +		dev_err(dev, "Media not active (%d)\n", rc);
> +		return rc;
> +	}
> +
> +	/*
> +	 * If DVSEC ranges are being used instead of HDM decoder registers there
> +	 * is no use in trying to manage those.
> +	 */
> +	if (!cxl_hdm_decode_init(cxlds)) {
> +		dev_err(dev,
> +			"Legacy range registers configuration prevents HDM operation.\n");
> +		return -EBUSY;
> +	}
> +
>  	/*
>  	 * The kernel may be operating out of CXL memory on this device,
>  	 * there is no spec defined way to determine whether this device
>