Re: [PATCH v13 04/22] cxl: move register/capability check to driver

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

On Mon, 14 Apr 2025 16:13:18 +0100
alejandro.lucero-palau@amd.com wrote:

> From: Alejandro Lucero <alucerop@amd.com>
> 
> Type3 has some mandatory capabilities which are optional for Type2.
> 
> In order to support same register/capability discovery code for both
> types, avoid any assumption about what capabilities should be there, and
> export the capabilities found for the caller doing the capabilities
> check based on the expected ones.
> 
> Add a function for facilitating the report of capabiities missing the
> expected ones.
> 
> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
Hi Alejandro.

A request if we end up with a v14 - please add notes on what changed
in each patch. It's really handy for reviewers to tell which patches
they need to take another look at.   More info that we get from
absence of our own tags!

One minor thing inline. 

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


> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index 0996e228b26a..7d94e81b2e3b 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -836,6 +836,8 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  {
>  	struct pci_host_bridge *host_bridge = pci_find_host_bridge(pdev->bus);
>  	struct cxl_dpa_info range_info = { 0 };
> +	DECLARE_BITMAP(expected, CXL_MAX_CAPS);
Trivial but can do
	DECLARE_BITMAP(expected, CXL_MAX_CAPS) = {};
to avoid need for the zeroing below.

> +	DECLARE_BITMAP(found, CXL_MAX_CAPS);
>  	struct cxl_memdev_state *mds;
>  	struct cxl_dev_state *cxlds;
>  	struct cxl_register_map map;
> @@ -871,7 +873,19 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  
>  	cxlds->rcd = is_cxl_restricted(pdev);
>  
> -	rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map);
> +	bitmap_zero(expected, CXL_MAX_CAPS);
> +	bitmap_zero(found, CXL_MAX_CAPS);
> +
> +	/*
> +	 * These are the mandatory capabilities for a Type3 device.
> +	 * Only checking capabilities used by current Linux drivers.
> +	 */
> +	set_bit(CXL_DEV_CAP_HDM, expected);
> +	set_bit(CXL_DEV_CAP_DEV_STATUS, expected);
> +	set_bit(CXL_DEV_CAP_MAILBOX_PRIMARY, expected);
> +	set_bit(CXL_DEV_CAP_MEMDEV, expected);
> +