Re: [PATCH v2 3/6] dm: core: Support multiple drivers with same compatibles

["Markus Schneider-Pargmann" <msp@baylibre.com>]

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

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)
> <msp@baylibre.com> 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) <msp@baylibre.com>
>> ---
>>  drivers/core/lists.c | 65 ++++++++++++++++++++++++++--------------------------
>>  1 file changed, 33 insertions(+), 32 deletions(-)
>>
>
> Reviewed-by: Simon Glass <simon.glass@canonical.com>
>
> 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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 289 bytes --]