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

[Neeraj Kumar <s.neeraj@samsung.com>]

[-- Attachment #1: Type: text/plain, Size: 3356 bytes --]

On 23/06/25 10:53AM, Jonathan Cameron wrote:
>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
>

Thanks, Will fix the commit message.

>> 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.
>

yes, its part of CXL_BUS block. I will update it as "select LIBNVDIMM".
Actually for CONFIG_CXL_PMEM_REGION=y we have dependency on CONFIG_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
>> +	  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.
>

Thanks for suggestion Jonathan. Sure will fix it in V1

>>  	case CXL_DECODER_RAM:
>>  		/*
>>  		 * The region can not be manged by CXL if any portion of
>
>

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]