Re: [PATCH 1/7] cxl/test: Refactor mock_init_hdm_decoder() to prep for type2 decoder

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

On Wed, Apr 22, 2026 at 04:02:31PM -0700, Dave Jiang wrote:
> Split mock_init_hdm_decoder() into different types of initialization.
> Currently the code deals with default initialization, saved decoders,
> and type3 auto region initialization. In the future, a type2 auto
> initialization will also be added. This refactor will make it cleaner
> to add additional init types.

Nice refactor! A couple of nits below...

> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  tools/testing/cxl/test/cxl.c | 184 ++++++++++++++++++++++-------------
>  1 file changed, 115 insertions(+), 69 deletions(-)
> 
> diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
> index cd47fdd7ccb5..c26c37cef12c 100644
> --- a/tools/testing/cxl/test/cxl.c
> +++ b/tools/testing/cxl/test/cxl.c
> @@ -1052,74 +1052,25 @@ static int first_decoder(struct device *dev, const void *data)
>  	return 0;
>  }
>  
> -/*
> - * Initialize a decoder during HDM enumeration.
> - *
> - * If a saved registry entry exists:
> - *   - enabled decoders are restored from the saved programming
> - *   - disabled decoders are initialized in a clean disabled state
> - *
> - * If no registry entry exists the decoder follows the normal mock
> - * initialization path, including the special auto-region setup for
> - * the first endpoints under host-bridge0.
> - *
> - * Returns true if decoder state was restored from the registry. In
> - * that case the saved decode configuration (including target mapping)
> - * has already been applied and the map_targets() is skipped.
> - */
> -static bool mock_init_hdm_decoder(struct cxl_decoder *cxld)
> +enum cxld_init_type {
> +	MOCK_DECODER_INIT_DEFAULT,
> +	MOCK_DECODER_INIT_SAVED,
> +	MOCK_DECODER_INIT_TYPE3_AUTO,
> +};

skip

> +/*
> + * Initialize a decoder during HDM enumeration.
> + *
> + * If a saved registry entry exists:
> + *   - enabled decoders are restored from the saved programming
> + *   - disabled decoders are initialized in a clean disabled state
> + *
> + * If no registry entry exists the decoder follows the normal mock
> + * initialization path, including the special auto-region setup for
> + * the first endpoints under host-bridge0.
> + *
> + * Returns true if decoder state was restored from the registry. In
> + * that case the saved decode configuration (including target mapping)
> + * has already been applied and the map_targets() is skipped.
> + */
> +static bool mock_init_hdm_decoder(struct cxl_decoder *cxld)
> +{
> +	struct cxl_endpoint_decoder *cxled = NULL;
> +	struct platform_device *pdev = NULL;
> +	struct cxl_test_decoder *td;
> +	struct cxl_memdev *cxlmd;
> +	struct cxl_port *port;
> +	bool hb0 = false;
> +
> +	if (is_endpoint_decoder(&cxld->dev)) {
> +		cxled = to_cxl_endpoint_decoder(&cxld->dev);
> +		cxlmd = cxled_to_memdev(cxled);
> +		WARN_ON(!dev_is_platform(cxlmd->dev.parent));
> +		pdev = to_platform_device(cxlmd->dev.parent);
> +
> +		/* check is endpoint is attach to host-bridge0 */
> +		port = cxled_to_port(cxled);
> +		do {
> +			if (port->uport_dev == &cxl_host_bridge[0]->dev) {
> +				hb0 = true;
> +				break;
> +			}
> +			if (is_cxl_port(port->dev.parent))
> +				port = to_cxl_port(port->dev.parent);
> +			else
> +				port = NULL;
> +		} while (port);
> +		port = cxled_to_port(cxled);
> +	} else {
> +		port = to_cxl_port(cxld->dev.parent);
> +	}
> +
> +	switch (get_decoder_init_type(cxld, pdev, hb0, &td)) {
> +	case MOCK_DECODER_INIT_SAVED:
> +		if (!td)
> +			return false;

get_decoder_init_type never returns NULL w _SAVED.

suggest replace w
		if (WARN_ON(!td))
			return false;


> +		return mock_decoder_handle_saved(cxld, td);
> +	case MOCK_DECODER_INIT_DEFAULT:
> +		/*
> +		 * The default path picks up all the decoders that are not
> +		 * endpoint.
> +		 */
> +		default_mock_decoder(cxld);
> +		return false;
> +	case MOCK_DECODER_INIT_TYPE3_AUTO:
> +		mock_init_hdm_type3_cxled(cxled, port, pdev);
> +		return false;
> +	default:
> +		return false;

default is unreachable since the enum has 4 names values.
suggest dropping it.


> +	}
>  }
>  
>  static int mock_cxl_enumerate_decoders(struct cxl_hdm *cxlhdm,
> -- 
> 2.53.0
>