On 11/7/25 5:39 AM, Neeraj Kumar wrote:
> On 06/10/25 08:55AM, Dave Jiang wrote:
>>
>>
>> On 9/29/25 6:30 AM, Neeraj Kumar wrote:
>>> On 23/09/25 03:37PM, Dave Jiang wrote:
>>>>
>>>>
>>>> On 9/17/25 6:41 AM, Neeraj Kumar wrote:
>>>>> In 84ec985944ef3, devm_cxl_add_nvdimm() sequence was changed and called
>>>>> before devm_cxl_add_endpoint(). It's because cxl pmem region auto-assembly
>>>>> used to get called at last in cxl_endpoint_port_probe(), which requires
>>>>> cxl_nvd presence.
>>>>>
>>>>> For cxl region persistency, region creation happens during nvdimm_probe
>>>>> which need the completion of endpoint probe.
>>>>>
>>>>> In order to accommodate both cxl pmem region auto-assembly and cxl region
>>>>> persistency, refactored following
>>>>>
>>>>> 1. Re-Sequence devm_cxl_add_nvdimm() after devm_cxl_add_endpoint(). This
>>>>> will be called only after successful completion of endpoint probe.
>>>>>
>>>>> 2. Moved cxl pmem region auto-assembly from cxl_endpoint_port_probe() to
>>>>> cxl_mem_probe() after devm_cxl_add_nvdimm(). It gurantees both the
>>>>> completion of endpoint probe and cxl_nvd presence before its call.
>>>>
>>>> Given that we are going forward with this implementation [1] from Dan and
>>>> drivers like the type2 enabling are going to be using it as well, can you
>>>> please consider converting this change to Dan's mechanism instead of
>>>> creating a whole new one?
>>>>
>>>> I think the region discovery can be done via the ops->probe() callback.
>>>> Thanks.
>>>>
>>>> [1]:
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/cxl/cxl.git/commit/?h=for-6.18/cxl-probe-order&id=88aec5ea7a24da00dc92c7778df4851fe4fd3ec6
>>>>
>>>> DJ
>>>>
>>>
>>> Sure, Let me revisit this.
>>> It seems [1] is there in seperate branch "for-6.18/cxl-probe-order", and
>>> not yet merged into next, right?
>>
>> Right. I believe Smita and Alejandro are using that as well. Depending on
>> who gets there first. We can setup an immutable branch at some point.
>>
>> [1]:
>> https://lore.kernel.org/linux-cxl/[email protected]/T/#t
>>
>> DJ
>
> Hi Dave,
>
> As per Dan's [1] newly introduced infra, Following is my understanding.
>
> Currently cxl_pci does not care about the attach state of the cxl_memdev
> because all generic memory expansion functionality can be handled by the
> cxl_core. For accelerators, the driver needs to know and perform driver
> specific initialization if CXL is available, or exectute a fallback to PCIe
> only operation.
>
> Dan's new infra is needed for CXL accelerator device drivers that need to
> make decisions about enabling CXL dependent functionality in the device, or
> falling back to PCIe-only operation.
>
> During cxl_pci_probe() we call devm_cxl_add_memdev(struct cxl_memdev_ops *ops)
> where function pointer as ops gets registered which gets called in
> cxl_mem_probe()
> using cxlmd->ops->probe()
>
> The probe callback runs after the port topology is successfully attached for
> the given memdev.
>
> So to use this infra we have to pass cxl_region_discovery() as ops parameter
> of devm_cxl_add_memdev() getting called from cxl_pci_probe().
>
> In this patch-set cxl_region_discovery() signature is different from
> cxlmd->ops->probe()
>
> {{{
> void cxl_region_discovery(struct cxl_port *port)
> {
> device_for_each_child(&port->dev, NULL, discover_region);
> }
>
> struct cxl_memdev_ops {
> int (*probe)(struct cxl_memdev *cxlmd);
> };
> }}}
>
> Even after changing the signature of cxl_region_discovery() as per
> cxlmd->ops->probe()
> may create problem as when the ops->probe() fails, then it will halts the
> probe sequence
> of cxl_pci_probe()
>
> It is because discover_region() may fail if two memdevs are participating
> into one region
While discover_region() may fail, the return value is ignored. The current code
disregards failures from device_for_each_child(). And also above,
cxl_region_discovery() returns void. So I don't follow how ops->probe() would
fail if we ignore errors from discover_region().
DJ
>
> Also, region auto assembly is mandatory functionality which creates region
> if (cxled->state == CXL_DECODER_STATE_AUTO) gets satisfied.
>
> Currently region auto assembly (added by a32320b71f085) happens after
> successfull
> enumeration of endpoint decoders at cxl_endpoint_port_probe(), which I have
> moved at
> cxl_mem_probe() after devm_cxl_add_nvdimm() which prepares cxl_nvd infra
> required by it.
>
> As discussed in [1], this patch-set does the movement of auto region assembly
> from
> cxl_endpoint_port_probe() to cxl_mem_probe() and resolved the conflicting
> dependency
> of cxl_nvd infra required by both region creation using LSA and auto region
> assembly.
>
> [1]:
> https://git.kernel.org/pub/scm/linux/kernel/git/cxl/cxl.git/commit/?h=for-6.18/cxl-probe-order&id=88aec5ea7a24da00dc92c7778df4851fe4fd3ec6
> [2]:
> https://lore.kernel.org/linux-cxl/1931444790.41752909482841.JavaMail.epsvc@epcpadp2new/
>
> Please let me know if my understanding is correct or I am missing something?
>
>
> Regards,
> Neeraj
>
>
