mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Arnd Bergmann" <arnd@arndb.de>
To: "James Clark" <jjc@jclark.com>,
	"Matthias Kaehlcke" <mka@chromium.org>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	"Doug Anderson" <dianders@chromium.org>,
	"Ravi Chandra Sadineni" <ravisadineni@chromium.org>,
	"Bartosz Golaszewski" <bartosz.golaszewski@oss.qualcomm.com>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	regressions@lists.linux.dev
Subject: Re: [PATCH] usb: misc: onboard_usb_dev: don't defer the USB device probe
Date: Fri, 30 Oct 2026 13:24:49 +0100	[thread overview]
Message-ID: <10ad94b3-66db-49b5-95c6-3ce9090f3278@app.fastmail.com> (raw)
In-Reply-To: <20261030114705.3159093-1-jjc@jclark.com>

On Fri, Oct 30, 2026, at 12:47, James Clark wrote:
> On a BeagleY-AI with BeagleBoard's device tree, kernels since commit
> f918e3804498 ("arm64: enable HAVE_SHARED_GPIOS for all ARM64 platforms")
> no longer boot to a usable system. The log repeats "hub 1-1:1.0: USB hub
> found" for the on-board TUSB8041 hub every few milliseconds, with no USB
> disconnect in between. The flood of uevents stalls udev and the boot ends
> in emergency mode. Reverting that commit makes the board boot normally
> again.
>
> The underlying problem is in onboard_usb_dev. Both peer hub nodes in this
> device tree carry the same reset-gpios, so the GPIO is now a shared one,
> and with CONFIG_GPIO_SHARED_PROXY=m (the default) the hub's platform
> probe is legitimately deferred until udev has loaded that module. The
> bootloader leaves the hub running, so the USB core enumerates it long
> before, and the USB device probe finds the platform device without
> driver data.

> At that point, onboard_dev_usbdev_probe() returns -EPROBE_DEFER. This is
> what causes the problem. The driver has generic_subclass set, so
> usb_probe_device() has already run the generic probe, which configured
> the device and bound the interface drivers. Each binding is a
> deferred-probe trigger, and the driver core re-triggers when a trigger
> happened during a probe that then deferred. So the device is re-probed
> straight away, reconfigured, its interfaces bound again, and so on for
> as long as the platform device remains unbound. This is not specific to
> shared GPIOs: it happens whenever the platform probe is deferred, and
> briefly whenever onboard_usb_dev is a module.

It sounds like the underlying problem should be fixed elsewhere,
possibly by preventing the deferred probe to be retried each time.
 
> +/*
> + * Attach a USB device that was bound by onboard_dev_usbdev_probe() 
> while its
> + * platform device was still unbound. The device lock serialises this 
> with
> + * the probe and with an unbind. This walks the USB bus, not the 
> driver's
> + * device list: unbinding a device holds its lock while it waits for 
> all
> + * references to its node on the driver's list to be dropped, so an 
> iterator
> + * over that list that takes the device lock would deadlock with it.
> + */
> +static int onboard_dev_attach_usbdev(struct usb_device *udev, void 
> *data)
> +{

This conceptually looks like a layering violation: no driver should
have to look through the list of devices like this.

> +	struct device *dev = &udev->dev;
> +	struct onboard_dev *onboard_dev;
> +	int err;
> +
> +	/* Unlocked pre-check, so as not to wait for every USB device's lock. 
> */
> +	if (READ_ONCE(dev->driver) != &onboard_dev_usbdev_driver.driver)
> +		return 0;
> +
> +	device_lock(dev);
> +
> +	if (dev->driver != &onboard_dev_usbdev_driver.driver ||
> +	    dev_get_drvdata(dev))
> +		goto out;
> +
> +	onboard_dev = _find_onboard_dev(dev);
> +	if (IS_ERR_OR_NULL(onboard_dev))
> +		goto out;

IS_ERR_OR_NULL() is almost always a bug, make sure you understan
the interface and what this can return. If the function can
actually return both error codes or NULL pointers, fix that
first.

     Arnd

  reply	other threads:[~2026-10-30 12:25 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-30 11:47 James Clark
2026-10-30 12:24 ` Arnd Bergmann [this message]
2026-10-30 13:07   ` James Clark
2026-10-30 13:34 ` Johan Hovold
2026-10-30 14:03   ` James Clark

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=10ad94b3-66db-49b5-95c6-3ce9090f3278@app.fastmail.com \
    --to=arnd@arndb.de \
    --cc=bartosz.golaszewski@oss.qualcomm.com \
    --cc=dianders@chromium.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jjc@jclark.com \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mka@chromium.org \
    --cc=ravisadineni@chromium.org \
    --cc=regressions@lists.linux.dev \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®