Re: [PATCH v8 04/11] cxl: Move port register setup to first dport appear

[Robert Richter <rrichter@amd.com>]

On 14.08.25 15:21:44, Dave Jiang wrote:
> This patch moves the port register setup to when the first dport appears
> via the memdev probe path. At this point, the CXL link should be
> established and the register access is expected to succeed. This change
> addresses an error message observed when PCIe hotplug is enabled on
> an Intel platform. The error messages "cxl portN: Couldn't locate the
> CXL.cache and CXL.mem capability array header" is observed for the
> hostbridge during cxl_acpi driver probe. If the cxl_acpi module
> probe is running before the CXL link between the endpoint device and the
> RP is established, then the platform may not have exposed DVSEC ID 3
> and/or DVSEC ID 7 blocks which will trigger the error message. This
> behavior is defined by the spec and not a hardware quirk.
> 
> This change also needs the dport enumeration to be moved to the memdev
> probe path in order to address the issue. This change is just part of
> the code refactoring and is not a wholly contained fix itself.
> 
> Suggested-by: Dan Williamsn <dan.j.williams@intel.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/cxl/core/port.c | 16 +++++++++++++---
>  drivers/cxl/cxl.h       |  2 ++
>  2 files changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 48e76673aaf3..25209952f469 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -867,9 +867,7 @@ static int cxl_port_add(struct cxl_port *port,
>  		if (rc)
>  			return rc;
>  
> -		rc = cxl_port_setup_regs(port, component_reg_phys);
> -		if (rc)
> -			return rc;
> +		port->component_reg_phys = component_reg_phys;
>  	} else {
>  		rc = dev_set_name(dev, "root%d", port->id);
>  		if (rc)
> @@ -1200,6 +1198,18 @@ __devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev,
>  
>  	cxl_debugfs_create_dport_dir(dport);
>  
> +	/*
> +	 * Setup port register if this is the first dport showed up. Having
> +	 * a dport also means that there is at least 1 active link.
> +	 */
> +	if (port->nr_dports == 1 &&
> +	    port->component_reg_phys != CXL_RESOURCE_NONE) {
> +		rc = cxl_port_setup_regs(port, port->component_reg_phys);

All that delays decoder enablement and visibility in sysfs. I think we
need a different approach to handle late CHBRC availablity. Let's see
your response to my other mail.

-Robert

> +		if (rc)
> +			return ERR_PTR(rc);
> +		port->component_reg_phys = CXL_RESOURCE_NONE;
> +	}
> +
>  	return dport;
>  }
>  
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 4b858f3d44c6..87a905db5ffb 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -599,6 +599,7 @@ struct cxl_dax_region {
>   * @cdat: Cached CDAT data
>   * @cdat_available: Should a CDAT attribute be available in sysfs
>   * @pci_latency: Upstream latency in picoseconds
> + * @component_reg_phys: Physical address of component register
>   */
>  struct cxl_port {
>  	struct device dev;
> @@ -622,6 +623,7 @@ struct cxl_port {
>  	} cdat;
>  	bool cdat_available;
>  	long pci_latency;
> +	resource_size_t component_reg_phys;
>  };
>  
>  /**
> -- 
> 2.50.1
>