Re: [PATCH v2 09/10] cxl: Create an xarray to tie a host bridge to the cxl_root

[Gregory Price <gourry@gourry.net>]

On Tue, May 06, 2025 at 05:43:09PM -0700, Dave Jiang wrote:
> Add helper functions to setup association of a host bridge device to a
> related cxl_root. Functions are in preparation to support the moving
> of host bridge ports creation from cxl_acpi to cxl_memdev probe path.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

mostly wording/naming nits.

I think "udev" is a bad naming choice, but i don't know what to replace
it with.  Most of the scenarios below are host-bridge-uport specifical,
so maybe like "hbup" (actually no, hbup is ugly, but you get the point).

> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 259b217e812f..a5a673d789f3 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
>  
> +/**
> + * cxl_udev_to_root - Retrieve cxl_root tied to the host bridge device
> + * @uport_dev: upstream port device that is the host bridge
                  "The host bridge's uport device connect to the root" ?

maybe instead of uport_dev, maybe hb_uport_dev? If this interface is
only intended to be used specifically with hb_uport.  This is a nit,
ignore it if you want.

> + *
> + * Return cxl_root on success or NULL on failure
> + *
> + * A reference is taken on the port device. Caller needs to call put_device()
> + * when done.
> + */
> +struct cxl_root *cxl_udev_to_root(struct device *uport_dev)

udev_to_root implies you can call this with any uport_dev.
hb_uport_to_root?

Functionality is fine.

> +{
> +	struct cxl_root *root;
> +
> +	root = xa_load(&cxl_root_ports, (unsigned long)uport_dev);
> +	if (!root)
> +		return NULL;
> +
> +	get_device(&root->port.dev);
> +
> +	return root;
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_udev_to_root, "CXL");
> +
> +/**
> + * devm_cxl_register_udev_root_port - Tie a hostbridge device to a root port

Similar naming questions as above.

At this point i'm a bit convinced "udev" is not what we want.

> + * @host: device that hosts the memory for the xarray entries
> + * @uport_dev: host bridge device that serves as the xarray index
> + * @root: cxl_root that serves as the xarray entry data
> + *
> + * Return 0 on success or -errno on failure.
> + */
> +int devm_cxl_register_udev_root_port(struct device *host,
> +				     struct device *uport_dev,
> +				     struct cxl_root *root)
> +{
> +	int rc;
> +
> +	rc = xa_insert(&cxl_root_ports, (unsigned long)uport_dev, root,
> +		       GFP_KERNEL);
> +	if (rc)
> +		return rc;
> +
> +	return devm_add_action_or_reset(host, unregister_udev_root_ports,
> +					uport_dev);
> +}
> +EXPORT_SYMBOL_NS_GPL(devm_cxl_register_udev_root_port, "CXL");
> +
>  bool dev_is_cxl_root_child(struct device *dev)
>  {
>  	struct cxl_port *port, *parent;
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 22f1a9542077..e0cba91803cc 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -736,6 +736,10 @@ struct pci_bus;
>  int devm_cxl_register_pci_bus(struct device *host, struct device *uport_dev,
>  			      struct pci_bus *bus);
>  struct pci_bus *cxl_port_to_pci_bus(struct cxl_port *port);
> +int devm_cxl_register_udev_root_port(struct device *host,
> +				     struct device *uport_dev,
> +				     struct cxl_root *root);
> +struct cxl_root *cxl_udev_to_root(struct device *uport_dev);
>  struct cxl_port *devm_cxl_add_port(struct device *host,
>  				   struct device *uport_dev,
>  				   resource_size_t component_reg_phys,
> -- 
> 2.49.0
>