[PATCH v2 1/3] drivers/cxl: introduce cxl_region_driver field for cxl_region

[PATCH v2 1/3] drivers/cxl: introduce cxl_region_driver field for cxl_region

[Gregory Price]

The CXL driver presently has 3 modes of managing a cxl_region:
  - no specific driver (bios-onlined SystemRAM)
  - dax_region (all other RAM regions, for now)
  - pmem_region (all PMEM regions)

Formalize these into specific "region drivers".

enum cxl_region_driver {
	CXL_REGION_DRIVER_NONE,
	CXL_REGION_DRIVER_DAX,
	CXL_REGION_DRIVER_PMEM
};

$cat regionN/region_driver
[none,dax,pmem]

The intent is to clarify how to to add additional drivers (sysram,
dynamic_capacity, etc) in the future, and to allow switching the
driver selection via a sysfs entry `regionN/region_driver`.

All RAM regions will be defaulted to CXL_CONTROL_DAX.

Auto-regions will either be static sysram (BIOS-onlined) and has no
region controller associated with it - or if the SP bit was set a
DAX device will be created.  This will be discovered at probe time.

Signed-off-by: Gregory Price <gourry@gourry.net>
---
 drivers/cxl/core/region.c | 113 ++++++++++++++++++++++++++++++--------
 drivers/cxl/cxl.h         |   8 +++
 2 files changed, 98 insertions(+), 23 deletions(-)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index ae899f68551f..f8262d2169ea 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -626,6 +626,57 @@ static ssize_t mode_show(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR_RO(mode);
 
+static ssize_t region_driver_show(struct device *dev,
+				  struct device_attribute *attr,
+				  char *buf)
+{
+	struct cxl_region *cxlr = to_cxl_region(dev);
+	const char *desc;
+
+	switch (cxlr->driver) {
+	case CXL_REGION_DRIVER_NONE:
+		desc = "none";
+		break;
+	case CXL_REGION_DRIVER_DAX:
+		desc = "dax";
+		break;
+	case CXL_REGION_DRIVER_PMEM:
+		desc = "pmem";
+		break;
+	}
+
+	return sysfs_emit(buf, "%s\n", desc);
+}
+
+static ssize_t region_driver_store(struct device *dev,
+				   struct device_attribute *attr,
+				   const char *buf, size_t len)
+{
+	struct cxl_region *cxlr = to_cxl_region(dev);
+	struct cxl_region_params *p = &cxlr->params;
+	int rc;
+
+	ACQUIRE(rwsem_write_kill, rwsem)(&cxl_rwsem.region);
+	if ((rc = ACQUIRE_ERR(rwsem_write_kill, &rwsem)))
+		return rc;
+
+	if (p->state >= CXL_CONFIG_COMMIT)
+		return -EBUSY;
+
+	/* PMEM drivers cannot be changed */
+	if (cxlr->mode == CXL_PARTMODE_PMEM)
+		return -EBUSY;
+
+	/* NONE type is not a valid selection for manually probed regions */
+	if (sysfs_streq(buf, "dax"))
+		cxlr->driver = CXL_REGION_DRIVER_DAX;
+	else
+		return -EINVAL;
+
+	return len;
+}
+static DEVICE_ATTR_RW(region_driver);
+
 static int alloc_hpa(struct cxl_region *cxlr, resource_size_t size)
 {
 	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent);
@@ -772,6 +823,7 @@ static struct attribute *cxl_region_attrs[] = {
 	&dev_attr_size.attr,
 	&dev_attr_mode.attr,
 	&dev_attr_extended_linear_cache_size.attr,
+	&dev_attr_region_driver.attr,
 	NULL,
 };
 
@@ -2599,6 +2651,16 @@ static struct cxl_region *devm_cxl_add_region(struct cxl_root_decoder *cxlrd,
 	cxlr->mode = mode;
 	cxlr->type = type;
 
+	/*
+	 * PMEM regions only have 1 driver: pmem_region
+	 * RAM regions default to DAX, but if the memory is already onlined by
+	 * BIOS as 'System-RAM', the DAX driver will be dropped during probe.
+	 */
+	if (mode == CXL_PARTMODE_PMEM)
+		cxlr->driver = CXL_REGION_DRIVER_PMEM;
+	else
+		cxlr->driver = CXL_REGION_DRIVER_DAX;
+
 	dev = &cxlr->dev;
 	rc = dev_set_name(dev, "region%d", id);
 	if (rc)
@@ -3951,33 +4013,38 @@ static int cxl_region_probe(struct device *dev)
 			return rc;
 	}
 
-	switch (cxlr->mode) {
-	case CXL_PARTMODE_PMEM:
-		rc = devm_cxl_region_edac_register(cxlr);
-		if (rc)
-			dev_dbg(&cxlr->dev, "CXL EDAC registration for region_id=%d failed\n",
-				cxlr->id);
+	if (cxlr->mode > CXL_PARTMODE_PMEM) {
+		dev_dbg(&cxlr->dev, "unsupported region mode: %d\n",
+			cxlr->mode);
+		return -ENXIO;
+	}
 
-		return devm_cxl_add_pmem_region(cxlr);
-	case CXL_PARTMODE_RAM:
-		rc = devm_cxl_region_edac_register(cxlr);
-		if (rc)
-			dev_dbg(&cxlr->dev, "CXL EDAC registration for region_id=%d failed\n",
-				cxlr->id);
+	/*
+	 * The region can not be managed by CXL if any portion of
+	 * it is already online as 'System RAM'.
+	 */
+	if (walk_iomem_res_desc(IORES_DESC_NONE,
+				IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY,
+				p->res->start, p->res->end, cxlr,
+					is_system_ram) > 0) {
+		cxlr->driver = CXL_REGION_DRIVER_NONE;
+		return 0;
+	}
 
-		/*
-		 * The region can not be manged by CXL if any portion of
-		 * it is already online as 'System RAM'
-		 */
-		if (walk_iomem_res_desc(IORES_DESC_NONE,
-					IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY,
-					p->res->start, p->res->end, cxlr,
-					is_system_ram) > 0)
-			return 0;
+	rc = devm_cxl_region_edac_register(cxlr);
+	dev_dbg(&cxlr->dev, "CXL EDAC registration for region_id=%d %s\n",
+			cxlr->id, rc ? "failed" : "succeeded");
+
+	switch (cxlr->driver) {
+	case CXL_REGION_DRIVER_NONE:
+		return 0;
+	case CXL_REGION_DRIVER_DAX:
 		return devm_cxl_add_dax_region(cxlr);
+	case CXL_REGION_DRIVER_PMEM:
+		return devm_cxl_add_pmem_region(cxlr);
 	default:
-		dev_dbg(&cxlr->dev, "unsupported region mode: %d\n",
-			cxlr->mode);
+		dev_dbg(&cxlr->dev, "unsupported region driver: %d\n",
+			cxlr->driver);
 		return -ENXIO;
 	}
 }
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index ba17fa86d249..e8256099de29 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -502,6 +502,13 @@ enum cxl_partition_mode {
 	CXL_PARTMODE_PMEM,
 };
 
+
+enum cxl_region_driver {
+	CXL_REGION_DRIVER_NONE,
+	CXL_REGION_DRIVER_DAX,
+	CXL_REGION_DRIVER_PMEM,
+};
+
 /*
  * Indicate whether this region has been assembled by autodetection or
  * userspace assembly. Prevent endpoint decoders outside of automatic
@@ -543,6 +550,7 @@ struct cxl_region {
 	struct device dev;
 	int id;
 	enum cxl_partition_mode mode;
+	enum cxl_region_driver driver;
 	enum cxl_decoder_type type;
 	struct cxl_nvdimm_bridge *cxl_nvb;
 	struct cxl_pmem_region *cxlr_pmem;
-- 
2.52.0

[PATCH v2 2/3] cxl/core/region: move pmem region driver logic into pmem_region

[Gregory Price]

Move the pmem region driver logic from region.c into pmem_region.c.

Signed-off-by: Gregory Price <gourry@gourry.net>
---
 drivers/cxl/core/Makefile      |   1 +
 drivers/cxl/core/core.h        |   1 +
 drivers/cxl/core/pmem_region.c | 191 +++++++++++++++++++++++++++++++++
 drivers/cxl/core/region.c      | 184 -------------------------------
 4 files changed, 193 insertions(+), 184 deletions(-)
 create mode 100644 drivers/cxl/core/pmem_region.c

diff --git a/drivers/cxl/core/Makefile b/drivers/cxl/core/Makefile
index 5ad8fef210b5..23269c81fd44 100644
--- a/drivers/cxl/core/Makefile
+++ b/drivers/cxl/core/Makefile
@@ -17,6 +17,7 @@ cxl_core-y += cdat.o
 cxl_core-y += ras.o
 cxl_core-$(CONFIG_TRACING) += trace.o
 cxl_core-$(CONFIG_CXL_REGION) += region.o
+cxl_core-$(CONFIG_CXL_REGION) += pmem_region.o
 cxl_core-$(CONFIG_CXL_MCE) += mce.o
 cxl_core-$(CONFIG_CXL_FEATURES) += features.o
 cxl_core-$(CONFIG_CXL_EDAC_MEM_FEATURES) += edac.o
diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
index 1fb66132b777..cd589e81f55e 100644
--- a/drivers/cxl/core/core.h
+++ b/drivers/cxl/core/core.h
@@ -42,6 +42,7 @@ int cxl_get_poison_by_endpoint(struct cxl_port *port);
 struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa);
 u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
 		   u64 dpa);
+int devm_cxl_add_pmem_region(struct cxl_region *cxlr);
 
 #else
 static inline u64 cxl_dpa_to_hpa(struct cxl_region *cxlr,
diff --git a/drivers/cxl/core/pmem_region.c b/drivers/cxl/core/pmem_region.c
new file mode 100644
index 000000000000..81b66e548bb5
--- /dev/null
+++ b/drivers/cxl/core/pmem_region.c
@@ -0,0 +1,191 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright(c) 2022 Intel Corporation. All rights reserved. */
+#include <linux/device.h>
+#include <linux/slab.h>
+#include <cxlmem.h>
+#include <cxl.h>
+#include "core.h"
+
+static void cxl_pmem_region_release(struct device *dev)
+{
+	struct cxl_pmem_region *cxlr_pmem = to_cxl_pmem_region(dev);
+	int i;
+
+	for (i = 0; i < cxlr_pmem->nr_mappings; i++) {
+		struct cxl_memdev *cxlmd = cxlr_pmem->mapping[i].cxlmd;
+
+		put_device(&cxlmd->dev);
+	}
+
+	kfree(cxlr_pmem);
+}
+
+static const struct attribute_group *cxl_pmem_region_attribute_groups[] = {
+	&cxl_base_attribute_group,
+	NULL,
+};
+
+const struct device_type cxl_pmem_region_type = {
+	.name = "cxl_pmem_region",
+	.release = cxl_pmem_region_release,
+	.groups = cxl_pmem_region_attribute_groups,
+};
+bool is_cxl_pmem_region(struct device *dev)
+{
+	return dev->type == &cxl_pmem_region_type;
+}
+EXPORT_SYMBOL_NS_GPL(is_cxl_pmem_region, "CXL");
+
+struct cxl_pmem_region *to_cxl_pmem_region(struct device *dev)
+{
+	if (dev_WARN_ONCE(dev, !is_cxl_pmem_region(dev),
+				"not a cxl_pmem_region device\n"))
+		return NULL;
+	return container_of(dev, struct cxl_pmem_region, dev);
+}
+EXPORT_SYMBOL_NS_GPL(to_cxl_pmem_region, "CXL");
+static struct lock_class_key cxl_pmem_region_key;
+
+static int cxl_pmem_region_alloc(struct cxl_region *cxlr)
+{
+	struct cxl_region_params *p = &cxlr->params;
+	struct cxl_nvdimm_bridge *cxl_nvb;
+	struct device *dev;
+	int i;
+
+	guard(rwsem_read)(&cxl_rwsem.region);
+	if (p->state != CXL_CONFIG_COMMIT)
+		return -ENXIO;
+
+	struct cxl_pmem_region *cxlr_pmem __free(kfree) =
+		kzalloc(struct_size(cxlr_pmem, mapping, p->nr_targets), GFP_KERNEL);
+	if (!cxlr_pmem)
+		return -ENOMEM;
+
+	cxlr_pmem->hpa_range.start = p->res->start;
+	cxlr_pmem->hpa_range.end = p->res->end;
+
+	/* Snapshot the region configuration underneath the cxl_rwsem.region */
+	cxlr_pmem->nr_mappings = p->nr_targets;
+	for (i = 0; i < p->nr_targets; i++) {
+		struct cxl_endpoint_decoder *cxled = p->targets[i];
+		struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
+		struct cxl_pmem_region_mapping *m = &cxlr_pmem->mapping[i];
+
+		/*
+		 * Regions never span CXL root devices, so by definition the
+		 * bridge for one device is the same for all.
+		 */
+		if (i == 0) {
+			cxl_nvb = cxl_find_nvdimm_bridge(cxlmd->endpoint);
+			if (!cxl_nvb)
+				return -ENODEV;
+			cxlr->cxl_nvb = cxl_nvb;
+		}
+		m->cxlmd = cxlmd;
+		get_device(&cxlmd->dev);
+		m->start = cxled->dpa_res->start;
+		m->size = resource_size(cxled->dpa_res);
+		m->position = i;
+	}
+
+	dev = &cxlr_pmem->dev;
+	device_initialize(dev);
+	lockdep_set_class(&dev->mutex, &cxl_pmem_region_key);
+	device_set_pm_not_required(dev);
+	dev->parent = &cxlr->dev;
+	dev->bus = &cxl_bus_type;
+	dev->type = &cxl_pmem_region_type;
+	cxlr_pmem->cxlr = cxlr;
+	cxlr->cxlr_pmem = no_free_ptr(cxlr_pmem);
+
+	return 0;
+}
+
+static void cxlr_pmem_unregister(void *_cxlr_pmem)
+{
+	struct cxl_pmem_region *cxlr_pmem = _cxlr_pmem;
+	struct cxl_region *cxlr = cxlr_pmem->cxlr;
+	struct cxl_nvdimm_bridge *cxl_nvb = cxlr->cxl_nvb;
+
+	/*
+	 * Either the bridge is in ->remove() context under the device_lock(),
+	 * or cxlr_release_nvdimm() is cancelling the bridge's release action
+	 * for @cxlr_pmem and doing it itself (while manually holding the bridge
+	 * lock).
+	 */
+	device_lock_assert(&cxl_nvb->dev);
+	cxlr->cxlr_pmem = NULL;
+	cxlr_pmem->cxlr = NULL;
+	device_unregister(&cxlr_pmem->dev);
+}
+
+static void cxlr_release_nvdimm(void *_cxlr)
+{
+	struct cxl_region *cxlr = _cxlr;
+	struct cxl_nvdimm_bridge *cxl_nvb = cxlr->cxl_nvb;
+
+	scoped_guard(device, &cxl_nvb->dev) {
+		if (cxlr->cxlr_pmem)
+			devm_release_action(&cxl_nvb->dev, cxlr_pmem_unregister,
+					cxlr->cxlr_pmem);
+	}
+	cxlr->cxl_nvb = NULL;
+	put_device(&cxl_nvb->dev);
+}
+
+/**
+ * devm_cxl_add_pmem_region() - add a cxl_region-to-nd_region bridge
+ * @cxlr: parent CXL region for this pmem region bridge device
+ *
+ * Return: 0 on success negative error code on failure.
+ */
+int devm_cxl_add_pmem_region(struct cxl_region *cxlr)
+{
+	struct cxl_pmem_region *cxlr_pmem;
+	struct cxl_nvdimm_bridge *cxl_nvb;
+	struct device *dev;
+	int rc;
+
+	rc = cxl_pmem_region_alloc(cxlr);
+	if (rc)
+		return rc;
+	cxlr_pmem = cxlr->cxlr_pmem;
+	cxl_nvb = cxlr->cxl_nvb;
+
+	dev = &cxlr_pmem->dev;
+	rc = dev_set_name(dev, "pmem_region%d", cxlr->id);
+	if (rc)
+		goto err;
+
+	rc = device_add(dev);
+	if (rc)
+		goto err;
+
+	dev_dbg(&cxlr->dev, "%s: register %s\n", dev_name(dev->parent),
+			dev_name(dev));
+
+	scoped_guard(device, &cxl_nvb->dev) {
+		if (cxl_nvb->dev.driver)
+			rc = devm_add_action_or_reset(&cxl_nvb->dev,
+					cxlr_pmem_unregister,
+					cxlr_pmem);
+		else
+			rc = -ENXIO;
+	}
+
+	if (rc)
+		goto err_bridge;
+
+	/* @cxlr carries a reference on @cxl_nvb until cxlr_release_nvdimm */
+	return devm_add_action_or_reset(&cxlr->dev, cxlr_release_nvdimm, cxlr);
+
+err:
+	put_device(dev);
+err_bridge:
+	put_device(&cxl_nvb->dev);
+	cxlr->cxl_nvb = NULL;
+	return rc;
+}
+
+
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index f8262d2169ea..a61805542b56 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -2808,46 +2808,6 @@ static ssize_t delete_region_store(struct device *dev,
 }
 DEVICE_ATTR_WO(delete_region);
 
-static void cxl_pmem_region_release(struct device *dev)
-{
-	struct cxl_pmem_region *cxlr_pmem = to_cxl_pmem_region(dev);
-	int i;
-
-	for (i = 0; i < cxlr_pmem->nr_mappings; i++) {
-		struct cxl_memdev *cxlmd = cxlr_pmem->mapping[i].cxlmd;
-
-		put_device(&cxlmd->dev);
-	}
-
-	kfree(cxlr_pmem);
-}
-
-static const struct attribute_group *cxl_pmem_region_attribute_groups[] = {
-	&cxl_base_attribute_group,
-	NULL,
-};
-
-const struct device_type cxl_pmem_region_type = {
-	.name = "cxl_pmem_region",
-	.release = cxl_pmem_region_release,
-	.groups = cxl_pmem_region_attribute_groups,
-};
-
-bool is_cxl_pmem_region(struct device *dev)
-{
-	return dev->type == &cxl_pmem_region_type;
-}
-EXPORT_SYMBOL_NS_GPL(is_cxl_pmem_region, "CXL");
-
-struct cxl_pmem_region *to_cxl_pmem_region(struct device *dev)
-{
-	if (dev_WARN_ONCE(dev, !is_cxl_pmem_region(dev),
-			  "not a cxl_pmem_region device\n"))
-		return NULL;
-	return container_of(dev, struct cxl_pmem_region, dev);
-}
-EXPORT_SYMBOL_NS_GPL(to_cxl_pmem_region, "CXL");
-
 struct cxl_poison_context {
 	struct cxl_port *port;
 	int part;
@@ -3279,64 +3239,6 @@ static int region_offset_to_dpa_result(struct cxl_region *cxlr, u64 offset,
 	return -ENXIO;
 }
 
-static struct lock_class_key cxl_pmem_region_key;
-
-static int cxl_pmem_region_alloc(struct cxl_region *cxlr)
-{
-	struct cxl_region_params *p = &cxlr->params;
-	struct cxl_nvdimm_bridge *cxl_nvb;
-	struct device *dev;
-	int i;
-
-	guard(rwsem_read)(&cxl_rwsem.region);
-	if (p->state != CXL_CONFIG_COMMIT)
-		return -ENXIO;
-
-	struct cxl_pmem_region *cxlr_pmem __free(kfree) =
-		kzalloc(struct_size(cxlr_pmem, mapping, p->nr_targets), GFP_KERNEL);
-	if (!cxlr_pmem)
-		return -ENOMEM;
-
-	cxlr_pmem->hpa_range.start = p->res->start;
-	cxlr_pmem->hpa_range.end = p->res->end;
-
-	/* Snapshot the region configuration underneath the cxl_rwsem.region */
-	cxlr_pmem->nr_mappings = p->nr_targets;
-	for (i = 0; i < p->nr_targets; i++) {
-		struct cxl_endpoint_decoder *cxled = p->targets[i];
-		struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
-		struct cxl_pmem_region_mapping *m = &cxlr_pmem->mapping[i];
-
-		/*
-		 * Regions never span CXL root devices, so by definition the
-		 * bridge for one device is the same for all.
-		 */
-		if (i == 0) {
-			cxl_nvb = cxl_find_nvdimm_bridge(cxlmd->endpoint);
-			if (!cxl_nvb)
-				return -ENODEV;
-			cxlr->cxl_nvb = cxl_nvb;
-		}
-		m->cxlmd = cxlmd;
-		get_device(&cxlmd->dev);
-		m->start = cxled->dpa_res->start;
-		m->size = resource_size(cxled->dpa_res);
-		m->position = i;
-	}
-
-	dev = &cxlr_pmem->dev;
-	device_initialize(dev);
-	lockdep_set_class(&dev->mutex, &cxl_pmem_region_key);
-	device_set_pm_not_required(dev);
-	dev->parent = &cxlr->dev;
-	dev->bus = &cxl_bus_type;
-	dev->type = &cxl_pmem_region_type;
-	cxlr_pmem->cxlr = cxlr;
-	cxlr->cxlr_pmem = no_free_ptr(cxlr_pmem);
-
-	return 0;
-}
-
 static void cxl_dax_region_release(struct device *dev)
 {
 	struct cxl_dax_region *cxlr_dax = to_cxl_dax_region(dev);
@@ -3400,92 +3302,6 @@ static struct cxl_dax_region *cxl_dax_region_alloc(struct cxl_region *cxlr)
 	return cxlr_dax;
 }
 
-static void cxlr_pmem_unregister(void *_cxlr_pmem)
-{
-	struct cxl_pmem_region *cxlr_pmem = _cxlr_pmem;
-	struct cxl_region *cxlr = cxlr_pmem->cxlr;
-	struct cxl_nvdimm_bridge *cxl_nvb = cxlr->cxl_nvb;
-
-	/*
-	 * Either the bridge is in ->remove() context under the device_lock(),
-	 * or cxlr_release_nvdimm() is cancelling the bridge's release action
-	 * for @cxlr_pmem and doing it itself (while manually holding the bridge
-	 * lock).
-	 */
-	device_lock_assert(&cxl_nvb->dev);
-	cxlr->cxlr_pmem = NULL;
-	cxlr_pmem->cxlr = NULL;
-	device_unregister(&cxlr_pmem->dev);
-}
-
-static void cxlr_release_nvdimm(void *_cxlr)
-{
-	struct cxl_region *cxlr = _cxlr;
-	struct cxl_nvdimm_bridge *cxl_nvb = cxlr->cxl_nvb;
-
-	scoped_guard(device, &cxl_nvb->dev) {
-		if (cxlr->cxlr_pmem)
-			devm_release_action(&cxl_nvb->dev, cxlr_pmem_unregister,
-					    cxlr->cxlr_pmem);
-	}
-	cxlr->cxl_nvb = NULL;
-	put_device(&cxl_nvb->dev);
-}
-
-/**
- * devm_cxl_add_pmem_region() - add a cxl_region-to-nd_region bridge
- * @cxlr: parent CXL region for this pmem region bridge device
- *
- * Return: 0 on success negative error code on failure.
- */
-static int devm_cxl_add_pmem_region(struct cxl_region *cxlr)
-{
-	struct cxl_pmem_region *cxlr_pmem;
-	struct cxl_nvdimm_bridge *cxl_nvb;
-	struct device *dev;
-	int rc;
-
-	rc = cxl_pmem_region_alloc(cxlr);
-	if (rc)
-		return rc;
-	cxlr_pmem = cxlr->cxlr_pmem;
-	cxl_nvb = cxlr->cxl_nvb;
-
-	dev = &cxlr_pmem->dev;
-	rc = dev_set_name(dev, "pmem_region%d", cxlr->id);
-	if (rc)
-		goto err;
-
-	rc = device_add(dev);
-	if (rc)
-		goto err;
-
-	dev_dbg(&cxlr->dev, "%s: register %s\n", dev_name(dev->parent),
-		dev_name(dev));
-
-	scoped_guard(device, &cxl_nvb->dev) {
-		if (cxl_nvb->dev.driver)
-			rc = devm_add_action_or_reset(&cxl_nvb->dev,
-						      cxlr_pmem_unregister,
-						      cxlr_pmem);
-		else
-			rc = -ENXIO;
-	}
-
-	if (rc)
-		goto err_bridge;
-
-	/* @cxlr carries a reference on @cxl_nvb until cxlr_release_nvdimm */
-	return devm_add_action_or_reset(&cxlr->dev, cxlr_release_nvdimm, cxlr);
-
-err:
-	put_device(dev);
-err_bridge:
-	put_device(&cxl_nvb->dev);
-	cxlr->cxl_nvb = NULL;
-	return rc;
-}
-
 static void cxlr_dax_unregister(void *_cxlr_dax)
 {
 	struct cxl_dax_region *cxlr_dax = _cxlr_dax;
-- 
2.52.0

Re: [PATCH v2 2/3] cxl/core/region: move pmem region driver logic into pmem_region

[Dave Jiang]

On 1/13/26 1:21 PM, Gregory Price wrote:
> Move the pmem region driver logic from region.c into pmem_region.c.
> 
> Signed-off-by: Gregory Price <gourry@gourry.net>

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

> ---
>  drivers/cxl/core/Makefile      |   1 +
>  drivers/cxl/core/core.h        |   1 +
>  drivers/cxl/core/pmem_region.c | 191 +++++++++++++++++++++++++++++++++
>  drivers/cxl/core/region.c      | 184 -------------------------------
>  4 files changed, 193 insertions(+), 184 deletions(-)
>  create mode 100644 drivers/cxl/core/pmem_region.c
> 
> diff --git a/drivers/cxl/core/Makefile b/drivers/cxl/core/Makefile
> index 5ad8fef210b5..23269c81fd44 100644
> --- a/drivers/cxl/core/Makefile
> +++ b/drivers/cxl/core/Makefile
> @@ -17,6 +17,7 @@ cxl_core-y += cdat.o
>  cxl_core-y += ras.o
>  cxl_core-$(CONFIG_TRACING) += trace.o
>  cxl_core-$(CONFIG_CXL_REGION) += region.o
> +cxl_core-$(CONFIG_CXL_REGION) += pmem_region.o
>  cxl_core-$(CONFIG_CXL_MCE) += mce.o
>  cxl_core-$(CONFIG_CXL_FEATURES) += features.o
>  cxl_core-$(CONFIG_CXL_EDAC_MEM_FEATURES) += edac.o
> diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
> index 1fb66132b777..cd589e81f55e 100644
> --- a/drivers/cxl/core/core.h
> +++ b/drivers/cxl/core/core.h
> @@ -42,6 +42,7 @@ int cxl_get_poison_by_endpoint(struct cxl_port *port);
>  struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa);
>  u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
>  		   u64 dpa);
> +int devm_cxl_add_pmem_region(struct cxl_region *cxlr);
>  
>  #else
>  static inline u64 cxl_dpa_to_hpa(struct cxl_region *cxlr,
> diff --git a/drivers/cxl/core/pmem_region.c b/drivers/cxl/core/pmem_region.c
> new file mode 100644
> index 000000000000..81b66e548bb5
> --- /dev/null
> +++ b/drivers/cxl/core/pmem_region.c
> @@ -0,0 +1,191 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright(c) 2022 Intel Corporation. All rights reserved. */
> +#include <linux/device.h>
> +#include <linux/slab.h>
> +#include <cxlmem.h>
> +#include <cxl.h>
> +#include "core.h"
> +
> +static void cxl_pmem_region_release(struct device *dev)
> +{
> +	struct cxl_pmem_region *cxlr_pmem = to_cxl_pmem_region(dev);
> +	int i;
> +
> +	for (i = 0; i < cxlr_pmem->nr_mappings; i++) {
> +		struct cxl_memdev *cxlmd = cxlr_pmem->mapping[i].cxlmd;
> +
> +		put_device(&cxlmd->dev);
> +	}
> +
> +	kfree(cxlr_pmem);
> +}
> +
> +static const struct attribute_group *cxl_pmem_region_attribute_groups[] = {
> +	&cxl_base_attribute_group,
> +	NULL,
> +};
> +
> +const struct device_type cxl_pmem_region_type = {
> +	.name = "cxl_pmem_region",
> +	.release = cxl_pmem_region_release,
> +	.groups = cxl_pmem_region_attribute_groups,
> +};
> +bool is_cxl_pmem_region(struct device *dev)
> +{
> +	return dev->type == &cxl_pmem_region_type;
> +}
> +EXPORT_SYMBOL_NS_GPL(is_cxl_pmem_region, "CXL");
> +
> +struct cxl_pmem_region *to_cxl_pmem_region(struct device *dev)
> +{
> +	if (dev_WARN_ONCE(dev, !is_cxl_pmem_region(dev),
> +				"not a cxl_pmem_region device\n"))
> +		return NULL;
> +	return container_of(dev, struct cxl_pmem_region, dev);
> +}
> +EXPORT_SYMBOL_NS_GPL(to_cxl_pmem_region, "CXL");
> +static struct lock_class_key cxl_pmem_region_key;
> +
> +static int cxl_pmem_region_alloc(struct cxl_region *cxlr)
> +{
> +	struct cxl_region_params *p = &cxlr->params;
> +	struct cxl_nvdimm_bridge *cxl_nvb;
> +	struct device *dev;
> +	int i;
> +
> +	guard(rwsem_read)(&cxl_rwsem.region);
> +	if (p->state != CXL_CONFIG_COMMIT)
> +		return -ENXIO;
> +
> +	struct cxl_pmem_region *cxlr_pmem __free(kfree) =
> +		kzalloc(struct_size(cxlr_pmem, mapping, p->nr_targets), GFP_KERNEL);
> +	if (!cxlr_pmem)
> +		return -ENOMEM;
> +
> +	cxlr_pmem->hpa_range.start = p->res->start;
> +	cxlr_pmem->hpa_range.end = p->res->end;
> +
> +	/* Snapshot the region configuration underneath the cxl_rwsem.region */
> +	cxlr_pmem->nr_mappings = p->nr_targets;
> +	for (i = 0; i < p->nr_targets; i++) {
> +		struct cxl_endpoint_decoder *cxled = p->targets[i];
> +		struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
> +		struct cxl_pmem_region_mapping *m = &cxlr_pmem->mapping[i];
> +
> +		/*
> +		 * Regions never span CXL root devices, so by definition the
> +		 * bridge for one device is the same for all.
> +		 */
> +		if (i == 0) {
> +			cxl_nvb = cxl_find_nvdimm_bridge(cxlmd->endpoint);
> +			if (!cxl_nvb)
> +				return -ENODEV;
> +			cxlr->cxl_nvb = cxl_nvb;
> +		}
> +		m->cxlmd = cxlmd;
> +		get_device(&cxlmd->dev);
> +		m->start = cxled->dpa_res->start;
> +		m->size = resource_size(cxled->dpa_res);
> +		m->position = i;
> +	}
> +
> +	dev = &cxlr_pmem->dev;
> +	device_initialize(dev);
> +	lockdep_set_class(&dev->mutex, &cxl_pmem_region_key);
> +	device_set_pm_not_required(dev);
> +	dev->parent = &cxlr->dev;
> +	dev->bus = &cxl_bus_type;
> +	dev->type = &cxl_pmem_region_type;
> +	cxlr_pmem->cxlr = cxlr;
> +	cxlr->cxlr_pmem = no_free_ptr(cxlr_pmem);
> +
> +	return 0;
> +}
> +
> +static void cxlr_pmem_unregister(void *_cxlr_pmem)
> +{
> +	struct cxl_pmem_region *cxlr_pmem = _cxlr_pmem;
> +	struct cxl_region *cxlr = cxlr_pmem->cxlr;
> +	struct cxl_nvdimm_bridge *cxl_nvb = cxlr->cxl_nvb;
> +
> +	/*
> +	 * Either the bridge is in ->remove() context under the device_lock(),
> +	 * or cxlr_release_nvdimm() is cancelling the bridge's release action
> +	 * for @cxlr_pmem and doing it itself (while manually holding the bridge
> +	 * lock).
> +	 */
> +	device_lock_assert(&cxl_nvb->dev);
> +	cxlr->cxlr_pmem = NULL;
> +	cxlr_pmem->cxlr = NULL;
> +	device_unregister(&cxlr_pmem->dev);
> +}
> +
> +static void cxlr_release_nvdimm(void *_cxlr)
> +{
> +	struct cxl_region *cxlr = _cxlr;
> +	struct cxl_nvdimm_bridge *cxl_nvb = cxlr->cxl_nvb;
> +
> +	scoped_guard(device, &cxl_nvb->dev) {
> +		if (cxlr->cxlr_pmem)
> +			devm_release_action(&cxl_nvb->dev, cxlr_pmem_unregister,
> +					cxlr->cxlr_pmem);
> +	}
> +	cxlr->cxl_nvb = NULL;
> +	put_device(&cxl_nvb->dev);
> +}
> +
> +/**
> + * devm_cxl_add_pmem_region() - add a cxl_region-to-nd_region bridge
> + * @cxlr: parent CXL region for this pmem region bridge device
> + *
> + * Return: 0 on success negative error code on failure.
> + */
> +int devm_cxl_add_pmem_region(struct cxl_region *cxlr)
> +{
> +	struct cxl_pmem_region *cxlr_pmem;
> +	struct cxl_nvdimm_bridge *cxl_nvb;
> +	struct device *dev;
> +	int rc;
> +
> +	rc = cxl_pmem_region_alloc(cxlr);
> +	if (rc)
> +		return rc;
> +	cxlr_pmem = cxlr->cxlr_pmem;
> +	cxl_nvb = cxlr->cxl_nvb;
> +
> +	dev = &cxlr_pmem->dev;
> +	rc = dev_set_name(dev, "pmem_region%d", cxlr->id);
> +	if (rc)
> +		goto err;
> +
> +	rc = device_add(dev);
> +	if (rc)
> +		goto err;
> +
> +	dev_dbg(&cxlr->dev, "%s: register %s\n", dev_name(dev->parent),
> +			dev_name(dev));
> +
> +	scoped_guard(device, &cxl_nvb->dev) {
> +		if (cxl_nvb->dev.driver)
> +			rc = devm_add_action_or_reset(&cxl_nvb->dev,
> +					cxlr_pmem_unregister,
> +					cxlr_pmem);
> +		else
> +			rc = -ENXIO;
> +	}
> +
> +	if (rc)
> +		goto err_bridge;
> +
> +	/* @cxlr carries a reference on @cxl_nvb until cxlr_release_nvdimm */
> +	return devm_add_action_or_reset(&cxlr->dev, cxlr_release_nvdimm, cxlr);
> +
> +err:
> +	put_device(dev);
> +err_bridge:
> +	put_device(&cxl_nvb->dev);
> +	cxlr->cxl_nvb = NULL;
> +	return rc;
> +}
> +
> +
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index f8262d2169ea..a61805542b56 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -2808,46 +2808,6 @@ static ssize_t delete_region_store(struct device *dev,
>  }
>  DEVICE_ATTR_WO(delete_region);
>  
> -static void cxl_pmem_region_release(struct device *dev)
> -{
> -	struct cxl_pmem_region *cxlr_pmem = to_cxl_pmem_region(dev);
> -	int i;
> -
> -	for (i = 0; i < cxlr_pmem->nr_mappings; i++) {
> -		struct cxl_memdev *cxlmd = cxlr_pmem->mapping[i].cxlmd;
> -
> -		put_device(&cxlmd->dev);
> -	}
> -
> -	kfree(cxlr_pmem);
> -}
> -
> -static const struct attribute_group *cxl_pmem_region_attribute_groups[] = {
> -	&cxl_base_attribute_group,
> -	NULL,
> -};
> -
> -const struct device_type cxl_pmem_region_type = {
> -	.name = "cxl_pmem_region",
> -	.release = cxl_pmem_region_release,
> -	.groups = cxl_pmem_region_attribute_groups,
> -};
> -
> -bool is_cxl_pmem_region(struct device *dev)
> -{
> -	return dev->type == &cxl_pmem_region_type;
> -}
> -EXPORT_SYMBOL_NS_GPL(is_cxl_pmem_region, "CXL");
> -
> -struct cxl_pmem_region *to_cxl_pmem_region(struct device *dev)
> -{
> -	if (dev_WARN_ONCE(dev, !is_cxl_pmem_region(dev),
> -			  "not a cxl_pmem_region device\n"))
> -		return NULL;
> -	return container_of(dev, struct cxl_pmem_region, dev);
> -}
> -EXPORT_SYMBOL_NS_GPL(to_cxl_pmem_region, "CXL");
> -
>  struct cxl_poison_context {
>  	struct cxl_port *port;
>  	int part;
> @@ -3279,64 +3239,6 @@ static int region_offset_to_dpa_result(struct cxl_region *cxlr, u64 offset,
>  	return -ENXIO;
>  }
>  
> -static struct lock_class_key cxl_pmem_region_key;
> -
> -static int cxl_pmem_region_alloc(struct cxl_region *cxlr)
> -{
> -	struct cxl_region_params *p = &cxlr->params;
> -	struct cxl_nvdimm_bridge *cxl_nvb;
> -	struct device *dev;
> -	int i;
> -
> -	guard(rwsem_read)(&cxl_rwsem.region);
> -	if (p->state != CXL_CONFIG_COMMIT)
> -		return -ENXIO;
> -
> -	struct cxl_pmem_region *cxlr_pmem __free(kfree) =
> -		kzalloc(struct_size(cxlr_pmem, mapping, p->nr_targets), GFP_KERNEL);
> -	if (!cxlr_pmem)
> -		return -ENOMEM;
> -
> -	cxlr_pmem->hpa_range.start = p->res->start;
> -	cxlr_pmem->hpa_range.end = p->res->end;
> -
> -	/* Snapshot the region configuration underneath the cxl_rwsem.region */
> -	cxlr_pmem->nr_mappings = p->nr_targets;
> -	for (i = 0; i < p->nr_targets; i++) {
> -		struct cxl_endpoint_decoder *cxled = p->targets[i];
> -		struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
> -		struct cxl_pmem_region_mapping *m = &cxlr_pmem->mapping[i];
> -
> -		/*
> -		 * Regions never span CXL root devices, so by definition the
> -		 * bridge for one device is the same for all.
> -		 */
> -		if (i == 0) {
> -			cxl_nvb = cxl_find_nvdimm_bridge(cxlmd->endpoint);
> -			if (!cxl_nvb)
> -				return -ENODEV;
> -			cxlr->cxl_nvb = cxl_nvb;
> -		}
> -		m->cxlmd = cxlmd;
> -		get_device(&cxlmd->dev);
> -		m->start = cxled->dpa_res->start;
> -		m->size = resource_size(cxled->dpa_res);
> -		m->position = i;
> -	}
> -
> -	dev = &cxlr_pmem->dev;
> -	device_initialize(dev);
> -	lockdep_set_class(&dev->mutex, &cxl_pmem_region_key);
> -	device_set_pm_not_required(dev);
> -	dev->parent = &cxlr->dev;
> -	dev->bus = &cxl_bus_type;
> -	dev->type = &cxl_pmem_region_type;
> -	cxlr_pmem->cxlr = cxlr;
> -	cxlr->cxlr_pmem = no_free_ptr(cxlr_pmem);
> -
> -	return 0;
> -}
> -
>  static void cxl_dax_region_release(struct device *dev)
>  {
>  	struct cxl_dax_region *cxlr_dax = to_cxl_dax_region(dev);
> @@ -3400,92 +3302,6 @@ static struct cxl_dax_region *cxl_dax_region_alloc(struct cxl_region *cxlr)
>  	return cxlr_dax;
>  }
>  
> -static void cxlr_pmem_unregister(void *_cxlr_pmem)
> -{
> -	struct cxl_pmem_region *cxlr_pmem = _cxlr_pmem;
> -	struct cxl_region *cxlr = cxlr_pmem->cxlr;
> -	struct cxl_nvdimm_bridge *cxl_nvb = cxlr->cxl_nvb;
> -
> -	/*
> -	 * Either the bridge is in ->remove() context under the device_lock(),
> -	 * or cxlr_release_nvdimm() is cancelling the bridge's release action
> -	 * for @cxlr_pmem and doing it itself (while manually holding the bridge
> -	 * lock).
> -	 */
> -	device_lock_assert(&cxl_nvb->dev);
> -	cxlr->cxlr_pmem = NULL;
> -	cxlr_pmem->cxlr = NULL;
> -	device_unregister(&cxlr_pmem->dev);
> -}
> -
> -static void cxlr_release_nvdimm(void *_cxlr)
> -{
> -	struct cxl_region *cxlr = _cxlr;
> -	struct cxl_nvdimm_bridge *cxl_nvb = cxlr->cxl_nvb;
> -
> -	scoped_guard(device, &cxl_nvb->dev) {
> -		if (cxlr->cxlr_pmem)
> -			devm_release_action(&cxl_nvb->dev, cxlr_pmem_unregister,
> -					    cxlr->cxlr_pmem);
> -	}
> -	cxlr->cxl_nvb = NULL;
> -	put_device(&cxl_nvb->dev);
> -}
> -
> -/**
> - * devm_cxl_add_pmem_region() - add a cxl_region-to-nd_region bridge
> - * @cxlr: parent CXL region for this pmem region bridge device
> - *
> - * Return: 0 on success negative error code on failure.
> - */
> -static int devm_cxl_add_pmem_region(struct cxl_region *cxlr)
> -{
> -	struct cxl_pmem_region *cxlr_pmem;
> -	struct cxl_nvdimm_bridge *cxl_nvb;
> -	struct device *dev;
> -	int rc;
> -
> -	rc = cxl_pmem_region_alloc(cxlr);
> -	if (rc)
> -		return rc;
> -	cxlr_pmem = cxlr->cxlr_pmem;
> -	cxl_nvb = cxlr->cxl_nvb;
> -
> -	dev = &cxlr_pmem->dev;
> -	rc = dev_set_name(dev, "pmem_region%d", cxlr->id);
> -	if (rc)
> -		goto err;
> -
> -	rc = device_add(dev);
> -	if (rc)
> -		goto err;
> -
> -	dev_dbg(&cxlr->dev, "%s: register %s\n", dev_name(dev->parent),
> -		dev_name(dev));
> -
> -	scoped_guard(device, &cxl_nvb->dev) {
> -		if (cxl_nvb->dev.driver)
> -			rc = devm_add_action_or_reset(&cxl_nvb->dev,
> -						      cxlr_pmem_unregister,
> -						      cxlr_pmem);
> -		else
> -			rc = -ENXIO;
> -	}
> -
> -	if (rc)
> -		goto err_bridge;
> -
> -	/* @cxlr carries a reference on @cxl_nvb until cxlr_release_nvdimm */
> -	return devm_add_action_or_reset(&cxlr->dev, cxlr_release_nvdimm, cxlr);
> -
> -err:
> -	put_device(dev);
> -err_bridge:
> -	put_device(&cxl_nvb->dev);
> -	cxlr->cxl_nvb = NULL;
> -	return rc;
> -}
> -
>  static void cxlr_dax_unregister(void *_cxlr_dax)
>  {
>  	struct cxl_dax_region *cxlr_dax = _cxlr_dax;

Re: [PATCH v2 2/3] cxl/core/region: move pmem region driver logic into pmem_region

[Fabio M. De Francesco]

On Tuesday, January 13, 2026 9:21:37 PM Central European Standard Time Gregory Price wrote:
> Move the pmem region driver logic from region.c into pmem_region.c.
> 
> Signed-off-by: Gregory Price <gourry@gourry.net>
> ---

Reviewed-by: Fabio M. De Francesco <fabio.m.de.francesco@linux.intel.com>

>  drivers/cxl/core/Makefile      |   1 +
>  drivers/cxl/core/core.h        |   1 +
>  drivers/cxl/core/pmem_region.c | 191 +++++++++++++++++++++++++++++++++
>  drivers/cxl/core/region.c      | 184 -------------------------------
>  4 files changed, 193 insertions(+), 184 deletions(-)
>  create mode 100644 drivers/cxl/core/pmem_region.c
> 
> diff --git a/drivers/cxl/core/Makefile b/drivers/cxl/core/Makefile
> index 5ad8fef210b5..23269c81fd44 100644
> --- a/drivers/cxl/core/Makefile
> +++ b/drivers/cxl/core/Makefile
> @@ -17,6 +17,7 @@ cxl_core-y += cdat.o
>  cxl_core-y += ras.o
>  cxl_core-$(CONFIG_TRACING) += trace.o
>  cxl_core-$(CONFIG_CXL_REGION) += region.o
> +cxl_core-$(CONFIG_CXL_REGION) += pmem_region.o
>  cxl_core-$(CONFIG_CXL_MCE) += mce.o
>  cxl_core-$(CONFIG_CXL_FEATURES) += features.o
>  cxl_core-$(CONFIG_CXL_EDAC_MEM_FEATURES) += edac.o
> diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
> index 1fb66132b777..cd589e81f55e 100644
> --- a/drivers/cxl/core/core.h
> +++ b/drivers/cxl/core/core.h
> @@ -42,6 +42,7 @@ int cxl_get_poison_by_endpoint(struct cxl_port *port);
>  struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa);
>  u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
>  		   u64 dpa);
> +int devm_cxl_add_pmem_region(struct cxl_region *cxlr);
>  
>  #else
>  static inline u64 cxl_dpa_to_hpa(struct cxl_region *cxlr,
> diff --git a/drivers/cxl/core/pmem_region.c b/drivers/cxl/core/pmem_region.c
> new file mode 100644
> index 000000000000..81b66e548bb5
> --- /dev/null
> +++ b/drivers/cxl/core/pmem_region.c
> @@ -0,0 +1,191 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright(c) 2022 Intel Corporation. All rights reserved. */
> +#include <linux/device.h>
> +#include <linux/slab.h>
> +#include <cxlmem.h>
> +#include <cxl.h>
> +#include "core.h"
> +
> +static void cxl_pmem_region_release(struct device *dev)
> +{
> +	struct cxl_pmem_region *cxlr_pmem = to_cxl_pmem_region(dev);
> +	int i;
> +
> +	for (i = 0; i < cxlr_pmem->nr_mappings; i++) {
> +		struct cxl_memdev *cxlmd = cxlr_pmem->mapping[i].cxlmd;
> +
> +		put_device(&cxlmd->dev);
> +	}
> +
> +	kfree(cxlr_pmem);
> +}
> +
> +static const struct attribute_group *cxl_pmem_region_attribute_groups[] = {
> +	&cxl_base_attribute_group,
> +	NULL,
> +};
> +
> +const struct device_type cxl_pmem_region_type = {
> +	.name = "cxl_pmem_region",
> +	.release = cxl_pmem_region_release,
> +	.groups = cxl_pmem_region_attribute_groups,
> +};
> +bool is_cxl_pmem_region(struct device *dev)
> +{
> +	return dev->type == &cxl_pmem_region_type;
> +}
> +EXPORT_SYMBOL_NS_GPL(is_cxl_pmem_region, "CXL");
> +
> +struct cxl_pmem_region *to_cxl_pmem_region(struct device *dev)
> +{
> +	if (dev_WARN_ONCE(dev, !is_cxl_pmem_region(dev),
> +				"not a cxl_pmem_region device\n"))
> +		return NULL;
> +	return container_of(dev, struct cxl_pmem_region, dev);
> +}
> +EXPORT_SYMBOL_NS_GPL(to_cxl_pmem_region, "CXL");
> +static struct lock_class_key cxl_pmem_region_key;
> +
> +static int cxl_pmem_region_alloc(struct cxl_region *cxlr)
> +{
> +	struct cxl_region_params *p = &cxlr->params;
> +	struct cxl_nvdimm_bridge *cxl_nvb;
> +	struct device *dev;
> +	int i;
> +
> +	guard(rwsem_read)(&cxl_rwsem.region);
> +	if (p->state != CXL_CONFIG_COMMIT)
> +		return -ENXIO;
> +
> +	struct cxl_pmem_region *cxlr_pmem __free(kfree) =
> +		kzalloc(struct_size(cxlr_pmem, mapping, p->nr_targets), GFP_KERNEL);
> +	if (!cxlr_pmem)
> +		return -ENOMEM;
> +
> +	cxlr_pmem->hpa_range.start = p->res->start;
> +	cxlr_pmem->hpa_range.end = p->res->end;
> +
> +	/* Snapshot the region configuration underneath the cxl_rwsem.region */
> +	cxlr_pmem->nr_mappings = p->nr_targets;
> +	for (i = 0; i < p->nr_targets; i++) {
> +		struct cxl_endpoint_decoder *cxled = p->targets[i];
> +		struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
> +		struct cxl_pmem_region_mapping *m = &cxlr_pmem->mapping[i];
> +
> +		/*
> +		 * Regions never span CXL root devices, so by definition the
> +		 * bridge for one device is the same for all.
> +		 */
> +		if (i == 0) {
> +			cxl_nvb = cxl_find_nvdimm_bridge(cxlmd->endpoint);
> +			if (!cxl_nvb)
> +				return -ENODEV;
> +			cxlr->cxl_nvb = cxl_nvb;
> +		}
> +		m->cxlmd = cxlmd;
> +		get_device(&cxlmd->dev);
> +		m->start = cxled->dpa_res->start;
> +		m->size = resource_size(cxled->dpa_res);
> +		m->position = i;
> +	}
> +
> +	dev = &cxlr_pmem->dev;
> +	device_initialize(dev);
> +	lockdep_set_class(&dev->mutex, &cxl_pmem_region_key);
> +	device_set_pm_not_required(dev);
> +	dev->parent = &cxlr->dev;
> +	dev->bus = &cxl_bus_type;
> +	dev->type = &cxl_pmem_region_type;
> +	cxlr_pmem->cxlr = cxlr;
> +	cxlr->cxlr_pmem = no_free_ptr(cxlr_pmem);
> +
> +	return 0;
> +}
> +
> +static void cxlr_pmem_unregister(void *_cxlr_pmem)
> +{
> +	struct cxl_pmem_region *cxlr_pmem = _cxlr_pmem;
> +	struct cxl_region *cxlr = cxlr_pmem->cxlr;
> +	struct cxl_nvdimm_bridge *cxl_nvb = cxlr->cxl_nvb;
> +
> +	/*
> +	 * Either the bridge is in ->remove() context under the device_lock(),
> +	 * or cxlr_release_nvdimm() is cancelling the bridge's release action
> +	 * for @cxlr_pmem and doing it itself (while manually holding the bridge
> +	 * lock).
> +	 */
> +	device_lock_assert(&cxl_nvb->dev);
> +	cxlr->cxlr_pmem = NULL;
> +	cxlr_pmem->cxlr = NULL;
> +	device_unregister(&cxlr_pmem->dev);
> +}
> +
> +static void cxlr_release_nvdimm(void *_cxlr)
> +{
> +	struct cxl_region *cxlr = _cxlr;
> +	struct cxl_nvdimm_bridge *cxl_nvb = cxlr->cxl_nvb;
> +
> +	scoped_guard(device, &cxl_nvb->dev) {
> +		if (cxlr->cxlr_pmem)
> +			devm_release_action(&cxl_nvb->dev, cxlr_pmem_unregister,
> +					cxlr->cxlr_pmem);
> +	}
> +	cxlr->cxl_nvb = NULL;
> +	put_device(&cxl_nvb->dev);
> +}
> +
> +/**
> + * devm_cxl_add_pmem_region() - add a cxl_region-to-nd_region bridge
> + * @cxlr: parent CXL region for this pmem region bridge device
> + *
> + * Return: 0 on success negative error code on failure.
> + */
> +int devm_cxl_add_pmem_region(struct cxl_region *cxlr)
> +{
> +	struct cxl_pmem_region *cxlr_pmem;
> +	struct cxl_nvdimm_bridge *cxl_nvb;
> +	struct device *dev;
> +	int rc;
> +
> +	rc = cxl_pmem_region_alloc(cxlr);
> +	if (rc)
> +		return rc;
> +	cxlr_pmem = cxlr->cxlr_pmem;
> +	cxl_nvb = cxlr->cxl_nvb;
> +
> +	dev = &cxlr_pmem->dev;
> +	rc = dev_set_name(dev, "pmem_region%d", cxlr->id);
> +	if (rc)
> +		goto err;
> +
> +	rc = device_add(dev);
> +	if (rc)
> +		goto err;
> +
> +	dev_dbg(&cxlr->dev, "%s: register %s\n", dev_name(dev->parent),
> +			dev_name(dev));
> +
> +	scoped_guard(device, &cxl_nvb->dev) {
> +		if (cxl_nvb->dev.driver)
> +			rc = devm_add_action_or_reset(&cxl_nvb->dev,
> +					cxlr_pmem_unregister,
> +					cxlr_pmem);
> +		else
> +			rc = -ENXIO;
> +	}
> +
> +	if (rc)
> +		goto err_bridge;
> +
> +	/* @cxlr carries a reference on @cxl_nvb until cxlr_release_nvdimm */
> +	return devm_add_action_or_reset(&cxlr->dev, cxlr_release_nvdimm, cxlr);
> +
> +err:
> +	put_device(dev);
> +err_bridge:
> +	put_device(&cxl_nvb->dev);
> +	cxlr->cxl_nvb = NULL;
> +	return rc;
> +}
> +
> +
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index f8262d2169ea..a61805542b56 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -2808,46 +2808,6 @@ static ssize_t delete_region_store(struct device *dev,
>  }
>  DEVICE_ATTR_WO(delete_region);
>  
> -static void cxl_pmem_region_release(struct device *dev)
> -{
> -	struct cxl_pmem_region *cxlr_pmem = to_cxl_pmem_region(dev);
> -	int i;
> -
> -	for (i = 0; i < cxlr_pmem->nr_mappings; i++) {
> -		struct cxl_memdev *cxlmd = cxlr_pmem->mapping[i].cxlmd;
> -
> -		put_device(&cxlmd->dev);
> -	}
> -
> -	kfree(cxlr_pmem);
> -}
> -
> -static const struct attribute_group *cxl_pmem_region_attribute_groups[] = {
> -	&cxl_base_attribute_group,
> -	NULL,
> -};
> -
> -const struct device_type cxl_pmem_region_type = {
> -	.name = "cxl_pmem_region",
> -	.release = cxl_pmem_region_release,
> -	.groups = cxl_pmem_region_attribute_groups,
> -};
> -
> -bool is_cxl_pmem_region(struct device *dev)
> -{
> -	return dev->type == &cxl_pmem_region_type;
> -}
> -EXPORT_SYMBOL_NS_GPL(is_cxl_pmem_region, "CXL");
> -
> -struct cxl_pmem_region *to_cxl_pmem_region(struct device *dev)
> -{
> -	if (dev_WARN_ONCE(dev, !is_cxl_pmem_region(dev),
> -			  "not a cxl_pmem_region device\n"))
> -		return NULL;
> -	return container_of(dev, struct cxl_pmem_region, dev);
> -}
> -EXPORT_SYMBOL_NS_GPL(to_cxl_pmem_region, "CXL");
> -
>  struct cxl_poison_context {
>  	struct cxl_port *port;
>  	int part;
> @@ -3279,64 +3239,6 @@ static int region_offset_to_dpa_result(struct cxl_region *cxlr, u64 offset,
>  	return -ENXIO;
>  }
>  
> -static struct lock_class_key cxl_pmem_region_key;
> -
> -static int cxl_pmem_region_alloc(struct cxl_region *cxlr)
> -{
> -	struct cxl_region_params *p = &cxlr->params;
> -	struct cxl_nvdimm_bridge *cxl_nvb;
> -	struct device *dev;
> -	int i;
> -
> -	guard(rwsem_read)(&cxl_rwsem.region);
> -	if (p->state != CXL_CONFIG_COMMIT)
> -		return -ENXIO;
> -
> -	struct cxl_pmem_region *cxlr_pmem __free(kfree) =
> -		kzalloc(struct_size(cxlr_pmem, mapping, p->nr_targets), GFP_KERNEL);
> -	if (!cxlr_pmem)
> -		return -ENOMEM;
> -
> -	cxlr_pmem->hpa_range.start = p->res->start;
> -	cxlr_pmem->hpa_range.end = p->res->end;
> -
> -	/* Snapshot the region configuration underneath the cxl_rwsem.region */
> -	cxlr_pmem->nr_mappings = p->nr_targets;
> -	for (i = 0; i < p->nr_targets; i++) {
> -		struct cxl_endpoint_decoder *cxled = p->targets[i];
> -		struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
> -		struct cxl_pmem_region_mapping *m = &cxlr_pmem->mapping[i];
> -
> -		/*
> -		 * Regions never span CXL root devices, so by definition the
> -		 * bridge for one device is the same for all.
> -		 */
> -		if (i == 0) {
> -			cxl_nvb = cxl_find_nvdimm_bridge(cxlmd->endpoint);
> -			if (!cxl_nvb)
> -				return -ENODEV;
> -			cxlr->cxl_nvb = cxl_nvb;
> -		}
> -		m->cxlmd = cxlmd;
> -		get_device(&cxlmd->dev);
> -		m->start = cxled->dpa_res->start;
> -		m->size = resource_size(cxled->dpa_res);
> -		m->position = i;
> -	}
> -
> -	dev = &cxlr_pmem->dev;
> -	device_initialize(dev);
> -	lockdep_set_class(&dev->mutex, &cxl_pmem_region_key);
> -	device_set_pm_not_required(dev);
> -	dev->parent = &cxlr->dev;
> -	dev->bus = &cxl_bus_type;
> -	dev->type = &cxl_pmem_region_type;
> -	cxlr_pmem->cxlr = cxlr;
> -	cxlr->cxlr_pmem = no_free_ptr(cxlr_pmem);
> -
> -	return 0;
> -}
> -
>  static void cxl_dax_region_release(struct device *dev)
>  {
>  	struct cxl_dax_region *cxlr_dax = to_cxl_dax_region(dev);
> @@ -3400,92 +3302,6 @@ static struct cxl_dax_region *cxl_dax_region_alloc(struct cxl_region *cxlr)
>  	return cxlr_dax;
>  }
>  
> -static void cxlr_pmem_unregister(void *_cxlr_pmem)
> -{
> -	struct cxl_pmem_region *cxlr_pmem = _cxlr_pmem;
> -	struct cxl_region *cxlr = cxlr_pmem->cxlr;
> -	struct cxl_nvdimm_bridge *cxl_nvb = cxlr->cxl_nvb;
> -
> -	/*
> -	 * Either the bridge is in ->remove() context under the device_lock(),
> -	 * or cxlr_release_nvdimm() is cancelling the bridge's release action
> -	 * for @cxlr_pmem and doing it itself (while manually holding the bridge
> -	 * lock).
> -	 */
> -	device_lock_assert(&cxl_nvb->dev);
> -	cxlr->cxlr_pmem = NULL;
> -	cxlr_pmem->cxlr = NULL;
> -	device_unregister(&cxlr_pmem->dev);
> -}
> -
> -static void cxlr_release_nvdimm(void *_cxlr)
> -{
> -	struct cxl_region *cxlr = _cxlr;
> -	struct cxl_nvdimm_bridge *cxl_nvb = cxlr->cxl_nvb;
> -
> -	scoped_guard(device, &cxl_nvb->dev) {
> -		if (cxlr->cxlr_pmem)
> -			devm_release_action(&cxl_nvb->dev, cxlr_pmem_unregister,
> -					    cxlr->cxlr_pmem);
> -	}
> -	cxlr->cxl_nvb = NULL;
> -	put_device(&cxl_nvb->dev);
> -}
> -
> -/**
> - * devm_cxl_add_pmem_region() - add a cxl_region-to-nd_region bridge
> - * @cxlr: parent CXL region for this pmem region bridge device
> - *
> - * Return: 0 on success negative error code on failure.
> - */
> -static int devm_cxl_add_pmem_region(struct cxl_region *cxlr)
> -{
> -	struct cxl_pmem_region *cxlr_pmem;
> -	struct cxl_nvdimm_bridge *cxl_nvb;
> -	struct device *dev;
> -	int rc;
> -
> -	rc = cxl_pmem_region_alloc(cxlr);
> -	if (rc)
> -		return rc;
> -	cxlr_pmem = cxlr->cxlr_pmem;
> -	cxl_nvb = cxlr->cxl_nvb;
> -
> -	dev = &cxlr_pmem->dev;
> -	rc = dev_set_name(dev, "pmem_region%d", cxlr->id);
> -	if (rc)
> -		goto err;
> -
> -	rc = device_add(dev);
> -	if (rc)
> -		goto err;
> -
> -	dev_dbg(&cxlr->dev, "%s: register %s\n", dev_name(dev->parent),
> -		dev_name(dev));
> -
> -	scoped_guard(device, &cxl_nvb->dev) {
> -		if (cxl_nvb->dev.driver)
> -			rc = devm_add_action_or_reset(&cxl_nvb->dev,
> -						      cxlr_pmem_unregister,
> -						      cxlr_pmem);
> -		else
> -			rc = -ENXIO;
> -	}
> -
> -	if (rc)
> -		goto err_bridge;
> -
> -	/* @cxlr carries a reference on @cxl_nvb until cxlr_release_nvdimm */
> -	return devm_add_action_or_reset(&cxlr->dev, cxlr_release_nvdimm, cxlr);
> -
> -err:
> -	put_device(dev);
> -err_bridge:
> -	put_device(&cxl_nvb->dev);
> -	cxlr->cxl_nvb = NULL;
> -	return rc;
> -}
> -
>  static void cxlr_dax_unregister(void *_cxlr_dax)
>  {
>  	struct cxl_dax_region *cxlr_dax = _cxlr_dax;
> -- 
> 2.52.0
> 
> 
> 

[PATCH v2 3/3] cxl/core/region: move dax region driver logic into dax_region

[Gregory Price]

Move the dax region driver logic from region.c into dax_region.c

Signed-off-by: Gregory Price <gourry@gourry.net>
---
 drivers/cxl/core/Makefile     |   1 +
 drivers/cxl/core/core.h       |   1 +
 drivers/cxl/core/dax_region.c | 106 ++++++++++++++++++++++++++++++++++
 drivers/cxl/core/region.c     |  99 -------------------------------
 4 files changed, 108 insertions(+), 99 deletions(-)
 create mode 100644 drivers/cxl/core/dax_region.c

diff --git a/drivers/cxl/core/Makefile b/drivers/cxl/core/Makefile
index 23269c81fd44..36f284d7c500 100644
--- a/drivers/cxl/core/Makefile
+++ b/drivers/cxl/core/Makefile
@@ -17,6 +17,7 @@ cxl_core-y += cdat.o
 cxl_core-y += ras.o
 cxl_core-$(CONFIG_TRACING) += trace.o
 cxl_core-$(CONFIG_CXL_REGION) += region.o
+cxl_core-$(CONFIG_CXL_REGION) += dax_region.o
 cxl_core-$(CONFIG_CXL_REGION) += pmem_region.o
 cxl_core-$(CONFIG_CXL_MCE) += mce.o
 cxl_core-$(CONFIG_CXL_FEATURES) += features.o
diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
index cd589e81f55e..3ea850408814 100644
--- a/drivers/cxl/core/core.h
+++ b/drivers/cxl/core/core.h
@@ -42,6 +42,7 @@ int cxl_get_poison_by_endpoint(struct cxl_port *port);
 struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa);
 u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
 		   u64 dpa);
+int devm_cxl_add_dax_region(struct cxl_region *cxlr);
 int devm_cxl_add_pmem_region(struct cxl_region *cxlr);
 
 #else
diff --git a/drivers/cxl/core/dax_region.c b/drivers/cxl/core/dax_region.c
new file mode 100644
index 000000000000..d7c38447f816
--- /dev/null
+++ b/drivers/cxl/core/dax_region.c
@@ -0,0 +1,106 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright(c) 2022 Intel Corporation. All rights reserved. */
+#include <linux/device.h>
+#include <linux/slab.h>
+#include <cxlmem.h>
+#include <cxl.h>
+#include "core.h"
+
+static void cxl_dax_region_release(struct device *dev)
+{
+	struct cxl_dax_region *cxlr_dax = to_cxl_dax_region(dev);
+
+	kfree(cxlr_dax);
+}
+
+static const struct attribute_group *cxl_dax_region_attribute_groups[] = {
+	&cxl_base_attribute_group,
+	NULL,
+};
+
+const struct device_type cxl_dax_region_type = {
+	.name = "cxl_dax_region",
+	.release = cxl_dax_region_release,
+	.groups = cxl_dax_region_attribute_groups,
+};
+
+static bool is_cxl_dax_region(struct device *dev)
+{
+	return dev->type == &cxl_dax_region_type;
+}
+
+struct cxl_dax_region *to_cxl_dax_region(struct device *dev)
+{
+	if (dev_WARN_ONCE(dev, !is_cxl_dax_region(dev),
+			  "not a cxl_dax_region device\n"))
+		return NULL;
+	return container_of(dev, struct cxl_dax_region, dev);
+}
+EXPORT_SYMBOL_NS_GPL(to_cxl_dax_region, "CXL");
+
+static struct lock_class_key cxl_dax_region_key;
+
+static struct cxl_dax_region *cxl_dax_region_alloc(struct cxl_region *cxlr)
+{
+	struct cxl_region_params *p = &cxlr->params;
+	struct cxl_dax_region *cxlr_dax;
+	struct device *dev;
+
+	guard(rwsem_read)(&cxl_rwsem.region);
+	if (p->state != CXL_CONFIG_COMMIT)
+		return ERR_PTR(-ENXIO);
+
+	cxlr_dax = kzalloc(sizeof(*cxlr_dax), GFP_KERNEL);
+	if (!cxlr_dax)
+		return ERR_PTR(-ENOMEM);
+
+	cxlr_dax->hpa_range.start = p->res->start;
+	cxlr_dax->hpa_range.end = p->res->end;
+
+	dev = &cxlr_dax->dev;
+	cxlr_dax->cxlr = cxlr;
+	device_initialize(dev);
+	lockdep_set_class(&dev->mutex, &cxl_dax_region_key);
+	device_set_pm_not_required(dev);
+	dev->parent = &cxlr->dev;
+	dev->bus = &cxl_bus_type;
+	dev->type = &cxl_dax_region_type;
+
+	return cxlr_dax;
+}
+
+static void cxlr_dax_unregister(void *_cxlr_dax)
+{
+	struct cxl_dax_region *cxlr_dax = _cxlr_dax;
+
+	device_unregister(&cxlr_dax->dev);
+}
+
+int devm_cxl_add_dax_region(struct cxl_region *cxlr)
+{
+	struct cxl_dax_region *cxlr_dax;
+	struct device *dev;
+	int rc;
+
+	cxlr_dax = cxl_dax_region_alloc(cxlr);
+	if (IS_ERR(cxlr_dax))
+		return PTR_ERR(cxlr_dax);
+
+	dev = &cxlr_dax->dev;
+	rc = dev_set_name(dev, "dax_region%d", cxlr->id);
+	if (rc)
+		goto err;
+
+	rc = device_add(dev);
+	if (rc)
+		goto err;
+
+	dev_dbg(&cxlr->dev, "%s: register %s\n", dev_name(dev->parent),
+		dev_name(dev));
+
+	return devm_add_action_or_reset(&cxlr->dev, cxlr_dax_unregister,
+					cxlr_dax);
+err:
+	put_device(dev);
+	return rc;
+}
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index a61805542b56..1ca5f5b02b22 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -3239,105 +3239,6 @@ static int region_offset_to_dpa_result(struct cxl_region *cxlr, u64 offset,
 	return -ENXIO;
 }
 
-static void cxl_dax_region_release(struct device *dev)
-{
-	struct cxl_dax_region *cxlr_dax = to_cxl_dax_region(dev);
-
-	kfree(cxlr_dax);
-}
-
-static const struct attribute_group *cxl_dax_region_attribute_groups[] = {
-	&cxl_base_attribute_group,
-	NULL,
-};
-
-const struct device_type cxl_dax_region_type = {
-	.name = "cxl_dax_region",
-	.release = cxl_dax_region_release,
-	.groups = cxl_dax_region_attribute_groups,
-};
-
-static bool is_cxl_dax_region(struct device *dev)
-{
-	return dev->type == &cxl_dax_region_type;
-}
-
-struct cxl_dax_region *to_cxl_dax_region(struct device *dev)
-{
-	if (dev_WARN_ONCE(dev, !is_cxl_dax_region(dev),
-			  "not a cxl_dax_region device\n"))
-		return NULL;
-	return container_of(dev, struct cxl_dax_region, dev);
-}
-EXPORT_SYMBOL_NS_GPL(to_cxl_dax_region, "CXL");
-
-static struct lock_class_key cxl_dax_region_key;
-
-static struct cxl_dax_region *cxl_dax_region_alloc(struct cxl_region *cxlr)
-{
-	struct cxl_region_params *p = &cxlr->params;
-	struct cxl_dax_region *cxlr_dax;
-	struct device *dev;
-
-	guard(rwsem_read)(&cxl_rwsem.region);
-	if (p->state != CXL_CONFIG_COMMIT)
-		return ERR_PTR(-ENXIO);
-
-	cxlr_dax = kzalloc(sizeof(*cxlr_dax), GFP_KERNEL);
-	if (!cxlr_dax)
-		return ERR_PTR(-ENOMEM);
-
-	cxlr_dax->hpa_range.start = p->res->start;
-	cxlr_dax->hpa_range.end = p->res->end;
-
-	dev = &cxlr_dax->dev;
-	cxlr_dax->cxlr = cxlr;
-	device_initialize(dev);
-	lockdep_set_class(&dev->mutex, &cxl_dax_region_key);
-	device_set_pm_not_required(dev);
-	dev->parent = &cxlr->dev;
-	dev->bus = &cxl_bus_type;
-	dev->type = &cxl_dax_region_type;
-
-	return cxlr_dax;
-}
-
-static void cxlr_dax_unregister(void *_cxlr_dax)
-{
-	struct cxl_dax_region *cxlr_dax = _cxlr_dax;
-
-	device_unregister(&cxlr_dax->dev);
-}
-
-static int devm_cxl_add_dax_region(struct cxl_region *cxlr)
-{
-	struct cxl_dax_region *cxlr_dax;
-	struct device *dev;
-	int rc;
-
-	cxlr_dax = cxl_dax_region_alloc(cxlr);
-	if (IS_ERR(cxlr_dax))
-		return PTR_ERR(cxlr_dax);
-
-	dev = &cxlr_dax->dev;
-	rc = dev_set_name(dev, "dax_region%d", cxlr->id);
-	if (rc)
-		goto err;
-
-	rc = device_add(dev);
-	if (rc)
-		goto err;
-
-	dev_dbg(&cxlr->dev, "%s: register %s\n", dev_name(dev->parent),
-		dev_name(dev));
-
-	return devm_add_action_or_reset(&cxlr->dev, cxlr_dax_unregister,
-					cxlr_dax);
-err:
-	put_device(dev);
-	return rc;
-}
-
 static int match_decoder_by_range(struct device *dev, const void *data)
 {
 	const struct range *r1, *r2 = data;
-- 
2.52.0

Re: [PATCH v2 3/3] cxl/core/region: move dax region driver logic into dax_region

[Dave Jiang]

On 1/13/26 1:21 PM, Gregory Price wrote:
> Move the dax region driver logic from region.c into dax_region.c
> 
> Signed-off-by: Gregory Price <gourry@gourry.net>

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


> ---
>  drivers/cxl/core/Makefile     |   1 +
>  drivers/cxl/core/core.h       |   1 +
>  drivers/cxl/core/dax_region.c | 106 ++++++++++++++++++++++++++++++++++
>  drivers/cxl/core/region.c     |  99 -------------------------------
>  4 files changed, 108 insertions(+), 99 deletions(-)
>  create mode 100644 drivers/cxl/core/dax_region.c
> 
> diff --git a/drivers/cxl/core/Makefile b/drivers/cxl/core/Makefile
> index 23269c81fd44..36f284d7c500 100644
> --- a/drivers/cxl/core/Makefile
> +++ b/drivers/cxl/core/Makefile
> @@ -17,6 +17,7 @@ cxl_core-y += cdat.o
>  cxl_core-y += ras.o
>  cxl_core-$(CONFIG_TRACING) += trace.o
>  cxl_core-$(CONFIG_CXL_REGION) += region.o
> +cxl_core-$(CONFIG_CXL_REGION) += dax_region.o
>  cxl_core-$(CONFIG_CXL_REGION) += pmem_region.o
>  cxl_core-$(CONFIG_CXL_MCE) += mce.o
>  cxl_core-$(CONFIG_CXL_FEATURES) += features.o
> diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
> index cd589e81f55e..3ea850408814 100644
> --- a/drivers/cxl/core/core.h
> +++ b/drivers/cxl/core/core.h
> @@ -42,6 +42,7 @@ int cxl_get_poison_by_endpoint(struct cxl_port *port);
>  struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa);
>  u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
>  		   u64 dpa);
> +int devm_cxl_add_dax_region(struct cxl_region *cxlr);
>  int devm_cxl_add_pmem_region(struct cxl_region *cxlr);
>  
>  #else
> diff --git a/drivers/cxl/core/dax_region.c b/drivers/cxl/core/dax_region.c
> new file mode 100644
> index 000000000000..d7c38447f816
> --- /dev/null
> +++ b/drivers/cxl/core/dax_region.c
> @@ -0,0 +1,106 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright(c) 2022 Intel Corporation. All rights reserved. */
> +#include <linux/device.h>
> +#include <linux/slab.h>
> +#include <cxlmem.h>
> +#include <cxl.h>
> +#include "core.h"
> +
> +static void cxl_dax_region_release(struct device *dev)
> +{
> +	struct cxl_dax_region *cxlr_dax = to_cxl_dax_region(dev);
> +
> +	kfree(cxlr_dax);
> +}
> +
> +static const struct attribute_group *cxl_dax_region_attribute_groups[] = {
> +	&cxl_base_attribute_group,
> +	NULL,
> +};
> +
> +const struct device_type cxl_dax_region_type = {
> +	.name = "cxl_dax_region",
> +	.release = cxl_dax_region_release,
> +	.groups = cxl_dax_region_attribute_groups,
> +};
> +
> +static bool is_cxl_dax_region(struct device *dev)
> +{
> +	return dev->type == &cxl_dax_region_type;
> +}
> +
> +struct cxl_dax_region *to_cxl_dax_region(struct device *dev)
> +{
> +	if (dev_WARN_ONCE(dev, !is_cxl_dax_region(dev),
> +			  "not a cxl_dax_region device\n"))
> +		return NULL;
> +	return container_of(dev, struct cxl_dax_region, dev);
> +}
> +EXPORT_SYMBOL_NS_GPL(to_cxl_dax_region, "CXL");
> +
> +static struct lock_class_key cxl_dax_region_key;
> +
> +static struct cxl_dax_region *cxl_dax_region_alloc(struct cxl_region *cxlr)
> +{
> +	struct cxl_region_params *p = &cxlr->params;
> +	struct cxl_dax_region *cxlr_dax;
> +	struct device *dev;
> +
> +	guard(rwsem_read)(&cxl_rwsem.region);
> +	if (p->state != CXL_CONFIG_COMMIT)
> +		return ERR_PTR(-ENXIO);
> +
> +	cxlr_dax = kzalloc(sizeof(*cxlr_dax), GFP_KERNEL);
> +	if (!cxlr_dax)
> +		return ERR_PTR(-ENOMEM);
> +
> +	cxlr_dax->hpa_range.start = p->res->start;
> +	cxlr_dax->hpa_range.end = p->res->end;
> +
> +	dev = &cxlr_dax->dev;
> +	cxlr_dax->cxlr = cxlr;
> +	device_initialize(dev);
> +	lockdep_set_class(&dev->mutex, &cxl_dax_region_key);
> +	device_set_pm_not_required(dev);
> +	dev->parent = &cxlr->dev;
> +	dev->bus = &cxl_bus_type;
> +	dev->type = &cxl_dax_region_type;
> +
> +	return cxlr_dax;
> +}
> +
> +static void cxlr_dax_unregister(void *_cxlr_dax)
> +{
> +	struct cxl_dax_region *cxlr_dax = _cxlr_dax;
> +
> +	device_unregister(&cxlr_dax->dev);
> +}
> +
> +int devm_cxl_add_dax_region(struct cxl_region *cxlr)
> +{
> +	struct cxl_dax_region *cxlr_dax;
> +	struct device *dev;
> +	int rc;
> +
> +	cxlr_dax = cxl_dax_region_alloc(cxlr);
> +	if (IS_ERR(cxlr_dax))
> +		return PTR_ERR(cxlr_dax);
> +
> +	dev = &cxlr_dax->dev;
> +	rc = dev_set_name(dev, "dax_region%d", cxlr->id);
> +	if (rc)
> +		goto err;
> +
> +	rc = device_add(dev);
> +	if (rc)
> +		goto err;
> +
> +	dev_dbg(&cxlr->dev, "%s: register %s\n", dev_name(dev->parent),
> +		dev_name(dev));
> +
> +	return devm_add_action_or_reset(&cxlr->dev, cxlr_dax_unregister,
> +					cxlr_dax);
> +err:
> +	put_device(dev);
> +	return rc;
> +}
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index a61805542b56..1ca5f5b02b22 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -3239,105 +3239,6 @@ static int region_offset_to_dpa_result(struct cxl_region *cxlr, u64 offset,
>  	return -ENXIO;
>  }
>  
> -static void cxl_dax_region_release(struct device *dev)
> -{
> -	struct cxl_dax_region *cxlr_dax = to_cxl_dax_region(dev);
> -
> -	kfree(cxlr_dax);
> -}
> -
> -static const struct attribute_group *cxl_dax_region_attribute_groups[] = {
> -	&cxl_base_attribute_group,
> -	NULL,
> -};
> -
> -const struct device_type cxl_dax_region_type = {
> -	.name = "cxl_dax_region",
> -	.release = cxl_dax_region_release,
> -	.groups = cxl_dax_region_attribute_groups,
> -};
> -
> -static bool is_cxl_dax_region(struct device *dev)
> -{
> -	return dev->type == &cxl_dax_region_type;
> -}
> -
> -struct cxl_dax_region *to_cxl_dax_region(struct device *dev)
> -{
> -	if (dev_WARN_ONCE(dev, !is_cxl_dax_region(dev),
> -			  "not a cxl_dax_region device\n"))
> -		return NULL;
> -	return container_of(dev, struct cxl_dax_region, dev);
> -}
> -EXPORT_SYMBOL_NS_GPL(to_cxl_dax_region, "CXL");
> -
> -static struct lock_class_key cxl_dax_region_key;
> -
> -static struct cxl_dax_region *cxl_dax_region_alloc(struct cxl_region *cxlr)
> -{
> -	struct cxl_region_params *p = &cxlr->params;
> -	struct cxl_dax_region *cxlr_dax;
> -	struct device *dev;
> -
> -	guard(rwsem_read)(&cxl_rwsem.region);
> -	if (p->state != CXL_CONFIG_COMMIT)
> -		return ERR_PTR(-ENXIO);
> -
> -	cxlr_dax = kzalloc(sizeof(*cxlr_dax), GFP_KERNEL);
> -	if (!cxlr_dax)
> -		return ERR_PTR(-ENOMEM);
> -
> -	cxlr_dax->hpa_range.start = p->res->start;
> -	cxlr_dax->hpa_range.end = p->res->end;
> -
> -	dev = &cxlr_dax->dev;
> -	cxlr_dax->cxlr = cxlr;
> -	device_initialize(dev);
> -	lockdep_set_class(&dev->mutex, &cxl_dax_region_key);
> -	device_set_pm_not_required(dev);
> -	dev->parent = &cxlr->dev;
> -	dev->bus = &cxl_bus_type;
> -	dev->type = &cxl_dax_region_type;
> -
> -	return cxlr_dax;
> -}
> -
> -static void cxlr_dax_unregister(void *_cxlr_dax)
> -{
> -	struct cxl_dax_region *cxlr_dax = _cxlr_dax;
> -
> -	device_unregister(&cxlr_dax->dev);
> -}
> -
> -static int devm_cxl_add_dax_region(struct cxl_region *cxlr)
> -{
> -	struct cxl_dax_region *cxlr_dax;
> -	struct device *dev;
> -	int rc;
> -
> -	cxlr_dax = cxl_dax_region_alloc(cxlr);
> -	if (IS_ERR(cxlr_dax))
> -		return PTR_ERR(cxlr_dax);
> -
> -	dev = &cxlr_dax->dev;
> -	rc = dev_set_name(dev, "dax_region%d", cxlr->id);
> -	if (rc)
> -		goto err;
> -
> -	rc = device_add(dev);
> -	if (rc)
> -		goto err;
> -
> -	dev_dbg(&cxlr->dev, "%s: register %s\n", dev_name(dev->parent),
> -		dev_name(dev));
> -
> -	return devm_add_action_or_reset(&cxlr->dev, cxlr_dax_unregister,
> -					cxlr_dax);
> -err:
> -	put_device(dev);
> -	return rc;
> -}
> -
>  static int match_decoder_by_range(struct device *dev, const void *data)
>  {
>  	const struct range *r1, *r2 = data;

Re: [PATCH v2 3/3] cxl/core/region: move dax region driver logic into dax_region

[Fabio M. De Francesco]

On Tuesday, January 13, 2026 9:21:38 PM Central European Standard Time Gregory Price wrote:
> Move the dax region driver logic from region.c into dax_region.c
> 
> Signed-off-by: Gregory Price <gourry@gourry.net>
> ---

Reviewed-by: Fabio M. De Francesco <fabio.m.de.francesco@linux.intel.com>

>  drivers/cxl/core/Makefile     |   1 +
>  drivers/cxl/core/core.h       |   1 +
>  drivers/cxl/core/dax_region.c | 106 ++++++++++++++++++++++++++++++++++
>  drivers/cxl/core/region.c     |  99 -------------------------------
>  4 files changed, 108 insertions(+), 99 deletions(-)
>  create mode 100644 drivers/cxl/core/dax_region.c
> 
> diff --git a/drivers/cxl/core/Makefile b/drivers/cxl/core/Makefile
> index 23269c81fd44..36f284d7c500 100644
> --- a/drivers/cxl/core/Makefile
> +++ b/drivers/cxl/core/Makefile
> @@ -17,6 +17,7 @@ cxl_core-y += cdat.o
>  cxl_core-y += ras.o
>  cxl_core-$(CONFIG_TRACING) += trace.o
>  cxl_core-$(CONFIG_CXL_REGION) += region.o
> +cxl_core-$(CONFIG_CXL_REGION) += dax_region.o
>  cxl_core-$(CONFIG_CXL_REGION) += pmem_region.o
>  cxl_core-$(CONFIG_CXL_MCE) += mce.o
>  cxl_core-$(CONFIG_CXL_FEATURES) += features.o
> diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
> index cd589e81f55e..3ea850408814 100644
> --- a/drivers/cxl/core/core.h
> +++ b/drivers/cxl/core/core.h
> @@ -42,6 +42,7 @@ int cxl_get_poison_by_endpoint(struct cxl_port *port);
>  struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa);
>  u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
>  		   u64 dpa);
> +int devm_cxl_add_dax_region(struct cxl_region *cxlr);
>  int devm_cxl_add_pmem_region(struct cxl_region *cxlr);
>  
>  #else
> diff --git a/drivers/cxl/core/dax_region.c b/drivers/cxl/core/dax_region.c
> new file mode 100644
> index 000000000000..d7c38447f816
> --- /dev/null
> +++ b/drivers/cxl/core/dax_region.c
> @@ -0,0 +1,106 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright(c) 2022 Intel Corporation. All rights reserved. */
> +#include <linux/device.h>
> +#include <linux/slab.h>
> +#include <cxlmem.h>
> +#include <cxl.h>
> +#include "core.h"
> +
> +static void cxl_dax_region_release(struct device *dev)
> +{
> +	struct cxl_dax_region *cxlr_dax = to_cxl_dax_region(dev);
> +
> +	kfree(cxlr_dax);
> +}
> +
> +static const struct attribute_group *cxl_dax_region_attribute_groups[] = {
> +	&cxl_base_attribute_group,
> +	NULL,
> +};
> +
> +const struct device_type cxl_dax_region_type = {
> +	.name = "cxl_dax_region",
> +	.release = cxl_dax_region_release,
> +	.groups = cxl_dax_region_attribute_groups,
> +};
> +
> +static bool is_cxl_dax_region(struct device *dev)
> +{
> +	return dev->type == &cxl_dax_region_type;
> +}
> +
> +struct cxl_dax_region *to_cxl_dax_region(struct device *dev)
> +{
> +	if (dev_WARN_ONCE(dev, !is_cxl_dax_region(dev),
> +			  "not a cxl_dax_region device\n"))
> +		return NULL;
> +	return container_of(dev, struct cxl_dax_region, dev);
> +}
> +EXPORT_SYMBOL_NS_GPL(to_cxl_dax_region, "CXL");
> +
> +static struct lock_class_key cxl_dax_region_key;
> +
> +static struct cxl_dax_region *cxl_dax_region_alloc(struct cxl_region *cxlr)
> +{
> +	struct cxl_region_params *p = &cxlr->params;
> +	struct cxl_dax_region *cxlr_dax;
> +	struct device *dev;
> +
> +	guard(rwsem_read)(&cxl_rwsem.region);
> +	if (p->state != CXL_CONFIG_COMMIT)
> +		return ERR_PTR(-ENXIO);
> +
> +	cxlr_dax = kzalloc(sizeof(*cxlr_dax), GFP_KERNEL);
> +	if (!cxlr_dax)
> +		return ERR_PTR(-ENOMEM);
> +
> +	cxlr_dax->hpa_range.start = p->res->start;
> +	cxlr_dax->hpa_range.end = p->res->end;
> +
> +	dev = &cxlr_dax->dev;
> +	cxlr_dax->cxlr = cxlr;
> +	device_initialize(dev);
> +	lockdep_set_class(&dev->mutex, &cxl_dax_region_key);
> +	device_set_pm_not_required(dev);
> +	dev->parent = &cxlr->dev;
> +	dev->bus = &cxl_bus_type;
> +	dev->type = &cxl_dax_region_type;
> +
> +	return cxlr_dax;
> +}
> +
> +static void cxlr_dax_unregister(void *_cxlr_dax)
> +{
> +	struct cxl_dax_region *cxlr_dax = _cxlr_dax;
> +
> +	device_unregister(&cxlr_dax->dev);
> +}
> +
> +int devm_cxl_add_dax_region(struct cxl_region *cxlr)
> +{
> +	struct cxl_dax_region *cxlr_dax;
> +	struct device *dev;
> +	int rc;
> +
> +	cxlr_dax = cxl_dax_region_alloc(cxlr);
> +	if (IS_ERR(cxlr_dax))
> +		return PTR_ERR(cxlr_dax);
> +
> +	dev = &cxlr_dax->dev;
> +	rc = dev_set_name(dev, "dax_region%d", cxlr->id);
> +	if (rc)
> +		goto err;
> +
> +	rc = device_add(dev);
> +	if (rc)
> +		goto err;
> +
> +	dev_dbg(&cxlr->dev, "%s: register %s\n", dev_name(dev->parent),
> +		dev_name(dev));
> +
> +	return devm_add_action_or_reset(&cxlr->dev, cxlr_dax_unregister,
> +					cxlr_dax);
> +err:
> +	put_device(dev);
> +	return rc;
> +}
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index a61805542b56..1ca5f5b02b22 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -3239,105 +3239,6 @@ static int region_offset_to_dpa_result(struct cxl_region *cxlr, u64 offset,
>  	return -ENXIO;
>  }
>  
> -static void cxl_dax_region_release(struct device *dev)
> -{
> -	struct cxl_dax_region *cxlr_dax = to_cxl_dax_region(dev);
> -
> -	kfree(cxlr_dax);
> -}
> -
> -static const struct attribute_group *cxl_dax_region_attribute_groups[] = {
> -	&cxl_base_attribute_group,
> -	NULL,
> -};
> -
> -const struct device_type cxl_dax_region_type = {
> -	.name = "cxl_dax_region",
> -	.release = cxl_dax_region_release,
> -	.groups = cxl_dax_region_attribute_groups,
> -};
> -
> -static bool is_cxl_dax_region(struct device *dev)
> -{
> -	return dev->type == &cxl_dax_region_type;
> -}
> -
> -struct cxl_dax_region *to_cxl_dax_region(struct device *dev)
> -{
> -	if (dev_WARN_ONCE(dev, !is_cxl_dax_region(dev),
> -			  "not a cxl_dax_region device\n"))
> -		return NULL;
> -	return container_of(dev, struct cxl_dax_region, dev);
> -}
> -EXPORT_SYMBOL_NS_GPL(to_cxl_dax_region, "CXL");
> -
> -static struct lock_class_key cxl_dax_region_key;
> -
> -static struct cxl_dax_region *cxl_dax_region_alloc(struct cxl_region *cxlr)
> -{
> -	struct cxl_region_params *p = &cxlr->params;
> -	struct cxl_dax_region *cxlr_dax;
> -	struct device *dev;
> -
> -	guard(rwsem_read)(&cxl_rwsem.region);
> -	if (p->state != CXL_CONFIG_COMMIT)
> -		return ERR_PTR(-ENXIO);
> -
> -	cxlr_dax = kzalloc(sizeof(*cxlr_dax), GFP_KERNEL);
> -	if (!cxlr_dax)
> -		return ERR_PTR(-ENOMEM);
> -
> -	cxlr_dax->hpa_range.start = p->res->start;
> -	cxlr_dax->hpa_range.end = p->res->end;
> -
> -	dev = &cxlr_dax->dev;
> -	cxlr_dax->cxlr = cxlr;
> -	device_initialize(dev);
> -	lockdep_set_class(&dev->mutex, &cxl_dax_region_key);
> -	device_set_pm_not_required(dev);
> -	dev->parent = &cxlr->dev;
> -	dev->bus = &cxl_bus_type;
> -	dev->type = &cxl_dax_region_type;
> -
> -	return cxlr_dax;
> -}
> -
> -static void cxlr_dax_unregister(void *_cxlr_dax)
> -{
> -	struct cxl_dax_region *cxlr_dax = _cxlr_dax;
> -
> -	device_unregister(&cxlr_dax->dev);
> -}
> -
> -static int devm_cxl_add_dax_region(struct cxl_region *cxlr)
> -{
> -	struct cxl_dax_region *cxlr_dax;
> -	struct device *dev;
> -	int rc;
> -
> -	cxlr_dax = cxl_dax_region_alloc(cxlr);
> -	if (IS_ERR(cxlr_dax))
> -		return PTR_ERR(cxlr_dax);
> -
> -	dev = &cxlr_dax->dev;
> -	rc = dev_set_name(dev, "dax_region%d", cxlr->id);
> -	if (rc)
> -		goto err;
> -
> -	rc = device_add(dev);
> -	if (rc)
> -		goto err;
> -
> -	dev_dbg(&cxlr->dev, "%s: register %s\n", dev_name(dev->parent),
> -		dev_name(dev));
> -
> -	return devm_add_action_or_reset(&cxlr->dev, cxlr_dax_unregister,
> -					cxlr_dax);
> -err:
> -	put_device(dev);
> -	return rc;
> -}
> -
>  static int match_decoder_by_range(struct device *dev, const void *data)
>  {
>  	const struct range *r1, *r2 = data;
> -- 
> 2.52.0
> 
> 
> 

Re: [PATCH v2 3/3] cxl/core/region: move dax region driver logic into dax_region

[Davidlohr Bueso]

On Tue, 13 Jan 2026, Gregory Price wrote:

>Move the dax region driver logic from region.c into dax_region.c
>
>Signed-off-by: Gregory Price <gourry@gourry.net>

Acked-by: Davidlohr Bueso <dave@stgolabs.net>

>---
> drivers/cxl/core/Makefile     |   1 +
> drivers/cxl/core/core.h       |   1 +
> drivers/cxl/core/dax_region.c | 106 ++++++++++++++++++++++++++++++++++
> drivers/cxl/core/region.c     |  99 -------------------------------
> 4 files changed, 108 insertions(+), 99 deletions(-)
> create mode 100644 drivers/cxl/core/dax_region.c
>
>diff --git a/drivers/cxl/core/Makefile b/drivers/cxl/core/Makefile
>index 23269c81fd44..36f284d7c500 100644
>--- a/drivers/cxl/core/Makefile
>+++ b/drivers/cxl/core/Makefile
>@@ -17,6 +17,7 @@ cxl_core-y += cdat.o
> cxl_core-y += ras.o
> cxl_core-$(CONFIG_TRACING) += trace.o
> cxl_core-$(CONFIG_CXL_REGION) += region.o
>+cxl_core-$(CONFIG_CXL_REGION) += dax_region.o
> cxl_core-$(CONFIG_CXL_REGION) += pmem_region.o
> cxl_core-$(CONFIG_CXL_MCE) += mce.o
> cxl_core-$(CONFIG_CXL_FEATURES) += features.o
>diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
>index cd589e81f55e..3ea850408814 100644
>--- a/drivers/cxl/core/core.h
>+++ b/drivers/cxl/core/core.h
>@@ -42,6 +42,7 @@ int cxl_get_poison_by_endpoint(struct cxl_port *port);
> struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa);
> u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
> 		   u64 dpa);
>+int devm_cxl_add_dax_region(struct cxl_region *cxlr);
> int devm_cxl_add_pmem_region(struct cxl_region *cxlr);
>
> #else
>diff --git a/drivers/cxl/core/dax_region.c b/drivers/cxl/core/dax_region.c
>new file mode 100644
>index 000000000000..d7c38447f816
>--- /dev/null
>+++ b/drivers/cxl/core/dax_region.c
>@@ -0,0 +1,106 @@
>+// SPDX-License-Identifier: GPL-2.0-only
>+/* Copyright(c) 2022 Intel Corporation. All rights reserved. */
>+#include <linux/device.h>
>+#include <linux/slab.h>
>+#include <cxlmem.h>
>+#include <cxl.h>
>+#include "core.h"
>+
>+static void cxl_dax_region_release(struct device *dev)
>+{
>+	struct cxl_dax_region *cxlr_dax = to_cxl_dax_region(dev);
>+
>+	kfree(cxlr_dax);
>+}
>+
>+static const struct attribute_group *cxl_dax_region_attribute_groups[] = {
>+	&cxl_base_attribute_group,
>+	NULL,
>+};
>+
>+const struct device_type cxl_dax_region_type = {
>+	.name = "cxl_dax_region",
>+	.release = cxl_dax_region_release,
>+	.groups = cxl_dax_region_attribute_groups,
>+};
>+
>+static bool is_cxl_dax_region(struct device *dev)
>+{
>+	return dev->type == &cxl_dax_region_type;
>+}
>+
>+struct cxl_dax_region *to_cxl_dax_region(struct device *dev)
>+{
>+	if (dev_WARN_ONCE(dev, !is_cxl_dax_region(dev),
>+			  "not a cxl_dax_region device\n"))
>+		return NULL;
>+	return container_of(dev, struct cxl_dax_region, dev);
>+}
>+EXPORT_SYMBOL_NS_GPL(to_cxl_dax_region, "CXL");
>+
>+static struct lock_class_key cxl_dax_region_key;
>+
>+static struct cxl_dax_region *cxl_dax_region_alloc(struct cxl_region *cxlr)
>+{
>+	struct cxl_region_params *p = &cxlr->params;
>+	struct cxl_dax_region *cxlr_dax;
>+	struct device *dev;
>+
>+	guard(rwsem_read)(&cxl_rwsem.region);
>+	if (p->state != CXL_CONFIG_COMMIT)
>+		return ERR_PTR(-ENXIO);
>+
>+	cxlr_dax = kzalloc(sizeof(*cxlr_dax), GFP_KERNEL);
>+	if (!cxlr_dax)
>+		return ERR_PTR(-ENOMEM);
>+
>+	cxlr_dax->hpa_range.start = p->res->start;
>+	cxlr_dax->hpa_range.end = p->res->end;
>+
>+	dev = &cxlr_dax->dev;
>+	cxlr_dax->cxlr = cxlr;
>+	device_initialize(dev);
>+	lockdep_set_class(&dev->mutex, &cxl_dax_region_key);
>+	device_set_pm_not_required(dev);
>+	dev->parent = &cxlr->dev;
>+	dev->bus = &cxl_bus_type;
>+	dev->type = &cxl_dax_region_type;
>+
>+	return cxlr_dax;
>+}
>+
>+static void cxlr_dax_unregister(void *_cxlr_dax)
>+{
>+	struct cxl_dax_region *cxlr_dax = _cxlr_dax;
>+
>+	device_unregister(&cxlr_dax->dev);
>+}
>+
>+int devm_cxl_add_dax_region(struct cxl_region *cxlr)
>+{
>+	struct cxl_dax_region *cxlr_dax;
>+	struct device *dev;
>+	int rc;
>+
>+	cxlr_dax = cxl_dax_region_alloc(cxlr);
>+	if (IS_ERR(cxlr_dax))
>+		return PTR_ERR(cxlr_dax);
>+
>+	dev = &cxlr_dax->dev;
>+	rc = dev_set_name(dev, "dax_region%d", cxlr->id);
>+	if (rc)
>+		goto err;
>+
>+	rc = device_add(dev);
>+	if (rc)
>+		goto err;
>+
>+	dev_dbg(&cxlr->dev, "%s: register %s\n", dev_name(dev->parent),
>+		dev_name(dev));
>+
>+	return devm_add_action_or_reset(&cxlr->dev, cxlr_dax_unregister,
>+					cxlr_dax);
>+err:
>+	put_device(dev);
>+	return rc;
>+}
>diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
>index a61805542b56..1ca5f5b02b22 100644
>--- a/drivers/cxl/core/region.c
>+++ b/drivers/cxl/core/region.c
>@@ -3239,105 +3239,6 @@ static int region_offset_to_dpa_result(struct cxl_region *cxlr, u64 offset,
> 	return -ENXIO;
> }
>
>-static void cxl_dax_region_release(struct device *dev)
>-{
>-	struct cxl_dax_region *cxlr_dax = to_cxl_dax_region(dev);
>-
>-	kfree(cxlr_dax);
>-}
>-
>-static const struct attribute_group *cxl_dax_region_attribute_groups[] = {
>-	&cxl_base_attribute_group,
>-	NULL,
>-};
>-
>-const struct device_type cxl_dax_region_type = {
>-	.name = "cxl_dax_region",
>-	.release = cxl_dax_region_release,
>-	.groups = cxl_dax_region_attribute_groups,
>-};
>-
>-static bool is_cxl_dax_region(struct device *dev)
>-{
>-	return dev->type == &cxl_dax_region_type;
>-}
>-
>-struct cxl_dax_region *to_cxl_dax_region(struct device *dev)
>-{
>-	if (dev_WARN_ONCE(dev, !is_cxl_dax_region(dev),
>-			  "not a cxl_dax_region device\n"))
>-		return NULL;
>-	return container_of(dev, struct cxl_dax_region, dev);
>-}
>-EXPORT_SYMBOL_NS_GPL(to_cxl_dax_region, "CXL");
>-
>-static struct lock_class_key cxl_dax_region_key;
>-
>-static struct cxl_dax_region *cxl_dax_region_alloc(struct cxl_region *cxlr)
>-{
>-	struct cxl_region_params *p = &cxlr->params;
>-	struct cxl_dax_region *cxlr_dax;
>-	struct device *dev;
>-
>-	guard(rwsem_read)(&cxl_rwsem.region);
>-	if (p->state != CXL_CONFIG_COMMIT)
>-		return ERR_PTR(-ENXIO);
>-
>-	cxlr_dax = kzalloc(sizeof(*cxlr_dax), GFP_KERNEL);
>-	if (!cxlr_dax)
>-		return ERR_PTR(-ENOMEM);
>-
>-	cxlr_dax->hpa_range.start = p->res->start;
>-	cxlr_dax->hpa_range.end = p->res->end;
>-
>-	dev = &cxlr_dax->dev;
>-	cxlr_dax->cxlr = cxlr;
>-	device_initialize(dev);
>-	lockdep_set_class(&dev->mutex, &cxl_dax_region_key);
>-	device_set_pm_not_required(dev);
>-	dev->parent = &cxlr->dev;
>-	dev->bus = &cxl_bus_type;
>-	dev->type = &cxl_dax_region_type;
>-
>-	return cxlr_dax;
>-}
>-
>-static void cxlr_dax_unregister(void *_cxlr_dax)
>-{
>-	struct cxl_dax_region *cxlr_dax = _cxlr_dax;
>-
>-	device_unregister(&cxlr_dax->dev);
>-}
>-
>-static int devm_cxl_add_dax_region(struct cxl_region *cxlr)
>-{
>-	struct cxl_dax_region *cxlr_dax;
>-	struct device *dev;
>-	int rc;
>-
>-	cxlr_dax = cxl_dax_region_alloc(cxlr);
>-	if (IS_ERR(cxlr_dax))
>-		return PTR_ERR(cxlr_dax);
>-
>-	dev = &cxlr_dax->dev;
>-	rc = dev_set_name(dev, "dax_region%d", cxlr->id);
>-	if (rc)
>-		goto err;
>-
>-	rc = device_add(dev);
>-	if (rc)
>-		goto err;
>-
>-	dev_dbg(&cxlr->dev, "%s: register %s\n", dev_name(dev->parent),
>-		dev_name(dev));
>-
>-	return devm_add_action_or_reset(&cxlr->dev, cxlr_dax_unregister,
>-					cxlr_dax);
>-err:
>-	put_device(dev);
>-	return rc;
>-}
>-
> static int match_decoder_by_range(struct device *dev, const void *data)
> {
> 	const struct range *r1, *r2 = data;
>-- 
>2.52.0
>

Re: [PATCH v2 1/3] drivers/cxl: introduce cxl_region_driver field for cxl_region

[Dave Jiang]

On 1/13/26 1:21 PM, Gregory Price wrote:
> The CXL driver presently has 3 modes of managing a cxl_region:
>   - no specific driver (bios-onlined SystemRAM)
>   - dax_region (all other RAM regions, for now)
>   - pmem_region (all PMEM regions)
> 
> Formalize these into specific "region drivers".
> 
> enum cxl_region_driver {
> 	CXL_REGION_DRIVER_NONE,
> 	CXL_REGION_DRIVER_DAX,
> 	CXL_REGION_DRIVER_PMEM
> };
> 
> $cat regionN/region_driver
> [none,dax,pmem]
> 
> The intent is to clarify how to to add additional drivers (sysram,
> dynamic_capacity, etc) in the future, and to allow switching the
> driver selection via a sysfs entry `regionN/region_driver`.
> 
> All RAM regions will be defaulted to CXL_CONTROL_DAX.
> 
> Auto-regions will either be static sysram (BIOS-onlined) and has no
> region controller associated with it - or if the SP bit was set a
> DAX device will be created.  This will be discovered at probe time.
> 
> Signed-off-by: Gregory Price <gourry@gourry.net>

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

> ---
>  drivers/cxl/core/region.c | 113 ++++++++++++++++++++++++++++++--------
>  drivers/cxl/cxl.h         |   8 +++
>  2 files changed, 98 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index ae899f68551f..f8262d2169ea 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -626,6 +626,57 @@ static ssize_t mode_show(struct device *dev, struct device_attribute *attr,
>  }
>  static DEVICE_ATTR_RO(mode);
>  
> +static ssize_t region_driver_show(struct device *dev,
> +				  struct device_attribute *attr,
> +				  char *buf)
> +{
> +	struct cxl_region *cxlr = to_cxl_region(dev);
> +	const char *desc;
> +
> +	switch (cxlr->driver) {
> +	case CXL_REGION_DRIVER_NONE:
> +		desc = "none";
> +		break;
> +	case CXL_REGION_DRIVER_DAX:
> +		desc = "dax";
> +		break;
> +	case CXL_REGION_DRIVER_PMEM:
> +		desc = "pmem";
> +		break;
> +	}
> +
> +	return sysfs_emit(buf, "%s\n", desc);
> +}
> +
> +static ssize_t region_driver_store(struct device *dev,
> +				   struct device_attribute *attr,
> +				   const char *buf, size_t len)
> +{
> +	struct cxl_region *cxlr = to_cxl_region(dev);
> +	struct cxl_region_params *p = &cxlr->params;
> +	int rc;
> +
> +	ACQUIRE(rwsem_write_kill, rwsem)(&cxl_rwsem.region);
> +	if ((rc = ACQUIRE_ERR(rwsem_write_kill, &rwsem)))
> +		return rc;
> +
> +	if (p->state >= CXL_CONFIG_COMMIT)
> +		return -EBUSY;
> +
> +	/* PMEM drivers cannot be changed */
> +	if (cxlr->mode == CXL_PARTMODE_PMEM)
> +		return -EBUSY;
> +
> +	/* NONE type is not a valid selection for manually probed regions */
> +	if (sysfs_streq(buf, "dax"))
> +		cxlr->driver = CXL_REGION_DRIVER_DAX;
> +	else
> +		return -EINVAL;
> +
> +	return len;
> +}
> +static DEVICE_ATTR_RW(region_driver);
> +
>  static int alloc_hpa(struct cxl_region *cxlr, resource_size_t size)
>  {
>  	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent);
> @@ -772,6 +823,7 @@ static struct attribute *cxl_region_attrs[] = {
>  	&dev_attr_size.attr,
>  	&dev_attr_mode.attr,
>  	&dev_attr_extended_linear_cache_size.attr,
> +	&dev_attr_region_driver.attr,
>  	NULL,
>  };
>  
> @@ -2599,6 +2651,16 @@ static struct cxl_region *devm_cxl_add_region(struct cxl_root_decoder *cxlrd,
>  	cxlr->mode = mode;
>  	cxlr->type = type;
>  
> +	/*
> +	 * PMEM regions only have 1 driver: pmem_region
> +	 * RAM regions default to DAX, but if the memory is already onlined by
> +	 * BIOS as 'System-RAM', the DAX driver will be dropped during probe.
> +	 */
> +	if (mode == CXL_PARTMODE_PMEM)
> +		cxlr->driver = CXL_REGION_DRIVER_PMEM;
> +	else
> +		cxlr->driver = CXL_REGION_DRIVER_DAX;
> +
>  	dev = &cxlr->dev;
>  	rc = dev_set_name(dev, "region%d", id);
>  	if (rc)
> @@ -3951,33 +4013,38 @@ static int cxl_region_probe(struct device *dev)
>  			return rc;
>  	}
>  
> -	switch (cxlr->mode) {
> -	case CXL_PARTMODE_PMEM:
> -		rc = devm_cxl_region_edac_register(cxlr);
> -		if (rc)
> -			dev_dbg(&cxlr->dev, "CXL EDAC registration for region_id=%d failed\n",
> -				cxlr->id);
> +	if (cxlr->mode > CXL_PARTMODE_PMEM) {
> +		dev_dbg(&cxlr->dev, "unsupported region mode: %d\n",
> +			cxlr->mode);
> +		return -ENXIO;
> +	}
>  
> -		return devm_cxl_add_pmem_region(cxlr);
> -	case CXL_PARTMODE_RAM:
> -		rc = devm_cxl_region_edac_register(cxlr);
> -		if (rc)
> -			dev_dbg(&cxlr->dev, "CXL EDAC registration for region_id=%d failed\n",
> -				cxlr->id);
> +	/*
> +	 * The region can not be managed by CXL if any portion of
> +	 * it is already online as 'System RAM'.
> +	 */
> +	if (walk_iomem_res_desc(IORES_DESC_NONE,
> +				IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY,
> +				p->res->start, p->res->end, cxlr,
> +					is_system_ram) > 0) {
> +		cxlr->driver = CXL_REGION_DRIVER_NONE;
> +		return 0;
> +	}
>  
> -		/*
> -		 * The region can not be manged by CXL if any portion of
> -		 * it is already online as 'System RAM'
> -		 */
> -		if (walk_iomem_res_desc(IORES_DESC_NONE,
> -					IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY,
> -					p->res->start, p->res->end, cxlr,
> -					is_system_ram) > 0)
> -			return 0;
> +	rc = devm_cxl_region_edac_register(cxlr);
> +	dev_dbg(&cxlr->dev, "CXL EDAC registration for region_id=%d %s\n",
> +			cxlr->id, rc ? "failed" : "succeeded");
> +
> +	switch (cxlr->driver) {
> +	case CXL_REGION_DRIVER_NONE:
> +		return 0;
> +	case CXL_REGION_DRIVER_DAX:
>  		return devm_cxl_add_dax_region(cxlr);
> +	case CXL_REGION_DRIVER_PMEM:
> +		return devm_cxl_add_pmem_region(cxlr);
>  	default:
> -		dev_dbg(&cxlr->dev, "unsupported region mode: %d\n",
> -			cxlr->mode);
> +		dev_dbg(&cxlr->dev, "unsupported region driver: %d\n",
> +			cxlr->driver);
>  		return -ENXIO;
>  	}
>  }
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index ba17fa86d249..e8256099de29 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -502,6 +502,13 @@ enum cxl_partition_mode {
>  	CXL_PARTMODE_PMEM,
>  };
>  
> +
> +enum cxl_region_driver {
> +	CXL_REGION_DRIVER_NONE,
> +	CXL_REGION_DRIVER_DAX,
> +	CXL_REGION_DRIVER_PMEM,
> +};
> +
>  /*
>   * Indicate whether this region has been assembled by autodetection or
>   * userspace assembly. Prevent endpoint decoders outside of automatic
> @@ -543,6 +550,7 @@ struct cxl_region {
>  	struct device dev;
>  	int id;
>  	enum cxl_partition_mode mode;
> +	enum cxl_region_driver driver;
>  	enum cxl_decoder_type type;
>  	struct cxl_nvdimm_bridge *cxl_nvb;
>  	struct cxl_pmem_region *cxlr_pmem;

Re: [PATCH v2 1/3] drivers/cxl: introduce cxl_region_driver field for cxl_region

[Alison Schofield]

On Tue, Jan 13, 2026 at 03:21:36PM -0500, Gregory Price wrote:
> The CXL driver presently has 3 modes of managing a cxl_region:
>   - no specific driver (bios-onlined SystemRAM)
>   - dax_region (all other RAM regions, for now)
>   - pmem_region (all PMEM regions)
> 
> Formalize these into specific "region drivers".
> 
> enum cxl_region_driver {
> 	CXL_REGION_DRIVER_NONE,
> 	CXL_REGION_DRIVER_DAX,
> 	CXL_REGION_DRIVER_PMEM
> };
> 
> $cat regionN/region_driver
> [none,dax,pmem]
> 
> The intent is to clarify how to to add additional drivers (sysram,
> dynamic_capacity, etc) in the future, and to allow switching the
> driver selection via a sysfs entry `regionN/region_driver`.

Needs description in Documentation/ABI/testing/sysfs-bus-cxl
I think that will help me understand the switching we expect
to support.


> All RAM regions will be defaulted to CXL_CONTROL_DAX.

CXL_CONTROL_DAX ?

> 
> Auto-regions will either be static sysram (BIOS-onlined) and has no
> region controller associated with it - or if the SP bit was set a
> DAX device will be created.  This will be discovered at probe time.
> 
> Signed-off-by: Gregory Price <gourry@gourry.net>

snip

> +static ssize_t region_driver_store(struct device *dev,
> +				   struct device_attribute *attr,
> +				   const char *buf, size_t len)
> +{
> +	struct cxl_region *cxlr = to_cxl_region(dev);
> +	struct cxl_region_params *p = &cxlr->params;
> +	int rc;
> +
> +	ACQUIRE(rwsem_write_kill, rwsem)(&cxl_rwsem.region);
> +	if ((rc = ACQUIRE_ERR(rwsem_write_kill, &rwsem)))
> +		return rc;
> +
> +	if (p->state >= CXL_CONFIG_COMMIT)
> +		return -EBUSY;
> +
> +	/* PMEM drivers cannot be changed */
> +	if (cxlr->mode == CXL_PARTMODE_PMEM)
> +		return -EBUSY;

why isn't above "if (cxlr->driver == CXL_REGION_DRIVER_PMEM)"

> +
> +	/* NONE type is not a valid selection for manually probed regions */
> +	if (sysfs_streq(buf, "dax"))
> +		cxlr->driver = CXL_REGION_DRIVER_DAX;
> +	else
> +		return -EINVAL;
> +
> +	return len;
> +}

Re: [PATCH v2 1/3] drivers/cxl: introduce cxl_region_driver field for cxl_region

[Gregory Price]

On Wed, Jan 14, 2026 at 10:15:28AM -0800, Alison Schofield wrote:
> On Tue, Jan 13, 2026 at 03:21:36PM -0500, Gregory Price wrote:
> > The CXL driver presently has 3 modes of managing a cxl_region:
> >   - no specific driver (bios-onlined SystemRAM)
> >   - dax_region (all other RAM regions, for now)
> >   - pmem_region (all PMEM regions)
> > 
> > Formalize these into specific "region drivers".
> > 
> > enum cxl_region_driver {
> > 	CXL_REGION_DRIVER_NONE,
> > 	CXL_REGION_DRIVER_DAX,
> > 	CXL_REGION_DRIVER_PMEM
> > };
> > 
> > $cat regionN/region_driver
> > [none,dax,pmem]
> > 
> > The intent is to clarify how to to add additional drivers (sysram,
> > dynamic_capacity, etc) in the future, and to allow switching the
> > driver selection via a sysfs entry `regionN/region_driver`.
> 
> Needs description in Documentation/ABI/testing/sysfs-bus-cxl
> I think that will help me understand the switching we expect
> to support.
>

Can do.

> > +	/* PMEM drivers cannot be changed */
> > +	if (cxlr->mode == CXL_PARTMODE_PMEM)
> > +		return -EBUSY;
> 
> why isn't above "if (cxlr->driver == CXL_REGION_DRIVER_PMEM)"
>

I wanted to future-proof against someone trying to do something silly
like changing PMEM to come up as REGION_DRIVER_NONE and then scootching
past the check here.  But I can change to

if (cxlr->mode == CXL_PARTMODE_PMEM ||
    cxlr->driver CXL_REGION_DRIVER_PMEM)

to make this even more explicit.

~Gregory

Re: [PATCH v2 1/3] drivers/cxl: introduce cxl_region_driver field for cxl_region

[Fabio M. De Francesco]

On Tuesday, January 13, 2026 9:21:36 PM Central European Standard Time Gregory Price wrote:
> The CXL driver presently has 3 modes of managing a cxl_region:
>   - no specific driver (bios-onlined SystemRAM)
>   - dax_region (all other RAM regions, for now)
>   - pmem_region (all PMEM regions)
> 
> Formalize these into specific "region drivers".
> 
> enum cxl_region_driver {
> 	CXL_REGION_DRIVER_NONE,
> 	CXL_REGION_DRIVER_DAX,
> 	CXL_REGION_DRIVER_PMEM
> };
> 
> $cat regionN/region_driver
> [none,dax,pmem]
> 
> The intent is to clarify how to to add additional drivers (sysram,
> dynamic_capacity, etc) in the future, and to allow switching the
> driver selection via a sysfs entry `regionN/region_driver`.
> 
> All RAM regions will be defaulted to CXL_CONTROL_DAX.
> 
> Auto-regions will either be static sysram (BIOS-onlined) and has no
> region controller associated with it - or if the SP bit was set a
> DAX device will be created.  This will be discovered at probe time.
> 
> Signed-off-by: Gregory Price <gourry@gourry.net>
> ---

Reviewed-by: Fabio M. De Francesco <fabio.m.de.francesco@linux.intel.com>

>  drivers/cxl/core/region.c | 113 ++++++++++++++++++++++++++++++--------
>  drivers/cxl/cxl.h         |   8 +++
>  2 files changed, 98 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index ae899f68551f..f8262d2169ea 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -626,6 +626,57 @@ static ssize_t mode_show(struct device *dev, struct device_attribute *attr,
>  }
>  static DEVICE_ATTR_RO(mode);
>  
> +static ssize_t region_driver_show(struct device *dev,
> +				  struct device_attribute *attr,
> +				  char *buf)
> +{
> +	struct cxl_region *cxlr = to_cxl_region(dev);
> +	const char *desc;
> +
> +	switch (cxlr->driver) {
> +	case CXL_REGION_DRIVER_NONE:
> +		desc = "none";
> +		break;
> +	case CXL_REGION_DRIVER_DAX:
> +		desc = "dax";
> +		break;
> +	case CXL_REGION_DRIVER_PMEM:
> +		desc = "pmem";
> +		break;
> +	}
> +
> +	return sysfs_emit(buf, "%s\n", desc);
> +}
> +
> +static ssize_t region_driver_store(struct device *dev,
> +				   struct device_attribute *attr,
> +				   const char *buf, size_t len)
> +{
> +	struct cxl_region *cxlr = to_cxl_region(dev);
> +	struct cxl_region_params *p = &cxlr->params;
> +	int rc;
> +
> +	ACQUIRE(rwsem_write_kill, rwsem)(&cxl_rwsem.region);
> +	if ((rc = ACQUIRE_ERR(rwsem_write_kill, &rwsem)))
> +		return rc;
> +
> +	if (p->state >= CXL_CONFIG_COMMIT)
> +		return -EBUSY;
> +
> +	/* PMEM drivers cannot be changed */
> +	if (cxlr->mode == CXL_PARTMODE_PMEM)
> +		return -EBUSY;
> +
> +	/* NONE type is not a valid selection for manually probed regions */
> +	if (sysfs_streq(buf, "dax"))
> +		cxlr->driver = CXL_REGION_DRIVER_DAX;
> +	else
> +		return -EINVAL;
> +
> +	return len;
> +}
> +static DEVICE_ATTR_RW(region_driver);
> +
>  static int alloc_hpa(struct cxl_region *cxlr, resource_size_t size)
>  {
>  	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent);
> @@ -772,6 +823,7 @@ static struct attribute *cxl_region_attrs[] = {
>  	&dev_attr_size.attr,
>  	&dev_attr_mode.attr,
>  	&dev_attr_extended_linear_cache_size.attr,
> +	&dev_attr_region_driver.attr,
>  	NULL,
>  };
>  
> @@ -2599,6 +2651,16 @@ static struct cxl_region *devm_cxl_add_region(struct cxl_root_decoder *cxlrd,
>  	cxlr->mode = mode;
>  	cxlr->type = type;
>  
> +	/*
> +	 * PMEM regions only have 1 driver: pmem_region
> +	 * RAM regions default to DAX, but if the memory is already onlined by
> +	 * BIOS as 'System-RAM', the DAX driver will be dropped during probe.
> +	 */
> +	if (mode == CXL_PARTMODE_PMEM)
> +		cxlr->driver = CXL_REGION_DRIVER_PMEM;
> +	else
> +		cxlr->driver = CXL_REGION_DRIVER_DAX;
> +
>  	dev = &cxlr->dev;
>  	rc = dev_set_name(dev, "region%d", id);
>  	if (rc)
> @@ -3951,33 +4013,38 @@ static int cxl_region_probe(struct device *dev)
>  			return rc;
>  	}
>  
> -	switch (cxlr->mode) {
> -	case CXL_PARTMODE_PMEM:
> -		rc = devm_cxl_region_edac_register(cxlr);
> -		if (rc)
> -			dev_dbg(&cxlr->dev, "CXL EDAC registration for region_id=%d failed\n",
> -				cxlr->id);
> +	if (cxlr->mode > CXL_PARTMODE_PMEM) {
> +		dev_dbg(&cxlr->dev, "unsupported region mode: %d\n",
> +			cxlr->mode);
> +		return -ENXIO;
> +	}
>  
> -		return devm_cxl_add_pmem_region(cxlr);
> -	case CXL_PARTMODE_RAM:
> -		rc = devm_cxl_region_edac_register(cxlr);
> -		if (rc)
> -			dev_dbg(&cxlr->dev, "CXL EDAC registration for region_id=%d failed\n",
> -				cxlr->id);
> +	/*
> +	 * The region can not be managed by CXL if any portion of
> +	 * it is already online as 'System RAM'.
> +	 */
> +	if (walk_iomem_res_desc(IORES_DESC_NONE,
> +				IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY,
> +				p->res->start, p->res->end, cxlr,
> +					is_system_ram) > 0) {
> +		cxlr->driver = CXL_REGION_DRIVER_NONE;
> +		return 0;
> +	}
>  
> -		/*
> -		 * The region can not be manged by CXL if any portion of
> -		 * it is already online as 'System RAM'
> -		 */
> -		if (walk_iomem_res_desc(IORES_DESC_NONE,
> -					IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY,
> -					p->res->start, p->res->end, cxlr,
> -					is_system_ram) > 0)
> -			return 0;
> +	rc = devm_cxl_region_edac_register(cxlr);
> +	dev_dbg(&cxlr->dev, "CXL EDAC registration for region_id=%d %s\n",
> +			cxlr->id, rc ? "failed" : "succeeded");
> +
> +	switch (cxlr->driver) {
> +	case CXL_REGION_DRIVER_NONE:
> +		return 0;
> +	case CXL_REGION_DRIVER_DAX:
>  		return devm_cxl_add_dax_region(cxlr);
> +	case CXL_REGION_DRIVER_PMEM:
> +		return devm_cxl_add_pmem_region(cxlr);
>  	default:
> -		dev_dbg(&cxlr->dev, "unsupported region mode: %d\n",
> -			cxlr->mode);
> +		dev_dbg(&cxlr->dev, "unsupported region driver: %d\n",
> +			cxlr->driver);
>  		return -ENXIO;
>  	}
>  }
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index ba17fa86d249..e8256099de29 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -502,6 +502,13 @@ enum cxl_partition_mode {
>  	CXL_PARTMODE_PMEM,
>  };
>  
> +
> +enum cxl_region_driver {
> +	CXL_REGION_DRIVER_NONE,
> +	CXL_REGION_DRIVER_DAX,
> +	CXL_REGION_DRIVER_PMEM,
> +};
> +
>  /*
>   * Indicate whether this region has been assembled by autodetection or
>   * userspace assembly. Prevent endpoint decoders outside of automatic
> @@ -543,6 +550,7 @@ struct cxl_region {
>  	struct device dev;
>  	int id;
>  	enum cxl_partition_mode mode;
> +	enum cxl_region_driver driver;
>  	enum cxl_decoder_type type;
>  	struct cxl_nvdimm_bridge *cxl_nvb;
>  	struct cxl_pmem_region *cxlr_pmem;
> -- 
> 2.52.0
> 
> 
> 

Re: [PATCH v2 1/3] drivers/cxl: introduce cxl_region_driver field for cxl_region

[Ira Weiny]

Gregory Price wrote:
> The CXL driver presently has 3 modes of managing a cxl_region:
>   - no specific driver (bios-onlined SystemRAM)
>   - dax_region (all other RAM regions, for now)
>   - pmem_region (all PMEM regions)
> 
> Formalize these into specific "region drivers".
> 
> enum cxl_region_driver {
> 	CXL_REGION_DRIVER_NONE,
> 	CXL_REGION_DRIVER_DAX,
> 	CXL_REGION_DRIVER_PMEM
> };

The problem I see with this series is that this is not actually telling
the user which driver is being used.  Only which device is being created.
Then it is assumed the proper driver attaches.

> 
> $cat regionN/region_driver
> [none,dax,pmem]

I feel like this will be more useful when CXL RAM regions can be driven by
different drivers.  I forget right now the rest of your other series.  But
I wonder if the actual driver in drivers/dax/cxl.c (the dax driver) should
be setting this field?

Also escaping my memory ATM, is why one can't relate the dax_region to the
cxl_region in user space already?

> 
> The intent is to clarify how to to add additional drivers (sysram,
> dynamic_capacity, etc) in the future, and to allow switching the
> driver selection via a sysfs entry `regionN/region_driver`.
> 
> All RAM regions will be defaulted to CXL_CONTROL_DAX.
> 
> Auto-regions will either be static sysram (BIOS-onlined) and has no
> region controller associated with it - or if the SP bit was set a
> DAX device will be created.  This will be discovered at probe time.

This seems like it adds information.  Since these BIOS controlled regions
kind of get 'lost'.

:-/

I'm not opposed to this idea but I'm worried that this adds to the already
very implicit nature of the CXL subsystem.

> 
> Signed-off-by: Gregory Price <gourry@gourry.net>
> ---
>  drivers/cxl/core/region.c | 113 ++++++++++++++++++++++++++++++--------
>  drivers/cxl/cxl.h         |   8 +++
>  2 files changed, 98 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index ae899f68551f..f8262d2169ea 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -626,6 +626,57 @@ static ssize_t mode_show(struct device *dev, struct device_attribute *attr,
>  }
>  static DEVICE_ATTR_RO(mode);
>  
> +static ssize_t region_driver_show(struct device *dev,
> +				  struct device_attribute *attr,
> +				  char *buf)
> +{
> +	struct cxl_region *cxlr = to_cxl_region(dev);
> +	const char *desc;
> +
> +	switch (cxlr->driver) {
> +	case CXL_REGION_DRIVER_NONE:
> +		desc = "none";
> +		break;
> +	case CXL_REGION_DRIVER_DAX:
> +		desc = "dax";
> +		break;
> +	case CXL_REGION_DRIVER_PMEM:
> +		desc = "pmem";
> +		break;
> +	}
> +
> +	return sysfs_emit(buf, "%s\n", desc);
> +}
> +
> +static ssize_t region_driver_store(struct device *dev,
> +				   struct device_attribute *attr,
> +				   const char *buf, size_t len)
> +{
> +	struct cxl_region *cxlr = to_cxl_region(dev);
> +	struct cxl_region_params *p = &cxlr->params;
> +	int rc;
> +
> +	ACQUIRE(rwsem_write_kill, rwsem)(&cxl_rwsem.region);
> +	if ((rc = ACQUIRE_ERR(rwsem_write_kill, &rwsem)))
> +		return rc;
> +
> +	if (p->state >= CXL_CONFIG_COMMIT)
> +		return -EBUSY;
> +
> +	/* PMEM drivers cannot be changed */
> +	if (cxlr->mode == CXL_PARTMODE_PMEM)
> +		return -EBUSY;
> +
> +	/* NONE type is not a valid selection for manually probed regions */
> +	if (sysfs_streq(buf, "dax"))
> +		cxlr->driver = CXL_REGION_DRIVER_DAX;

How does this work?  Doesn't this have to create a dax_region device for
the driver to attach to?  That is the equal to CXL_REGION_DRIVER_DAX being
set at region probe time.

Hindsight: It seems like this driver should have been called the CXL
partition driver.  As it is triggered by the partitions being found...  I
think.  I'm probably really confused right now.

Ira

> +	else
> +		return -EINVAL;
> +
> +	return len;
> +}
> +static DEVICE_ATTR_RW(region_driver);
> +
>  static int alloc_hpa(struct cxl_region *cxlr, resource_size_t size)
>  {
>  	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent);
> @@ -772,6 +823,7 @@ static struct attribute *cxl_region_attrs[] = {
>  	&dev_attr_size.attr,
>  	&dev_attr_mode.attr,
>  	&dev_attr_extended_linear_cache_size.attr,
> +	&dev_attr_region_driver.attr,
>  	NULL,
>  };
>  
> @@ -2599,6 +2651,16 @@ static struct cxl_region *devm_cxl_add_region(struct cxl_root_decoder *cxlrd,
>  	cxlr->mode = mode;
>  	cxlr->type = type;
>  
> +	/*
> +	 * PMEM regions only have 1 driver: pmem_region
> +	 * RAM regions default to DAX, but if the memory is already onlined by
> +	 * BIOS as 'System-RAM', the DAX driver will be dropped during probe.
> +	 */
> +	if (mode == CXL_PARTMODE_PMEM)
> +		cxlr->driver = CXL_REGION_DRIVER_PMEM;
> +	else
> +		cxlr->driver = CXL_REGION_DRIVER_DAX;
> +
>  	dev = &cxlr->dev;
>  	rc = dev_set_name(dev, "region%d", id);
>  	if (rc)
> @@ -3951,33 +4013,38 @@ static int cxl_region_probe(struct device *dev)
>  			return rc;
>  	}
>  
> -	switch (cxlr->mode) {
> -	case CXL_PARTMODE_PMEM:
> -		rc = devm_cxl_region_edac_register(cxlr);
> -		if (rc)
> -			dev_dbg(&cxlr->dev, "CXL EDAC registration for region_id=%d failed\n",
> -				cxlr->id);
> +	if (cxlr->mode > CXL_PARTMODE_PMEM) {
> +		dev_dbg(&cxlr->dev, "unsupported region mode: %d\n",
> +			cxlr->mode);
> +		return -ENXIO;
> +	}
>  
> -		return devm_cxl_add_pmem_region(cxlr);
> -	case CXL_PARTMODE_RAM:
> -		rc = devm_cxl_region_edac_register(cxlr);
> -		if (rc)
> -			dev_dbg(&cxlr->dev, "CXL EDAC registration for region_id=%d failed\n",
> -				cxlr->id);
> +	/*
> +	 * The region can not be managed by CXL if any portion of
> +	 * it is already online as 'System RAM'.
> +	 */
> +	if (walk_iomem_res_desc(IORES_DESC_NONE,
> +				IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY,
> +				p->res->start, p->res->end, cxlr,
> +					is_system_ram) > 0) {
> +		cxlr->driver = CXL_REGION_DRIVER_NONE;
> +		return 0;
> +	}
>  
> -		/*
> -		 * The region can not be manged by CXL if any portion of
> -		 * it is already online as 'System RAM'
> -		 */
> -		if (walk_iomem_res_desc(IORES_DESC_NONE,
> -					IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY,
> -					p->res->start, p->res->end, cxlr,
> -					is_system_ram) > 0)
> -			return 0;
> +	rc = devm_cxl_region_edac_register(cxlr);
> +	dev_dbg(&cxlr->dev, "CXL EDAC registration for region_id=%d %s\n",
> +			cxlr->id, rc ? "failed" : "succeeded");
> +
> +	switch (cxlr->driver) {
> +	case CXL_REGION_DRIVER_NONE:
> +		return 0;
> +	case CXL_REGION_DRIVER_DAX:
>  		return devm_cxl_add_dax_region(cxlr);
> +	case CXL_REGION_DRIVER_PMEM:
> +		return devm_cxl_add_pmem_region(cxlr);
>  	default:
> -		dev_dbg(&cxlr->dev, "unsupported region mode: %d\n",
> -			cxlr->mode);
> +		dev_dbg(&cxlr->dev, "unsupported region driver: %d\n",
> +			cxlr->driver);
>  		return -ENXIO;
>  	}
>  }
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index ba17fa86d249..e8256099de29 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -502,6 +502,13 @@ enum cxl_partition_mode {
>  	CXL_PARTMODE_PMEM,
>  };
>  
> +
> +enum cxl_region_driver {
> +	CXL_REGION_DRIVER_NONE,
> +	CXL_REGION_DRIVER_DAX,
> +	CXL_REGION_DRIVER_PMEM,
> +};
> +
>  /*
>   * Indicate whether this region has been assembled by autodetection or
>   * userspace assembly. Prevent endpoint decoders outside of automatic
> @@ -543,6 +550,7 @@ struct cxl_region {
>  	struct device dev;
>  	int id;
>  	enum cxl_partition_mode mode;
> +	enum cxl_region_driver driver;
>  	enum cxl_decoder_type type;
>  	struct cxl_nvdimm_bridge *cxl_nvb;
>  	struct cxl_pmem_region *cxlr_pmem;
> -- 
> 2.52.0
> 

Re: [PATCH v2 1/3] drivers/cxl: introduce cxl_region_driver field for cxl_region

[Gregory Price]

On Wed, Jan 21, 2026 at 04:27:23PM -0600, Ira Weiny wrote:
> > enum cxl_region_driver {
> > 	CXL_REGION_DRIVER_NONE,
> > 	CXL_REGION_DRIVER_DAX,
> > 	CXL_REGION_DRIVER_PMEM
> > };
> 
> The problem I see with this series is that this is not actually telling
> the user which driver is being used.  Only which device is being created.
> Then it is assumed the proper driver attaches.
> 

I'm not fully following this comment, apologies.

> > 
> > $cat regionN/region_driver
> > [none,dax,pmem]
> 
> I feel like this will be more useful when CXL RAM regions can be driven by
> different drivers.  I forget right now the rest of your other series.  But
> I wonder if the actual driver in drivers/dax/cxl.c (the dax driver) should
> be setting this field?
> 

the code in drivers/dax/cxl.c gets called by region code at probe() time

So by the point you get to there, you've already made the decision to
engage the dax_region glue - i.e. this is actually needed to be set
prior to `echo region0 > cxl/drivers/bind`

so the workflow looks like this


   - echo region0 > decoder0.0/create_ram_region
   - /* do all the decoder programming and commit */
   - echo [dax,sysram,...] > region0/region_driver
   - echo region0 > cxl/drivers/region/bind
     -> region probe() creates dax_region
     -> dax_region probe()
     -> dax probe()

so region probe() then sees you selected DAX and goes down the dax
rabbit hole.

> Also escaping my memory ATM, is why one can't relate the dax_region to the
> cxl_region in user space already?
> 

you can, it's linked in `region0/dax_region` after probe()

> > Auto-regions will either be static sysram (BIOS-onlined) and has no
> > region controller associated with it - or if the SP bit was set a
> > DAX device will be created.  This will be discovered at probe time.
> 
> This seems like it adds information.  Since these BIOS controlled regions
> kind of get 'lost'.
>

It actually just formally encodes: "Auto-regions have only two
reasonable end-points:

    auto-SysRAM region:  no driver control (it's already online)
    EFI_MEMORY_SP     :  DAX

This is what the current implicit behavior is.

> :-/
> 
> I'm not opposed to this idea but I'm worried that this adds to the already
> very implicit nature of the CXL subsystem.
> 

Point of clarification:
    "All RAM regions will be defaulted to CXL_REGION_DRIVER_DAX"
                      |
		      V
    "All Auto-regions in a RAM partition will be defaulted to
     CXL_REGION_DRIVER_DAX" 


Auto (BIOS-configured) regions should be considered evil for exactly this
reason. They make the software model much more difficult to discern.

If a user wants to change an auto-decoder region, they would have to:

  1) unbind daxdev
  2) unbind dax_region
  3) unbind region0
  4) echo 'new_driver' > region0/region_driver
  5) echo region0 > cxl/drivers/region/bind

Unless we want to change the existing default pattern for auto-regions
to just not auto-probe, which would break existing systems.

> > +	/* NONE type is not a valid selection for manually probed regions */
> > +	if (sysfs_streq(buf, "dax"))
> > +		cxlr->driver = CXL_REGION_DRIVER_DAX;
> 
> How does this work?  Doesn't this have to create a dax_region device for
> the driver to attach to?  That is the equal to CXL_REGION_DRIVER_DAX being
> set at region probe time.
> 

This gets read at probe() time essentially to generate dax_region

This is essentially how it "already works" - the information is just
obfuscated by the default nature of:

pmem region?  ->  nvdimm glue
ram  region?  ->  dax glue

> Hindsight: It seems like this driver should have been called the CXL
> partition driver.  As it is triggered by the partitions being found...  I
> think.  I'm probably really confused right now.

I was trying to say this on the recent dax and CXL calls - the current
region driver (region.c) is really the PARTITION driver, and what i'm
trying to break out is the ACTUAL region driver pieces.

This is why the original series called this a region_controller, because
both region and driver here is just overloaded :[ 

~Gregory

Re: [PATCH v2 1/3] drivers/cxl: introduce cxl_region_driver field for cxl_region

[Gregory Price]

> > :-/
> > 
> > I'm not opposed to this idea but I'm worried that this adds to the already
> > very implicit nature of the CXL subsystem.
> > 
> 

Took me a bit, but i follow this now.

The current flow of things:

   echo region0 > cxl/drivers/cxl_region/bind
      cxl_region_probe()
        - devm_cxl_add_dax_region()
        - dax_region is created and probed

      cxl_dax_region_probe()
        - devm_create_dev_dax(&data));
	  dax0.0 is created with IORESOURCE_DAX_KMEM

      dax/bus.c discovery
        - dax bus discovers new dev_dax
	- dax bus auto-sets driver type to dax_kmem
	  dax_bus_match() -> dax_match_type() 
          if (dev_dax->region->res.flags & IORESOURCE_DAX_KMEM)
                type = DAXDRV_KMEM_TYPE;
        - dax_bus_probe() calls dax_drv->probe() unconditionally
	  
      dev_dax_kmem_probe()
        - kmem driver starts memory hotplug procedure


So to my (your? our?) point - the following is implicit in cxl:

	if (IS_ENABLED(CONFIG_DEV_DAX_KMEM))
		create_ram_region  ===>  dax_kmem
	else
		create_ram_region  ===>  device_dax


What would have been nice:

echo region0 > decoder0.0/create_ram_region
  - region0 is created
  - program decoders

echo region0 > cxl/drivers/dax_region/bind
  - dax_region0 is created
  - dax_region0 creates dax0.0
  - dax/bus.c *does not* auto-probe the driver type
  - user configures dax0.0

echo dax0.0 > dax/drivers/kmem/bind
  - dax/kmem.c does the whole hotplug song and dance

But in an effort to make auto-onlining capacity user-friendly,
the middle piece was automated away.

I don't see how we dig ourself out of the former flow without either:

1) Breaking the current auto-config flow by adding:

      cxl/drivers/[xxx_region]/bind

2) Adding something akin to `region0/region_driver` which is basically
   used to replace the `echo region0 > cxl/drivers/[xxx_region]/bind

   you just get
   switch (region->region_driver) {
   case dax: do_dax_thing(); break;
   case sysram: do_sysram_thing(); break;
   ...
   }

   do_dax_thing() is otherwise just dax_region_probe()
   do_sysram_thing() is otherwise just sysram_region_probe()


~Gregory

Re: [PATCH v2 1/3] drivers/cxl: introduce cxl_region_driver field for cxl_region

[Davidlohr Bueso]

On Tue, 13 Jan 2026, Gregory Price wrote:

>The CXL driver presently has 3 modes of managing a cxl_region:
>  - no specific driver (bios-onlined SystemRAM)
>  - dax_region (all other RAM regions, for now)
>  - pmem_region (all PMEM regions)
>
>Formalize these into specific "region drivers".

I think your overall idea of these region driver backends to isolate use
cases makes a lot of sense and helps untangle things. This series is a good
step and nicely moves ad-hoc complexity out of region.c.

>
>enum cxl_region_driver {
>	CXL_REGION_DRIVER_NONE,
>	CXL_REGION_DRIVER_DAX,
>	CXL_REGION_DRIVER_PMEM
>};
>
>$cat regionN/region_driver
>[none,dax,pmem]
>
>The intent is to clarify how to to add additional drivers (sysram,
>dynamic_capacity, etc) in the future, and to allow switching the
>driver selection via a sysfs entry `regionN/region_driver`.
>
>All RAM regions will be defaulted to CXL_CONTROL_DAX.

s/CXL_CONTROL_DAX/CXL_REGION_DRIVER_DAX?

>
>Auto-regions will either be static sysram (BIOS-onlined) and has no
>region controller associated with it - or if the SP bit was set a
>DAX device will be created.  This will be discovered at probe time.
>
>Signed-off-by: Gregory Price <gourry@gourry.net>

Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>

>---
> drivers/cxl/core/region.c | 113 ++++++++++++++++++++++++++++++--------
> drivers/cxl/cxl.h         |   8 +++
> 2 files changed, 98 insertions(+), 23 deletions(-)
>
>diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
>index ae899f68551f..f8262d2169ea 100644
>--- a/drivers/cxl/core/region.c
>+++ b/drivers/cxl/core/region.c
>@@ -626,6 +626,57 @@ static ssize_t mode_show(struct device *dev, struct device_attribute *attr,
> }
> static DEVICE_ATTR_RO(mode);
>
>+static ssize_t region_driver_show(struct device *dev,
>+				  struct device_attribute *attr,
>+				  char *buf)
>+{
>+	struct cxl_region *cxlr = to_cxl_region(dev);
>+	const char *desc;
>+
>+	switch (cxlr->driver) {
>+	case CXL_REGION_DRIVER_NONE:
>+		desc = "none";
>+		break;
>+	case CXL_REGION_DRIVER_DAX:
>+		desc = "dax";
>+		break;
>+	case CXL_REGION_DRIVER_PMEM:
>+		desc = "pmem";
>+		break;
>+	}
>+
>+	return sysfs_emit(buf, "%s\n", desc);
>+}
>+
>+static ssize_t region_driver_store(struct device *dev,
>+				   struct device_attribute *attr,
>+				   const char *buf, size_t len)
>+{
>+	struct cxl_region *cxlr = to_cxl_region(dev);
>+	struct cxl_region_params *p = &cxlr->params;
>+	int rc;
>+
>+	ACQUIRE(rwsem_write_kill, rwsem)(&cxl_rwsem.region);
>+	if ((rc = ACQUIRE_ERR(rwsem_write_kill, &rwsem)))
>+		return rc;
>+
>+	if (p->state >= CXL_CONFIG_COMMIT)
>+		return -EBUSY;
>+
>+	/* PMEM drivers cannot be changed */
>+	if (cxlr->mode == CXL_PARTMODE_PMEM)
>+		return -EBUSY;
>+
>+	/* NONE type is not a valid selection for manually probed regions */
>+	if (sysfs_streq(buf, "dax"))
>+		cxlr->driver = CXL_REGION_DRIVER_DAX;
>+	else
>+		return -EINVAL;
>+
>+	return len;
>+}
>+static DEVICE_ATTR_RW(region_driver);
>+
> static int alloc_hpa(struct cxl_region *cxlr, resource_size_t size)
> {
> 	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent);
>@@ -772,6 +823,7 @@ static struct attribute *cxl_region_attrs[] = {
> 	&dev_attr_size.attr,
> 	&dev_attr_mode.attr,
> 	&dev_attr_extended_linear_cache_size.attr,
>+	&dev_attr_region_driver.attr,
> 	NULL,
> };
>
>@@ -2599,6 +2651,16 @@ static struct cxl_region *devm_cxl_add_region(struct cxl_root_decoder *cxlrd,
> 	cxlr->mode = mode;
> 	cxlr->type = type;
>
>+	/*
>+	 * PMEM regions only have 1 driver: pmem_region
>+	 * RAM regions default to DAX, but if the memory is already onlined by
>+	 * BIOS as 'System-RAM', the DAX driver will be dropped during probe.
>+	 */
>+	if (mode == CXL_PARTMODE_PMEM)
>+		cxlr->driver = CXL_REGION_DRIVER_PMEM;
>+	else
>+		cxlr->driver = CXL_REGION_DRIVER_DAX;
>+
> 	dev = &cxlr->dev;
> 	rc = dev_set_name(dev, "region%d", id);
> 	if (rc)
>@@ -3951,33 +4013,38 @@ static int cxl_region_probe(struct device *dev)
> 			return rc;
> 	}
>
>-	switch (cxlr->mode) {
>-	case CXL_PARTMODE_PMEM:
>-		rc = devm_cxl_region_edac_register(cxlr);
>-		if (rc)
>-			dev_dbg(&cxlr->dev, "CXL EDAC registration for region_id=%d failed\n",
>-				cxlr->id);
>+	if (cxlr->mode > CXL_PARTMODE_PMEM) {
>+		dev_dbg(&cxlr->dev, "unsupported region mode: %d\n",
>+			cxlr->mode);
>+		return -ENXIO;
>+	}
>
>-		return devm_cxl_add_pmem_region(cxlr);
>-	case CXL_PARTMODE_RAM:
>-		rc = devm_cxl_region_edac_register(cxlr);
>-		if (rc)
>-			dev_dbg(&cxlr->dev, "CXL EDAC registration for region_id=%d failed\n",
>-				cxlr->id);
>+	/*
>+	 * The region can not be managed by CXL if any portion of
>+	 * it is already online as 'System RAM'.
>+	 */
>+	if (walk_iomem_res_desc(IORES_DESC_NONE,
>+				IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY,
>+				p->res->start, p->res->end, cxlr,
>+					is_system_ram) > 0) {
>+		cxlr->driver = CXL_REGION_DRIVER_NONE;
>+		return 0;
>+	}
>
>-		/*
>-		 * The region can not be manged by CXL if any portion of
>-		 * it is already online as 'System RAM'
>-		 */
>-		if (walk_iomem_res_desc(IORES_DESC_NONE,
>-					IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY,
>-					p->res->start, p->res->end, cxlr,
>-					is_system_ram) > 0)
>-			return 0;
>+	rc = devm_cxl_region_edac_register(cxlr);
>+	dev_dbg(&cxlr->dev, "CXL EDAC registration for region_id=%d %s\n",
>+			cxlr->id, rc ? "failed" : "succeeded");
>+
>+	switch (cxlr->driver) {
>+	case CXL_REGION_DRIVER_NONE:
>+		return 0;
>+	case CXL_REGION_DRIVER_DAX:
> 		return devm_cxl_add_dax_region(cxlr);
>+	case CXL_REGION_DRIVER_PMEM:
>+		return devm_cxl_add_pmem_region(cxlr);
> 	default:
>-		dev_dbg(&cxlr->dev, "unsupported region mode: %d\n",
>-			cxlr->mode);
>+		dev_dbg(&cxlr->dev, "unsupported region driver: %d\n",
>+			cxlr->driver);
> 		return -ENXIO;
> 	}
> }
>diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
>index ba17fa86d249..e8256099de29 100644
>--- a/drivers/cxl/cxl.h
>+++ b/drivers/cxl/cxl.h
>@@ -502,6 +502,13 @@ enum cxl_partition_mode {
> 	CXL_PARTMODE_PMEM,
> };
>
>+
>+enum cxl_region_driver {
>+	CXL_REGION_DRIVER_NONE,
>+	CXL_REGION_DRIVER_DAX,
>+	CXL_REGION_DRIVER_PMEM,
>+};
>+
> /*
>  * Indicate whether this region has been assembled by autodetection or
>  * userspace assembly. Prevent endpoint decoders outside of automatic
>@@ -543,6 +550,7 @@ struct cxl_region {
> 	struct device dev;
> 	int id;
> 	enum cxl_partition_mode mode;
>+	enum cxl_region_driver driver;
> 	enum cxl_decoder_type type;
> 	struct cxl_nvdimm_bridge *cxl_nvb;
> 	struct cxl_pmem_region *cxlr_pmem;
>-- 
>2.52.0
>

Re: [PATCH v2 1/3] drivers/cxl: introduce cxl_region_driver field for cxl_region

[Gregory Price]

On Thu, Jan 29, 2026 at 11:10:39AM -0800, Davidlohr Bueso wrote:
> On Tue, 13 Jan 2026, Gregory Price wrote:
> 
> > The CXL driver presently has 3 modes of managing a cxl_region:
> >  - no specific driver (bios-onlined SystemRAM)
> >  - dax_region (all other RAM regions, for now)
> >  - pmem_region (all PMEM regions)
> > 
> > Formalize these into specific "region drivers".
> 
> I think your overall idea of these region driver backends to isolate use
> cases makes a lot of sense and helps untangle things. This series is a good
> step and nicely moves ad-hoc complexity out of region.c.
> 

fyi this is being respun entirely to include the end-to-end solution
described here:

https://lore.kernel.org/linux-cxl/aXErSP1zKnv4koKG@gourry-fedora-PF4VCD3F/

Posting shortly

~Gregory