Re: [RFC PATCH 13/20] cxl/mem: Refactor cxl pmem region auto-assembling

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

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

On 23/06/25 10:20AM, Jonathan Cameron wrote:
>On Tue, 17 Jun 2025 18:09:37 +0530
>Neeraj Kumar <s.neeraj@samsung.com> wrote:
>
>> In 84ec985944ef3, For cxl pmem region auto-assembly after endpoint port
>> probing, cxl_nvd presence was required. And for cxl region persistency,
>> region creation happens during nvdimm_probe which need the completion
>> of endpoint probe.
>>
>> It is therefore refactored cxl pmem region auto-assembly after endpoint
>> probing to cxl mem probing
>>
>> Signed-off-by: Neeraj Kumar <s.neeraj@samsung.com>
>For ordering requirements this needs more eyes.  I've never cared that
>much about he persistency and auto assembly code so not something I have
>a good mental model of!
>

Sure, Will see how other maintainers responds to it. Thanks for your
feedback.

>
>> ---
>>  drivers/cxl/core/port.c | 38 ++++++++++++++++++++++++++++++++++++++
>>  drivers/cxl/cxl.h       |  1 +
>>  drivers/cxl/mem.c       | 27 ++++++++++++++++++---------
>>  drivers/cxl/port.c      | 38 --------------------------------------
>>  4 files changed, 57 insertions(+), 47 deletions(-)
>
>> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
>> index 2f03a4d5606e..aaea4eb178ef 100644
>> --- a/drivers/cxl/mem.c
>> +++ b/drivers/cxl/mem.c
>
>> @@ -180,6 +171,24 @@ static int cxl_mem_probe(struct device *dev)
>>  			return rc;
>>  	}
>>
>> +	if (resource_size(&cxlds->pmem_res) && IS_ENABLED(CONFIG_CXL_PMEM)) {
>> +		rc = devm_cxl_add_nvdimm(parent_port, cxlmd);
>> +		if (rc) {
>> +			if (rc == -ENODEV)
>> +				dev_info(dev, "PMEM disabled by platform\n");
>> +			return rc;
>> +		}
>> +	}
>> +
>> +	/*
>> +	 * Now that all endpoint decoders are successfully enumerated, try to
>> +	 * assemble region autodiscovery from committed decoders.
>> +	 * Earlier it was part of cxl_endpoint_port_probe, So moved it here
>
>I would drop this history statement. Good to have in the patch description
>but no point in keeping it in the code. Just state what the requirements
>are now.
>

Okay, I will remove this and update commit message with the same

>> +	 * as cxl_nvd of the memdev needs to be available during the pmem
>> +	 * region auto-assembling
>> +	 */
>> +	cxl_region_discovery(cxlmd->endpoint);
>> +
>>  	/*
>>  	 * The kernel may be operating out of CXL memory on this device,
>
>
>

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