Re: [PATCH v9 05/10] cxl/test: Refactor decoder setup to reduce cxl_test burden

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

On Fri, 29 Aug 2025 11:09:23 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> Group the decoder setup code in switch and endpoint port probe into a
> single function for each to reduce the number of functions to be mocked
> in cxl_test. Introduce devm_cxl_switch_port_decoders_setup() and
> devm_cxl_endpoint_decoders_setup(). These two functions will be mocked
> instead with some functions optimized out since the mock version does
> not do anything. Remove devm_cxl_setup_hdm(),
> devm_cxl_add_passthrough_decoder(), and devm_cxl_enumerate_decoders() in
> cxl_test mock code. In turn, mock_cxl_add_passthrough_decoder() can be
> removed since cxl_test does not setup passthrough decoders.
> __wrap_cxl_hdm_decode_init() and __wrap_cxl_dvsec_rr_decode() can be
> removed as well since they only return 0 when called.
> 
> Suggested-by: Robert Richter <rrichter@amd.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

There was a bit of code reordering in here that made it a little hard
to check but I can't see an easy way to make that more obvious.
A precursor patch shuffling the code would perhaps allow it to be
discussed in the patch description, but perhaps that's overkill

Anyhow, I'm fairly sure it is fine.

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

> diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
> index cf32dc50b7a6..d8cae2b5bac6 100644
> --- a/drivers/cxl/port.c
> +++ b/drivers/cxl/port.c

>  
>  static int cxl_endpoint_port_probe(struct cxl_port *port)
>  {
> -	struct cxl_endpoint_dvsec_info info = { .port = port };
>  	struct cxl_memdev *cxlmd = to_cxl_memdev(port->uport_dev);
> -	struct cxl_dev_state *cxlds = cxlmd->cxlds;
> -	struct cxl_hdm *cxlhdm;
>  	int rc;
>  
> -	rc = cxl_dvsec_rr_decode(cxlds, &info);
> -	if (rc < 0)
> -		return rc;
> -
> -	cxlhdm = devm_cxl_setup_hdm(port, &info);
> -	if (IS_ERR(cxlhdm)) {
> -		if (PTR_ERR(cxlhdm) == -ENODEV)
> -			dev_err(&port->dev, "HDM decoder registers not found\n");
> -		return PTR_ERR(cxlhdm);
> -	}
> -
>  	/* Cache the data early to ensure is_visible() works */
>  	read_cdat_data(port);
>  	cxl_endpoint_parse_cdat(port);
> @@ -117,11 +87,7 @@ static int cxl_endpoint_port_probe(struct cxl_port *port)
>  	if (rc)
>  		return rc;

Hmm. The reordering makes me a little nervous but I 'think' it's ok as the
code in between is all related to the cxlmd which none of the
code above is directly related to.
 
>  
> -	rc = cxl_hdm_decode_init(cxlds, cxlhdm, &info);
> -	if (rc)
> -		return rc;
> -
> -	rc = devm_cxl_enumerate_decoders(cxlhdm, &info);
> +	rc = devm_cxl_endpoint_decoders_setup(port);
>  	if (rc)
>  		return rc;