Re: [PATCH v1 1/4] driver core: Rely on supplier driver binding at link creation

["Lucero Palau, Alejandro" <alejandro.lucero-palau@amd.com>]

On 23/09/2026 09:49, Lucero Palau, Alejandro wrote:
>
> On 22/09/2026 22:39, Maxime Chevallier wrote:
>> Hi,
>>
>> On 9/21/26 21:12, alucerop@amd.com wrote:
>>> From: Alejandro Lucero <alucerop@amd.com>
>>>
>>> Differentiate between driver binding from PM dependency when a device
>>> link is created. Rely on the device being bound to a driver for 
>>> validating
>>> the supplier as some device drivers could not support PM.
>>>
>>> Check for supplier's PM state only if consumer specifies PM_RUNTIME 
>>> flag.
>
>
> Hi Maxime,
>
>
>> Hmmm this patch seems to have broken pretty much all the boards I'm 
>> running that
>> boot with DT. I'm getting logs such as :
>>
>> platform 16100000.serial: Failed to create device link (0x124) with 
>> supplier
>>    16000400.clock-controller
>>
>> Some are just hanging at "Starting kernel..." from u-boot.
>>
>> Then boards go silent as the uart dies, and I'm using that uart to 
>> access the
>> device's console :(
>>
>> Found with a WIP stmmac runner for netdev CI.
>>
>> Unfortunately I don't have much logs to share, as this just prevents 
>> the boards
>> from booting :( With this patch reverted, all boards boot fine
>
>
> It is obvious I did not understand well the implications ...
>
>
> I think the supplier device lock could be the problem behind the 
> hanging and how the supplier initialization is checked now based on 
> the driver bound behind the console issue ...
>
>
> I have a couple of embedded boards to play with so I will try to 
> reproduce the problem for getting more info. FWIW, all was fine with 
> the server I tested this, no device link errors at all.
>

I have to amend my words ... the server had not exactly the same code 
:-)  I have to copy things in and out due to security measures and 
sometimes is hard to have all synced.


I think the sashiko reports could be a good start to fixing this.


Apologies for the any inconvenient.

Thanks,

Alejandro.


>
> Thank you for testing it!
>
>
>
>>
>> Maxime
>>
>>> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
>>> ---
>>>   drivers/base/core.c | 27 +++++++++++++++++++--------
>>>   1 file changed, 19 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/base/core.c b/drivers/base/core.c
>>> index 4c0c373998a1..eb6d87e35d76 100644
>>> --- a/drivers/base/core.c
>>> +++ b/drivers/base/core.c
>>> @@ -834,15 +834,26 @@ struct device_link *device_link_add(struct 
>>> device *consumer,
>>>       device_pm_lock();
>>>         /*
>>> -     * If the supplier has not been fully registered yet or there is a
>>> -     * reverse (non-SYNC_STATE_ONLY) dependency between the 
>>> consumer and
>>> -     * the supplier already in the graph, return NULL. If the link 
>>> is a
>>> -     * SYNC_STATE_ONLY link, we don't check for reverse dependencies
>>> -     * because it only affects sync_state() callbacks.
>>> +     * If the supplier has not been fully registered yet with a driver
>>> +     * return NULL.
>>>        */
>>> -    if (!device_pm_initialized(supplier)
>>> -        || (!(flags & DL_FLAG_SYNC_STATE_ONLY) &&
>>> -          device_is_dependent(consumer, supplier))) {
>>> +    scoped_guard(device, supplier) {
>>> +        if (!device_is_bound(supplier)) {
>>> +            link = NULL;
>>> +            goto out;
>>> +        }
>>> +    }
>>> +    /*
>>> +     * If consumer asks for PM to use the link and the supplier has 
>>> not
>>> +     * PM initialized, or if there is a reverse (non-SYNC_STATE_ONLY)
>>> +     * dependency between the consumer and the supplier already in the
>>> +     * graph, return NULL. If the link is a SYNC_STATE_ONLY link, we
>>> +     * don't check for reverse dependencies because it only affects
>>> +     * sync_state() callbacks.
>>> +     */
>>> +    if (((flags & DL_FLAG_PM_RUNTIME) && 
>>> !device_pm_initialized(supplier)) ||
>>> +        (!(flags & DL_FLAG_SYNC_STATE_ONLY) &&
>>> +         device_is_dependent(consumer, supplier))) {
>>>           link = NULL;
>>>           goto out;
>>>       }