Re: [PATCH 4/7] cxl/test: Add hierarchy enumeration support for type2 device

[Alison Schofield <alison.schofield@intel.com>]

On Wed, Apr 22, 2026 at 04:02:34PM -0700, Dave Jiang wrote:
> Add enumeration of type2 device hierarchy in cxl-test. The type2 device
> is setup to be directly attached to a root port instead of rp -> switch
> -> device that type3 hierarchy is setup..
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  tools/testing/cxl/test/cxl.c | 110 ++++++++++++++++++++++++++++++++---
>  1 file changed, 103 insertions(+), 7 deletions(-)
> 

skip


> +static int cxl_type2_mem_init(void)
> +{
> +	int i, rc;
> +
> +	for (i = 0; i < NR_CXL_ROOT_PORTS; i++) {

I'd like to drop the artifact we carry here after copying type3 setup.
Type 2 doesn't use the second pdev. I think we can make the one accel
one region constraint explicit.

How about:

	#define NR_CXL_TYPE2_ACCEL 1  /* One Type2 dev, One Region */

and then in cxl_type2_mem_init():

replace this:
	for (i = 0; i < NR_CXL_ROOT_PORTS; i++) {
with this:
	for (i = 0; i < NR_CXL_TYPE2_ACCEL; i++) {



> +		struct platform_device *dport = cxl_root_port[i];
> +		struct platform_device *pdev;
> +
> +		pdev = platform_device_alloc("cxl_type2_accel", i);
> +		if (!pdev)
> +			goto err_mem;
> +		pdev->dev.parent = &dport->dev;
> +		set_dev_node(&pdev->dev, i % 2);
> +
> +		rc = platform_device_add(pdev);
> +		if (rc) {
> +			platform_device_put(pdev);
> +			goto err_mem;
> +		}
> +		cxl_mem[i] = pdev;
> +	}
> +
> +	return 0;
> +
> +err_mem:
> +	for (i = NR_CXL_ROOT_PORTS - 1; i >= 0; i--) {
> +		struct platform_device *pdev = cxl_mem[i];
> +
> +		if (!pdev)
> +			break;
>  		platform_device_unregister(cxl_mem[i]);
> +	}
> +	return rc;
>  }

snip

>  
> +static int cxl_mem_init(void)
> +{
> +	if (type2_test)
> +		return cxl_type2_mem_init();
> +	return cxl_type3_mem_init();
> +}

The precedent above would be good to follow for the topology helpers.
Now we have scattered 'if (type2_test) return 0;' at 7 sites that need
to know which topology this is. How about:

static int cxl_type3_topo_init(void)
{
	/* inits all the things */
}

static int cxl_type2_topo_init(void)
{
	return cxl_rootports_populate();
}

static int cxl_topo_init(void)
{
	if (type2_test)
		return cxl_type2_topo_init();
	return cxl_type3_topo_init();
}

static void cxl_topo_exit(void)
{
	if (type2_test) {
		cxl_rootports_remove();
	return;
	}

	cxl_rch_topo_exit();
	cxl_single_topo_exit();
	cxl_switches_remove();
	cxl_rootports_remove();
}

> +
>  static ssize_t
>  decoder_reset_preserve_registry_show(struct device *dev,
>  				     struct device_attribute *attr, char *buf)
> @@ -2008,6 +2098,9 @@ static int cxl_dsps_populate(void)
>  
>  static void cxl_switches_remove(void)
>  {
> +	if (type2_test)
> +		return;
> +
>  	cxl_usps_remove();
>  	cxl_dsps_remove();
>  }
> @@ -2016,6 +2109,9 @@ static int cxl_switches_populate(void)
>  {
>  	int rc;
>  
> +	if (type2_test)
> +		return 0;
> +
>  	BUILD_BUG_ON(ARRAY_SIZE(cxl_switch_uport) != ARRAY_SIZE(cxl_root_port));
>  	rc = cxl_usps_populate();
>  	if (rc)
> -- 
> 2.53.0
>