Linux USB
 help / color / mirror / Atom feed
From: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
To: Elson Serrao <elson.serrao@oss.qualcomm.com>
Cc: Thinh Nguyen <Thinh.Nguyen@synopsys.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"jack.pham@oss.qualcomm.com" <jack.pham@oss.qualcomm.com>,
	"wesley.cheng@oss.qualcomm.com" <wesley.cheng@oss.qualcomm.com>
Subject: Re: [PATCH] usb: dwc3: avoid probe deferral when USB power supply is not available
Date: Thu, 7 May 2026 23:02:23 +0000	[thread overview]
Message-ID: <af0WXtFvDwbI9-9-@vbox> (raw)
In-Reply-To: <4db536fd-9b20-49ec-a5f3-1fa3f2876c1e@oss.qualcomm.com>

On Wed, May 06, 2026, Elson Serrao wrote:
> 
> 
> On 5/1/2026 5:55 PM, Thinh Nguyen wrote:
> > On Tue, Apr 07, 2026, Elson Serrao wrote:
> >> The dwc3 driver currently defers probe if the USB power supply is not yet
> >> registered. On some platforms, even though charging and power supply
> >> functionality is available during normal operation, there may exist
> >> minimal booting modes (such as recovery or diagnostic environments) where
> >> the relevant USB power supply device is not registered. In such cases,
> >> probe deferral prevents USB gadget operation entirely.
> >>
> >> USB data functionality for basic operation does not inherently depend on
> >> the power supply framework, which is only required for enforcing VBUS
> >> current control. The configured VBUS current limit is typically enforced
> >> through the charger or PMIC power path. When charging functionality is
> >> unavailable, applying a current limit has no practical effect, reducing
> >> the benefit of strict probe-time enforcement in these environments.
> >>
> >> Instead of deferring probe, register a power supply notifier when the
> >> USB power supply is not yet available. Cache the requested VBUS current
> >> limit and apply it once the matching power supply becomes available, as
> >> notified through the registered callback.
> >>
> > 
> > This gets a bit cumbersome now that we need to consider some corner
> > cases around the notifier callback.
> > 
> >> Signed-off-by: Elson Serrao <elson.serrao@oss.qualcomm.com>
> >> ---
> >>  drivers/usb/dwc3/core.c   | 82 ++++++++++++++++++++++++++++++++-------
> >>  drivers/usb/dwc3/core.h   |  4 ++
> >>  drivers/usb/dwc3/gadget.c | 10 ++++-
> >>  3 files changed, 80 insertions(+), 16 deletions(-)
> >>
> >> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> >> index 161a4d58b2ce..20df0b287623 100644
> >> --- a/drivers/usb/dwc3/core.c
> >> +++ b/drivers/usb/dwc3/core.c
> >> @@ -2167,24 +2167,72 @@ static void dwc3_vbus_draw_work(struct work_struct *work)
> >>  	if (ret < 0)
> >>  		dev_dbg(dwc->dev, "Error (%d) setting vbus draw (%d mA)\n",
> >>  			ret, dwc->current_limit);
> >> +
> >> +	/* Unregister the psy notifier now that we have the power_supply reference */
> >> +	if (dwc->psy_nb.notifier_call) {
> > 
> > Is it possible for dwc3_core_remove() to happen here? If so, should we
> > do something about it?
> > 
> 
> Hi Thinh
> 
> Thanks for the review.
> 
> Yes dwc3_core_remove() could race with this path.
> 
> To simplify things, I’m planning to unregister the notifier only
> from dwc3_core_remove(), so we don’t need to handle this case here
> and the notifier lifetime remains tied to the device lifecycle.
> 
> Let me know if you’d prefer a different approach.
> 

That's fine to me. Just make sure to return early if the power supply is
registered.

<snip>

> >> +
> >> +	dwc->usb_psy = power_supply_get_by_name(dwc->usb_psy_name);
> >> +	if (!dwc->usb_psy) {
> > 
> > Is it possible for the power supply to register here?
> > 
> 
> The power_supply framework introduces a ~10 ms delay [1]
> before invoking notifiers after registration. So for the race described
> above to occur, our probe would need to stall for more than that
> duration between the initial power_supply_get_by_name() call
> and notifier registration, which seems highly unlikely under normal
> conditions.
> 
> That said, there is still a theoretical window where the power
> supply could register in that gap. If you think it's worth being
> defensive here, we could re-check power_supply_get_by_name() after
> registering the notifier and handle the case accordingly.
> 
> [1] https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/power/supply/power_supply_core.c?h=v7.1-rc2*n40__;Iw!!A4F2R9G_pg!dUiPW6mibrvsk4uGO4MnGVg3R1zR3EmxxIROrw4N-ytHZq7N9q-V6irNAWrBrolUR2HABsAGSQoMPzGnEGQsvWdhzWzcVHOU$ 
> 

For my own sanity, can we have that extra check? Otherwise, every time I
scan through this I would need to recall why it wasn't needed.

Thanks,
Thinh

  reply	other threads:[~2026-05-07 23:02 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-07 23:24 [PATCH] usb: dwc3: avoid probe deferral when USB power supply is not available Elson Serrao
2026-04-29 17:52 ` Elson Serrao
2026-04-30  7:43   ` Thinh Nguyen
2026-05-02  0:55 ` Thinh Nguyen
2026-05-06 22:59   ` Elson Serrao
2026-05-07 23:02     ` Thinh Nguyen [this message]
2026-05-08  0:28       ` Elson Serrao

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=af0WXtFvDwbI9-9-@vbox \
    --to=thinh.nguyen@synopsys.com \
    --cc=elson.serrao@oss.qualcomm.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jack.pham@oss.qualcomm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=wesley.cheng@oss.qualcomm.com \
    /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