Re: [PATCH v4 05/14] cxl: Simplify cxl_root_ops allocation and handling

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

On 11/3/25 11:47 AM, Robert Richter wrote:
> A root port's callback handlers are collected in struct cxl_root_ops.
> The structure is dynamically allocated, though it contains only a
> single pointer in it. This also requires to check two pointers to
> check for the existance of a callback.
> 
> Simplify the allocation, release and handler check by embedding the
> ops statical in struct cxl_root.
> 
> Signed-off-by: Robert Richter <rrichter@amd.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>> ---
>  drivers/cxl/acpi.c      |  7 ++-----
>  drivers/cxl/core/cdat.c |  8 ++++----
>  drivers/cxl/core/port.c |  9 ++-------
>  drivers/cxl/cxl.h       | 19 ++++++++++---------
>  4 files changed, 18 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index bd2e282ca93a..1ab780edf141 100644
> --- a/drivers/cxl/acpi.c
> +++ b/drivers/cxl/acpi.c
> @@ -299,10 +299,6 @@ static int cxl_acpi_qos_class(struct cxl_root *cxl_root,
>  	return cxl_acpi_evaluate_qtg_dsm(handle, coord, entries, qos_class);
>  }
>  
> -static const struct cxl_root_ops acpi_root_ops = {
> -	.qos_class = cxl_acpi_qos_class,
> -};
> -
>  static void del_cxl_resource(struct resource *res)
>  {
>  	if (!res)
> @@ -914,9 +910,10 @@ static int cxl_acpi_probe(struct platform_device *pdev)
>  	cxl_res->end = -1;
>  	cxl_res->flags = IORESOURCE_MEM;
>  
> -	cxl_root = devm_cxl_add_root(host, &acpi_root_ops);
> +	cxl_root = devm_cxl_add_root(host);
>  	if (IS_ERR(cxl_root))
>  		return PTR_ERR(cxl_root);
> +	cxl_root->ops.qos_class = cxl_acpi_qos_class;
>  	root_port = &cxl_root->port;
>  
>  	rc = bus_for_each_dev(adev->dev.bus, NULL, root_port,
> diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
> index c4bd6e8a0cf0..b84a9b52942c 100644
> --- a/drivers/cxl/core/cdat.c
> +++ b/drivers/cxl/core/cdat.c
> @@ -213,7 +213,7 @@ static int cxl_port_perf_data_calculate(struct cxl_port *port,
>  	if (!cxl_root)
>  		return -ENODEV;
>  
> -	if (!cxl_root->ops || !cxl_root->ops->qos_class)
> +	if (!cxl_root->ops.qos_class)
>  		return -EOPNOTSUPP;
>  
>  	xa_for_each(dsmas_xa, index, dent) {
> @@ -221,9 +221,9 @@ static int cxl_port_perf_data_calculate(struct cxl_port *port,
>  
>  		cxl_coordinates_combine(dent->coord, dent->cdat_coord, ep_c);
>  		dent->entries = 1;
> -		rc = cxl_root->ops->qos_class(cxl_root,
> -					      &dent->coord[ACCESS_COORDINATE_CPU],
> -					      1, &qos_class);
> +		rc = cxl_root->ops.qos_class(cxl_root,
> +					     &dent->coord[ACCESS_COORDINATE_CPU],
> +					     1, &qos_class);
>  		if (rc != 1)
>  			continue;
>  
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 8128fd2b5b31..2338d146577c 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -459,7 +459,6 @@ static void cxl_root_decoder_release(struct device *dev)
>  	if (atomic_read(&cxlrd->region_id) >= 0)
>  		memregion_free(atomic_read(&cxlrd->region_id));
>  	__cxl_decoder_release(&cxlrd->cxlsd.cxld);
> -	kfree(cxlrd->ops);
>  	kfree(cxlrd);
>  }
>  
> @@ -955,19 +954,15 @@ struct cxl_port *devm_cxl_add_port(struct device *host,
>  }
>  EXPORT_SYMBOL_NS_GPL(devm_cxl_add_port, "CXL");
>  
> -struct cxl_root *devm_cxl_add_root(struct device *host,
> -				   const struct cxl_root_ops *ops)
> +struct cxl_root *devm_cxl_add_root(struct device *host)
>  {
> -	struct cxl_root *cxl_root;
>  	struct cxl_port *port;
>  
>  	port = devm_cxl_add_port(host, host, CXL_RESOURCE_NONE, NULL);
>  	if (IS_ERR(port))
>  		return ERR_CAST(port);
>  
> -	cxl_root = to_cxl_root(port);
> -	cxl_root->ops = ops;
> -	return cxl_root;
> +	return to_cxl_root(port);
>  }
>  EXPORT_SYMBOL_NS_GPL(devm_cxl_add_root, "CXL");
>  
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index b57cfa4273b9..9a381c827416 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -638,6 +638,14 @@ struct cxl_port {
>  	resource_size_t component_reg_phys;
>  };
>  
> +struct cxl_root;
> +
> +struct cxl_root_ops {
> +	int (*qos_class)(struct cxl_root *cxl_root,
> +			 struct access_coordinate *coord, int entries,
> +			 int *qos_class);
> +};
> +
>  /**
>   * struct cxl_root - logical collection of root cxl_port items
>   *
> @@ -646,7 +654,7 @@ struct cxl_port {
>   */
>  struct cxl_root {
>  	struct cxl_port port;
> -	const struct cxl_root_ops *ops;
> +	struct cxl_root_ops ops;
>  };
>  
>  static inline struct cxl_root *
> @@ -655,12 +663,6 @@ to_cxl_root(const struct cxl_port *port)
>  	return container_of(port, struct cxl_root, port);
>  }
>  
> -struct cxl_root_ops {
> -	int (*qos_class)(struct cxl_root *cxl_root,
> -			 struct access_coordinate *coord, int entries,
> -			 int *qos_class);
> -};
> -
>  static inline struct cxl_dport *
>  cxl_find_dport_by_dev(struct cxl_port *port, const struct device *dport_dev)
>  {
> @@ -755,8 +757,7 @@ struct cxl_port *devm_cxl_add_port(struct device *host,
>  				   struct device *uport_dev,
>  				   resource_size_t component_reg_phys,
>  				   struct cxl_dport *parent_dport);
> -struct cxl_root *devm_cxl_add_root(struct device *host,
> -				   const struct cxl_root_ops *ops);
> +struct cxl_root *devm_cxl_add_root(struct device *host);
>  struct cxl_root *find_cxl_root(struct cxl_port *port);
>  
>  DEFINE_FREE(put_cxl_root, struct cxl_root *, if (_T) put_device(&_T->port.dev))