On Tue, 17 Jun 2025 18:09:43 +0530
Neeraj Kumar <s.nee...@samsung.com> wrote:
> Created a separate file core/pmem_region.c along with CONFIG_PMEM_REGION
> Moved pmem_region related code from core/region.c to core/pmem_region.c
> For region label update, need to create device attribute, which calls
> nvdimm exported function thus making pmem_region dependent on libnvdimm.
> Because of this dependency of pmem region on libnvdimm, segregated pmem
segregate
> region related code from core/region.c
>
> Signed-off-by: Neeraj Kumar <s.nee...@samsung.com>
> ---
> drivers/cxl/Kconfig | 12 ++
> drivers/cxl/core/Makefile | 1 +
> drivers/cxl/core/core.h | 8 +-
> drivers/cxl/core/pmem_region.c | 222 +++++++++++++++++++++++++++++++++
> drivers/cxl/core/port.c | 2 +-
> drivers/cxl/core/region.c | 217 ++------------------------------
> drivers/cxl/cxl.h | 42 +++++--
> tools/testing/cxl/Kbuild | 1 +
> 8 files changed, 283 insertions(+), 222 deletions(-)
> create mode 100644 drivers/cxl/core/pmem_region.c
>
> diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
> index 876469e23f7a..f0cbb096bfe7 100644
> --- a/drivers/cxl/Kconfig
> +++ b/drivers/cxl/Kconfig
> @@ -128,6 +128,18 @@ config CXL_REGION
>
> If unsure say 'y'
>
> +config CXL_PMEM_REGION
> + bool "CXL: Pmem Region Support"
> + default CXL_BUS
> + depends on CXL_REGION
> + select LIBNVDIMM if CXL_BUS = y
This is in the block covered by if CXL_BUS so I think you can simplify this
check.
> + 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
> + updation/deletion 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/pmem_region.c b/drivers/cxl/core/pmem_region.c
> new file mode 100644
> index 000000000000..a29526c27d40
> --- /dev/null
> +++ b/drivers/cxl/core/pmem_region.c
> @@ -0,0 +1,222 @@
> @@ -3273,92 +3155,6 @@ static struct cxl_dax_region
> *cxl_dax_region_alloc(struct cxl_region *cxlr)
> return cxlr_dax;
> }
> static void cxlr_dax_unregister(void *_cxlr_dax)
> {
> struct cxl_dax_region *cxlr_dax = _cxlr_dax;
> @@ -3646,7 +3442,10 @@ static int cxl_region_probe(struct device *dev)
>
> switch (cxlr->mode) {
> case CXL_DECODER_PMEM:
> - return devm_cxl_add_pmem_region(cxlr);
> + if (IS_ENABLED(CONFIG_CXL_PMEM_REGION))
> + return devm_cxl_add_pmem_region(cxlr);
> + else
> + return -EINVAL;
if (!IS_ENABLED(CONFIG_CXL_PMEM_REGION))
return -EINVAL;
return devm_cxl_add_pmem_region()
Where ever possible keep the error conditions as the out of line ones.
That generally improves code readability and is common practice in the kernel.
> case CXL_DECODER_RAM:
> /*
> * The region can not be manged by CXL if any portion of
