Re: [PATCH V4 14/17] cxl/pmem_region: Introduce CONFIG_CXL_PMEM_REGION for core/pmem_region.c

2026-01-09 Thread Neeraj Kumar

On 17/12/25 03:38PM, Jonathan Cameron wrote:

On Wed, 19 Nov 2025 13:22:52 +0530
Neeraj Kumar  wrote:


As pmem region label update/delete has hard dependency on libnvdimm.
It is therefore put core/pmem_region.c under CONFIG_CXL_PMEM_REGION
control. It handles the dependency by selecting CONFIG_LIBNVDIMM
if not enabled.

Signed-off-by: Neeraj Kumar 




diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
index 5ebbc3d3dde5..beeb9b7527b8 100644
--- a/drivers/cxl/core/core.h
+++ b/drivers/cxl/core/core.h
@@ -89,17 +88,23 @@ static inline struct cxl_region *to_cxl_region(struct 
device *dev)
 {
return NULL;
 }
-static inline int devm_cxl_add_pmem_region(struct cxl_region *cxlr)
-{
-   return 0;
-}
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 9798120b208e..408e139718f1 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -3918,6 +3918,8 @@ static int cxl_region_probe(struct device *dev)
dev_dbg(&cxlr->dev, "CXL EDAC registration for region_id=%d 
failed\n",
cxlr->id);

+   if (!IS_ENABLED(CONFIG_CXL_PMEM_REGION))
+   return -EINVAL;
return devm_cxl_add_pmem_region(cxlr);


Why not have the stub return -EINVAL if it never makes sense to call without
the CONFIG being enabled?


I have removed this config check and handled it in
devm_cxl_add_pmem_region() itself.



Regards,
Neeraj


Re: [PATCH V4 14/17] cxl/pmem_region: Introduce CONFIG_CXL_PMEM_REGION for core/pmem_region.c

2025-12-17 Thread Jonathan Cameron
On Wed, 19 Nov 2025 13:22:52 +0530
Neeraj Kumar  wrote:

> As pmem region label update/delete has hard dependency on libnvdimm.
> It is therefore put core/pmem_region.c under CONFIG_CXL_PMEM_REGION
> control. It handles the dependency by selecting CONFIG_LIBNVDIMM
> if not enabled.
> 
> Signed-off-by: Neeraj Kumar 


> diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
> index 5ebbc3d3dde5..beeb9b7527b8 100644
> --- a/drivers/cxl/core/core.h
> +++ b/drivers/cxl/core/core.h
> @@ -34,7 +34,6 @@ int cxl_decoder_detach(struct cxl_region *cxlr,
>  #define CXL_REGION_ATTR(x) (&dev_attr_##x.attr)
>  #define CXL_REGION_TYPE(x) (&cxl_region_type)
>  #define SET_CXL_REGION_ATTR(x) (&dev_attr_##x.attr),
> -#define CXL_PMEM_REGION_TYPE(x) (&cxl_pmem_region_type)
>  #define CXL_DAX_REGION_TYPE(x) (&cxl_dax_region_type)
>  int cxl_region_init(void);
>  void cxl_region_exit(void);
> @@ -89,17 +88,23 @@ static inline struct cxl_region *to_cxl_region(struct 
> device *dev)
>  {
>   return NULL;
>  }
> -static inline int devm_cxl_add_pmem_region(struct cxl_region *cxlr)
> -{
> - return 0;
> -}
>  #define CXL_REGION_ATTR(x) NULL
>  #define CXL_REGION_TYPE(x) NULL
>  #define SET_CXL_REGION_ATTR(x)
> -#define CXL_PMEM_REGION_TYPE(x) NULL
>  #define CXL_DAX_REGION_TYPE(x) NULL
>  #endif
>  
> +#ifdef CONFIG_CXL_PMEM_REGION
> +#define CXL_PMEM_REGION_TYPE(x) (&cxl_pmem_region_type)
> +int devm_cxl_add_pmem_region(struct cxl_region *cxlr);
> +#else
> +#define CXL_PMEM_REGION_TYPE(x) NULL
> +static inline int devm_cxl_add_pmem_region(struct cxl_region *cxlr)
> +{
> + return 0;
> +}
> +#endif
> +
>  struct cxl_send_command;
>  struct cxl_mem_query_commands;
>  int cxl_query_cmd(struct cxl_mailbox *cxl_mbox,
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 9798120b208e..408e139718f1 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -3918,6 +3918,8 @@ static int cxl_region_probe(struct device *dev)
>   dev_dbg(&cxlr->dev, "CXL EDAC registration for 
> region_id=%d failed\n",
>   cxlr->id);
>  
> + if (!IS_ENABLED(CONFIG_CXL_PMEM_REGION))
> + return -EINVAL;
>   return devm_cxl_add_pmem_region(cxlr);

Why not have the stub return -EINVAL if it never makes sense to call without
the CONFIG being enabled?

>   case CXL_PARTMODE_RAM:
>   rc = devm_cxl_region_edac_register(cxlr);





Re: [PATCH V4 14/17] cxl/pmem_region: Introduce CONFIG_CXL_PMEM_REGION for core/pmem_region.c

2025-11-19 Thread Dave Jiang



On 11/19/25 12:52 AM, Neeraj Kumar wrote:
> As pmem region label update/delete has hard dependency on libnvdimm.
> It is therefore put core/pmem_region.c under CONFIG_CXL_PMEM_REGION
> control. It handles the dependency by selecting CONFIG_LIBNVDIMM
> if not enabled.
> 
> Signed-off-by: Neeraj Kumar 

Reviewed-by: Dave Jiang 

> ---
>  drivers/cxl/Kconfig   | 15 +++
>  drivers/cxl/core/Makefile |  3 ++-
>  drivers/cxl/core/core.h   | 17 +++--
>  drivers/cxl/core/region.c |  2 ++
>  drivers/cxl/cxl.h | 24 ++--
>  tools/testing/cxl/Kbuild  |  3 ++-
>  6 files changed, 46 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
> index f1361ed6a0d4..307fed8f1f56 100644
> --- a/drivers/cxl/Kconfig
> +++ b/drivers/cxl/Kconfig
> @@ -211,6 +211,21 @@ config CXL_REGION
>  
> If unsure say 'y'
>  
> +config CXL_PMEM_REGION
> + bool "CXL: Pmem Region Support"
> + default CXL_BUS
> + depends on CXL_REGION
> + depends on ARCH_HAS_PMEM_API
> + depends on PHYS_ADDR_T_64BIT
> + depends on BLK_DEV
> + select LIBNVDIMM
> + help
> +Enable the CXL core to enumerate and provision CXL pmem regions.
> +A CXL pmem region need to update region label into LSA. For LSA
> +update/delete libnvdimm is required.
> +
> +If unsure say 'y'
> +
>  config CXL_REGION_INVALIDATION_TEST
>   bool "CXL: Region Cache Management Bypass (TEST)"
>   depends on CXL_REGION
> diff --git a/drivers/cxl/core/Makefile b/drivers/cxl/core/Makefile
> index fe0fcab6d730..399157beb917 100644
> --- a/drivers/cxl/core/Makefile
> +++ b/drivers/cxl/core/Makefile
> @@ -16,7 +16,8 @@ cxl_core-y += pmu.o
>  cxl_core-y += cdat.o
>  cxl_core-y += ras.o
>  cxl_core-$(CONFIG_TRACING) += trace.o
> -cxl_core-$(CONFIG_CXL_REGION) += region.o pmem_region.o
> +cxl_core-$(CONFIG_CXL_REGION) += region.o
> +cxl_core-$(CONFIG_CXL_PMEM_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 5ebbc3d3dde5..beeb9b7527b8 100644
> --- a/drivers/cxl/core/core.h
> +++ b/drivers/cxl/core/core.h
> @@ -34,7 +34,6 @@ int cxl_decoder_detach(struct cxl_region *cxlr,
>  #define CXL_REGION_ATTR(x) (&dev_attr_##x.attr)
>  #define CXL_REGION_TYPE(x) (&cxl_region_type)
>  #define SET_CXL_REGION_ATTR(x) (&dev_attr_##x.attr),
> -#define CXL_PMEM_REGION_TYPE(x) (&cxl_pmem_region_type)
>  #define CXL_DAX_REGION_TYPE(x) (&cxl_dax_region_type)
>  int cxl_region_init(void);
>  void cxl_region_exit(void);
> @@ -89,17 +88,23 @@ static inline struct cxl_region *to_cxl_region(struct 
> device *dev)
>  {
>   return NULL;
>  }
> -static inline int devm_cxl_add_pmem_region(struct cxl_region *cxlr)
> -{
> - return 0;
> -}
>  #define CXL_REGION_ATTR(x) NULL
>  #define CXL_REGION_TYPE(x) NULL
>  #define SET_CXL_REGION_ATTR(x)
> -#define CXL_PMEM_REGION_TYPE(x) NULL
>  #define CXL_DAX_REGION_TYPE(x) NULL
>  #endif
>  
> +#ifdef CONFIG_CXL_PMEM_REGION
> +#define CXL_PMEM_REGION_TYPE(x) (&cxl_pmem_region_type)
> +int devm_cxl_add_pmem_region(struct cxl_region *cxlr);
> +#else
> +#define CXL_PMEM_REGION_TYPE(x) NULL
> +static inline int devm_cxl_add_pmem_region(struct cxl_region *cxlr)
> +{
> + return 0;
> +}
> +#endif
> +
>  struct cxl_send_command;
>  struct cxl_mem_query_commands;
>  int cxl_query_cmd(struct cxl_mailbox *cxl_mbox,
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 9798120b208e..408e139718f1 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -3918,6 +3918,8 @@ static int cxl_region_probe(struct device *dev)
>   dev_dbg(&cxlr->dev, "CXL EDAC registration for 
> region_id=%d failed\n",
>   cxlr->id);
>  
> + if (!IS_ENABLED(CONFIG_CXL_PMEM_REGION))
> + return -EINVAL;
>   return devm_cxl_add_pmem_region(cxlr);
>   case CXL_PARTMODE_RAM:
>   rc = devm_cxl_region_edac_register(cxlr);
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 684a0d1b441a..6ac3b40cb5ff 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -899,21 +899,11 @@ int devm_cxl_add_nvdimm(struct cxl_port *parent_port, 
> struct cxl_memdev *cxlmd);
>  struct cxl_nvdimm_bridge *cxl_find_nvdimm_bridge(struct cxl_port *port);
>  
>  #ifdef CONFIG_CXL_REGION
> -bool is_cxl_pmem_region(struct device *dev);
> -struct cxl_pmem_region *to_cxl_pmem_region(struct device *dev);
>  int cxl_add_to_region(struct cxl_endpoint_decoder *cxled);
>  struct cxl_dax_region *to_cxl_dax_region(struct device *dev);
>  u64 cxl_port_get_spa_cache_alias(struct cxl_port *endpoint, u64 spa);
>  int cxl_region_discovery(struct cxl_memdev *cxlmd);
>  #else
> -static inline bool is_cxl_pmem_region(struct device *dev)
> -{
> -

[PATCH V4 14/17] cxl/pmem_region: Introduce CONFIG_CXL_PMEM_REGION for core/pmem_region.c

2025-11-19 Thread Neeraj Kumar
As pmem region label update/delete has hard dependency on libnvdimm.
It is therefore put core/pmem_region.c under CONFIG_CXL_PMEM_REGION
control. It handles the dependency by selecting CONFIG_LIBNVDIMM
if not enabled.

Signed-off-by: Neeraj Kumar 
---
 drivers/cxl/Kconfig   | 15 +++
 drivers/cxl/core/Makefile |  3 ++-
 drivers/cxl/core/core.h   | 17 +++--
 drivers/cxl/core/region.c |  2 ++
 drivers/cxl/cxl.h | 24 ++--
 tools/testing/cxl/Kbuild  |  3 ++-
 6 files changed, 46 insertions(+), 18 deletions(-)

diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
index f1361ed6a0d4..307fed8f1f56 100644
--- a/drivers/cxl/Kconfig
+++ b/drivers/cxl/Kconfig
@@ -211,6 +211,21 @@ config CXL_REGION
 
  If unsure say 'y'
 
+config CXL_PMEM_REGION
+   bool "CXL: Pmem Region Support"
+   default CXL_BUS
+   depends on CXL_REGION
+   depends on ARCH_HAS_PMEM_API
+   depends on PHYS_ADDR_T_64BIT
+   depends on BLK_DEV
+   select LIBNVDIMM
+   help
+  Enable the CXL core to enumerate and provision CXL pmem regions.
+  A CXL pmem region need to update region label into LSA. For LSA
+  update/delete libnvdimm is required.
+
+  If unsure say 'y'
+
 config CXL_REGION_INVALIDATION_TEST
bool "CXL: Region Cache Management Bypass (TEST)"
depends on CXL_REGION
diff --git a/drivers/cxl/core/Makefile b/drivers/cxl/core/Makefile
index fe0fcab6d730..399157beb917 100644
--- a/drivers/cxl/core/Makefile
+++ b/drivers/cxl/core/Makefile
@@ -16,7 +16,8 @@ cxl_core-y += pmu.o
 cxl_core-y += cdat.o
 cxl_core-y += ras.o
 cxl_core-$(CONFIG_TRACING) += trace.o
-cxl_core-$(CONFIG_CXL_REGION) += region.o pmem_region.o
+cxl_core-$(CONFIG_CXL_REGION) += region.o
+cxl_core-$(CONFIG_CXL_PMEM_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 5ebbc3d3dde5..beeb9b7527b8 100644
--- a/drivers/cxl/core/core.h
+++ b/drivers/cxl/core/core.h
@@ -34,7 +34,6 @@ int cxl_decoder_detach(struct cxl_region *cxlr,
 #define CXL_REGION_ATTR(x) (&dev_attr_##x.attr)
 #define CXL_REGION_TYPE(x) (&cxl_region_type)
 #define SET_CXL_REGION_ATTR(x) (&dev_attr_##x.attr),
-#define CXL_PMEM_REGION_TYPE(x) (&cxl_pmem_region_type)
 #define CXL_DAX_REGION_TYPE(x) (&cxl_dax_region_type)
 int cxl_region_init(void);
 void cxl_region_exit(void);
@@ -89,17 +88,23 @@ static inline struct cxl_region *to_cxl_region(struct 
device *dev)
 {
return NULL;
 }
-static inline int devm_cxl_add_pmem_region(struct cxl_region *cxlr)
-{
-   return 0;
-}
 #define CXL_REGION_ATTR(x) NULL
 #define CXL_REGION_TYPE(x) NULL
 #define SET_CXL_REGION_ATTR(x)
-#define CXL_PMEM_REGION_TYPE(x) NULL
 #define CXL_DAX_REGION_TYPE(x) NULL
 #endif
 
+#ifdef CONFIG_CXL_PMEM_REGION
+#define CXL_PMEM_REGION_TYPE(x) (&cxl_pmem_region_type)
+int devm_cxl_add_pmem_region(struct cxl_region *cxlr);
+#else
+#define CXL_PMEM_REGION_TYPE(x) NULL
+static inline int devm_cxl_add_pmem_region(struct cxl_region *cxlr)
+{
+   return 0;
+}
+#endif
+
 struct cxl_send_command;
 struct cxl_mem_query_commands;
 int cxl_query_cmd(struct cxl_mailbox *cxl_mbox,
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 9798120b208e..408e139718f1 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -3918,6 +3918,8 @@ static int cxl_region_probe(struct device *dev)
dev_dbg(&cxlr->dev, "CXL EDAC registration for 
region_id=%d failed\n",
cxlr->id);
 
+   if (!IS_ENABLED(CONFIG_CXL_PMEM_REGION))
+   return -EINVAL;
return devm_cxl_add_pmem_region(cxlr);
case CXL_PARTMODE_RAM:
rc = devm_cxl_region_edac_register(cxlr);
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 684a0d1b441a..6ac3b40cb5ff 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -899,21 +899,11 @@ int devm_cxl_add_nvdimm(struct cxl_port *parent_port, 
struct cxl_memdev *cxlmd);
 struct cxl_nvdimm_bridge *cxl_find_nvdimm_bridge(struct cxl_port *port);
 
 #ifdef CONFIG_CXL_REGION
-bool is_cxl_pmem_region(struct device *dev);
-struct cxl_pmem_region *to_cxl_pmem_region(struct device *dev);
 int cxl_add_to_region(struct cxl_endpoint_decoder *cxled);
 struct cxl_dax_region *to_cxl_dax_region(struct device *dev);
 u64 cxl_port_get_spa_cache_alias(struct cxl_port *endpoint, u64 spa);
 int cxl_region_discovery(struct cxl_memdev *cxlmd);
 #else
-static inline bool is_cxl_pmem_region(struct device *dev)
-{
-   return false;
-}
-static inline struct cxl_pmem_region *to_cxl_pmem_region(struct device *dev)
-{
-   return NULL;
-}
 static inline int cxl_add_to_region(struct cxl_endpoint_decoder *cxled)
 {
return 0;
@@ -933,6 +923,20 @@ static inline int cxl_region_discovery(str