Re: [PATCH v2] cxl: factor out cxl_await_range_active() and cxl_media_ready()

[Ira Weiny <ira.weiny@intel.com>]

Zhi Wang wrote:
> Before accessing the CXL device memory after reset/power-on, the driver
> needs to ensure the device memory media is ready.
> 
> However, not every CXL device implements the CXL memory device register
> groups. E.g. a CXL type-2 device. Thus calling cxl_await_media_ready()
> on these devcie will lead to a kernel panic. This problem was found when
> testing the emulated CXL type-2 device without a CXL memory device
> register.
> 

[snip]

> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index 4be35dc22202..6fbbaae4a79c 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -846,8 +846,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	if (rc)
>  		dev_dbg(&pdev->dev, "Failed to map RAS capability.\n");
>  
> -	rc = cxl_await_media_ready(cxlds);
> -	if (rc == 0)
> +	if (!cxl_await_range_active(cxlds) && !cxl_media_ready(cxlds))

FWIW this logic is awkward for me.  To me this says 'range not active' and
'media not ready' sets the media_ready flag to true.

Why not make cxl_await_range_active() and cxl_media_ready() return bool?

	if (cxl_await_range_active(cxlds) && cxl_media_ready(cxlds))
		...

?

Ira

>  		cxlds->media_ready = true;
>  	else
>  		dev_warn(&pdev->dev, "Media not active (%d)\n", rc);
> -- 
> 2.34.1
>