Re: [PATCH V4 13/17] cxl/pmem_region: Prep patch to accommodate pmem_region attributes

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

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

On 17/12/25 03:35PM, Jonathan Cameron wrote:
>On Wed, 19 Nov 2025 13:22:51 +0530
>Neeraj Kumar <s.neeraj@samsung.com> wrote:
>
>> For region label update, need to create device attribute, which calls
>> nvdimm exported routine thus making pmem_region dependent on libnvdimm.
>> Because of this dependency of pmem region on libnvdimm, segregate pmem
>> region related code from core/region.c to core/pmem_region.c
>>
>> This patch has no functionality change. Its just code movement from
>> core/region.c to core/pmem_region.c
>>
>> Signed-off-by: Neeraj Kumar <s.neeraj@samsung.com>
>
>Minor stuff below.
>Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

Thanks Jonathan for RB tag.

>
>
>>  #define SET_CXL_REGION_ATTR(x)
>> diff --git a/drivers/cxl/core/pmem_region.c b/drivers/cxl/core/pmem_region.c
>> new file mode 100644
>> index 000000000000..b45e60f04ff4
>> --- /dev/null
>> +++ b/drivers/cxl/core/pmem_region.c
>> @@ -0,0 +1,202 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/* Copyright(c) 2020 Intel Corporation. */
>> +#include <linux/device.h>
>> +#include <linux/memregion.h>
>> +#include <cxlmem.h>
>> +#include <cxl.h>
>> +#include "core.h"
>> +
>> +/**
>> + * DOC: cxl pmem region
>> + *
>> + * The core CXL PMEM region infrastructure supports persistent memory
>> + * region creation using LIBNVDIMM subsystem. It has dependency on
>> + * LIBNVDIMM, pmem region need updation of cxl region information into
>
>Perhaps reword as:
>
>pmem region needs to update the cxl region information in the LSA.
>

Fixed it accrodingly in V5

>> + * LSA. LIBNVDIMM dependency is only for pmem region, it is therefore
>> + * need this separate file.
>
>This seems like an explanation for the patch. Not sure we need it
>in the final code.  Anyone who considers changing this will rapidly
>spot that in the build files.
>
>...

Fixed it in V5

>
>> +static const struct attribute_group *cxl_pmem_region_attribute_groups[] = {
>> +	&cxl_base_attribute_group,
>> +	NULL,
>
>Whilst here, perhaps drop that trailing , there shouldn't be one on a terminating
>entry like this.

Fixed it in V5


Regards,
Neeraj

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