On 10/1/26 9:32 PM, Lucero Palau, Alejandro wrote:
> 
> On 01/10/2026 21:31, Dave Jiang wrote:
>>
>> On 10/1/26 6:20 AM, alucerop@amd.com wrote:
>>> From: Alejandro Lucero <alucerop@amd.com>
>>>
>>> PM initialization could not be necessary for some devices.
>>>
>>> Avoid checking for supplier PM initialization if so.
>>>
>>> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
>>> ---
>>>   drivers/base/core.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/base/core.c b/drivers/base/core.c
>>> index 4c0c373998a1..bf0513beafad 100644
>>> --- a/drivers/base/core.c
>>> +++ b/drivers/base/core.c
>>> @@ -840,7 +840,7 @@ struct device_link *device_link_add(struct device *consumer,
>>>        * SYNC_STATE_ONLY link, we don't check for reverse dependencies
>>>        * because it only affects sync_state() callbacks.
>>>        */
>>> -    if (!device_pm_initialized(supplier)
>>> +    if ((!device_pm_not_required(supplier) && !device_pm_initialized(supplier))
>>>           || (!(flags & DL_FLAG_SYNC_STATE_ONLY) &&
>>>             device_is_dependent(consumer, supplier))) {
>>>           link = NULL;
>> A no PM supplier can now be linked at any point: before device_add(), while it fails, or after device_del(). Maybe replace with a helper like this?
> 
> 
> I would say you can not use a device as supplier before device_add() happens for such a supplier, and if it does happen after device_del(), something is wrong with the caller.

Yes that would be a bug, and device_link_add() is suppose to catch it. For PM devices, the device_pm_initialized() test is the gate. The change you made skips that test for no PM device case. I had LLM created a test module for verification, attached.

Essentially the logic in this patch removed the check for 2 states that the original code used to block.
1. before device_add(supplier)
2. after device_del(supplier) 

> 
> 
> Your suggestion is likely making the code more legible, but it does not change the functionality I added. Does it? Not saying it would not help, but I can not understand your comment for suggesting it which seems to point to potential problems I did not see.
> 

It does.
- before device_add(supplier): this patch creates the link, and the helper refuses it.
- after device_add(supplier): both create it.
- after device_del(supplier), before last put_device: this patch creates the link, and the helper refuses it.

delete_region() or root decoder teardown can unregister the region while the endpoint is still bound. cxl_get_range_and_link() can still be called on a region that has already been through device_del().

Without the helper, it's possible where the PFx driver can device_link_add() a deleted region that is still around due to endpoint still holds a reference.

DJ


> 
>> static bool device_link_supplier_ready(struct device *supplier)
>> {
>>        /* no PM devices never enter dpm_list, so check registration directly */
>>        if (device_pm_not_required(supplier))
>>                return device_is_registered(supplier);
>>
>>        return device_pm_initialized(supplier);
>> }
>>
>> ...
>>
>>        if (!device_link_supplier_ready(supplier) ||
>>            (!(flags & DL_FLAG_SYNC_STATE_ONLY) &&
>>             device_is_dependent(consumer, supplier))) {
>>                link = NULL;
>>                goto out;
>>        }