From: Dan Williams <dan.j.williams@intel.com>
With dport addition moving out of cxl_switch_port_probe() it is no longer
the case that a single dport-add failure will cause all dport resources
to be automatically unwound.
devm still helps all dport resources get cleaned up when the port is
detached, but setup now needs to avoid leaking resources if an early exit
occurs during setup.
Convert from a "devm add" model, to an "auto remove" model that makes the
caller responsible for registering devm reclaim after the object is fully
instantiated.
As a side of effect of this reorganization port->nr_dports is now always
consistent with the number of entries in the port->dports xarray, and this
can stop playing games with ida_is_empty() which is unreliable as a
detector of whether decoders are setup. I.e. consider how
CONFIG_DEBUG_KOBJECT_RELEASE might wreak havoc with this approach.
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Terry Bowman <terry.bowman@amd.com>
---
Changes in v13 -> v14:
- New patch
---
drivers/cxl/acpi.c | 11 +-
drivers/cxl/core/pci.c | 10 +-
drivers/cxl/core/port.c | 225 ++++++++++++++++-----------
drivers/cxl/cxl.h | 23 +--
drivers/cxl/port.c | 8 +-
tools/testing/cxl/Kbuild | 3 +-
tools/testing/cxl/cxl_core_exports.c | 13 +-
tools/testing/cxl/exports.h | 4 +-
tools/testing/cxl/test/cxl.c | 6 +-
tools/testing/cxl/test/mock.c | 25 ++-
tools/testing/cxl/test/mock.h | 4 +-
11 files changed, 188 insertions(+), 144 deletions(-)
diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
index 77ac940e3013..1e1383eb9bd5 100644
--- a/drivers/cxl/acpi.c
+++ b/drivers/cxl/acpi.c
@@ -679,16 +679,19 @@ static int add_host_bridge_dport(struct device *match, void *arg)
if (ctx.cxl_version == ACPI_CEDT_CHBS_VERSION_CXL11) {
dev_dbg(match, "RCRB found for UID %lld: %pa\n", ctx.uid,
&ctx.base);
- dport = devm_cxl_add_rch_dport(root_port, bridge, ctx.uid,
- ctx.base);
+ dport = cxl_add_rch_dport(root_port, bridge, ctx.uid, ctx.base);
} else {
- dport = devm_cxl_add_dport(root_port, bridge, ctx.uid,
- CXL_RESOURCE_NONE);
+ dport = cxl_add_dport(root_port, bridge, ctx.uid,
+ CXL_RESOURCE_NONE);
}
if (IS_ERR(dport))
return PTR_ERR(dport);
+ ret = cxl_dport_autoremove(dport);
+ if (ret)
+ return ret;
+
ret = get_genport_coordinates(match, dport);
if (ret)
dev_dbg(match, "Failed to get generic port perf coordinates.\n");
diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index 0305a421504e..512a3e29a095 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -41,14 +41,14 @@ static int pci_get_port_num(struct pci_dev *pdev)
}
/**
- * __devm_cxl_add_dport_by_dev - allocate a dport by dport device
+ * __cxl_add_dport_by_dev - allocate a dport by dport device
* @port: cxl_port that hosts the dport
* @dport_dev: 'struct device' of the dport
*
* Returns the allocated dport on success or ERR_PTR() of -errno on error
*/
-struct cxl_dport *__devm_cxl_add_dport_by_dev(struct cxl_port *port,
- struct device *dport_dev)
+struct cxl_dport *__cxl_add_dport_by_dev(struct cxl_port *port,
+ struct device *dport_dev)
{
struct cxl_register_map map;
struct pci_dev *pdev;
@@ -67,9 +67,9 @@ struct cxl_dport *__devm_cxl_add_dport_by_dev(struct cxl_port *port,
return ERR_PTR(rc);
device_lock_assert(&port->dev);
- return devm_cxl_add_dport(port, dport_dev, port_num, map.resource);
+ return cxl_add_dport(port, dport_dev, port_num, map.resource);
}
-EXPORT_SYMBOL_NS_GPL(__devm_cxl_add_dport_by_dev, "CXL");
+EXPORT_SYMBOL_NS_GPL(__cxl_add_dport_by_dev, "CXL");
struct cxl_walk_context {
struct pci_bus *bus;
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index fef3aa0c6680..a05a1812bb6e 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -1051,7 +1051,8 @@ static struct cxl_dport *find_dport(struct cxl_port *port, int id)
return NULL;
}
-static int add_dport(struct cxl_port *port, struct cxl_dport *dport)
+static struct cxl_dport *add_dport(struct cxl_port *port,
+ struct cxl_dport *dport)
{
struct cxl_dport *dup;
int rc;
@@ -1063,16 +1064,33 @@ static int add_dport(struct cxl_port *port, struct cxl_dport *dport)
"unable to add dport%d-%s non-unique port id (%s)\n",
dport->port_id, dev_name(dport->dport_dev),
dev_name(dup->dport_dev));
- return -EBUSY;
+ return ERR_PTR(-EBUSY);
+ }
+
+ /*
+ * Unlike CXL switch upstream ports where it can train a CXL link
+ * independent of its downstream ports, a host bridge upstream port may
+ * not enable CXL registers until at least one downstream port (root
+ * port) trains CXL. Enumerate registers once when the number of dports
+ * transitions from zero to one.
+ */
+ if (!port->nr_dports) {
+ rc = cxl_port_setup_regs(port, port->component_reg_phys);
+ if (rc)
+ return ERR_PTR(rc);
}
+ /* 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;
+ return ERR_PTR(rc);
+ retain_and_null_ptr(dev);
port->nr_dports++;
- return 0;
+ return dport;
}
/*
@@ -1094,51 +1112,32 @@ static void cond_cxl_root_unlock(struct cxl_port *port)
device_unlock(&port->dev);
}
-static void cxl_dport_remove(void *data)
+static void remove_dport(struct cxl_dport *dport)
{
- 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);
}
-static void cxl_dport_unlink(void *data)
-{
- struct cxl_dport *dport = data;
- struct cxl_port *port = dport->port;
- char link_name[CXL_TARGET_STRLEN];
+DEFINE_FREE(remove_dport, struct cxl_dport *,
+ if (!IS_ERR_OR_NULL(_T)) remove_dport(_T))
- sprintf(link_name, "dport%d", dport->port_id);
- sysfs_remove_link(&port->dev.kobj, link_name);
-}
-
-static struct cxl_dport *
-__devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev,
- int port_id, resource_size_t component_reg_phys,
- resource_size_t rcrb)
+static struct cxl_dport *__cxl_add_dport(struct cxl_port *port,
+ struct device *dport_dev, int port_id,
+ resource_size_t component_reg_phys,
+ resource_size_t rcrb)
{
char link_name[CXL_TARGET_STRLEN];
- struct cxl_dport *dport;
- struct device *host;
int rc;
- if (is_cxl_root(port))
- host = port->uport_dev;
- else
- host = &port->dev;
-
- if (!host->driver) {
- dev_WARN_ONCE(&port->dev, 1, "dport:%s bad devm context\n",
- dev_name(dport_dev));
- return ERR_PTR(-ENXIO);
- }
-
if (snprintf(link_name, CXL_TARGET_STRLEN, "dport%d", port_id) >=
CXL_TARGET_STRLEN)
return ERR_PTR(-EINVAL);
- dport = devm_kzalloc(host, sizeof(*dport), GFP_KERNEL);
+ struct cxl_dport *dport __free(kfree) =
+ kzalloc(sizeof(*dport), GFP_KERNEL);
if (!dport)
return ERR_PTR(-ENOMEM);
@@ -1176,48 +1175,27 @@ __devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev,
&component_reg_phys);
cond_cxl_root_lock(port);
- rc = add_dport(port, dport);
+ struct cxl_dport *dport_add __free(remove_dport) =
+ add_dport(port, dport);
cond_cxl_root_unlock(port);
- 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;
- }
+ if (IS_ERR(dport_add))
+ return dport_add;
- get_device(dport_dev);
- rc = devm_add_action_or_reset(host, cxl_dport_remove, dport);
- if (rc)
- return ERR_PTR(rc);
+ if (dev_is_pci(dport_dev))
+ dport->link_latency = cxl_pci_get_latency(to_pci_dev(dport_dev));
rc = sysfs_create_link(&port->dev.kobj, &dport_dev->kobj, link_name);
if (rc)
return ERR_PTR(rc);
- rc = devm_add_action_or_reset(host, cxl_dport_unlink, dport);
- if (rc)
- return ERR_PTR(rc);
-
- if (dev_is_pci(dport_dev))
- dport->link_latency = cxl_pci_get_latency(to_pci_dev(dport_dev));
-
cxl_debugfs_create_dport_dir(dport);
- return dport;
+ retain_and_null_ptr(dport_add);
+ return no_free_ptr(dport);
}
/**
- * devm_cxl_add_dport - append VH downstream port data to a cxl_port
+ * cxl_add_dport - append VH downstream port data to a cxl_port
* @port: the cxl_port that references this dport
* @dport_dev: firmware or PCI device representing the dport
* @port_id: identifier for this dport in a decoder's target list
@@ -1227,14 +1205,13 @@ __devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev,
* either the port's host (for root ports), or the port itself (for
* switch ports)
*/
-struct cxl_dport *devm_cxl_add_dport(struct cxl_port *port,
- struct device *dport_dev, int port_id,
- resource_size_t component_reg_phys)
+struct cxl_dport *cxl_add_dport(struct cxl_port *port, struct device *dport_dev,
+ int port_id, resource_size_t component_reg_phys)
{
struct cxl_dport *dport;
- dport = __devm_cxl_add_dport(port, dport_dev, port_id,
- component_reg_phys, CXL_RESOURCE_NONE);
+ dport = __cxl_add_dport(port, dport_dev, port_id, component_reg_phys,
+ CXL_RESOURCE_NONE);
if (IS_ERR(dport)) {
dev_dbg(dport_dev, "failed to add dport to %s: %ld\n",
dev_name(&port->dev), PTR_ERR(dport));
@@ -1245,10 +1222,10 @@ struct cxl_dport *devm_cxl_add_dport(struct cxl_port *port,
return dport;
}
-EXPORT_SYMBOL_NS_GPL(devm_cxl_add_dport, "CXL");
+EXPORT_SYMBOL_NS_GPL(cxl_add_dport, "CXL");
/**
- * devm_cxl_add_rch_dport - append RCH downstream port data to a cxl_port
+ * cxl_add_rch_dport - append RCH downstream port data to a cxl_port
* @port: the cxl_port that references this dport
* @dport_dev: firmware or PCI device representing the dport
* @port_id: identifier for this dport in a decoder's target list
@@ -1256,9 +1233,9 @@ EXPORT_SYMBOL_NS_GPL(devm_cxl_add_dport, "CXL");
*
* See CXL 3.0 9.11.8 CXL Devices Attached to an RCH
*/
-struct cxl_dport *devm_cxl_add_rch_dport(struct cxl_port *port,
- struct device *dport_dev, int port_id,
- resource_size_t rcrb)
+struct cxl_dport *cxl_add_rch_dport(struct cxl_port *port,
+ struct device *dport_dev, int port_id,
+ resource_size_t rcrb)
{
struct cxl_dport *dport;
@@ -1267,8 +1244,8 @@ struct cxl_dport *devm_cxl_add_rch_dport(struct cxl_port *port,
return ERR_PTR(-EINVAL);
}
- dport = __devm_cxl_add_dport(port, dport_dev, port_id,
- CXL_RESOURCE_NONE, rcrb);
+ dport = __cxl_add_dport(port, dport_dev, port_id, CXL_RESOURCE_NONE,
+ rcrb);
if (IS_ERR(dport)) {
dev_dbg(dport_dev, "failed to add RCH dport to %s: %ld\n",
dev_name(&port->dev), PTR_ERR(dport));
@@ -1279,7 +1256,7 @@ struct cxl_dport *devm_cxl_add_rch_dport(struct cxl_port *port,
return dport;
}
-EXPORT_SYMBOL_NS_GPL(devm_cxl_add_rch_dport, "CXL");
+EXPORT_SYMBOL_NS_GPL(cxl_add_rch_dport, "CXL");
static int add_ep(struct cxl_ep *new)
{
@@ -1439,13 +1416,42 @@ static void delete_switch_port(struct cxl_port *port)
devm_release_action(port->dev.parent, unregister_port, port);
}
+static void unlink_dport(void *data)
+{
+ struct cxl_dport *dport = data;
+ struct cxl_port *port = dport->port;
+ char link_name[CXL_TARGET_STRLEN];
+
+ sprintf(link_name, "dport%d", dport->port_id);
+ sysfs_remove_link(&port->dev.kobj, link_name);
+ remove_dport(dport);
+ kfree(dport);
+}
+
+int cxl_dport_autoremove(struct cxl_dport *dport)
+{
+ struct cxl_port *port = dport->port;
+ struct device *host;
+
+ if (is_cxl_root(port))
+ host = port->uport_dev;
+ else
+ host = &port->dev;
+
+ return devm_add_action_or_reset(host, unlink_dport, dport);
+}
+EXPORT_SYMBOL_NS_GPL(cxl_dport_autoremove, "CXL");
+
+/*
+ * Note: this only services dynamic removal of mid-level ports, root ports are
+ * always removed by the platform driver (e.g. cxl_acpi). @host can be
+ * hard-coded to &port->dev.
+ */
static void del_dport(struct cxl_dport *dport)
{
struct cxl_port *port = dport->port;
- devm_release_action(&port->dev, cxl_dport_unlink, dport);
- devm_release_action(&port->dev, cxl_dport_remove, dport);
- devm_kfree(&port->dev, dport);
+ devm_release_action(&port->dev, unlink_dport, dport);
}
static void del_dports(struct cxl_port *port)
@@ -1597,10 +1603,24 @@ static int update_decoder_targets(struct device *dev, void *data)
return 0;
}
-DEFINE_FREE(del_cxl_dport, struct cxl_dport *, if (!IS_ERR_OR_NULL(_T)) del_dport(_T))
+static struct cxl_port *cxl_port_devres_group(struct cxl_port *port)
+{
+ if (!devres_open_group(&port->dev, port, GFP_KERNEL))
+ return ERR_PTR(-ENOMEM);
+ return port;
+}
+DEFINE_FREE(cxl_port_group_free, struct cxl_port *,
+ if (!IS_ERR_OR_NULL(_T)) devres_release_group(&(_T)->dev, _T))
+
+static void cxl_port_group_close(struct cxl_port *port)
+{
+ devres_remove_group(&port->dev, port);
+}
+
static struct cxl_dport *cxl_port_add_dport(struct cxl_port *port,
struct device *dport_dev)
{
+ struct cxl_dport *new_dport;
struct cxl_dport *dport;
int rc;
@@ -1615,29 +1635,46 @@ static struct cxl_dport *cxl_port_add_dport(struct cxl_port *port,
return ERR_PTR(-EBUSY);
}
- struct cxl_dport *new_dport __free(del_cxl_dport) =
- devm_cxl_add_dport_by_dev(port, dport_dev);
- if (IS_ERR(new_dport))
- return new_dport;
-
- cxl_switch_parse_cdat(new_dport);
+ /*
+ * With the first dport arrival it is now safe to start looking at
+ * component registers. Be careful to not strand resources if dport
+ * creation ultimately fails.
+ */
+ struct cxl_port *port_group __free(cxl_port_group_free) =
+ cxl_port_devres_group(port);
+ if (IS_ERR(port_group))
+ return ERR_CAST(port_group);
- if (ida_is_empty(&port->decoder_ida)) {
+ if (port->nr_dports == 0) {
rc = devm_cxl_switch_port_decoders_setup(port);
if (rc)
return ERR_PTR(rc);
- dev_dbg(&port->dev, "first dport%d:%s added with decoders\n",
- new_dport->port_id, dev_name(dport_dev));
- return no_free_ptr(new_dport);
+ /*
+ * Note, when nr_dports returns to zero the port is unregistered
+ * and triggers cleanup. I.e. no need for open-coded release
+ * action on dport removal. See cxl_detach_ep() for that logic.
+ */
}
+ new_dport = cxl_add_dport_by_dev(port, dport_dev);
+ if (IS_ERR(new_dport))
+ return new_dport;
+
+ rc = cxl_dport_autoremove(new_dport);
+ if (rc)
+ return ERR_PTR(rc);
+
+ cxl_switch_parse_cdat(new_dport);
+
+ cxl_port_group_close(no_free_ptr(port_group));
+
+ dev_dbg(&port->dev, "dport[%d] id:%d dport_dev: %s added\n",
+ port->nr_dports - 1, new_dport->port_id, dev_name(dport_dev));
+
/* New dport added, update the decoder targets */
device_for_each_child(&port->dev, new_dport, update_decoder_targets);
- dev_dbg(&port->dev, "dport%d:%s added\n", new_dport->port_id,
- dev_name(dport_dev));
-
- return no_free_ptr(new_dport);
+ return new_dport;
}
static struct cxl_dport *devm_cxl_create_port(struct device *ep_dev,
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 6f3741a57932..47ee06c95433 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -796,12 +796,12 @@ struct cxl_port *cxl_mem_find_port(struct cxl_memdev *cxlmd,
struct cxl_dport **dport);
bool schedule_cxl_memdev_detach(struct cxl_memdev *cxlmd);
-struct cxl_dport *devm_cxl_add_dport(struct cxl_port *port,
- struct device *dport, int port_id,
- resource_size_t component_reg_phys);
-struct cxl_dport *devm_cxl_add_rch_dport(struct cxl_port *port,
- struct device *dport_dev, int port_id,
- resource_size_t rcrb);
+struct cxl_dport *cxl_add_dport(struct cxl_port *port, struct device *dport,
+ int port_id,
+ resource_size_t component_reg_phys);
+struct cxl_dport *cxl_add_rch_dport(struct cxl_port *port,
+ struct device *dport_dev, int port_id,
+ resource_size_t rcrb);
struct cxl_decoder *to_cxl_decoder(struct device *dev);
struct cxl_root_decoder *to_cxl_root_decoder(struct device *dev);
@@ -824,6 +824,7 @@ static inline int cxl_root_decoder_autoremove(struct device *host,
return cxl_decoder_autoremove(host, &cxlrd->cxlsd.cxld);
}
int cxl_endpoint_autoremove(struct cxl_memdev *cxlmd, struct cxl_port *endpoint);
+int cxl_dport_autoremove(struct cxl_dport *dport);
/**
* struct cxl_endpoint_dvsec_info - Cached DVSEC info
@@ -937,10 +938,10 @@ void cxl_coordinates_combine(struct access_coordinate *out,
struct access_coordinate *c2);
bool cxl_endpoint_decoder_reset_detected(struct cxl_port *port);
-struct cxl_dport *devm_cxl_add_dport_by_dev(struct cxl_port *port,
- struct device *dport_dev);
-struct cxl_dport *__devm_cxl_add_dport_by_dev(struct cxl_port *port,
- struct device *dport_dev);
+struct cxl_dport *cxl_add_dport_by_dev(struct cxl_port *port,
+ struct device *dport_dev);
+struct cxl_dport *__cxl_add_dport_by_dev(struct cxl_port *port,
+ struct device *dport_dev);
/*
* Unit test builds overrides this to __weak, find the 'strong' version
@@ -964,7 +965,7 @@ u16 cxl_gpf_get_dvsec(struct device *dev);
*/
#ifndef CXL_TEST_ENABLE
#define DECLARE_TESTABLE(x) __##x
-#define devm_cxl_add_dport_by_dev DECLARE_TESTABLE(devm_cxl_add_dport_by_dev)
+#define cxl_add_dport_by_dev DECLARE_TESTABLE(cxl_add_dport_by_dev)
#define devm_cxl_switch_port_decoders_setup DECLARE_TESTABLE(devm_cxl_switch_port_decoders_setup)
#endif
diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
index 51c8f2f84717..167cc0a87484 100644
--- a/drivers/cxl/port.c
+++ b/drivers/cxl/port.c
@@ -59,8 +59,12 @@ static int discover_region(struct device *dev, void *unused)
static int cxl_switch_port_probe(struct cxl_port *port)
{
- /* Reset nr_dports for rebind of driver */
- port->nr_dports = 0;
+ /*
+ * Unfortunately, typical driver operations like "find and map
+ * registers", can not be done at port device attach time and must wait
+ * for dport arrival. See cxl_port_add_dport() and the comments in
+ * add_dport() for details.
+ */
/* Cache the data early to ensure is_visible() works */
read_cdat_data(port);
diff --git a/tools/testing/cxl/Kbuild b/tools/testing/cxl/Kbuild
index 6eceefefb0e0..4d740392aac5 100644
--- a/tools/testing/cxl/Kbuild
+++ b/tools/testing/cxl/Kbuild
@@ -5,7 +5,8 @@ ldflags-y += --wrap=acpi_evaluate_integer
ldflags-y += --wrap=acpi_pci_find_root
ldflags-y += --wrap=nvdimm_bus_register
ldflags-y += --wrap=cxl_await_media_ready
-ldflags-y += --wrap=devm_cxl_add_rch_dport
+ldflags-y += --wrap=cxl_add_rch_dport
+ldflags-y += --wrap=cxl_rcd_component_reg_phys
ldflags-y += --wrap=cxl_endpoint_parse_cdat
ldflags-y += --wrap=cxl_dport_init_ras_reporting
ldflags-y += --wrap=devm_cxl_endpoint_decoders_setup
diff --git a/tools/testing/cxl/cxl_core_exports.c b/tools/testing/cxl/cxl_core_exports.c
index 6754de35598d..02d479867a12 100644
--- a/tools/testing/cxl/cxl_core_exports.c
+++ b/tools/testing/cxl/cxl_core_exports.c
@@ -7,16 +7,15 @@
/* Exporting of cxl_core symbols that are only used by cxl_test */
EXPORT_SYMBOL_NS_GPL(cxl_num_decoders_committed, "CXL");
-cxl_add_dport_by_dev_fn _devm_cxl_add_dport_by_dev =
- __devm_cxl_add_dport_by_dev;
-EXPORT_SYMBOL_NS_GPL(_devm_cxl_add_dport_by_dev, "CXL");
+cxl_add_dport_by_dev_fn _cxl_add_dport_by_dev = __cxl_add_dport_by_dev;
+EXPORT_SYMBOL_NS_GPL(_cxl_add_dport_by_dev, "CXL");
-struct cxl_dport *devm_cxl_add_dport_by_dev(struct cxl_port *port,
- struct device *dport_dev)
+struct cxl_dport *cxl_add_dport_by_dev(struct cxl_port *port,
+ struct device *dport_dev)
{
- return _devm_cxl_add_dport_by_dev(port, dport_dev);
+ return _cxl_add_dport_by_dev(port, dport_dev);
}
-EXPORT_SYMBOL_NS_GPL(devm_cxl_add_dport_by_dev, "CXL");
+EXPORT_SYMBOL_NS_GPL(cxl_add_dport_by_dev, "CXL");
cxl_switch_decoders_setup_fn _devm_cxl_switch_port_decoders_setup =
__devm_cxl_switch_port_decoders_setup;
diff --git a/tools/testing/cxl/exports.h b/tools/testing/cxl/exports.h
index 7ebee7c0bd67..cbb16073be18 100644
--- a/tools/testing/cxl/exports.h
+++ b/tools/testing/cxl/exports.h
@@ -4,8 +4,8 @@
#define __MOCK_CXL_EXPORTS_H_
typedef struct cxl_dport *(*cxl_add_dport_by_dev_fn)(struct cxl_port *port,
- struct device *dport_dev);
-extern cxl_add_dport_by_dev_fn _devm_cxl_add_dport_by_dev;
+ struct device *dport_dev);
+extern cxl_add_dport_by_dev_fn _cxl_add_dport_by_dev;
typedef int(*cxl_switch_decoders_setup_fn)(struct cxl_port *port);
extern cxl_switch_decoders_setup_fn _devm_cxl_switch_port_decoders_setup;
diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
index 81e2aef3627a..b7a2b550c0b0 100644
--- a/tools/testing/cxl/test/cxl.c
+++ b/tools/testing/cxl/test/cxl.c
@@ -1060,8 +1060,8 @@ static struct cxl_dport *mock_cxl_add_dport_by_dev(struct cxl_port *port,
if (&pdev->dev != dport_dev)
continue;
- return devm_cxl_add_dport(port, &pdev->dev, pdev->id,
- CXL_RESOURCE_NONE);
+ return cxl_add_dport(port, &pdev->dev, pdev->id,
+ CXL_RESOURCE_NONE);
}
return ERR_PTR(-ENODEV);
@@ -1126,9 +1126,9 @@ static struct cxl_mock_ops cxl_mock_ops = {
.devm_cxl_switch_port_decoders_setup = mock_cxl_switch_port_decoders_setup,
.devm_cxl_endpoint_decoders_setup = mock_cxl_endpoint_decoders_setup,
.cxl_endpoint_parse_cdat = mock_cxl_endpoint_parse_cdat,
- .devm_cxl_add_dport_by_dev = mock_cxl_add_dport_by_dev,
.hmat_get_extended_linear_cache_size =
mock_hmat_get_extended_linear_cache_size,
+ .cxl_add_dport_by_dev = mock_cxl_add_dport_by_dev,
.list = LIST_HEAD_INIT(cxl_mock_ops.list),
};
diff --git a/tools/testing/cxl/test/mock.c b/tools/testing/cxl/test/mock.c
index 44bce80ef3ff..660e8402189c 100644
--- a/tools/testing/cxl/test/mock.c
+++ b/tools/testing/cxl/test/mock.c
@@ -15,14 +15,13 @@
static LIST_HEAD(mock);
static struct cxl_dport *
-redirect_devm_cxl_add_dport_by_dev(struct cxl_port *port,
- struct device *dport_dev);
+redirect_cxl_add_dport_by_dev(struct cxl_port *port, struct device *dport_dev);
static int redirect_devm_cxl_switch_port_decoders_setup(struct cxl_port *port);
void register_cxl_mock_ops(struct cxl_mock_ops *ops)
{
list_add_rcu(&ops->list, &mock);
- _devm_cxl_add_dport_by_dev = redirect_devm_cxl_add_dport_by_dev;
+ _cxl_add_dport_by_dev = redirect_cxl_add_dport_by_dev;
_devm_cxl_switch_port_decoders_setup =
redirect_devm_cxl_switch_port_decoders_setup;
}
@@ -34,7 +33,7 @@ void unregister_cxl_mock_ops(struct cxl_mock_ops *ops)
{
_devm_cxl_switch_port_decoders_setup =
__devm_cxl_switch_port_decoders_setup;
- _devm_cxl_add_dport_by_dev = __devm_cxl_add_dport_by_dev;
+ _cxl_add_dport_by_dev = __cxl_add_dport_by_dev;
list_del_rcu(&ops->list);
synchronize_srcu(&cxl_mock_srcu);
}
@@ -207,7 +206,7 @@ int __wrap_cxl_await_media_ready(struct cxl_dev_state *cxlds)
}
EXPORT_SYMBOL_NS_GPL(__wrap_cxl_await_media_ready, "CXL");
-struct cxl_dport *__wrap_devm_cxl_add_rch_dport(struct cxl_port *port,
+struct cxl_dport *__wrap_cxl_add_rch_dport(struct cxl_port *port,
struct device *dport_dev,
int port_id,
resource_size_t rcrb)
@@ -217,19 +216,19 @@ struct cxl_dport *__wrap_devm_cxl_add_rch_dport(struct cxl_port *port,
struct cxl_mock_ops *ops = get_cxl_mock_ops(&index);
if (ops && ops->is_mock_port(dport_dev)) {
- dport = devm_cxl_add_dport(port, dport_dev, port_id,
- CXL_RESOURCE_NONE);
+ dport = cxl_add_dport(port, dport_dev, port_id,
+ CXL_RESOURCE_NONE);
if (!IS_ERR(dport)) {
dport->rcrb.base = rcrb;
dport->rch = true;
}
} else
- dport = devm_cxl_add_rch_dport(port, dport_dev, port_id, rcrb);
+ dport = cxl_add_rch_dport(port, dport_dev, port_id, rcrb);
put_cxl_mock_ops(index);
return dport;
}
-EXPORT_SYMBOL_NS_GPL(__wrap_devm_cxl_add_rch_dport, "CXL");
+EXPORT_SYMBOL_NS_GPL(__wrap_cxl_add_rch_dport, "CXL");
void __wrap_cxl_endpoint_parse_cdat(struct cxl_port *port)
{
@@ -257,17 +256,17 @@ void __wrap_cxl_dport_init_ras_reporting(struct cxl_dport *dport, struct device
}
EXPORT_SYMBOL_NS_GPL(__wrap_cxl_dport_init_ras_reporting, "CXL");
-struct cxl_dport *redirect_devm_cxl_add_dport_by_dev(struct cxl_port *port,
- struct device *dport_dev)
+struct cxl_dport *redirect_cxl_add_dport_by_dev(struct cxl_port *port,
+ struct device *dport_dev)
{
int index;
struct cxl_mock_ops *ops = get_cxl_mock_ops(&index);
struct cxl_dport *dport;
if (ops && ops->is_mock_port(port->uport_dev))
- dport = ops->devm_cxl_add_dport_by_dev(port, dport_dev);
+ dport = ops->cxl_add_dport_by_dev(port, dport_dev);
else
- dport = __devm_cxl_add_dport_by_dev(port, dport_dev);
+ dport = __cxl_add_dport_by_dev(port, dport_dev);
put_cxl_mock_ops(index);
return dport;
diff --git a/tools/testing/cxl/test/mock.h b/tools/testing/cxl/test/mock.h
index 2684b89c8aa2..fa13aca4e260 100644
--- a/tools/testing/cxl/test/mock.h
+++ b/tools/testing/cxl/test/mock.h
@@ -22,8 +22,8 @@ struct cxl_mock_ops {
int (*devm_cxl_switch_port_decoders_setup)(struct cxl_port *port);
int (*devm_cxl_endpoint_decoders_setup)(struct cxl_port *port);
void (*cxl_endpoint_parse_cdat)(struct cxl_port *port);
- struct cxl_dport *(*devm_cxl_add_dport_by_dev)(struct cxl_port *port,
- struct device *dport_dev);
+ struct cxl_dport *(*cxl_add_dport_by_dev)(struct cxl_port *port,
+ struct device *dport_dev);
int (*hmat_get_extended_linear_cache_size)(struct resource *backing_res,
int nid,
resource_size_t *cache_size);
--
2.34.1On Wed, 14 Jan 2026 12:20:40 -0600
Terry Bowman <terry.bowman@amd.com> wrote:
> From: Dan Williams <dan.j.williams@intel.com>
>
> With dport addition moving out of cxl_switch_port_probe() it is no longer
> the case that a single dport-add failure will cause all dport resources
> to be automatically unwound.
>
> devm still helps all dport resources get cleaned up when the port is
> detached, but setup now needs to avoid leaking resources if an early exit
> occurs during setup.
>
> Convert from a "devm add" model, to an "auto remove" model that makes the
> caller responsible for registering devm reclaim after the object is fully
> instantiated.
>
> As a side of effect of this reorganization port->nr_dports is now always
> consistent with the number of entries in the port->dports xarray, and this
> can stop playing games with ida_is_empty() which is unreliable as a
> detector of whether decoders are setup. I.e. consider how
> CONFIG_DEBUG_KOBJECT_RELEASE might wreak havoc with this approach.
>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> Reviewed-by: Terry Bowman <terry.bowman@amd.com>
>
> ---
>
> Changes in v13 -> v14:
> - New patch
Hi Dan, Terry,
I think this needs a little reorganization to ensure we don't have
dport and dport_add both being the same pointer for different free
reasons. Adding a helper and we can combine them with a clear
hand over of ownership.
Wrapping devres_remove_group() in a function that is called close_group()
rings alarm bells.
Jonathan
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index fef3aa0c6680..a05a1812bb6e 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> -static struct cxl_dport *
> -__devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev,
> - int port_id, resource_size_t component_reg_phys,
> - resource_size_t rcrb)
> +static struct cxl_dport *__cxl_add_dport(struct cxl_port *port,
> + struct device *dport_dev, int port_id,
> + resource_size_t component_reg_phys,
> + resource_size_t rcrb)
> {
> char link_name[CXL_TARGET_STRLEN];
> - struct cxl_dport *dport;
> - struct device *host;
> int rc;
>
> - if (is_cxl_root(port))
> - host = port->uport_dev;
> - else
> - host = &port->dev;
> -
> - if (!host->driver) {
> - dev_WARN_ONCE(&port->dev, 1, "dport:%s bad devm context\n",
> - dev_name(dport_dev));
> - return ERR_PTR(-ENXIO);
> - }
> -
> if (snprintf(link_name, CXL_TARGET_STRLEN, "dport%d", port_id) >=
> CXL_TARGET_STRLEN)
> return ERR_PTR(-EINVAL);
>
> - dport = devm_kzalloc(host, sizeof(*dport), GFP_KERNEL);
> + struct cxl_dport *dport __free(kfree) =
> + kzalloc(sizeof(*dport), GFP_KERNEL);
> if (!dport)
> return ERR_PTR(-ENOMEM);
>
> @@ -1176,48 +1175,27 @@ __devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev,
> &component_reg_phys);
>
> cond_cxl_root_lock(port);
> - rc = add_dport(port, dport);
> + struct cxl_dport *dport_add __free(remove_dport) =
> + add_dport(port, dport);
This pattern of having both dport and dport_add effectively
pointing to the same pointer concerns me from a readability / maintainability
point of view. We've often made use of helper functions to avoid doing
this and I think that would make sense here as well.
Take everything down to and including dport_add() as a helper called
something like (naming needs work!)
struct dport_dev *dport __free(remove_and_free_dport) =
add_dport_wrapper();
With the __free doing the kfree as well as remove.
> cond_cxl_root_unlock(port);
> - 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;
> - }
> + if (IS_ERR(dport_add))
> + return dport_add;
>
> - get_device(dport_dev);
> - rc = devm_add_action_or_reset(host, cxl_dport_remove, dport);
> - if (rc)
> - return ERR_PTR(rc);
> + if (dev_is_pci(dport_dev))
> + dport->link_latency = cxl_pci_get_latency(to_pci_dev(dport_dev));
>
> rc = sysfs_create_link(&port->dev.kobj, &dport_dev->kobj, link_name);
> if (rc)
> return ERR_PTR(rc);
>
> - rc = devm_add_action_or_reset(host, cxl_dport_unlink, dport);
> - if (rc)
> - return ERR_PTR(rc);
> -
> - if (dev_is_pci(dport_dev))
> - dport->link_latency = cxl_pci_get_latency(to_pci_dev(dport_dev));
> -
> cxl_debugfs_create_dport_dir(dport);
>
> - return dport;
> + retain_and_null_ptr(dport_add);
> + return no_free_ptr(dport);
> }
> +
> +/*
> + * Note: this only services dynamic removal of mid-level ports, root ports are
> + * always removed by the platform driver (e.g. cxl_acpi). @host can be
> + * hard-coded to &port->dev.
> + */
> static void del_dport(struct cxl_dport *dport)
> {
> struct cxl_port *port = dport->port;
>
> - devm_release_action(&port->dev, cxl_dport_unlink, dport);
> - devm_release_action(&port->dev, cxl_dport_remove, dport);
> - devm_kfree(&port->dev, dport);
> + devm_release_action(&port->dev, unlink_dport, dport);
> }
>
> static void del_dports(struct cxl_port *port)
> @@ -1597,10 +1603,24 @@ static int update_decoder_targets(struct device *dev, void *data)
> return 0;
> }
>
> -DEFINE_FREE(del_cxl_dport, struct cxl_dport *, if (!IS_ERR_OR_NULL(_T)) del_dport(_T))
> +static struct cxl_port *cxl_port_devres_group(struct cxl_port *port)
> +{
> + if (!devres_open_group(&port->dev, port, GFP_KERNEL))
> + return ERR_PTR(-ENOMEM);
> + return port;
> +}
> +DEFINE_FREE(cxl_port_group_free, struct cxl_port *,
> + if (!IS_ERR_OR_NULL(_T)) devres_release_group(&(_T)->dev, _T))
> +
> +static void cxl_port_group_close(struct cxl_port *port)
This feels like misleading naming and I'm not sure what intent is.
Would have expected it to call devres_close_group()
> +{
> + devres_remove_group(&port->dev, port);
> +}
> +
> static struct cxl_dport *cxl_port_add_dport(struct cxl_port *port,
> struct device *dport_dev)
> {
> + struct cxl_dport *new_dport;
> struct cxl_dport *dport;
> int rc;
>
> @@ -1615,29 +1635,46 @@ static struct cxl_dport *cxl_port_add_dport(struct cxl_port *port,
> return ERR_PTR(-EBUSY);
> }
>
> - struct cxl_dport *new_dport __free(del_cxl_dport) =
> - devm_cxl_add_dport_by_dev(port, dport_dev);
> - if (IS_ERR(new_dport))
> - return new_dport;
> -
> - cxl_switch_parse_cdat(new_dport);
> + /*
> + * With the first dport arrival it is now safe to start looking at
> + * component registers. Be careful to not strand resources if dport
> + * creation ultimately fails.
> + */
> + struct cxl_port *port_group __free(cxl_port_group_free) =
> + cxl_port_devres_group(port);
> + if (IS_ERR(port_group))
> + return ERR_CAST(port_group);
>
> - if (ida_is_empty(&port->decoder_ida)) {
> + if (port->nr_dports == 0) {
> rc = devm_cxl_switch_port_decoders_setup(port);
> if (rc)
> return ERR_PTR(rc);
> - dev_dbg(&port->dev, "first dport%d:%s added with decoders\n",
> - new_dport->port_id, dev_name(dport_dev));
> - return no_free_ptr(new_dport);
> + /*
> + * Note, when nr_dports returns to zero the port is unregistered
> + * and triggers cleanup. I.e. no need for open-coded release
> + * action on dport removal. See cxl_detach_ep() for that logic.
> + */
> }
>
> + new_dport = cxl_add_dport_by_dev(port, dport_dev);
> + if (IS_ERR(new_dport))
> + return new_dport;
> +
> + rc = cxl_dport_autoremove(new_dport);
> + if (rc)
> + return ERR_PTR(rc);
> +
> + cxl_switch_parse_cdat(new_dport);
> +
> + cxl_port_group_close(no_free_ptr(port_group));
Give name vs what it does I'm not sure how this currently works.
> +
> + dev_dbg(&port->dev, "dport[%d] id:%d dport_dev: %s added\n",
> + port->nr_dports - 1, new_dport->port_id, dev_name(dport_dev));
> +
> /* New dport added, update the decoder targets */
> device_for_each_child(&port->dev, new_dport, update_decoder_targets);
>
> - dev_dbg(&port->dev, "dport%d:%s added\n", new_dport->port_id,
> - dev_name(dport_dev));
> -
> - return no_free_ptr(new_dport);
> + return new_dport;
> }
>
> static struct cxl_dport *devm_cxl_create_port(struct device *ep_dev,
Jonathan Cameron wrote:
> On Wed, 14 Jan 2026 12:20:40 -0600
> Terry Bowman <terry.bowman@amd.com> wrote:
>
> > From: Dan Williams <dan.j.williams@intel.com>
> >
> > With dport addition moving out of cxl_switch_port_probe() it is no longer
> > the case that a single dport-add failure will cause all dport resources
> > to be automatically unwound.
> >
> > devm still helps all dport resources get cleaned up when the port is
> > detached, but setup now needs to avoid leaking resources if an early exit
> > occurs during setup.
> >
> > Convert from a "devm add" model, to an "auto remove" model that makes the
> > caller responsible for registering devm reclaim after the object is fully
> > instantiated.
> >
> > As a side of effect of this reorganization port->nr_dports is now always
> > consistent with the number of entries in the port->dports xarray, and this
> > can stop playing games with ida_is_empty() which is unreliable as a
> > detector of whether decoders are setup. I.e. consider how
> > CONFIG_DEBUG_KOBJECT_RELEASE might wreak havoc with this approach.
> >
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > Reviewed-by: Terry Bowman <terry.bowman@amd.com>
> >
> > ---
> >
> > Changes in v13 -> v14:
> > - New patch
> Hi Dan, Terry,
>
> I think this needs a little reorganization to ensure we don't have
> dport and dport_add both being the same pointer for different free
> reasons. Adding a helper and we can combine them with a clear
> hand over of ownership.
>
> Wrapping devres_remove_group() in a function that is called close_group()
> rings alarm bells.
>
> Jonathan
[..]
>
> > @@ -1176,48 +1175,27 @@ __devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev,
> > &component_reg_phys);
> >
> > cond_cxl_root_lock(port);
> > - rc = add_dport(port, dport);
> > + struct cxl_dport *dport_add __free(remove_dport) =
> > + add_dport(port, dport);
>
> This pattern of having both dport and dport_add effectively
> pointing to the same pointer concerns me from a readability / maintainability
> point of view. We've often made use of helper functions to avoid doing
> this and I think that would make sense here as well.
Yeah, while I do think the multi-variable pattern is useful for
many-step object construction, I can usually easily be persuaded to
consider a helper function.
> Take everything down to and including dport_add() as a helper called
> something like (naming needs work!)
> struct dport_dev *dport __free(remove_and_free_dport) =
> add_dport_wrapper();
I ended up with the patch below which is similar in spirit to this
without a new DEFINE_FREE().
>
> > cond_cxl_root_unlock(port);
> > - 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;
> > - }
> > + if (IS_ERR(dport_add))
> > + return dport_add;
> >
> > - get_device(dport_dev);
> > - rc = devm_add_action_or_reset(host, cxl_dport_remove, dport);
> > - if (rc)
> > - return ERR_PTR(rc);
> > + if (dev_is_pci(dport_dev))
> > + dport->link_latency = cxl_pci_get_latency(to_pci_dev(dport_dev));
> >
> > rc = sysfs_create_link(&port->dev.kobj, &dport_dev->kobj, link_name);
> > if (rc)
> > return ERR_PTR(rc);
> >
> > - rc = devm_add_action_or_reset(host, cxl_dport_unlink, dport);
> > - if (rc)
> > - return ERR_PTR(rc);
> > -
> > - if (dev_is_pci(dport_dev))
> > - dport->link_latency = cxl_pci_get_latency(to_pci_dev(dport_dev));
> > -
> > cxl_debugfs_create_dport_dir(dport);
> >
> > - return dport;
> > + retain_and_null_ptr(dport_add);
> > + return no_free_ptr(dport);
> > }
>
>
>
> > +
> > +/*
> > + * Note: this only services dynamic removal of mid-level ports, root ports are
> > + * always removed by the platform driver (e.g. cxl_acpi). @host can be
> > + * hard-coded to &port->dev.
> > + */
> > static void del_dport(struct cxl_dport *dport)
> > {
> > struct cxl_port *port = dport->port;
> >
> > - devm_release_action(&port->dev, cxl_dport_unlink, dport);
> > - devm_release_action(&port->dev, cxl_dport_remove, dport);
> > - devm_kfree(&port->dev, dport);
> > + devm_release_action(&port->dev, unlink_dport, dport);
> > }
> >
> > static void del_dports(struct cxl_port *port)
> > @@ -1597,10 +1603,24 @@ static int update_decoder_targets(struct device *dev, void *data)
> > return 0;
> > }
> >
> > -DEFINE_FREE(del_cxl_dport, struct cxl_dport *, if (!IS_ERR_OR_NULL(_T)) del_dport(_T))
> > +static struct cxl_port *cxl_port_devres_group(struct cxl_port *port)
> > +{
> > + if (!devres_open_group(&port->dev, port, GFP_KERNEL))
> > + return ERR_PTR(-ENOMEM);
> > + return port;
> > +}
> > +DEFINE_FREE(cxl_port_group_free, struct cxl_port *,
> > + if (!IS_ERR_OR_NULL(_T)) devres_release_group(&(_T)->dev, _T))
> > +
> > +static void cxl_port_group_close(struct cxl_port *port)
>
> This feels like misleading naming and I'm not sure what intent is.
> Would have expected it to call devres_close_group()
Agree. The hastiness of this patch shows. Switched all the naming to not
be surprising. The flow is:
cxl_port_open_group(): start recording devres resource acquisition
cxl_port_remove_group(): on success, stop tracking the group, leave the resources
cxl_port_release_group(): on failure, destroy the group, free the resources
New patch, added a Fixes: tag.
-- 8< --
From 9731bb6cb5638a0d2141dc072f90db0d00400680 Mon Sep 17 00:00:00 2001
From: Dan Williams <dan.j.williams@intel.com>
Date: Wed, 14 Jan 2026 12:20:40 -0600
Subject: [PATCH] cxl/port: Fix devm resource leaks with dport management
With dport addition moving out of cxl_switch_port_probe() it is no longer
the case that a single dport-add failure will cause all dport resources
to be automatically unwound.
devm still helps all dport resources get cleaned up when the port is
detached, but setup now needs to avoid leaking resources if an early exit
occurs during setup.
Convert from a "devm add" model, to an "auto remove" model that makes the
caller responsible for registering devm reclaim after the object is fully
instantiated.
As a side of effect of this reorganization port->nr_dports is now always
consistent with the number of entries in the port->dports xarray, and this
can stop playing games with ida_is_empty() which is unreliable as a
detector of whether decoders are setup. I.e. consider how
CONFIG_DEBUG_KOBJECT_RELEASE might wreak havoc with this approach.
Cc: <stable@vger.kernel.org>
Fixes: 4f06d81e7c6a ("cxl: Defer dport allocation for switch ports")
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/cxl/cxl.h | 23 +--
tools/testing/cxl/exports.h | 4 +-
tools/testing/cxl/test/mock.h | 4 +-
drivers/cxl/acpi.c | 11 +-
drivers/cxl/core/pci.c | 10 +-
drivers/cxl/core/port.c | 252 ++++++++++++++++-----------
drivers/cxl/port.c | 8 +-
tools/testing/cxl/cxl_core_exports.c | 13 +-
tools/testing/cxl/test/cxl.c | 6 +-
tools/testing/cxl/test/mock.c | 25 ++-
tools/testing/cxl/Kbuild | 3 +-
11 files changed, 209 insertions(+), 150 deletions(-)
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 6f3741a57932..47ee06c95433 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -796,12 +796,12 @@ struct cxl_port *cxl_mem_find_port(struct cxl_memdev *cxlmd,
struct cxl_dport **dport);
bool schedule_cxl_memdev_detach(struct cxl_memdev *cxlmd);
-struct cxl_dport *devm_cxl_add_dport(struct cxl_port *port,
- struct device *dport, int port_id,
- resource_size_t component_reg_phys);
-struct cxl_dport *devm_cxl_add_rch_dport(struct cxl_port *port,
- struct device *dport_dev, int port_id,
- resource_size_t rcrb);
+struct cxl_dport *cxl_add_dport(struct cxl_port *port, struct device *dport,
+ int port_id,
+ resource_size_t component_reg_phys);
+struct cxl_dport *cxl_add_rch_dport(struct cxl_port *port,
+ struct device *dport_dev, int port_id,
+ resource_size_t rcrb);
struct cxl_decoder *to_cxl_decoder(struct device *dev);
struct cxl_root_decoder *to_cxl_root_decoder(struct device *dev);
@@ -824,6 +824,7 @@ static inline int cxl_root_decoder_autoremove(struct device *host,
return cxl_decoder_autoremove(host, &cxlrd->cxlsd.cxld);
}
int cxl_endpoint_autoremove(struct cxl_memdev *cxlmd, struct cxl_port *endpoint);
+int cxl_dport_autoremove(struct cxl_dport *dport);
/**
* struct cxl_endpoint_dvsec_info - Cached DVSEC info
@@ -937,10 +938,10 @@ void cxl_coordinates_combine(struct access_coordinate *out,
struct access_coordinate *c2);
bool cxl_endpoint_decoder_reset_detected(struct cxl_port *port);
-struct cxl_dport *devm_cxl_add_dport_by_dev(struct cxl_port *port,
- struct device *dport_dev);
-struct cxl_dport *__devm_cxl_add_dport_by_dev(struct cxl_port *port,
- struct device *dport_dev);
+struct cxl_dport *cxl_add_dport_by_dev(struct cxl_port *port,
+ struct device *dport_dev);
+struct cxl_dport *__cxl_add_dport_by_dev(struct cxl_port *port,
+ struct device *dport_dev);
/*
* Unit test builds overrides this to __weak, find the 'strong' version
@@ -964,7 +965,7 @@ u16 cxl_gpf_get_dvsec(struct device *dev);
*/
#ifndef CXL_TEST_ENABLE
#define DECLARE_TESTABLE(x) __##x
-#define devm_cxl_add_dport_by_dev DECLARE_TESTABLE(devm_cxl_add_dport_by_dev)
+#define cxl_add_dport_by_dev DECLARE_TESTABLE(cxl_add_dport_by_dev)
#define devm_cxl_switch_port_decoders_setup DECLARE_TESTABLE(devm_cxl_switch_port_decoders_setup)
#endif
diff --git a/tools/testing/cxl/exports.h b/tools/testing/cxl/exports.h
index 7ebee7c0bd67..cbb16073be18 100644
--- a/tools/testing/cxl/exports.h
+++ b/tools/testing/cxl/exports.h
@@ -4,8 +4,8 @@
#define __MOCK_CXL_EXPORTS_H_
typedef struct cxl_dport *(*cxl_add_dport_by_dev_fn)(struct cxl_port *port,
- struct device *dport_dev);
-extern cxl_add_dport_by_dev_fn _devm_cxl_add_dport_by_dev;
+ struct device *dport_dev);
+extern cxl_add_dport_by_dev_fn _cxl_add_dport_by_dev;
typedef int(*cxl_switch_decoders_setup_fn)(struct cxl_port *port);
extern cxl_switch_decoders_setup_fn _devm_cxl_switch_port_decoders_setup;
diff --git a/tools/testing/cxl/test/mock.h b/tools/testing/cxl/test/mock.h
index 2684b89c8aa2..fa13aca4e260 100644
--- a/tools/testing/cxl/test/mock.h
+++ b/tools/testing/cxl/test/mock.h
@@ -22,8 +22,8 @@ struct cxl_mock_ops {
int (*devm_cxl_switch_port_decoders_setup)(struct cxl_port *port);
int (*devm_cxl_endpoint_decoders_setup)(struct cxl_port *port);
void (*cxl_endpoint_parse_cdat)(struct cxl_port *port);
- struct cxl_dport *(*devm_cxl_add_dport_by_dev)(struct cxl_port *port,
- struct device *dport_dev);
+ struct cxl_dport *(*cxl_add_dport_by_dev)(struct cxl_port *port,
+ struct device *dport_dev);
int (*hmat_get_extended_linear_cache_size)(struct resource *backing_res,
int nid,
resource_size_t *cache_size);
diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
index 77ac940e3013..1e1383eb9bd5 100644
--- a/drivers/cxl/acpi.c
+++ b/drivers/cxl/acpi.c
@@ -679,16 +679,19 @@ static int add_host_bridge_dport(struct device *match, void *arg)
if (ctx.cxl_version == ACPI_CEDT_CHBS_VERSION_CXL11) {
dev_dbg(match, "RCRB found for UID %lld: %pa\n", ctx.uid,
&ctx.base);
- dport = devm_cxl_add_rch_dport(root_port, bridge, ctx.uid,
- ctx.base);
+ dport = cxl_add_rch_dport(root_port, bridge, ctx.uid, ctx.base);
} else {
- dport = devm_cxl_add_dport(root_port, bridge, ctx.uid,
- CXL_RESOURCE_NONE);
+ dport = cxl_add_dport(root_port, bridge, ctx.uid,
+ CXL_RESOURCE_NONE);
}
if (IS_ERR(dport))
return PTR_ERR(dport);
+ ret = cxl_dport_autoremove(dport);
+ if (ret)
+ return ret;
+
ret = get_genport_coordinates(match, dport);
if (ret)
dev_dbg(match, "Failed to get generic port perf coordinates.\n");
diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index b838c59d7a3c..ce117812e5c8 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -41,14 +41,14 @@ static int pci_get_port_num(struct pci_dev *pdev)
}
/**
- * __devm_cxl_add_dport_by_dev - allocate a dport by dport device
+ * __cxl_add_dport_by_dev - allocate a dport by dport device
* @port: cxl_port that hosts the dport
* @dport_dev: 'struct device' of the dport
*
* Returns the allocated dport on success or ERR_PTR() of -errno on error
*/
-struct cxl_dport *__devm_cxl_add_dport_by_dev(struct cxl_port *port,
- struct device *dport_dev)
+struct cxl_dport *__cxl_add_dport_by_dev(struct cxl_port *port,
+ struct device *dport_dev)
{
struct cxl_register_map map;
struct pci_dev *pdev;
@@ -67,9 +67,9 @@ struct cxl_dport *__devm_cxl_add_dport_by_dev(struct cxl_port *port,
return ERR_PTR(rc);
device_lock_assert(&port->dev);
- return devm_cxl_add_dport(port, dport_dev, port_num, map.resource);
+ return cxl_add_dport(port, dport_dev, port_num, map.resource);
}
-EXPORT_SYMBOL_NS_GPL(__devm_cxl_add_dport_by_dev, "CXL");
+EXPORT_SYMBOL_NS_GPL(__cxl_add_dport_by_dev, "CXL");
static int cxl_dvsec_mem_range_valid(struct cxl_dev_state *cxlds, int id)
{
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index fef3aa0c6680..41b65babd057 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -1066,11 +1066,28 @@ static int add_dport(struct cxl_port *port, struct cxl_dport *dport)
return -EBUSY;
}
+ /*
+ * Unlike CXL switch upstream ports where it can train a CXL link
+ * independent of its downstream ports, a host bridge upstream port may
+ * not enable CXL registers until at least one downstream port (root
+ * port) trains CXL. Enumerate registers once when the number of dports
+ * transitions from zero to one.
+ */
+ if (!port->nr_dports) {
+ rc = cxl_port_setup_regs(port, port->component_reg_phys);
+ if (rc)
+ return rc;
+ }
+
+ /* 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;
}
@@ -1094,51 +1111,64 @@ static void cond_cxl_root_unlock(struct cxl_port *port)
device_unlock(&port->dev);
}
-static void cxl_dport_remove(void *data)
+static void remove_dport(struct cxl_dport *dport)
{
- 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);
}
-static void cxl_dport_unlink(void *data)
+static struct cxl_dport *__register_dport(struct cxl_dport *dport)
{
- struct cxl_dport *dport = data;
- struct cxl_port *port = dport->port;
+ int rc;
char link_name[CXL_TARGET_STRLEN];
+ struct cxl_port *port = dport->port;
+ struct device *dport_dev = dport->dport_dev;
- sprintf(link_name, "dport%d", dport->port_id);
- sysfs_remove_link(&port->dev.kobj, link_name);
-}
+ if (snprintf(link_name, CXL_TARGET_STRLEN, "dport%d", dport->port_id) >=
+ CXL_TARGET_STRLEN)
+ return ERR_PTR(-EINVAL);
-static struct cxl_dport *
-__devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev,
- int port_id, resource_size_t component_reg_phys,
- resource_size_t rcrb)
-{
- char link_name[CXL_TARGET_STRLEN];
- struct cxl_dport *dport;
- struct device *host;
- int rc;
+ cond_cxl_root_lock(port);
+ rc = add_dport(port, dport);
+ cond_cxl_root_unlock(port);
+ if (rc)
+ return ERR_PTR(rc);
- if (is_cxl_root(port))
- host = port->uport_dev;
- else
- host = &port->dev;
+ if (dev_is_pci(dport_dev))
+ dport->link_latency = cxl_pci_get_latency(to_pci_dev(dport_dev));
- if (!host->driver) {
- dev_WARN_ONCE(&port->dev, 1, "dport:%s bad devm context\n",
- dev_name(dport_dev));
- return ERR_PTR(-ENXIO);
+ rc = sysfs_create_link(&port->dev.kobj, &dport_dev->kobj, link_name);
+ if (rc) {
+ remove_dport(dport);
+ return ERR_PTR(rc);
}
- if (snprintf(link_name, CXL_TARGET_STRLEN, "dport%d", port_id) >=
- CXL_TARGET_STRLEN)
- return ERR_PTR(-EINVAL);
+ cxl_debugfs_create_dport_dir(dport);
- dport = devm_kzalloc(host, sizeof(*dport), GFP_KERNEL);
+ return dport;
+}
+
+static struct cxl_dport *register_or_free_dport(struct cxl_dport *dport)
+{
+ struct cxl_dport *result = __register_dport(dport);
+
+ if (IS_ERR(result))
+ kfree(dport);
+ return result;
+}
+
+static struct cxl_dport *__cxl_add_dport(struct cxl_port *port,
+ struct device *dport_dev, int port_id,
+ resource_size_t component_reg_phys,
+ resource_size_t rcrb)
+{
+ int rc;
+
+ struct cxl_dport *dport __free(kfree) =
+ kzalloc(sizeof(*dport), GFP_KERNEL);
if (!dport)
return ERR_PTR(-ENOMEM);
@@ -1175,49 +1205,11 @@ __devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev,
dev_dbg(dport_dev, "Component Registers found for dport: %pa\n",
&component_reg_phys);
- cond_cxl_root_lock(port);
- rc = add_dport(port, dport);
- cond_cxl_root_unlock(port);
- 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);
-
- rc = sysfs_create_link(&port->dev.kobj, &dport_dev->kobj, link_name);
- if (rc)
- return ERR_PTR(rc);
-
- rc = devm_add_action_or_reset(host, cxl_dport_unlink, dport);
- if (rc)
- return ERR_PTR(rc);
-
- if (dev_is_pci(dport_dev))
- dport->link_latency = cxl_pci_get_latency(to_pci_dev(dport_dev));
-
- cxl_debugfs_create_dport_dir(dport);
-
- return dport;
+ return register_or_free_dport(no_free_ptr(dport));
}
/**
- * devm_cxl_add_dport - append VH downstream port data to a cxl_port
+ * cxl_add_dport - append VH downstream port data to a cxl_port
* @port: the cxl_port that references this dport
* @dport_dev: firmware or PCI device representing the dport
* @port_id: identifier for this dport in a decoder's target list
@@ -1227,14 +1219,13 @@ __devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev,
* either the port's host (for root ports), or the port itself (for
* switch ports)
*/
-struct cxl_dport *devm_cxl_add_dport(struct cxl_port *port,
- struct device *dport_dev, int port_id,
- resource_size_t component_reg_phys)
+struct cxl_dport *cxl_add_dport(struct cxl_port *port, struct device *dport_dev,
+ int port_id, resource_size_t component_reg_phys)
{
struct cxl_dport *dport;
- dport = __devm_cxl_add_dport(port, dport_dev, port_id,
- component_reg_phys, CXL_RESOURCE_NONE);
+ dport = __cxl_add_dport(port, dport_dev, port_id, component_reg_phys,
+ CXL_RESOURCE_NONE);
if (IS_ERR(dport)) {
dev_dbg(dport_dev, "failed to add dport to %s: %ld\n",
dev_name(&port->dev), PTR_ERR(dport));
@@ -1245,10 +1236,10 @@ struct cxl_dport *devm_cxl_add_dport(struct cxl_port *port,
return dport;
}
-EXPORT_SYMBOL_NS_GPL(devm_cxl_add_dport, "CXL");
+EXPORT_SYMBOL_NS_GPL(cxl_add_dport, "CXL");
/**
- * devm_cxl_add_rch_dport - append RCH downstream port data to a cxl_port
+ * cxl_add_rch_dport - append RCH downstream port data to a cxl_port
* @port: the cxl_port that references this dport
* @dport_dev: firmware or PCI device representing the dport
* @port_id: identifier for this dport in a decoder's target list
@@ -1256,9 +1247,9 @@ EXPORT_SYMBOL_NS_GPL(devm_cxl_add_dport, "CXL");
*
* See CXL 3.0 9.11.8 CXL Devices Attached to an RCH
*/
-struct cxl_dport *devm_cxl_add_rch_dport(struct cxl_port *port,
- struct device *dport_dev, int port_id,
- resource_size_t rcrb)
+struct cxl_dport *cxl_add_rch_dport(struct cxl_port *port,
+ struct device *dport_dev, int port_id,
+ resource_size_t rcrb)
{
struct cxl_dport *dport;
@@ -1267,8 +1258,8 @@ struct cxl_dport *devm_cxl_add_rch_dport(struct cxl_port *port,
return ERR_PTR(-EINVAL);
}
- dport = __devm_cxl_add_dport(port, dport_dev, port_id,
- CXL_RESOURCE_NONE, rcrb);
+ dport = __cxl_add_dport(port, dport_dev, port_id, CXL_RESOURCE_NONE,
+ rcrb);
if (IS_ERR(dport)) {
dev_dbg(dport_dev, "failed to add RCH dport to %s: %ld\n",
dev_name(&port->dev), PTR_ERR(dport));
@@ -1279,7 +1270,7 @@ struct cxl_dport *devm_cxl_add_rch_dport(struct cxl_port *port,
return dport;
}
-EXPORT_SYMBOL_NS_GPL(devm_cxl_add_rch_dport, "CXL");
+EXPORT_SYMBOL_NS_GPL(cxl_add_rch_dport, "CXL");
static int add_ep(struct cxl_ep *new)
{
@@ -1439,13 +1430,42 @@ static void delete_switch_port(struct cxl_port *port)
devm_release_action(port->dev.parent, unregister_port, port);
}
+static void unlink_dport(void *data)
+{
+ struct cxl_dport *dport = data;
+ struct cxl_port *port = dport->port;
+ char link_name[CXL_TARGET_STRLEN];
+
+ sprintf(link_name, "dport%d", dport->port_id);
+ sysfs_remove_link(&port->dev.kobj, link_name);
+ remove_dport(dport);
+ kfree(dport);
+}
+
+int cxl_dport_autoremove(struct cxl_dport *dport)
+{
+ struct cxl_port *port = dport->port;
+ struct device *host;
+
+ if (is_cxl_root(port))
+ host = port->uport_dev;
+ else
+ host = &port->dev;
+
+ return devm_add_action_or_reset(host, unlink_dport, dport);
+}
+EXPORT_SYMBOL_NS_GPL(cxl_dport_autoremove, "CXL");
+
+/*
+ * Note: this only services dynamic removal of mid-level ports, root ports are
+ * always removed by the platform driver (e.g. cxl_acpi). @host can be
+ * hard-coded to &port->dev.
+ */
static void del_dport(struct cxl_dport *dport)
{
struct cxl_port *port = dport->port;
- devm_release_action(&port->dev, cxl_dport_unlink, dport);
- devm_release_action(&port->dev, cxl_dport_remove, dport);
- devm_kfree(&port->dev, dport);
+ devm_release_action(&port->dev, unlink_dport, dport);
}
static void del_dports(struct cxl_port *port)
@@ -1597,10 +1617,24 @@ static int update_decoder_targets(struct device *dev, void *data)
return 0;
}
-DEFINE_FREE(del_cxl_dport, struct cxl_dport *, if (!IS_ERR_OR_NULL(_T)) del_dport(_T))
+static struct cxl_port *cxl_port_open_group(struct cxl_port *port)
+{
+ if (!devres_open_group(&port->dev, port, GFP_KERNEL))
+ return ERR_PTR(-ENOMEM);
+ return port;
+}
+DEFINE_FREE(cxl_port_release_group, struct cxl_port *,
+ if (!IS_ERR_OR_NULL(_T)) devres_release_group(&(_T)->dev, _T))
+
+static void cxl_port_remove_group(struct cxl_port *port)
+{
+ devres_remove_group(&port->dev, port);
+}
+
static struct cxl_dport *cxl_port_add_dport(struct cxl_port *port,
struct device *dport_dev)
{
+ struct cxl_dport *new_dport;
struct cxl_dport *dport;
int rc;
@@ -1615,29 +1649,47 @@ static struct cxl_dport *cxl_port_add_dport(struct cxl_port *port,
return ERR_PTR(-EBUSY);
}
- struct cxl_dport *new_dport __free(del_cxl_dport) =
- devm_cxl_add_dport_by_dev(port, dport_dev);
- if (IS_ERR(new_dport))
- return new_dport;
-
- cxl_switch_parse_cdat(new_dport);
+ /*
+ * With the first dport arrival it is now safe to start looking at
+ * component registers. Be careful to not strand resources if dport
+ * creation ultimately fails.
+ */
+ struct cxl_port *port_group __free(cxl_port_release_group) =
+ cxl_port_open_group(port);
+ if (IS_ERR(port_group))
+ return ERR_CAST(port_group);
- if (ida_is_empty(&port->decoder_ida)) {
+ if (port->nr_dports == 0) {
rc = devm_cxl_switch_port_decoders_setup(port);
if (rc)
return ERR_PTR(rc);
- dev_dbg(&port->dev, "first dport%d:%s added with decoders\n",
- new_dport->port_id, dev_name(dport_dev));
- return no_free_ptr(new_dport);
+ /*
+ * Note, when nr_dports returns to zero the port is unregistered
+ * and triggers cleanup. I.e. no need for open-coded release
+ * action on dport removal. See cxl_detach_ep() for that logic.
+ */
}
+ new_dport = cxl_add_dport_by_dev(port, dport_dev);
+ if (IS_ERR(new_dport))
+ return new_dport;
+
+ rc = cxl_dport_autoremove(new_dport);
+ if (rc)
+ return ERR_PTR(rc);
+
+ cxl_switch_parse_cdat(new_dport);
+
+ /* group tracking no longer needed, dport successfully added */
+ cxl_port_remove_group(no_free_ptr(port_group));
+
+ dev_dbg(&port->dev, "dport[%d] id:%d dport_dev: %s added\n",
+ port->nr_dports - 1, new_dport->port_id, dev_name(dport_dev));
+
/* New dport added, update the decoder targets */
device_for_each_child(&port->dev, new_dport, update_decoder_targets);
- dev_dbg(&port->dev, "dport%d:%s added\n", new_dport->port_id,
- dev_name(dport_dev));
-
- return no_free_ptr(new_dport);
+ return new_dport;
}
static struct cxl_dport *devm_cxl_create_port(struct device *ep_dev,
diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
index 51c8f2f84717..167cc0a87484 100644
--- a/drivers/cxl/port.c
+++ b/drivers/cxl/port.c
@@ -59,8 +59,12 @@ static int discover_region(struct device *dev, void *unused)
static int cxl_switch_port_probe(struct cxl_port *port)
{
- /* Reset nr_dports for rebind of driver */
- port->nr_dports = 0;
+ /*
+ * Unfortunately, typical driver operations like "find and map
+ * registers", can not be done at port device attach time and must wait
+ * for dport arrival. See cxl_port_add_dport() and the comments in
+ * add_dport() for details.
+ */
/* Cache the data early to ensure is_visible() works */
read_cdat_data(port);
diff --git a/tools/testing/cxl/cxl_core_exports.c b/tools/testing/cxl/cxl_core_exports.c
index 6754de35598d..02d479867a12 100644
--- a/tools/testing/cxl/cxl_core_exports.c
+++ b/tools/testing/cxl/cxl_core_exports.c
@@ -7,16 +7,15 @@
/* Exporting of cxl_core symbols that are only used by cxl_test */
EXPORT_SYMBOL_NS_GPL(cxl_num_decoders_committed, "CXL");
-cxl_add_dport_by_dev_fn _devm_cxl_add_dport_by_dev =
- __devm_cxl_add_dport_by_dev;
-EXPORT_SYMBOL_NS_GPL(_devm_cxl_add_dport_by_dev, "CXL");
+cxl_add_dport_by_dev_fn _cxl_add_dport_by_dev = __cxl_add_dport_by_dev;
+EXPORT_SYMBOL_NS_GPL(_cxl_add_dport_by_dev, "CXL");
-struct cxl_dport *devm_cxl_add_dport_by_dev(struct cxl_port *port,
- struct device *dport_dev)
+struct cxl_dport *cxl_add_dport_by_dev(struct cxl_port *port,
+ struct device *dport_dev)
{
- return _devm_cxl_add_dport_by_dev(port, dport_dev);
+ return _cxl_add_dport_by_dev(port, dport_dev);
}
-EXPORT_SYMBOL_NS_GPL(devm_cxl_add_dport_by_dev, "CXL");
+EXPORT_SYMBOL_NS_GPL(cxl_add_dport_by_dev, "CXL");
cxl_switch_decoders_setup_fn _devm_cxl_switch_port_decoders_setup =
__devm_cxl_switch_port_decoders_setup;
diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
index 81e2aef3627a..b7a2b550c0b0 100644
--- a/tools/testing/cxl/test/cxl.c
+++ b/tools/testing/cxl/test/cxl.c
@@ -1060,8 +1060,8 @@ static struct cxl_dport *mock_cxl_add_dport_by_dev(struct cxl_port *port,
if (&pdev->dev != dport_dev)
continue;
- return devm_cxl_add_dport(port, &pdev->dev, pdev->id,
- CXL_RESOURCE_NONE);
+ return cxl_add_dport(port, &pdev->dev, pdev->id,
+ CXL_RESOURCE_NONE);
}
return ERR_PTR(-ENODEV);
@@ -1126,9 +1126,9 @@ static struct cxl_mock_ops cxl_mock_ops = {
.devm_cxl_switch_port_decoders_setup = mock_cxl_switch_port_decoders_setup,
.devm_cxl_endpoint_decoders_setup = mock_cxl_endpoint_decoders_setup,
.cxl_endpoint_parse_cdat = mock_cxl_endpoint_parse_cdat,
- .devm_cxl_add_dport_by_dev = mock_cxl_add_dport_by_dev,
.hmat_get_extended_linear_cache_size =
mock_hmat_get_extended_linear_cache_size,
+ .cxl_add_dport_by_dev = mock_cxl_add_dport_by_dev,
.list = LIST_HEAD_INIT(cxl_mock_ops.list),
};
diff --git a/tools/testing/cxl/test/mock.c b/tools/testing/cxl/test/mock.c
index 44bce80ef3ff..660e8402189c 100644
--- a/tools/testing/cxl/test/mock.c
+++ b/tools/testing/cxl/test/mock.c
@@ -15,14 +15,13 @@
static LIST_HEAD(mock);
static struct cxl_dport *
-redirect_devm_cxl_add_dport_by_dev(struct cxl_port *port,
- struct device *dport_dev);
+redirect_cxl_add_dport_by_dev(struct cxl_port *port, struct device *dport_dev);
static int redirect_devm_cxl_switch_port_decoders_setup(struct cxl_port *port);
void register_cxl_mock_ops(struct cxl_mock_ops *ops)
{
list_add_rcu(&ops->list, &mock);
- _devm_cxl_add_dport_by_dev = redirect_devm_cxl_add_dport_by_dev;
+ _cxl_add_dport_by_dev = redirect_cxl_add_dport_by_dev;
_devm_cxl_switch_port_decoders_setup =
redirect_devm_cxl_switch_port_decoders_setup;
}
@@ -34,7 +33,7 @@ void unregister_cxl_mock_ops(struct cxl_mock_ops *ops)
{
_devm_cxl_switch_port_decoders_setup =
__devm_cxl_switch_port_decoders_setup;
- _devm_cxl_add_dport_by_dev = __devm_cxl_add_dport_by_dev;
+ _cxl_add_dport_by_dev = __cxl_add_dport_by_dev;
list_del_rcu(&ops->list);
synchronize_srcu(&cxl_mock_srcu);
}
@@ -207,7 +206,7 @@ int __wrap_cxl_await_media_ready(struct cxl_dev_state *cxlds)
}
EXPORT_SYMBOL_NS_GPL(__wrap_cxl_await_media_ready, "CXL");
-struct cxl_dport *__wrap_devm_cxl_add_rch_dport(struct cxl_port *port,
+struct cxl_dport *__wrap_cxl_add_rch_dport(struct cxl_port *port,
struct device *dport_dev,
int port_id,
resource_size_t rcrb)
@@ -217,19 +216,19 @@ struct cxl_dport *__wrap_devm_cxl_add_rch_dport(struct cxl_port *port,
struct cxl_mock_ops *ops = get_cxl_mock_ops(&index);
if (ops && ops->is_mock_port(dport_dev)) {
- dport = devm_cxl_add_dport(port, dport_dev, port_id,
- CXL_RESOURCE_NONE);
+ dport = cxl_add_dport(port, dport_dev, port_id,
+ CXL_RESOURCE_NONE);
if (!IS_ERR(dport)) {
dport->rcrb.base = rcrb;
dport->rch = true;
}
} else
- dport = devm_cxl_add_rch_dport(port, dport_dev, port_id, rcrb);
+ dport = cxl_add_rch_dport(port, dport_dev, port_id, rcrb);
put_cxl_mock_ops(index);
return dport;
}
-EXPORT_SYMBOL_NS_GPL(__wrap_devm_cxl_add_rch_dport, "CXL");
+EXPORT_SYMBOL_NS_GPL(__wrap_cxl_add_rch_dport, "CXL");
void __wrap_cxl_endpoint_parse_cdat(struct cxl_port *port)
{
@@ -257,17 +256,17 @@ void __wrap_cxl_dport_init_ras_reporting(struct cxl_dport *dport, struct device
}
EXPORT_SYMBOL_NS_GPL(__wrap_cxl_dport_init_ras_reporting, "CXL");
-struct cxl_dport *redirect_devm_cxl_add_dport_by_dev(struct cxl_port *port,
- struct device *dport_dev)
+struct cxl_dport *redirect_cxl_add_dport_by_dev(struct cxl_port *port,
+ struct device *dport_dev)
{
int index;
struct cxl_mock_ops *ops = get_cxl_mock_ops(&index);
struct cxl_dport *dport;
if (ops && ops->is_mock_port(port->uport_dev))
- dport = ops->devm_cxl_add_dport_by_dev(port, dport_dev);
+ dport = ops->cxl_add_dport_by_dev(port, dport_dev);
else
- dport = __devm_cxl_add_dport_by_dev(port, dport_dev);
+ dport = __cxl_add_dport_by_dev(port, dport_dev);
put_cxl_mock_ops(index);
return dport;
diff --git a/tools/testing/cxl/Kbuild b/tools/testing/cxl/Kbuild
index 6eceefefb0e0..4d740392aac5 100644
--- a/tools/testing/cxl/Kbuild
+++ b/tools/testing/cxl/Kbuild
@@ -5,7 +5,8 @@ ldflags-y += --wrap=acpi_evaluate_integer
ldflags-y += --wrap=acpi_pci_find_root
ldflags-y += --wrap=nvdimm_bus_register
ldflags-y += --wrap=cxl_await_media_ready
-ldflags-y += --wrap=devm_cxl_add_rch_dport
+ldflags-y += --wrap=cxl_add_rch_dport
+ldflags-y += --wrap=cxl_rcd_component_reg_phys
ldflags-y += --wrap=cxl_endpoint_parse_cdat
ldflags-y += --wrap=cxl_dport_init_ras_reporting
ldflags-y += --wrap=devm_cxl_endpoint_decoders_setup
--
2.52.0
On Thu, 15 Jan 2026 20:45:20 -0800
dan.j.williams@intel.com wrote:
> Jonathan Cameron wrote:
> > On Wed, 14 Jan 2026 12:20:40 -0600
> > Terry Bowman <terry.bowman@amd.com> wrote:
> >
> > > From: Dan Williams <dan.j.williams@intel.com>
> > >
> > > With dport addition moving out of cxl_switch_port_probe() it is no longer
> > > the case that a single dport-add failure will cause all dport resources
> > > to be automatically unwound.
> > >
> > > devm still helps all dport resources get cleaned up when the port is
> > > detached, but setup now needs to avoid leaking resources if an early exit
> > > occurs during setup.
> > >
> > > Convert from a "devm add" model, to an "auto remove" model that makes the
> > > caller responsible for registering devm reclaim after the object is fully
> > > instantiated.
> > >
> > > As a side of effect of this reorganization port->nr_dports is now always
> > > consistent with the number of entries in the port->dports xarray, and this
> > > can stop playing games with ida_is_empty() which is unreliable as a
> > > detector of whether decoders are setup. I.e. consider how
> > > CONFIG_DEBUG_KOBJECT_RELEASE might wreak havoc with this approach.
> > >
> > > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > > Reviewed-by: Terry Bowman <terry.bowman@amd.com>
> > >
> > > ---
> > >
> > > Changes in v13 -> v14:
> > > - New patch
> > Hi Dan, Terry,
> >
> > I think this needs a little reorganization to ensure we don't have
> > dport and dport_add both being the same pointer for different free
> > reasons. Adding a helper and we can combine them with a clear
> > hand over of ownership.
> >
> > Wrapping devres_remove_group() in a function that is called close_group()
> > rings alarm bells.
> >
> > Jonathan
> [..]
> >
> > > @@ -1176,48 +1175,27 @@ __devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev,
> > > &component_reg_phys);
> > >
> > > cond_cxl_root_lock(port);
> > > - rc = add_dport(port, dport);
> > > + struct cxl_dport *dport_add __free(remove_dport) =
> > > + add_dport(port, dport);
> >
> > This pattern of having both dport and dport_add effectively
> > pointing to the same pointer concerns me from a readability / maintainability
> > point of view. We've often made use of helper functions to avoid doing
> > this and I think that would make sense here as well.
>
> Yeah, while I do think the multi-variable pattern is useful for
> many-step object construction, I can usually easily be persuaded to
> consider a helper function.
>
> > Take everything down to and including dport_add() as a helper called
> > something like (naming needs work!)
> > struct dport_dev *dport __free(remove_and_free_dport) =
> > add_dport_wrapper();
>
> I ended up with the patch below which is similar in spirit to this
> without a new DEFINE_FREE().
>
>
> >
> > > cond_cxl_root_unlock(port);
> > > - 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;
> > > - }
> > > + if (IS_ERR(dport_add))
> > > + return dport_add;
> > >
> > > - get_device(dport_dev);
> > > - rc = devm_add_action_or_reset(host, cxl_dport_remove, dport);
> > > - if (rc)
> > > - return ERR_PTR(rc);
> > > + if (dev_is_pci(dport_dev))
> > > + dport->link_latency = cxl_pci_get_latency(to_pci_dev(dport_dev));
> > >
> > > rc = sysfs_create_link(&port->dev.kobj, &dport_dev->kobj, link_name);
> > > if (rc)
> > > return ERR_PTR(rc);
> > >
> > > - rc = devm_add_action_or_reset(host, cxl_dport_unlink, dport);
> > > - if (rc)
> > > - return ERR_PTR(rc);
> > > -
> > > - if (dev_is_pci(dport_dev))
> > > - dport->link_latency = cxl_pci_get_latency(to_pci_dev(dport_dev));
> > > -
> > > cxl_debugfs_create_dport_dir(dport);
> > >
> > > - return dport;
> > > + retain_and_null_ptr(dport_add);
> > > + return no_free_ptr(dport);
> > > }
> >
> >
> >
> > > +
> > > +/*
> > > + * Note: this only services dynamic removal of mid-level ports, root ports are
> > > + * always removed by the platform driver (e.g. cxl_acpi). @host can be
> > > + * hard-coded to &port->dev.
> > > + */
> > > static void del_dport(struct cxl_dport *dport)
> > > {
> > > struct cxl_port *port = dport->port;
> > >
> > > - devm_release_action(&port->dev, cxl_dport_unlink, dport);
> > > - devm_release_action(&port->dev, cxl_dport_remove, dport);
> > > - devm_kfree(&port->dev, dport);
> > > + devm_release_action(&port->dev, unlink_dport, dport);
> > > }
> > >
> > > static void del_dports(struct cxl_port *port)
> > > @@ -1597,10 +1603,24 @@ static int update_decoder_targets(struct device *dev, void *data)
> > > return 0;
> > > }
> > >
> > > -DEFINE_FREE(del_cxl_dport, struct cxl_dport *, if (!IS_ERR_OR_NULL(_T)) del_dport(_T))
> > > +static struct cxl_port *cxl_port_devres_group(struct cxl_port *port)
> > > +{
> > > + if (!devres_open_group(&port->dev, port, GFP_KERNEL))
> > > + return ERR_PTR(-ENOMEM);
> > > + return port;
> > > +}
> > > +DEFINE_FREE(cxl_port_group_free, struct cxl_port *,
> > > + if (!IS_ERR_OR_NULL(_T)) devres_release_group(&(_T)->dev, _T))
> > > +
> > > +static void cxl_port_group_close(struct cxl_port *port)
> >
> > This feels like misleading naming and I'm not sure what intent is.
> > Would have expected it to call devres_close_group()
>
> Agree. The hastiness of this patch shows. Switched all the naming to not
> be surprising. The flow is:
>
> cxl_port_open_group(): start recording devres resource acquisition
> cxl_port_remove_group(): on success, stop tracking the group, leave the resources
> cxl_port_release_group(): on failure, destroy the group, free the resources
Hi Dan, thanks for getting back on this so quickly!
Ok. So I'd misunderstood intent. If we don't have the option of close_group()
then these are just for temporary tracking of potential cleanup stuff
rather than because we want to optionally roll back part of the main
devres stuff (the bit that gets cleaned up on driver unbind /
just after remove())
I was thinking this was odd usage, but it is documented in devres.rst as one
or the two group examples, so I'm less concerned about that. Maybe
sprinkle a comment or two on the temporary nature of the devres group?
However, this makes me wonder why the other model in devres.rst
https://elixir.bootlin.com/linux/v6.19-rc5/source/Documentation/driver-api/driver-model/devres.rst#L187
int my_midlayer_create_something()
{
if (!devres_open_group(dev, my_midlayer_create_something, GFP_KERNEL))
return -ENOMEM;
...
devres_close_group(dev, my_midlayer_create_something);
return 0;
}
void my_midlayer_destroy_something()
{
devres_release_group(dev, my_midlayer_create_something);
}
isn't more appropriate here.
Did you give that approach a go? Assuming unlink_dport() cleans up all the
same stuff as was covered by the group (it doesn't quite because of the
last few things that can't fail) it should be a much less invasive
change. A small complexity is you'd need group to be created on the right dev
so that it matches what is done in the new autoremove code.
I'll give this a go, but might take a while so sending this in the meantime.
Anyhow, some of the comments that follow are on what you have done, and
a few others are on what the devres_close_group approach would look like.
This might the hardest to review patch I've looked at in a while...
Not sure what you could do about that though!
>
> New patch, added a Fixes: tag.
>
> -- 8< --
> From 9731bb6cb5638a0d2141dc072f90db0d00400680 Mon Sep 17 00:00:00 2001
> From: Dan Williams <dan.j.williams@intel.com>
> Date: Wed, 14 Jan 2026 12:20:40 -0600
> Subject: [PATCH] cxl/port: Fix devm resource leaks with dport management
>
> With dport addition moving out of cxl_switch_port_probe() it is no longer
> the case that a single dport-add failure will cause all dport resources
> to be automatically unwound.
>
> devm still helps all dport resources get cleaned up when the port is
> detached, but setup now needs to avoid leaking resources if an early exit
> occurs during setup.
>
> Convert from a "devm add" model, to an "auto remove" model that makes the
> caller responsible for registering devm reclaim after the object is fully
> instantiated.
>
> As a side of effect of this reorganization port->nr_dports is now always
> consistent with the number of entries in the port->dports xarray, and this
> can stop playing games with ida_is_empty() which is unreliable as a
> detector of whether decoders are setup. I.e. consider how
> CONFIG_DEBUG_KOBJECT_RELEASE might wreak havoc with this approach.
Given complexity of ownership, can we have a flow chart of who owns what when?
>
> Cc: <stable@vger.kernel.org>
> Fixes: 4f06d81e7c6a ("cxl: Defer dport allocation for switch ports")
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index b838c59d7a3c..ce117812e5c8 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -41,14 +41,14 @@ static int pci_get_port_num(struct pci_dev *pdev)
> }
>
> /**
> - * __devm_cxl_add_dport_by_dev - allocate a dport by dport device
> + * __cxl_add_dport_by_dev - allocate a dport by dport device
> * @port: cxl_port that hosts the dport
> * @dport_dev: 'struct device' of the dport
> *
> * Returns the allocated dport on success or ERR_PTR() of -errno on error
> */
> -struct cxl_dport *__devm_cxl_add_dport_by_dev(struct cxl_port *port,
> - struct device *dport_dev)
> +struct cxl_dport *__cxl_add_dport_by_dev(struct cxl_port *port,
> + struct device *dport_dev)
> {
> struct cxl_register_map map;
> struct pci_dev *pdev;
> @@ -67,9 +67,9 @@ struct cxl_dport *__devm_cxl_add_dport_by_dev(struct cxl_port *port,
> return ERR_PTR(rc);
>
> device_lock_assert(&port->dev);
> - return devm_cxl_add_dport(port, dport_dev, port_num, map.resource);
> + return cxl_add_dport(port, dport_dev, port_num, map.resource);
> }
> -EXPORT_SYMBOL_NS_GPL(__devm_cxl_add_dport_by_dev, "CXL");
> +EXPORT_SYMBOL_NS_GPL(__cxl_add_dport_by_dev, "CXL");
>
> static int cxl_dvsec_mem_range_valid(struct cxl_dev_state *cxlds, int id)
> {
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index fef3aa0c6680..41b65babd057 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -1066,11 +1066,28 @@ static int add_dport(struct cxl_port *port, struct cxl_dport *dport)
> return -EBUSY;
> }
>
> + /*
> + * Unlike CXL switch upstream ports where it can train a CXL link
> + * independent of its downstream ports, a host bridge upstream port may
> + * not enable CXL registers until at least one downstream port (root
> + * port) trains CXL. Enumerate registers once when the number of dports
> + * transitions from zero to one.
> + */
> + if (!port->nr_dports) {
> + rc = cxl_port_setup_regs(port, port->component_reg_phys);
> + if (rc)
> + return rc;
> + }
> +
> + /* 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;
> }
> @@ -1094,51 +1111,64 @@ static void cond_cxl_root_unlock(struct cxl_port *port)
> device_unlock(&port->dev);
> }
>
> -static void cxl_dport_remove(void *data)
> +static void remove_dport(struct cxl_dport *dport)
> {
> - 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);
> }
>
> -static void cxl_dport_unlink(void *data)
> +static struct cxl_dport *__register_dport(struct cxl_dport *dport)
> {
> - struct cxl_dport *dport = data;
> - struct cxl_port *port = dport->port;
> + int rc;
> char link_name[CXL_TARGET_STRLEN];
> + struct cxl_port *port = dport->port;
> + struct device *dport_dev = dport->dport_dev;
>
> - sprintf(link_name, "dport%d", dport->port_id);
> - sysfs_remove_link(&port->dev.kobj, link_name);
> -}
> + if (snprintf(link_name, CXL_TARGET_STRLEN, "dport%d", dport->port_id) >=
> + CXL_TARGET_STRLEN)
> + return ERR_PTR(-EINVAL);
>
> -static struct cxl_dport *
> -__devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev,
> - int port_id, resource_size_t component_reg_phys,
> - resource_size_t rcrb)
> -{
> - char link_name[CXL_TARGET_STRLEN];
> - struct cxl_dport *dport;
> - struct device *host;
> - int rc;
> + cond_cxl_root_lock(port);
> + rc = add_dport(port, dport);
> + cond_cxl_root_unlock(port);
> + if (rc)
> + return ERR_PTR(rc);
>
> - if (is_cxl_root(port))
> - host = port->uport_dev;
> - else
> - host = &port->dev;
> + if (dev_is_pci(dport_dev))
> + dport->link_latency = cxl_pci_get_latency(to_pci_dev(dport_dev));
>
> - if (!host->driver) {
> - dev_WARN_ONCE(&port->dev, 1, "dport:%s bad devm context\n",
> - dev_name(dport_dev));
> - return ERR_PTR(-ENXIO);
> + rc = sysfs_create_link(&port->dev.kobj, &dport_dev->kobj, link_name);
> + if (rc) {
There are several operations in here that get undone in unlink_dport()
I'd wrap those up in an __unregister_dport() function so it is easy to see
what pairs with what.
> + remove_dport(dport);
> + return ERR_PTR(rc);
> }
>
> - if (snprintf(link_name, CXL_TARGET_STRLEN, "dport%d", port_id) >=
> - CXL_TARGET_STRLEN)
> - return ERR_PTR(-EINVAL);
> + cxl_debugfs_create_dport_dir(dport);
>
> - dport = devm_kzalloc(host, sizeof(*dport), GFP_KERNEL);
> + return dport;
> +}
> @@ -1439,13 +1430,42 @@ static void delete_switch_port(struct cxl_port *port)
> devm_release_action(port->dev.parent, unregister_port, port);
> }
>
> +static void unlink_dport(void *data)
> +{
> + struct cxl_dport *dport = data;
> + struct cxl_port *port = dport->port;
> + char link_name[CXL_TARGET_STRLEN];
> +
> + sprintf(link_name, "dport%d", dport->port_id);
> + sysfs_remove_link(&port->dev.kobj, link_name);
> + remove_dport(dport);
> + kfree(dport);
To me this removes half the advantage of devres which is
that we don't need to be careful to remove things in the right
order. Ah well, perhaps a price we need to pay.
> +}
> +
> +int cxl_dport_autoremove(struct cxl_dport *dport)
> +{
> + struct cxl_port *port = dport->port;
> + struct device *host;
> +
> + if (is_cxl_root(port))
> + host = port->uport_dev;
> + else
> + host = &port->dev;
> +
> + return devm_add_action_or_reset(host, unlink_dport, dport);
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_dport_autoremove, "CXL");
> +
> +/*
> + * Note: this only services dynamic removal of mid-level ports, root ports are
> + * always removed by the platform driver (e.g. cxl_acpi). @host can be
> + * hard-coded to &port->dev.
> + */
> static void del_dport(struct cxl_dport *dport)
> {
> struct cxl_port *port = dport->port;
>
> - devm_release_action(&port->dev, cxl_dport_unlink, dport);
> - devm_release_action(&port->dev, cxl_dport_remove, dport);
> - devm_kfree(&port->dev, dport);
> + devm_release_action(&port->dev, unlink_dport, dport);
If you did go with the devres_close_group() suggestion I think you could
then call devres_remove_group() to undo the dport stuff leaving the
rest of the devres on port-dev in place.
> }
>
> static void del_dports(struct cxl_port *port)
> @@ -1597,10 +1617,24 @@ static int update_decoder_targets(struct device *dev, void *data)
> return 0;
> }
>
> -DEFINE_FREE(del_cxl_dport, struct cxl_dport *, if (!IS_ERR_OR_NULL(_T)) del_dport(_T))
> +static struct cxl_port *cxl_port_open_group(struct cxl_port *port)
> +{
> + if (!devres_open_group(&port->dev, port, GFP_KERNEL))
The use of port as the ID is tiny bit nasty but necessary I guess for the DEFINE_FREE to
work (you could use &port->dev) but that doesn't really help.
Disadvantage is you can't stack these groups without breaking the advice
in devres not to reuse IDs. In practice I think that actually works but
it's the sort of advice comment that makes me think it might not always do so!
> + return ERR_PTR(-ENOMEM);
> + return port;
> +}
> +DEFINE_FREE(cxl_port_release_group, struct cxl_port *,
> + if (!IS_ERR_OR_NULL(_T)) devres_release_group(&(_T)->dev, _T))
> +
> +static void cxl_port_remove_group(struct cxl_port *port)
> +{
> + devres_remove_group(&port->dev, port);
> +}
> +
> static struct cxl_dport *cxl_port_add_dport(struct cxl_port *port,
> struct device *dport_dev)
> {
> + struct cxl_dport *new_dport;
> struct cxl_dport *dport;
Too many dports. Given the existing one is only used for a sanity check, can we
have a precursor that gets rid of it via a helper
int check_no_existing_dport(struct cxl_dport *port, struct device *dport_dev)
{
struct cxl_dport *dport = cxl_find_dport_by_dev(port, dport_dev);
if (dport) {
dev_dbg(&port->dev, "dport%d:%s already exists\n",
dport->port_id, dev_name(dport_dev));
return -EBUSY;
}
return 0;
}
...
rc = check_no_existing_dport(port, dport_dev);
if (rc)
return ERR_PTR(rc);
then can rename new_dport to dport for this patch.
Or don't bother hiding it and just use dport variable for both.
> int rc;
>
> @@ -1615,29 +1649,47 @@ static struct cxl_dport *cxl_port_add_dport(struct cxl_port *port,
> return ERR_PTR(-EBUSY);
> }
>
> - struct cxl_dport *new_dport __free(del_cxl_dport) =
> - devm_cxl_add_dport_by_dev(port, dport_dev);
> - if (IS_ERR(new_dport))
> - return new_dport;
> -
> - cxl_switch_parse_cdat(new_dport);
> + /*
> + * With the first dport arrival it is now safe to start looking at
> + * component registers. Be careful to not strand resources if dport
> + * creation ultimately fails.
> + */
> + struct cxl_port *port_group __free(cxl_port_release_group) =
> + cxl_port_open_group(port);
So this is relies on everything being registered against port->dev, whereas
for root ports this then gets handed off to port->uport_dev.
That's a bit obscure - hence request for some patch description text
on who owns what resources + when.
> + if (IS_ERR(port_group))
> + return ERR_CAST(port_group);
>
> - if (ida_is_empty(&port->decoder_ida)) {
> + if (port->nr_dports == 0) {
> rc = devm_cxl_switch_port_decoders_setup(port);
> if (rc)
> return ERR_PTR(rc);
> - dev_dbg(&port->dev, "first dport%d:%s added with decoders\n",
> - new_dport->port_id, dev_name(dport_dev));
> - return no_free_ptr(new_dport);
> + /*
> + * Note, when nr_dports returns to zero the port is unregistered
> + * and triggers cleanup. I.e. no need for open-coded release
> + * action on dport removal. See cxl_detach_ep() for that logic.
> + */
> }
>
> + new_dport = cxl_add_dport_by_dev(port, dport_dev);
> + if (IS_ERR(new_dport))
> + return new_dport;
> +
> + rc = cxl_dport_autoremove(new_dport);
> + if (rc)
> + return ERR_PTR(rc);
> +
> + cxl_switch_parse_cdat(new_dport);
> +
> + /* group tracking no longer needed, dport successfully added */
> + cxl_port_remove_group(no_free_ptr(port_group));
I think this is a tiny bit too late (though my head hurts so could be wrong).
If we hit the error condition just above, we already freed the stuff
that this group controls.
I'd be tempted to have a helper for the entire region the group is held for
helper()
{
struct cxl_port *port_group __free(cxl_port_release_group) =
cxl_port_open_group(port);
if (IS_ERR(port_group))
return ERR_CAST(port_group);
...
cxl_port_remove_group(no_free_ptr(port_group));
return something good;
}
so that the scope is clear.
> +
> + dev_dbg(&port->dev, "dport[%d] id:%d dport_dev: %s added\n",
> + port->nr_dports - 1, new_dport->port_id, dev_name(dport_dev));
> +
> /* New dport added, update the decoder targets */
> device_for_each_child(&port->dev, new_dport, update_decoder_targets);
>
> - dev_dbg(&port->dev, "dport%d:%s added\n", new_dport->port_id,
> - dev_name(dport_dev));
> -
> - return no_free_ptr(new_dport);
> + return new_dport;
> }
>
> static struct cxl_dport *devm_cxl_create_port(struct device *ep_dev,
Jonathan Cameron wrote: [..] > This might the hardest to review patch I've looked at in a while... Well, that *is* fatal feedback. I think this simply needs to be broken up into smaller to digest pieces. Will send that shortly.
> rc = check_no_existing_dport(port, dport_dev);
> if (rc)
> return ERR_PTR(rc);
>
> then can rename new_dport to dport for this patch.
> Or don't bother hiding it and just use dport variable for both.
>
>
> > int rc;
> >
> > @@ -1615,29 +1649,47 @@ static struct cxl_dport *cxl_port_add_dport(struct cxl_port *port,
> > return ERR_PTR(-EBUSY);
> > }
> >
> > - struct cxl_dport *new_dport __free(del_cxl_dport) =
> > - devm_cxl_add_dport_by_dev(port, dport_dev);
> > - if (IS_ERR(new_dport))
> > - return new_dport;
> > -
> > - cxl_switch_parse_cdat(new_dport);
> > + /*
> > + * With the first dport arrival it is now safe to start looking at
> > + * component registers. Be careful to not strand resources if dport
> > + * creation ultimately fails.
> > + */
> > + struct cxl_port *port_group __free(cxl_port_release_group) =
> > + cxl_port_open_group(port);
>
> So this is relies on everything being registered against port->dev, whereas
> for root ports this then gets handed off to port->uport_dev.
> That's a bit obscure - hence request for some patch description text
> on who owns what resources + when.
>
> > + if (IS_ERR(port_group))
> > + return ERR_CAST(port_group);
> >
> > - if (ida_is_empty(&port->decoder_ida)) {
> > + if (port->nr_dports == 0) {
> > rc = devm_cxl_switch_port_decoders_setup(port);
> > if (rc)
> > return ERR_PTR(rc);
I'm not totally sure I have an appropriate base for messing with this but
with just this patch on top of cxl/for-7.0/cxl-init I'm getting problems
with the reorder here as the devm_cxl_switch_port fails to map HDM decoders.
On a simple single switch couple of devices test.
I'm probably suffering Friday afternoon syndrome (and naughty testing just
a middle patch in a series)
My 'guess' is that it's because we no longer add the port first.
I couldn't spot any other patch messing with related logic earlier in Terry's series
(and wanted to keep what I was messing with for trying alternative fixes to a minimum)
Any thoughts on what I'm missing?
Jonathan
> > - dev_dbg(&port->dev, "first dport%d:%s added with decoders\n",
> > - new_dport->port_id, dev_name(dport_dev));
> > - return no_free_ptr(new_dport);
> > + /*
> > + * Note, when nr_dports returns to zero the port is unregistered
> > + * and triggers cleanup. I.e. no need for open-coded release
> > + * action on dport removal. See cxl_detach_ep() for that logic.
> > + */
> > }
> >
> > + new_dport = cxl_add_dport_by_dev(port, dport_dev);
> > + if (IS_ERR(new_dport))
> > + return new_dport;
> > +
> > + rc = cxl_dport_autoremove(new_dport);
> > + if (rc)
> > + return ERR_PTR(rc);
> > +
> > + cxl_switch_parse_cdat(new_dport);
> > +
> > + /* group tracking no longer needed, dport successfully added */
> > + cxl_port_remove_group(no_free_ptr(port_group));
Jonathan Cameron wrote: [..] > Any thoughts on what I'm missing? Probably: cxl/port: Map Port component registers before switchport init ...which really should not be a distinct patch from the one that changes the ordering. I will send out a more patient series to settle this so Terry does not need to keep carrying it.
On Mon, 19 Jan 2026 15:02:09 -0800
dan.j.williams@intel.com wrote:
> Jonathan Cameron wrote:
> [..]
> > Any thoughts on what I'm missing?
>
> Probably:
>
> cxl/port: Map Port component registers before switchport init
>
> ...which really should not be a distinct patch from the one that changes
> the ordering. I will send out a more patient series to settle this so
> Terry does not need to keep carrying it.
>
Ah. I stopped looking when I got to this patch :) Spot on - thanks!
Below is my suggestion of another approach. I think it ends up
a fair bit simpler, but you know this code a lot better than I do, so I may
be missing some problems! I like the simpler reaping. That approach looks
like it can be used elsewhere in this file.
Testing so far is one representative config only.
The patch you highlight above is squashed into this. At least one comment
I made on your patch applies here too (naughty me :)
From a95567d9e2e3809ca1953a9aec24324ae13f6145 Mon Sep 17 00:00:00 2001
From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Date: Tue, 20 Jan 2026 10:53:14 +0000
Subject: [PATCH] Alternative to Dan's approach for managing dport resources.
Create a devres group to manage a subset of resources.
On success, close the devres group, but also stash a copy in
struct cxl_dport so we can use it for dport reaping.
On error, when group open release it.
Somewhat tested, but not what I'd consider exhaustive yet!
Used a 2 port switch, 2 EP test config.
1. Unbind endpoints, checked reap happened.
2. Unbind port1, check tear down happened.
3. Add some errors so some calls fail (stepping through each
group that was created) Some cases get setup by
it having another try based on a different downstream device
arrival triggering the flow, but it seems fine.
---
drivers/cxl/core/port.c | 76 ++++++++++++++++++++---------------
drivers/cxl/cxl.h | 2 +
drivers/cxl/cxlpci.h | 4 ++
tools/testing/cxl/test/mock.c | 1 +
4 files changed, 51 insertions(+), 32 deletions(-)
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index fef3aa0c6680..e3c53b2fc78f 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -778,7 +778,7 @@ static int cxl_setup_comp_regs(struct device *host, struct cxl_register_map *map
return cxl_setup_regs(map);
}
-static int cxl_port_setup_regs(struct cxl_port *port,
+int cxl_port_setup_regs(struct cxl_port *port,
resource_size_t component_reg_phys)
{
if (dev_is_platform(port->uport_dev))
@@ -786,6 +786,7 @@ static int cxl_port_setup_regs(struct cxl_port *port,
return cxl_setup_comp_regs(&port->dev, &port->reg_map,
component_reg_phys);
}
+EXPORT_SYMBOL_NS_GPL(cxl_port_setup_regs, "CXL");
static int cxl_dport_setup_regs(struct device *host, struct cxl_dport *dport,
resource_size_t component_reg_phys)
@@ -1065,7 +1066,12 @@ static int add_dport(struct cxl_port *port, struct cxl_dport *dport)
dev_name(dup->dport_dev));
return -EBUSY;
}
-
+ //comment from original patch here.
+ if (!port->nr_dports) {
+ rc = cxl_port_setup_regs(port, port->component_reg_phys);
+ if (rc)
+ return rc;
+ }
rc = xa_insert(&port->dports, (unsigned long)dport->dport_dev, dport,
GFP_KERNEL);
if (rc)
@@ -1181,20 +1187,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)
@@ -1441,11 +1433,7 @@ static void delete_switch_port(struct cxl_port *port)
static void del_dport(struct cxl_dport *dport)
{
- struct cxl_port *port = dport->port;
-
- devm_release_action(&port->dev, cxl_dport_unlink, dport);
- devm_release_action(&port->dev, cxl_dport_remove, dport);
- devm_kfree(&port->dev, dport);
+ devres_release_group(&dport->port->dev, dport->devres_group);
}
static void del_dports(struct cxl_port *port)
@@ -1602,6 +1590,9 @@ static struct cxl_dport *cxl_port_add_dport(struct cxl_port *port,
struct device *dport_dev)
{
struct cxl_dport *dport;
+ struct cxl_dport *new_dport;
+ struct device *host;
+ void *devres_group;
int rc;
device_lock_assert(&port->dev);
@@ -1615,21 +1606,31 @@ static struct cxl_dport *cxl_port_add_dport(struct cxl_port *port,
return ERR_PTR(-EBUSY);
}
- struct cxl_dport *new_dport __free(del_cxl_dport) =
- devm_cxl_add_dport_by_dev(port, dport_dev);
- if (IS_ERR(new_dport))
- return new_dport;
+ if (is_cxl_root(port))
+ host = port->uport_dev;
+ else
+ host = &port->dev;
+
+ devres_group = devres_open_group(host, NULL, GFP_KERNEL);
+ if (!devres_group)
+ return ERR_PTR(-ENOMEM);
- cxl_switch_parse_cdat(new_dport);
+ if (port->nr_dports == 0) {
+ rc = cxl_port_setup_regs(port, port->component_reg_phys);
+ if (rc)
+ goto release_group;
- if (ida_is_empty(&port->decoder_ida)) {
rc = devm_cxl_switch_port_decoders_setup(port);
if (rc)
- return ERR_PTR(rc);
- dev_dbg(&port->dev, "first dport%d:%s added with decoders\n",
- new_dport->port_id, dev_name(dport_dev));
- return no_free_ptr(new_dport);
+ goto release_group;
+ }
+
+ new_dport = devm_cxl_add_dport_by_dev(port, dport_dev);
+ if (IS_ERR(new_dport)) {
+ rc = PTR_ERR(new_dport);
+ goto release_group;
}
+ cxl_switch_parse_cdat(new_dport);
/* New dport added, update the decoder targets */
device_for_each_child(&port->dev, new_dport, update_decoder_targets);
@@ -1637,7 +1638,18 @@ static struct cxl_dport *cxl_port_add_dport(struct cxl_port *port,
dev_dbg(&port->dev, "dport%d:%s added\n", new_dport->port_id,
dev_name(dport_dev));
- return no_free_ptr(new_dport);
+ /*
+ * Stash the group for use during reaping when all downstream devices
+ * go away.
+ */
+ new_dport->devres_group = devres_group;
+ devres_close_group(host, devres_group);
+
+ return new_dport;
+
+release_group:
+ devres_release_group(host, devres_group);
+ return ERR_PTR(rc);
}
static struct cxl_dport *devm_cxl_create_port(struct device *ep_dev,
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index c796c3db36e0..855f7629f5c4 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -688,6 +688,7 @@ struct cxl_rcrb_info {
* @coord: access coordinates (bandwidth and latency performance attributes)
* @link_latency: calculated PCIe downstream latency
* @gpf_dvsec: Cached GPF port DVSEC
+ * @devres_group: Used to simplify reaping ports.
*/
struct cxl_dport {
struct device *dport_dev;
@@ -700,6 +701,7 @@ struct cxl_dport {
struct access_coordinate coord[ACCESS_COORDINATE_MAX];
long link_latency;
int gpf_dvsec;
+ void *devres_group;
};
/**
diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
index 1d526bea8431..a3fbedb66dbb 100644
--- a/drivers/cxl/cxlpci.h
+++ b/drivers/cxl/cxlpci.h
@@ -132,4 +132,8 @@ void read_cdat_data(struct cxl_port *port);
void cxl_cor_error_detected(struct pci_dev *pdev);
pci_ers_result_t cxl_error_detected(struct pci_dev *pdev,
pci_channel_state_t state);
+
+int cxl_port_setup_regs(struct cxl_port *port,
+ resource_size_t component_reg_phys);
+
#endif /* __CXL_PCI_H__ */
On 1/14/26 11:20 AM, Terry Bowman wrote:
> From: Dan Williams <dan.j.williams@intel.com>
>
> With dport addition moving out of cxl_switch_port_probe() it is no longer
> the case that a single dport-add failure will cause all dport resources
> to be automatically unwound.
>
> devm still helps all dport resources get cleaned up when the port is
> detached, but setup now needs to avoid leaking resources if an early exit
> occurs during setup.
>
> Convert from a "devm add" model, to an "auto remove" model that makes the
> caller responsible for registering devm reclaim after the object is fully
> instantiated.
>
> As a side of effect of this reorganization port->nr_dports is now always
> consistent with the number of entries in the port->dports xarray, and this
> can stop playing games with ida_is_empty() which is unreliable as a
> detector of whether decoders are setup. I.e. consider how
> CONFIG_DEBUG_KOBJECT_RELEASE might wreak havoc with this approach.
>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> Reviewed-by: Terry Bowman <terry.bowman@amd.com>
Missing sign off tag
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
>
> ---
>
> Changes in v13 -> v14:
> - New patch
> ---
> drivers/cxl/acpi.c | 11 +-
> drivers/cxl/core/pci.c | 10 +-
> drivers/cxl/core/port.c | 225 ++++++++++++++++-----------
> drivers/cxl/cxl.h | 23 +--
> drivers/cxl/port.c | 8 +-
> tools/testing/cxl/Kbuild | 3 +-
> tools/testing/cxl/cxl_core_exports.c | 13 +-
> tools/testing/cxl/exports.h | 4 +-
> tools/testing/cxl/test/cxl.c | 6 +-
> tools/testing/cxl/test/mock.c | 25 ++-
> tools/testing/cxl/test/mock.h | 4 +-
> 11 files changed, 188 insertions(+), 144 deletions(-)
>
> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index 77ac940e3013..1e1383eb9bd5 100644
> --- a/drivers/cxl/acpi.c
> +++ b/drivers/cxl/acpi.c
> @@ -679,16 +679,19 @@ static int add_host_bridge_dport(struct device *match, void *arg)
> if (ctx.cxl_version == ACPI_CEDT_CHBS_VERSION_CXL11) {
> dev_dbg(match, "RCRB found for UID %lld: %pa\n", ctx.uid,
> &ctx.base);
> - dport = devm_cxl_add_rch_dport(root_port, bridge, ctx.uid,
> - ctx.base);
> + dport = cxl_add_rch_dport(root_port, bridge, ctx.uid, ctx.base);
> } else {
> - dport = devm_cxl_add_dport(root_port, bridge, ctx.uid,
> - CXL_RESOURCE_NONE);
> + dport = cxl_add_dport(root_port, bridge, ctx.uid,
> + CXL_RESOURCE_NONE);
> }
>
> if (IS_ERR(dport))
> return PTR_ERR(dport);
>
> + ret = cxl_dport_autoremove(dport);
> + if (ret)
> + return ret;
> +
> ret = get_genport_coordinates(match, dport);
> if (ret)
> dev_dbg(match, "Failed to get generic port perf coordinates.\n");
> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index 0305a421504e..512a3e29a095 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -41,14 +41,14 @@ static int pci_get_port_num(struct pci_dev *pdev)
> }
>
> /**
> - * __devm_cxl_add_dport_by_dev - allocate a dport by dport device
> + * __cxl_add_dport_by_dev - allocate a dport by dport device
> * @port: cxl_port that hosts the dport
> * @dport_dev: 'struct device' of the dport
> *
> * Returns the allocated dport on success or ERR_PTR() of -errno on error
> */
> -struct cxl_dport *__devm_cxl_add_dport_by_dev(struct cxl_port *port,
> - struct device *dport_dev)
> +struct cxl_dport *__cxl_add_dport_by_dev(struct cxl_port *port,
> + struct device *dport_dev)
> {
> struct cxl_register_map map;
> struct pci_dev *pdev;
> @@ -67,9 +67,9 @@ struct cxl_dport *__devm_cxl_add_dport_by_dev(struct cxl_port *port,
> return ERR_PTR(rc);
>
> device_lock_assert(&port->dev);
> - return devm_cxl_add_dport(port, dport_dev, port_num, map.resource);
> + return cxl_add_dport(port, dport_dev, port_num, map.resource);
> }
> -EXPORT_SYMBOL_NS_GPL(__devm_cxl_add_dport_by_dev, "CXL");
> +EXPORT_SYMBOL_NS_GPL(__cxl_add_dport_by_dev, "CXL");
>
> struct cxl_walk_context {
> struct pci_bus *bus;
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index fef3aa0c6680..a05a1812bb6e 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -1051,7 +1051,8 @@ static struct cxl_dport *find_dport(struct cxl_port *port, int id)
> return NULL;
> }
>
> -static int add_dport(struct cxl_port *port, struct cxl_dport *dport)
> +static struct cxl_dport *add_dport(struct cxl_port *port,
> + struct cxl_dport *dport)
> {
> struct cxl_dport *dup;
> int rc;
> @@ -1063,16 +1064,33 @@ static int add_dport(struct cxl_port *port, struct cxl_dport *dport)
> "unable to add dport%d-%s non-unique port id (%s)\n",
> dport->port_id, dev_name(dport->dport_dev),
> dev_name(dup->dport_dev));
> - return -EBUSY;
> + return ERR_PTR(-EBUSY);
> + }
> +
> + /*
> + * Unlike CXL switch upstream ports where it can train a CXL link
> + * independent of its downstream ports, a host bridge upstream port may
> + * not enable CXL registers until at least one downstream port (root
> + * port) trains CXL. Enumerate registers once when the number of dports
> + * transitions from zero to one.
> + */
> + if (!port->nr_dports) {
> + rc = cxl_port_setup_regs(port, port->component_reg_phys);
> + if (rc)
> + return ERR_PTR(rc);
> }
>
> + /* 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;
> + return ERR_PTR(rc);
>
> + retain_and_null_ptr(dev);
> port->nr_dports++;
> - return 0;
> + return dport;
> }
>
> /*
> @@ -1094,51 +1112,32 @@ static void cond_cxl_root_unlock(struct cxl_port *port)
> device_unlock(&port->dev);
> }
>
> -static void cxl_dport_remove(void *data)
> +static void remove_dport(struct cxl_dport *dport)
> {
> - 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);
> }
>
> -static void cxl_dport_unlink(void *data)
> -{
> - struct cxl_dport *dport = data;
> - struct cxl_port *port = dport->port;
> - char link_name[CXL_TARGET_STRLEN];
> +DEFINE_FREE(remove_dport, struct cxl_dport *,
> + if (!IS_ERR_OR_NULL(_T)) remove_dport(_T))
>
> - sprintf(link_name, "dport%d", dport->port_id);
> - sysfs_remove_link(&port->dev.kobj, link_name);
> -}
> -
> -static struct cxl_dport *
> -__devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev,
> - int port_id, resource_size_t component_reg_phys,
> - resource_size_t rcrb)
> +static struct cxl_dport *__cxl_add_dport(struct cxl_port *port,
> + struct device *dport_dev, int port_id,
> + resource_size_t component_reg_phys,
> + resource_size_t rcrb)
> {
> char link_name[CXL_TARGET_STRLEN];
> - struct cxl_dport *dport;
> - struct device *host;
> int rc;
>
> - if (is_cxl_root(port))
> - host = port->uport_dev;
> - else
> - host = &port->dev;
> -
> - if (!host->driver) {
> - dev_WARN_ONCE(&port->dev, 1, "dport:%s bad devm context\n",
> - dev_name(dport_dev));
> - return ERR_PTR(-ENXIO);
> - }
> -
> if (snprintf(link_name, CXL_TARGET_STRLEN, "dport%d", port_id) >=
> CXL_TARGET_STRLEN)
> return ERR_PTR(-EINVAL);
>
> - dport = devm_kzalloc(host, sizeof(*dport), GFP_KERNEL);
> + struct cxl_dport *dport __free(kfree) =
> + kzalloc(sizeof(*dport), GFP_KERNEL);
> if (!dport)
> return ERR_PTR(-ENOMEM);
>
> @@ -1176,48 +1175,27 @@ __devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev,
> &component_reg_phys);
>
> cond_cxl_root_lock(port);
> - rc = add_dport(port, dport);
> + struct cxl_dport *dport_add __free(remove_dport) =
> + add_dport(port, dport);
> cond_cxl_root_unlock(port);
> - 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;
> - }
> + if (IS_ERR(dport_add))
> + return dport_add;
>
> - get_device(dport_dev);
> - rc = devm_add_action_or_reset(host, cxl_dport_remove, dport);
> - if (rc)
> - return ERR_PTR(rc);
> + if (dev_is_pci(dport_dev))
> + dport->link_latency = cxl_pci_get_latency(to_pci_dev(dport_dev));
>
> rc = sysfs_create_link(&port->dev.kobj, &dport_dev->kobj, link_name);
> if (rc)
> return ERR_PTR(rc);
>
> - rc = devm_add_action_or_reset(host, cxl_dport_unlink, dport);
> - if (rc)
> - return ERR_PTR(rc);
> -
> - if (dev_is_pci(dport_dev))
> - dport->link_latency = cxl_pci_get_latency(to_pci_dev(dport_dev));
> -
> cxl_debugfs_create_dport_dir(dport);
>
> - return dport;
> + retain_and_null_ptr(dport_add);
> + return no_free_ptr(dport);
> }
>
> /**
> - * devm_cxl_add_dport - append VH downstream port data to a cxl_port
> + * cxl_add_dport - append VH downstream port data to a cxl_port
> * @port: the cxl_port that references this dport
> * @dport_dev: firmware or PCI device representing the dport
> * @port_id: identifier for this dport in a decoder's target list
> @@ -1227,14 +1205,13 @@ __devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev,
> * either the port's host (for root ports), or the port itself (for
> * switch ports)
> */
> -struct cxl_dport *devm_cxl_add_dport(struct cxl_port *port,
> - struct device *dport_dev, int port_id,
> - resource_size_t component_reg_phys)
> +struct cxl_dport *cxl_add_dport(struct cxl_port *port, struct device *dport_dev,
> + int port_id, resource_size_t component_reg_phys)
> {
> struct cxl_dport *dport;
>
> - dport = __devm_cxl_add_dport(port, dport_dev, port_id,
> - component_reg_phys, CXL_RESOURCE_NONE);
> + dport = __cxl_add_dport(port, dport_dev, port_id, component_reg_phys,
> + CXL_RESOURCE_NONE);
> if (IS_ERR(dport)) {
> dev_dbg(dport_dev, "failed to add dport to %s: %ld\n",
> dev_name(&port->dev), PTR_ERR(dport));
> @@ -1245,10 +1222,10 @@ struct cxl_dport *devm_cxl_add_dport(struct cxl_port *port,
>
> return dport;
> }
> -EXPORT_SYMBOL_NS_GPL(devm_cxl_add_dport, "CXL");
> +EXPORT_SYMBOL_NS_GPL(cxl_add_dport, "CXL");
>
> /**
> - * devm_cxl_add_rch_dport - append RCH downstream port data to a cxl_port
> + * cxl_add_rch_dport - append RCH downstream port data to a cxl_port
> * @port: the cxl_port that references this dport
> * @dport_dev: firmware or PCI device representing the dport
> * @port_id: identifier for this dport in a decoder's target list
> @@ -1256,9 +1233,9 @@ EXPORT_SYMBOL_NS_GPL(devm_cxl_add_dport, "CXL");
> *
> * See CXL 3.0 9.11.8 CXL Devices Attached to an RCH
> */
> -struct cxl_dport *devm_cxl_add_rch_dport(struct cxl_port *port,
> - struct device *dport_dev, int port_id,
> - resource_size_t rcrb)
> +struct cxl_dport *cxl_add_rch_dport(struct cxl_port *port,
> + struct device *dport_dev, int port_id,
> + resource_size_t rcrb)
> {
> struct cxl_dport *dport;
>
> @@ -1267,8 +1244,8 @@ struct cxl_dport *devm_cxl_add_rch_dport(struct cxl_port *port,
> return ERR_PTR(-EINVAL);
> }
>
> - dport = __devm_cxl_add_dport(port, dport_dev, port_id,
> - CXL_RESOURCE_NONE, rcrb);
> + dport = __cxl_add_dport(port, dport_dev, port_id, CXL_RESOURCE_NONE,
> + rcrb);
> if (IS_ERR(dport)) {
> dev_dbg(dport_dev, "failed to add RCH dport to %s: %ld\n",
> dev_name(&port->dev), PTR_ERR(dport));
> @@ -1279,7 +1256,7 @@ struct cxl_dport *devm_cxl_add_rch_dport(struct cxl_port *port,
>
> return dport;
> }
> -EXPORT_SYMBOL_NS_GPL(devm_cxl_add_rch_dport, "CXL");
> +EXPORT_SYMBOL_NS_GPL(cxl_add_rch_dport, "CXL");
>
> static int add_ep(struct cxl_ep *new)
> {
> @@ -1439,13 +1416,42 @@ static void delete_switch_port(struct cxl_port *port)
> devm_release_action(port->dev.parent, unregister_port, port);
> }
>
> +static void unlink_dport(void *data)
> +{
> + struct cxl_dport *dport = data;
> + struct cxl_port *port = dport->port;
> + char link_name[CXL_TARGET_STRLEN];
> +
> + sprintf(link_name, "dport%d", dport->port_id);
> + sysfs_remove_link(&port->dev.kobj, link_name);
> + remove_dport(dport);
> + kfree(dport);
> +}
> +
> +int cxl_dport_autoremove(struct cxl_dport *dport)
> +{
> + struct cxl_port *port = dport->port;
> + struct device *host;
> +
> + if (is_cxl_root(port))
> + host = port->uport_dev;
> + else
> + host = &port->dev;
> +
> + return devm_add_action_or_reset(host, unlink_dport, dport);
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_dport_autoremove, "CXL");
> +
> +/*
> + * Note: this only services dynamic removal of mid-level ports, root ports are
> + * always removed by the platform driver (e.g. cxl_acpi). @host can be
> + * hard-coded to &port->dev.
> + */
> static void del_dport(struct cxl_dport *dport)
> {
> struct cxl_port *port = dport->port;
>
> - devm_release_action(&port->dev, cxl_dport_unlink, dport);
> - devm_release_action(&port->dev, cxl_dport_remove, dport);
> - devm_kfree(&port->dev, dport);
> + devm_release_action(&port->dev, unlink_dport, dport);
> }
>
> static void del_dports(struct cxl_port *port)
> @@ -1597,10 +1603,24 @@ static int update_decoder_targets(struct device *dev, void *data)
> return 0;
> }
>
> -DEFINE_FREE(del_cxl_dport, struct cxl_dport *, if (!IS_ERR_OR_NULL(_T)) del_dport(_T))
> +static struct cxl_port *cxl_port_devres_group(struct cxl_port *port)
> +{
> + if (!devres_open_group(&port->dev, port, GFP_KERNEL))
> + return ERR_PTR(-ENOMEM);
> + return port;
> +}
> +DEFINE_FREE(cxl_port_group_free, struct cxl_port *,
> + if (!IS_ERR_OR_NULL(_T)) devres_release_group(&(_T)->dev, _T))
> +
> +static void cxl_port_group_close(struct cxl_port *port)
> +{
> + devres_remove_group(&port->dev, port);
> +}
> +
> static struct cxl_dport *cxl_port_add_dport(struct cxl_port *port,
> struct device *dport_dev)
> {
> + struct cxl_dport *new_dport;
> struct cxl_dport *dport;
> int rc;
>
> @@ -1615,29 +1635,46 @@ static struct cxl_dport *cxl_port_add_dport(struct cxl_port *port,
> return ERR_PTR(-EBUSY);
> }
>
> - struct cxl_dport *new_dport __free(del_cxl_dport) =
> - devm_cxl_add_dport_by_dev(port, dport_dev);
> - if (IS_ERR(new_dport))
> - return new_dport;
> -
> - cxl_switch_parse_cdat(new_dport);
> + /*
> + * With the first dport arrival it is now safe to start looking at
> + * component registers. Be careful to not strand resources if dport
> + * creation ultimately fails.
> + */
> + struct cxl_port *port_group __free(cxl_port_group_free) =
> + cxl_port_devres_group(port);
> + if (IS_ERR(port_group))
> + return ERR_CAST(port_group);
>
> - if (ida_is_empty(&port->decoder_ida)) {
> + if (port->nr_dports == 0) {
> rc = devm_cxl_switch_port_decoders_setup(port);
> if (rc)
> return ERR_PTR(rc);
> - dev_dbg(&port->dev, "first dport%d:%s added with decoders\n",
> - new_dport->port_id, dev_name(dport_dev));
> - return no_free_ptr(new_dport);
> + /*
> + * Note, when nr_dports returns to zero the port is unregistered
> + * and triggers cleanup. I.e. no need for open-coded release
> + * action on dport removal. See cxl_detach_ep() for that logic.
> + */
> }
>
> + new_dport = cxl_add_dport_by_dev(port, dport_dev);
> + if (IS_ERR(new_dport))
> + return new_dport;
> +
> + rc = cxl_dport_autoremove(new_dport);
> + if (rc)
> + return ERR_PTR(rc);
> +
> + cxl_switch_parse_cdat(new_dport);
> +
> + cxl_port_group_close(no_free_ptr(port_group));
> +
> + dev_dbg(&port->dev, "dport[%d] id:%d dport_dev: %s added\n",
> + port->nr_dports - 1, new_dport->port_id, dev_name(dport_dev));
> +
> /* New dport added, update the decoder targets */
> device_for_each_child(&port->dev, new_dport, update_decoder_targets);
>
> - dev_dbg(&port->dev, "dport%d:%s added\n", new_dport->port_id,
> - dev_name(dport_dev));
> -
> - return no_free_ptr(new_dport);
> + return new_dport;
> }
>
> static struct cxl_dport *devm_cxl_create_port(struct device *ep_dev,
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 6f3741a57932..47ee06c95433 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -796,12 +796,12 @@ struct cxl_port *cxl_mem_find_port(struct cxl_memdev *cxlmd,
> struct cxl_dport **dport);
> bool schedule_cxl_memdev_detach(struct cxl_memdev *cxlmd);
>
> -struct cxl_dport *devm_cxl_add_dport(struct cxl_port *port,
> - struct device *dport, int port_id,
> - resource_size_t component_reg_phys);
> -struct cxl_dport *devm_cxl_add_rch_dport(struct cxl_port *port,
> - struct device *dport_dev, int port_id,
> - resource_size_t rcrb);
> +struct cxl_dport *cxl_add_dport(struct cxl_port *port, struct device *dport,
> + int port_id,
> + resource_size_t component_reg_phys);
> +struct cxl_dport *cxl_add_rch_dport(struct cxl_port *port,
> + struct device *dport_dev, int port_id,
> + resource_size_t rcrb);
>
> struct cxl_decoder *to_cxl_decoder(struct device *dev);
> struct cxl_root_decoder *to_cxl_root_decoder(struct device *dev);
> @@ -824,6 +824,7 @@ static inline int cxl_root_decoder_autoremove(struct device *host,
> return cxl_decoder_autoremove(host, &cxlrd->cxlsd.cxld);
> }
> int cxl_endpoint_autoremove(struct cxl_memdev *cxlmd, struct cxl_port *endpoint);
> +int cxl_dport_autoremove(struct cxl_dport *dport);
>
> /**
> * struct cxl_endpoint_dvsec_info - Cached DVSEC info
> @@ -937,10 +938,10 @@ void cxl_coordinates_combine(struct access_coordinate *out,
> struct access_coordinate *c2);
>
> bool cxl_endpoint_decoder_reset_detected(struct cxl_port *port);
> -struct cxl_dport *devm_cxl_add_dport_by_dev(struct cxl_port *port,
> - struct device *dport_dev);
> -struct cxl_dport *__devm_cxl_add_dport_by_dev(struct cxl_port *port,
> - struct device *dport_dev);
> +struct cxl_dport *cxl_add_dport_by_dev(struct cxl_port *port,
> + struct device *dport_dev);
> +struct cxl_dport *__cxl_add_dport_by_dev(struct cxl_port *port,
> + struct device *dport_dev);
>
> /*
> * Unit test builds overrides this to __weak, find the 'strong' version
> @@ -964,7 +965,7 @@ u16 cxl_gpf_get_dvsec(struct device *dev);
> */
> #ifndef CXL_TEST_ENABLE
> #define DECLARE_TESTABLE(x) __##x
> -#define devm_cxl_add_dport_by_dev DECLARE_TESTABLE(devm_cxl_add_dport_by_dev)
> +#define cxl_add_dport_by_dev DECLARE_TESTABLE(cxl_add_dport_by_dev)
> #define devm_cxl_switch_port_decoders_setup DECLARE_TESTABLE(devm_cxl_switch_port_decoders_setup)
> #endif
>
> diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
> index 51c8f2f84717..167cc0a87484 100644
> --- a/drivers/cxl/port.c
> +++ b/drivers/cxl/port.c
> @@ -59,8 +59,12 @@ static int discover_region(struct device *dev, void *unused)
>
> static int cxl_switch_port_probe(struct cxl_port *port)
> {
> - /* Reset nr_dports for rebind of driver */
> - port->nr_dports = 0;
> + /*
> + * Unfortunately, typical driver operations like "find and map
> + * registers", can not be done at port device attach time and must wait
> + * for dport arrival. See cxl_port_add_dport() and the comments in
> + * add_dport() for details.
> + */
>
> /* Cache the data early to ensure is_visible() works */
> read_cdat_data(port);
> diff --git a/tools/testing/cxl/Kbuild b/tools/testing/cxl/Kbuild
> index 6eceefefb0e0..4d740392aac5 100644
> --- a/tools/testing/cxl/Kbuild
> +++ b/tools/testing/cxl/Kbuild
> @@ -5,7 +5,8 @@ ldflags-y += --wrap=acpi_evaluate_integer
> ldflags-y += --wrap=acpi_pci_find_root
> ldflags-y += --wrap=nvdimm_bus_register
> ldflags-y += --wrap=cxl_await_media_ready
> -ldflags-y += --wrap=devm_cxl_add_rch_dport
> +ldflags-y += --wrap=cxl_add_rch_dport
> +ldflags-y += --wrap=cxl_rcd_component_reg_phys
> ldflags-y += --wrap=cxl_endpoint_parse_cdat
> ldflags-y += --wrap=cxl_dport_init_ras_reporting
> ldflags-y += --wrap=devm_cxl_endpoint_decoders_setup
> diff --git a/tools/testing/cxl/cxl_core_exports.c b/tools/testing/cxl/cxl_core_exports.c
> index 6754de35598d..02d479867a12 100644
> --- a/tools/testing/cxl/cxl_core_exports.c
> +++ b/tools/testing/cxl/cxl_core_exports.c
> @@ -7,16 +7,15 @@
> /* Exporting of cxl_core symbols that are only used by cxl_test */
> EXPORT_SYMBOL_NS_GPL(cxl_num_decoders_committed, "CXL");
>
> -cxl_add_dport_by_dev_fn _devm_cxl_add_dport_by_dev =
> - __devm_cxl_add_dport_by_dev;
> -EXPORT_SYMBOL_NS_GPL(_devm_cxl_add_dport_by_dev, "CXL");
> +cxl_add_dport_by_dev_fn _cxl_add_dport_by_dev = __cxl_add_dport_by_dev;
> +EXPORT_SYMBOL_NS_GPL(_cxl_add_dport_by_dev, "CXL");
>
> -struct cxl_dport *devm_cxl_add_dport_by_dev(struct cxl_port *port,
> - struct device *dport_dev)
> +struct cxl_dport *cxl_add_dport_by_dev(struct cxl_port *port,
> + struct device *dport_dev)
> {
> - return _devm_cxl_add_dport_by_dev(port, dport_dev);
> + return _cxl_add_dport_by_dev(port, dport_dev);
> }
> -EXPORT_SYMBOL_NS_GPL(devm_cxl_add_dport_by_dev, "CXL");
> +EXPORT_SYMBOL_NS_GPL(cxl_add_dport_by_dev, "CXL");
>
> cxl_switch_decoders_setup_fn _devm_cxl_switch_port_decoders_setup =
> __devm_cxl_switch_port_decoders_setup;
> diff --git a/tools/testing/cxl/exports.h b/tools/testing/cxl/exports.h
> index 7ebee7c0bd67..cbb16073be18 100644
> --- a/tools/testing/cxl/exports.h
> +++ b/tools/testing/cxl/exports.h
> @@ -4,8 +4,8 @@
> #define __MOCK_CXL_EXPORTS_H_
>
> typedef struct cxl_dport *(*cxl_add_dport_by_dev_fn)(struct cxl_port *port,
> - struct device *dport_dev);
> -extern cxl_add_dport_by_dev_fn _devm_cxl_add_dport_by_dev;
> + struct device *dport_dev);
> +extern cxl_add_dport_by_dev_fn _cxl_add_dport_by_dev;
>
> typedef int(*cxl_switch_decoders_setup_fn)(struct cxl_port *port);
> extern cxl_switch_decoders_setup_fn _devm_cxl_switch_port_decoders_setup;
> diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
> index 81e2aef3627a..b7a2b550c0b0 100644
> --- a/tools/testing/cxl/test/cxl.c
> +++ b/tools/testing/cxl/test/cxl.c
> @@ -1060,8 +1060,8 @@ static struct cxl_dport *mock_cxl_add_dport_by_dev(struct cxl_port *port,
> if (&pdev->dev != dport_dev)
> continue;
>
> - return devm_cxl_add_dport(port, &pdev->dev, pdev->id,
> - CXL_RESOURCE_NONE);
> + return cxl_add_dport(port, &pdev->dev, pdev->id,
> + CXL_RESOURCE_NONE);
> }
>
> return ERR_PTR(-ENODEV);
> @@ -1126,9 +1126,9 @@ static struct cxl_mock_ops cxl_mock_ops = {
> .devm_cxl_switch_port_decoders_setup = mock_cxl_switch_port_decoders_setup,
> .devm_cxl_endpoint_decoders_setup = mock_cxl_endpoint_decoders_setup,
> .cxl_endpoint_parse_cdat = mock_cxl_endpoint_parse_cdat,
> - .devm_cxl_add_dport_by_dev = mock_cxl_add_dport_by_dev,
> .hmat_get_extended_linear_cache_size =
> mock_hmat_get_extended_linear_cache_size,
> + .cxl_add_dport_by_dev = mock_cxl_add_dport_by_dev,
> .list = LIST_HEAD_INIT(cxl_mock_ops.list),
> };
>
> diff --git a/tools/testing/cxl/test/mock.c b/tools/testing/cxl/test/mock.c
> index 44bce80ef3ff..660e8402189c 100644
> --- a/tools/testing/cxl/test/mock.c
> +++ b/tools/testing/cxl/test/mock.c
> @@ -15,14 +15,13 @@
> static LIST_HEAD(mock);
>
> static struct cxl_dport *
> -redirect_devm_cxl_add_dport_by_dev(struct cxl_port *port,
> - struct device *dport_dev);
> +redirect_cxl_add_dport_by_dev(struct cxl_port *port, struct device *dport_dev);
> static int redirect_devm_cxl_switch_port_decoders_setup(struct cxl_port *port);
>
> void register_cxl_mock_ops(struct cxl_mock_ops *ops)
> {
> list_add_rcu(&ops->list, &mock);
> - _devm_cxl_add_dport_by_dev = redirect_devm_cxl_add_dport_by_dev;
> + _cxl_add_dport_by_dev = redirect_cxl_add_dport_by_dev;
> _devm_cxl_switch_port_decoders_setup =
> redirect_devm_cxl_switch_port_decoders_setup;
> }
> @@ -34,7 +33,7 @@ void unregister_cxl_mock_ops(struct cxl_mock_ops *ops)
> {
> _devm_cxl_switch_port_decoders_setup =
> __devm_cxl_switch_port_decoders_setup;
> - _devm_cxl_add_dport_by_dev = __devm_cxl_add_dport_by_dev;
> + _cxl_add_dport_by_dev = __cxl_add_dport_by_dev;
> list_del_rcu(&ops->list);
> synchronize_srcu(&cxl_mock_srcu);
> }
> @@ -207,7 +206,7 @@ int __wrap_cxl_await_media_ready(struct cxl_dev_state *cxlds)
> }
> EXPORT_SYMBOL_NS_GPL(__wrap_cxl_await_media_ready, "CXL");
>
> -struct cxl_dport *__wrap_devm_cxl_add_rch_dport(struct cxl_port *port,
> +struct cxl_dport *__wrap_cxl_add_rch_dport(struct cxl_port *port,
> struct device *dport_dev,
> int port_id,
> resource_size_t rcrb)
> @@ -217,19 +216,19 @@ struct cxl_dport *__wrap_devm_cxl_add_rch_dport(struct cxl_port *port,
> struct cxl_mock_ops *ops = get_cxl_mock_ops(&index);
>
> if (ops && ops->is_mock_port(dport_dev)) {
> - dport = devm_cxl_add_dport(port, dport_dev, port_id,
> - CXL_RESOURCE_NONE);
> + dport = cxl_add_dport(port, dport_dev, port_id,
> + CXL_RESOURCE_NONE);
> if (!IS_ERR(dport)) {
> dport->rcrb.base = rcrb;
> dport->rch = true;
> }
> } else
> - dport = devm_cxl_add_rch_dport(port, dport_dev, port_id, rcrb);
> + dport = cxl_add_rch_dport(port, dport_dev, port_id, rcrb);
> put_cxl_mock_ops(index);
>
> return dport;
> }
> -EXPORT_SYMBOL_NS_GPL(__wrap_devm_cxl_add_rch_dport, "CXL");
> +EXPORT_SYMBOL_NS_GPL(__wrap_cxl_add_rch_dport, "CXL");
>
> void __wrap_cxl_endpoint_parse_cdat(struct cxl_port *port)
> {
> @@ -257,17 +256,17 @@ void __wrap_cxl_dport_init_ras_reporting(struct cxl_dport *dport, struct device
> }
> EXPORT_SYMBOL_NS_GPL(__wrap_cxl_dport_init_ras_reporting, "CXL");
>
> -struct cxl_dport *redirect_devm_cxl_add_dport_by_dev(struct cxl_port *port,
> - struct device *dport_dev)
> +struct cxl_dport *redirect_cxl_add_dport_by_dev(struct cxl_port *port,
> + struct device *dport_dev)
> {
> int index;
> struct cxl_mock_ops *ops = get_cxl_mock_ops(&index);
> struct cxl_dport *dport;
>
> if (ops && ops->is_mock_port(port->uport_dev))
> - dport = ops->devm_cxl_add_dport_by_dev(port, dport_dev);
> + dport = ops->cxl_add_dport_by_dev(port, dport_dev);
> else
> - dport = __devm_cxl_add_dport_by_dev(port, dport_dev);
> + dport = __cxl_add_dport_by_dev(port, dport_dev);
> put_cxl_mock_ops(index);
>
> return dport;
> diff --git a/tools/testing/cxl/test/mock.h b/tools/testing/cxl/test/mock.h
> index 2684b89c8aa2..fa13aca4e260 100644
> --- a/tools/testing/cxl/test/mock.h
> +++ b/tools/testing/cxl/test/mock.h
> @@ -22,8 +22,8 @@ struct cxl_mock_ops {
> int (*devm_cxl_switch_port_decoders_setup)(struct cxl_port *port);
> int (*devm_cxl_endpoint_decoders_setup)(struct cxl_port *port);
> void (*cxl_endpoint_parse_cdat)(struct cxl_port *port);
> - struct cxl_dport *(*devm_cxl_add_dport_by_dev)(struct cxl_port *port,
> - struct device *dport_dev);
> + struct cxl_dport *(*cxl_add_dport_by_dev)(struct cxl_port *port,
> + struct device *dport_dev);
> int (*hmat_get_extended_linear_cache_size)(struct resource *backing_res,
> int nid,
> resource_size_t *cache_size);
© 2016 - 2026 Red Hat, Inc.