Re: [PATCH] cxl/port: Fix cxl_test register enumeration regression

[Dave Jiang <dave.jiang@intel.com>]

On 9/15/23 01:07, Dan Williams wrote:
> The cxl_test unit test environment models a CXL topology for
> sysfs/user-ABI regression testing. It uses interface mocking via the
> "--wrap=" linker option to redirect cxl_core routines that parse
> hardware registers with versions that just publish objects, like
> devm_cxl_enumerate_decoders().
> 
> Starting with:
> 
> Commit 19ab69a60e3b ("cxl/port: Store the port's Component Register mappings in struct cxl_port")
> 
> ...port register enumeration is moved into devm_cxl_add_port(). This
> conflicts with the "cxl_test avoids emulating registers stance" so
> either the port code needs to be refactored (too violent), or modified
> so that register enumeration is skipped on "fake" cxl_test ports
> (annoying, but straightforward).
> 
> This conflict has happened previously and the "check for platform
> device" workaround to avoid instrusive refactoring was deployed in those
> scenarios. In general, refactoring should only benefit production code,
> test code needs to remain minimally instrusive to the greatest extent
> possible.
> 
> This was missed previously because it may sometimes just cause warning
> messages to be emitted, but it can also cause test failures. The
> backport to -stable is only nice to have for clean cxl_test runs.
> 
> Fixes: 19ab69a60e3b ("cxl/port: Store the port's Component Register mappings in struct cxl_port")
> Cc: <stable@vger.kernel.org>
> Reported-by: Alison Schofield <alison.schofield@intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Tested-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/cxl/core/port.c |   13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 724be8448eb4..7ca01a834e18 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -1,5 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /* Copyright(c) 2020 Intel Corporation. All rights reserved. */
> +#include <linux/platform_device.h>
>  #include <linux/memregion.h>
>  #include <linux/workqueue.h>
>  #include <linux/debugfs.h>
> @@ -706,16 +707,20 @@ static int cxl_setup_comp_regs(struct device *dev, struct cxl_register_map *map,
>  	return cxl_setup_regs(map);
>  }
>  
> -static inline int cxl_port_setup_regs(struct cxl_port *port,
> -				      resource_size_t component_reg_phys)
> +static int cxl_port_setup_regs(struct cxl_port *port,
> +			resource_size_t component_reg_phys)
>  {
> +	if (dev_is_platform(port->uport_dev))
> +		return 0;
>  	return cxl_setup_comp_regs(&port->dev, &port->comp_map,
>  				   component_reg_phys);
>  }
>  
> -static inline int cxl_dport_setup_regs(struct cxl_dport *dport,
> -				       resource_size_t component_reg_phys)
> +static int cxl_dport_setup_regs(struct cxl_dport *dport,
> +				resource_size_t component_reg_phys)
>  {
> +	if (dev_is_platform(dport->dport_dev))
> +		return 0;
>  	return cxl_setup_comp_regs(dport->dport_dev, &dport->comp_map,
>  				   component_reg_phys);
>  }
>