All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mattijs Korpershoek <mkorpershoek@kernel.org>
To: "Markus Schneider-Pargmann (TI.com)" <msp@baylibre.com>,
	u-boot@lists.denx.de,
	Mattijs Korpershoek <mkorpershoek@kernel.org>
Cc: Simon Glass <sjg@chromium.org>, Tom Rini <trini@konsulko.com>,
	Marek Vasut <marex@denx.de>,
	Andrew Goodbody <andrew.goodbody@linaro.org>,
	Kory Maincent <kory.maincent@bootlin.com>,
	Svyatoslav Ryhel <clamor95@gmail.com>,
	Christian Marangi <ansuelsmth@gmail.com>,
	Dinesh Maniyam <dinesh.maniyam@altera.com>,
	Heiko Schocher <hs@nabladev.com>,
	"Markus Schneider-Pargmann (TI.com)" <msp@baylibre.com>,
	Simon Glass <simon.glass@canonical.com>
Subject: Re: [PATCH v3 3/6] dm: core: Support multiple drivers with same compatibles
Date: Fri, 16 Jan 2026 11:50:39 +0100	[thread overview]
Message-ID: <87bjitesqo.fsf@kernel.org> (raw)
In-Reply-To: <20260114-topic-musb-probing-v2026-01-v3-3-ebb8d990b9df@baylibre.com>

Hi Markus,

Thank you for the patch.

On Wed, Jan 14, 2026 at 09:33, "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.

Nitpick: where -> were.

Please don't do any refactor/tweaks in this patch. drivers/core is
already difficult enough to review without formatting changes (to me at
least)

I see a couple below.

>
> The modification will only add additional loop iterations if -ENODEV is
> returned. Otherwise the exit and continue conditions for the loop stay
> the same and do not cause any additional iterations and should not
> impact performance.
>
> 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

Nitpick: UCLASS_USB_GADGET_GENERIC (not GENERIc)

> function and returning -ENODEV the other driver can probe instead.
>
> Reviewed-by: Simon Glass <simon.glass@canonical.com>
> Signed-off-by: Markus Schneider-Pargmann (TI.com) <msp@baylibre.com>
> ---
>  drivers/core/lists.c | 65 ++++++++++++++++++++++++++--------------------------
>  1 file changed, 33 insertions(+), 32 deletions(-)
>
> 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");


This was changed from :

  log_debug("Skipping device pre-relocation\n");

Please move log formatting to a separate, cosmetic patch. This way, this
patch stays purely functional, and simpler.

See: https://docs.u-boot.org/en/latest/develop/sending_patches.html#general-patch-submission-rules


> +					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);

Same here. We added "   -" to the log, making the diff overly
complicated.

Locally, discarded these message formatting changes and the diff was
much simpler.

Can we please re-submit by moving the formatting changes in a separate
patch?

Thanks
Mattijs



  reply	other threads:[~2026-01-16 10:50 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-14  8:33 [PATCH v3 0/6] dm: core: Support same compatible in host/gadget musb drivers Markus Schneider-Pargmann (TI.com)
2026-01-14  8:33 ` [PATCH v3 1/6] dm: core: lists_bind_fdt: Remove unused variable Markus Schneider-Pargmann (TI.com)
2026-01-16  8:22   ` Mattijs Korpershoek
2026-01-14  8:33 ` [PATCH v3 2/6] dm: core: lists_bind_fdt: Replace found variable Markus Schneider-Pargmann (TI.com)
2026-01-16  8:23   ` Mattijs Korpershoek
2026-01-14  8:33 ` [PATCH v3 3/6] dm: core: Support multiple drivers with same compatibles Markus Schneider-Pargmann (TI.com)
2026-01-16 10:50   ` Mattijs Korpershoek [this message]
2026-01-21 16:45     ` Maniyam, Dinesh
2026-01-21 17:00       ` Markus Schneider-Pargmann
2026-01-23 11:24         ` Maniyam, Dinesh
2026-01-14  8:33 ` [PATCH v3 4/6] test: dm: Add compatible multimatch test Markus Schneider-Pargmann (TI.com)
2026-01-16  8:35   ` Mattijs Korpershoek
2026-01-14  8:33 ` [PATCH v3 5/6] usb: musb-new: Relative ctrl_mod address parsing Markus Schneider-Pargmann (TI.com)
2026-01-16  8:41   ` Mattijs Korpershoek
2026-01-16 10:14     ` Markus Schneider-Pargmann
2026-01-14  8:33 ` [PATCH v3 6/6] usb: musb-new: Add compatibles for ti,musb-am33xx Markus Schneider-Pargmann (TI.com)
2026-01-16  8:44   ` Mattijs Korpershoek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87bjitesqo.fsf@kernel.org \
    --to=mkorpershoek@kernel.org \
    --cc=andrew.goodbody@linaro.org \
    --cc=ansuelsmth@gmail.com \
    --cc=clamor95@gmail.com \
    --cc=dinesh.maniyam@altera.com \
    --cc=hs@nabladev.com \
    --cc=kory.maincent@bootlin.com \
    --cc=marex@denx.de \
    --cc=msp@baylibre.com \
    --cc=simon.glass@canonical.com \
    --cc=sjg@chromium.org \
    --cc=trini@konsulko.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.