Re: [PATCH v20 02/22] cxl/port: Arrange for always synchronous endpoint attach

Dave Jiang
2025-11-13T23:01:37Z

On 11/10/25 8:36 AM, alejandro.lucero-palau@amd.com wrote:
> From: Dan Williams <dan.j.williams@intel.com>
>
> Make it so that upon return from devm_cxl_add_endpoint() that
> cxl_mem_probe() can assume that the endpoint has had a chance to complete
> cxl_port_probe().
>
> I.e. cxl_port module loading has completed prior to device registration.
>
> MODULE_SOFTDEP() is not sufficient for this purpose, but a hard link-time
> dependency is reliable.
>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

With your sign off tag added,
Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
> drivers/cxl/mem.c | 38 --------------------------------------
> drivers/cxl/port.c | 41 +++++++++++++++++++++++++++++++++++++++++
> drivers/cxl/private.h | 7 ++++++-
> 3 files changed, 47 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> index fa5d901ee817..01a8e808196e 100644
> --- a/drivers/cxl/mem.c
> +++ b/drivers/cxl/mem.c
> @@ -46,44 +46,6 @@ static int cxl_mem_dpa_show(struct seq_file *file, void *data)
> return 0;
> }
>
> -static int devm_cxl_add_endpoint(struct device *host, struct cxl_memdev *cxlmd,
> - struct cxl_dport *parent_dport)
> -{
> - struct cxl_port *parent_port = parent_dport->port;
> - struct cxl_port *endpoint, *iter, *down;
> - int rc;
> -
> - /*
> - * Now that the path to the root is established record all the
> - * intervening ports in the chain.
> - */
> - for (iter = parent_port, down = NULL; !is_cxl_root(iter);
> - down = iter, iter = to_cxl_port(iter->dev.parent)) {
> - struct cxl_ep *ep;
> -
> - ep = cxl_ep_load(iter, cxlmd);
> - ep->next = down;
> - }
> -
> - /* Note: endpoint port component registers are derived from @cxlds */
> - endpoint = devm_cxl_add_port(host, &cxlmd->dev, CXL_RESOURCE_NONE,
> - parent_dport);
> - if (IS_ERR(endpoint))
> - return PTR_ERR(endpoint);
> -
> - rc = cxl_endpoint_autoremove(cxlmd, endpoint);
> - if (rc)
> - return rc;
> -
> - if (!endpoint->dev.driver) {
> - dev_err(&cxlmd->dev, "%s failed probe\n",
> - dev_name(&endpoint->dev));
> - return -ENXIO;
> - }
> -
> - return 0;
> -}
> -
> static int cxl_debugfs_poison_inject(void *data, u64 dpa)
> {
> struct cxl_memdev *cxlmd = data;
> diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
> index 51c8f2f84717..ef65d983e1c8 100644
> --- a/drivers/cxl/port.c
> +++ b/drivers/cxl/port.c
> @@ -6,6 +6,7 @@
>
> #include "cxlmem.h"
> #include "cxlpci.h"
> +#include "private.h"
>
> /**
> * DOC: cxl port
> @@ -156,10 +157,50 @@ static struct cxl_driver cxl_port_driver = {
> .probe = cxl_port_probe,
> .id = CXL_DEVICE_PORT,
> .drv = {
> + .probe_type = PROBE_FORCE_SYNCHRONOUS,
> .dev_groups = cxl_port_attribute_groups,
> },
> };
>
> +int devm_cxl_add_endpoint(struct device *host, struct cxl_memdev *cxlmd,
> + struct cxl_dport *parent_dport)
> +{
> + struct cxl_port *parent_port = parent_dport->port;
> + struct cxl_port *endpoint, *iter, *down;
> + int rc;
> +
> + /*
> + * Now that the path to the root is established record all the
> + * intervening ports in the chain.
> + */
> + for (iter = parent_port, down = NULL; !is_cxl_root(iter);
> + down = iter, iter = to_cxl_port(iter->dev.parent)) {
> + struct cxl_ep *ep;
> +
> + ep = cxl_ep_load(iter, cxlmd);
> + ep->next = down;
> + }
> +
> + /* Note: endpoint port component registers are derived from @cxlds */
> + endpoint = devm_cxl_add_port(host, &cxlmd->dev, CXL_RESOURCE_NONE,
> + parent_dport);
> + if (IS_ERR(endpoint))
> + return PTR_ERR(endpoint);
> +
> + rc = cxl_endpoint_autoremove(cxlmd, endpoint);
> + if (rc)
> + return rc;
> +
> + if (!endpoint->dev.driver) {
> + dev_err(&cxlmd->dev, "%s failed probe\n",
> + dev_name(&endpoint->dev));
> + return -ENXIO;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_NS_GPL(devm_cxl_add_endpoint, "CXL");
> +
> static int __init cxl_port_init(void)
> {
> return cxl_driver_register(&cxl_port_driver);
> diff --git a/drivers/cxl/private.h b/drivers/cxl/private.h
> index 50c2ac57afb5..f8d1ff64f534 100644
> --- a/drivers/cxl/private.h
> +++ b/drivers/cxl/private.h
> @@ -1,10 +1,15 @@
> /* SPDX-License-Identifier: GPL-2.0 */
> /* Copyright(c) 2025 Intel Corporation. */
>
> -/* Private interfaces betwen common drivers ("cxl_mem") and the cxl_core */
> +/*
> + * Private interfaces betwen common drivers ("cxl_mem", "cxl_port") and
> + * the cxl_core.
> + */
>
> #ifndef __CXL_PRIVATE_H__
> #define __CXL_PRIVATE_H__
> struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds);
> int devm_cxl_memdev_add_or_reset(struct device *host, struct cxl_memdev *cxlmd);
> +int devm_cxl_add_endpoint(struct device *host, struct cxl_memdev *cxlmd,
> + struct cxl_dport *parent_dport);
> #endif /* __CXL_PRIVATE_H__ */