Re: [PATCH 1/9] cxl/port: Cleanup handling of the nr_dports 0 -> 1 transition

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

On 1/21/26 8:33 PM, Dan Williams wrote:
> There are multiple setup actions that can occur for a switch port after it
> is known that it has at least one active downstream link. That work is
> currently split between __devm_cxl_add_dport(), the add_dport() helper, and
> cxl_port_add_dport() where decoder setup occurs.
> 
> Clean this up by moving all @dport object setup responsibilities into
> add_dport() and all port effects into cxl_port_add_dport().
> 
> add_dport() handles taking a reference on @dport->dport_dev, and
> cxl_port_add_dport() grows the awareness to setup the port component
> registers. This removes an awkward open-coded xa_erase() from the middle of
> __devm_cxl_add_dport() and instead tasks cxl_port_add_dport() with calling
> the common @dport destruction path if anything goes wrong.
> 
> After this @port->nr_dports is always the count of @dports in the
> @port->dports xarray, and cxl_dport_remove() is symmetric with add_dport().
> With ->nr_dports now reliably tracking the number of dports the use of
> ida_is_empty() can be dropped. Recall that the ida is only cleared on
> "release" of decoder objects, and release can be arbitrarily delayed past
> unregistration.
> 
> Lastly port->component_reg_phys is no longer reset to CXL_RESOURCE_NONE
> post setup, no reason is seen to carry that forward.
> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

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

> ---
>  drivers/cxl/core/port.c | 31 +++++++++++++++----------------
>  1 file changed, 15 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index fef3aa0c6680..ff899c690d85 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -1066,11 +1066,15 @@ static int add_dport(struct cxl_port *port, struct cxl_dport *dport)
>  		return -EBUSY;
>  	}
>  
> +	/* Arrange for dport_dev to be valid through remove_dport() */
> +	struct device *dev __free(put_device) = get_device(dport->dport_dev);
> +
>  	rc = xa_insert(&port->dports, (unsigned long)dport->dport_dev, dport,
>  		       GFP_KERNEL);
>  	if (rc)
>  		return rc;
>  
> +	retain_and_null_ptr(dev);
>  	port->nr_dports++;
>  	return 0;
>  }
> @@ -1099,6 +1103,7 @@ static void cxl_dport_remove(void *data)
>  	struct cxl_dport *dport = data;
>  	struct cxl_port *port = dport->port;
>  
> +	port->nr_dports--;
>  	xa_erase(&port->dports, (unsigned long) dport->dport_dev);
>  	put_device(dport->dport_dev);
>  }
> @@ -1181,21 +1186,6 @@ __devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev,
>  	if (rc)
>  		return ERR_PTR(rc);
>  
> -	/*
> -	 * 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);
> -		if (rc) {
> -			xa_erase(&port->dports, (unsigned long)dport->dport_dev);
> -			return ERR_PTR(rc);
> -		}
> -		port->component_reg_phys = CXL_RESOURCE_NONE;
> -	}
> -
> -	get_device(dport_dev);
>  	rc = devm_add_action_or_reset(host, cxl_dport_remove, dport);
>  	if (rc)
>  		return ERR_PTR(rc);
> @@ -1622,7 +1612,16 @@ static struct cxl_dport *cxl_port_add_dport(struct cxl_port *port,
>  
>  	cxl_switch_parse_cdat(new_dport);
>  
> -	if (ida_is_empty(&port->decoder_ida)) {
> +	if (port->nr_dports == 1) {
> +		/*
> +		 * Some host bridges are known to not have component regsisters
> +		 * available until a root port has trained CXL. Perform that
> +		 * setup now.
> +		 */
> +		rc = cxl_port_setup_regs(port, port->component_reg_phys);
> +		if (rc)
> +			return ERR_PTR(rc);
> +
>  		rc = devm_cxl_switch_port_decoders_setup(port);
>  		if (rc)
>  			return ERR_PTR(rc);