Re: [RFC PATCH 19/20] cxl/pmem_region: Prep patch to accommodate pmem_region attributes

[Jonathan Cameron <Jonathan.Cameron@huawei.com>]

On Tue, 17 Jun 2025 18:09:43 +0530
Neeraj Kumar <s.neeraj@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.neeraj@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