Re: [PATCH V5 16/17] cxl/pmem_region: Create pmem region using information parsed from LSA

Neeraj Kumar
2026-01-23T11:28:38Z

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

On 15/01/26 06:28PM, Jonathan Cameron wrote:
>On Fri, 9 Jan 2026 18:14:36 +0530
>Neeraj Kumar <s.neeraj@samsung.com> wrote:
>
>> create_pmem_region() creates CXL region based on region information
>> parsed from the Label Storage Area (LSA). This routine requires cxl
>> endpoint decoder and root decoder. Add cxl_find_root_decoder_by_port()
>> and cxl_find_free_ep_decoder() to find the root decoder and a free
>> endpoint decoder respectively.
>>
>> Signed-off-by: Neeraj Kumar <s.neeraj@samsung.com>
>Hi Neeraj,
>
>Just a few minor things.
>
>Jonathan
>
>> diff --git a/drivers/cxl/core/pmem_region.c b/drivers/cxl/core/pmem_region.c
>> index 53d3d81e9676..4a8cf8322cf0 100644
>> --- a/drivers/cxl/core/pmem_region.c
>> +++ b/drivers/cxl/core/pmem_region.c
>> @@ -287,3 +287,139 @@ int devm_cxl_add_pmem_region(struct cxl_region *cxlr)
>> cxlr->cxl_nvb = NULL;
>> return rc;
>> }
>> +
>> +static int match_root_decoder_by_dport(struct device *dev, const void *data)
>> +{
>> + const struct cxl_port *ep_port = data;
>> + struct cxl_root_decoder *cxlrd;
>> + struct cxl_port *root_port;
>> + struct cxl_decoder *cxld;
>> + struct cxl_dport *dport;
>> + bool dport_matched = false;
>> +
>> + if (!is_root_decoder(dev))
>> + return 0;
>> +
>> + cxld = to_cxl_decoder(dev);
>> + if (!(cxld->flags & CXL_DECODER_F_PMEM))
>> + return 0;
>> +
>> + cxlrd = to_cxl_root_decoder(dev);
>> +
>> + root_port = cxlrd_to_port(cxlrd);
>> + dport = cxl_find_dport_by_dev(root_port, ep_port->host_bridge);
>> + if (!dport)
>> + return 0;
>> +
>There is a fairly standard way to check if a loop matched without
>needing a boolean. Just check if the exit condition was reached.
>
>drop declaration of i out of here.
>> + for (int i = 0; i < cxlrd->cxlsd.nr_targets; i++) {
>> + if (dport == cxlrd->cxlsd.target[i]) {
>> + dport_matched = true;
>No need for this.
>> + break;
>> + }
>> + }
>> +
>> + if (!dport_matched)
> if (i == cxlrd->cxlsd.nr_targets)
> return 0;

Thanks for your suggestion. I have fixed it accordingly in V6.

>
>> + return 0;
>> +
>> + return is_root_decoder(dev);
>> +}
>> +
>> +/**
>> + * cxl_find_root_decoder_by_port() - find a cxl root decoder on cxl bus
>> + * @port: any descendant port in CXL port topology
>> + * @cxled: endpoint decoder
>> + *
>> + * Caller of this function must call put_device() when done as a device ref
>> + * is taken via device_find_child()
>> + */
>> +static struct cxl_root_decoder *
>> +cxl_find_root_decoder_by_port(struct cxl_port *port,
>> + struct cxl_endpoint_decoder *cxled)
>> +{
>> + struct cxl_root *cxl_root __free(put_cxl_root) = find_cxl_root(port);
>> + struct cxl_port *ep_port = cxled_to_port(cxled);
>> + struct device *dev;
>> +
>> + if (!cxl_root)
>> + return NULL;
>> +
>> + dev = device_find_child(&cxl_root->port.dev, ep_port,
>> + match_root_decoder_by_dport);
>> + if (!dev)
>> + return NULL;
>> +
>> + return to_cxl_root_decoder(dev);
>> +}
>
>> +void create_pmem_region(struct nvdimm *nvdimm)
>> +{
>> + struct cxl_region *cxlr;
>> + struct cxl_memdev *cxlmd;
>> + struct cxl_nvdimm *cxl_nvd;
>> + struct cxl_endpoint_decoder *cxled;
>> + struct cxl_pmem_region_params *params;
>
>CXL tends to be reverse xmas tree and good to stick to local style.

Changed them in reverse xmas tree style.

Regards,
Neeraj

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