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

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

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>


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

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

...

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

> +};