Hi Simon,

On Fri Jan 9, 2026 at 12:36 PM CET, Simon Glass wrote:
> Hi Markus,
>
> On Thu, 8 Jan 2026 at 04:17, Markus Schneider-Pargmann (TI.com)
> <[email protected]> wrote:
>>
>> Currently once a driver matched the compatible string of a device, other
>> drivers are ignored. If the first matching driver returns -ENODEV, no
>> other possibly matching drivers are iterated with that compatible of the
>> device. Instead the next compatible in the list of compatibles is
>> selected, assuming only one driver matches one compatible at a time.
>>
>> To be able to use the bind function to return -ENODEV and continue
>> matching other drivers with the same compatible, move the for loop a bit
>> to continue the for loop after -ENODEV was returned. The loop had to be
>> adjusted a bit to still support the 'drv' argument properly. Some
>> simplifications where done as well.
>>
>> This is required for ti-musb-host and ti-musb-peripheral which both
>> match on the same device but differ based on the dr_mode DT property.
>> Depending on this property, the driver is either UCLASS_USB or
>> UCLASS_USB_GADGET_GENERIc. By checking the DT property in the bind
>> function and returning -ENODEV the other driver can probe instead.
>>
>> Signed-off-by: Markus Schneider-Pargmann (TI.com) <[email protected]>
>> ---
>>  drivers/core/lists.c | 65 
>> ++++++++++++++++++++++++++--------------------------
>>  1 file changed, 33 insertions(+), 32 deletions(-)
>>
>
> Reviewed-by: Simon Glass <[email protected]>
>
> I would like to see some discussion about performance here. It seems
> that all you are doing here is not exiting the loop early, so the
> impact should be small?

See inline.

>
>> diff --git a/drivers/core/lists.c b/drivers/core/lists.c
>> index 
>> 9d1ca38212ee7f53b8894f964f096611c8ec20a5..3d9f9bc93954efdd624dddb1833f3a855c3c28de
>>  100644
>> --- a/drivers/core/lists.c
>> +++ b/drivers/core/lists.c
>> @@ -235,50 +235,51 @@ int lists_bind_fdt(struct udevice *parent, ofnode 
>> node, struct udevice **devp,
>>                 log_debug("   - attempt to match compatible string '%s'\n",
>>                           compat);
>>
>> -               id = NULL;
>>                 for (entry = driver; entry != driver + n_ents; entry++) {
>> +                       /* Search for drivers with matching drv or existing 
>> of_match */
>>                         if (drv) {
>>                                 if (drv != entry)
>>                                         continue;
>> -                               if (!entry->of_match)
>> -                                       break;
>> +                       } else if (!entry->of_match) {
>> +                               continue;
>>                         }
>> -                       ret = driver_check_compatible(entry->of_match, &id,
>> -                                                     compat);
>> -                       if (!ret)
>> -                               break;
>> -               }
>> -               if (entry == driver + n_ents)
>> -                       continue;
>>
>> -               if (pre_reloc_only) {
>> -                       if (!ofnode_pre_reloc(node) &&
>> -                           !(entry->flags & DM_FLAG_PRE_RELOC)) {
>> -                               log_debug("Skipping device 
>> pre-relocation\n");
>> -                               return 0;
>> +                       id = NULL;
>> +                       if (entry->of_match) {
>> +                               ret = 
>> driver_check_compatible(entry->of_match, &id,
>> +                                                             compat);
>> +                               if (ret)
>> +                                       continue;
>> +                               log_debug("   - found match at driver '%s' 
>> for '%s'\n",
>> +                                         entry->name, id->compatible);
>> +                       }
>> +
>> +                       if (pre_reloc_only) {
>> +                               if (!ofnode_pre_reloc(node) &&
>> +                                   !(entry->flags & DM_FLAG_PRE_RELOC)) {
>> +                                       log_debug("   - Skipping device 
>> pre-relocation\n");
>> +                                       return 0;
>> +                               }
>> +                       }
>> +
>> +                       ret = device_bind_with_driver_data(parent, entry, 
>> name,
>> +                                                          id ? id->data : 
>> 0, node,
>> +                                                          &dev);
>> +                       if (!drv && ret == -ENODEV) {
>> +                               log_debug("   - Driver '%s' refuses to 
>> bind\n", entry->name);
>> +                               continue;

So for performance this continue is the only new way to continue this
loop which wasn't done before this patch. And that only happens when the
driver returns -ENODEV. All other ways through this for loop below
return in some way. On error we return, on success we return 0.

Finding the right driver has the same continue and exit conditions as
before.

I will add a short paragraph to the commit message as well.

Best
Markus

Attachment: signature.asc
Description: PGP signature

Reply via email to