On Thu, Feb 20, 2025 at 10:09:38AM -0800, William McVicker wrote:
> Hi Prashanth,
>
> On 02/17/2025, Prashanth K wrote:
> > Currently the USB gadget will be set as bus-powered based solely
> > on whether its bMaxPower is greater than 100mA, but this may miss
> > devices that may legitimately draw less than 100mA but still want
> > to report as bus-powered. Similarly during suspend & resume, USB
> > gadget is incorrectly marked as bus/self powered without checking
> > the bmAttributes field. Fix these by configuring the USB gadget
> > as self or bus powered based on bmAttributes, and explicitly set
> > it as bus-powered if it draws more than 100mA.
> >
> > Cc: stable@xxxxxxxxxxxxxxx
> > Fixes: 5e5caf4fa8d3 ("usb: gadget: composite: Inform controller driver of self-powered")
> > Signed-off-by: Prashanth K <prashanth.k@xxxxxxxxxxxxxxxx>
> > ---
> > Changes in v2:
> > - Didn't change anything from RFC.
> > - Link to RFC: https://lore.kernel.org/all/20250204105908.2255686-1-prashanth.k@xxxxxxxxxxxxxxxx/
> >
> > drivers/usb/gadget/composite.c | 16 +++++++++++-----
> > 1 file changed, 11 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> > index bdda8c74602d..1fb28bbf6c45 100644
> > --- a/drivers/usb/gadget/composite.c
> > +++ b/drivers/usb/gadget/composite.c
> > @@ -1050,10 +1050,11 @@ static int set_config(struct usb_composite_dev *cdev,
> > else
> > usb_gadget_set_remote_wakeup(gadget, 0);
> > done:
> > - if (power <= USB_SELF_POWER_VBUS_MAX_DRAW)
> > - usb_gadget_set_selfpowered(gadget);
> > - else
> > + if (power > USB_SELF_POWER_VBUS_MAX_DRAW ||
> > + !(c->bmAttributes & USB_CONFIG_ATT_SELFPOWER))
> > usb_gadget_clear_selfpowered(gadget);
> > + else
> > + usb_gadget_set_selfpowered(gadget);
> >
> > usb_gadget_vbus_draw(gadget, power);
> > if (result >= 0 && cdev->delayed_status)
> > @@ -2615,7 +2616,9 @@ void composite_suspend(struct usb_gadget *gadget)
> >
> > cdev->suspended = 1;
> >
> > - usb_gadget_set_selfpowered(gadget);
> > + if (cdev->config->bmAttributes & USB_CONFIG_ATT_SELFPOWER)
> > + usb_gadget_set_selfpowered(gadget);
>
> I'm hitting a null pointer derefence here on my Pixel 6 device on suspend. I
> haven't dug deep into it how we get here, but in my case `cdev->config` is
> NULL. This happens immediate after booting my device. I verified that just
> adding a NULL check fixes the issue and dwc3 gadget can successfully suspend.
This was just fixed in my tree today with this commit:
https://lore.kernel.org/r/20250220120314.3614330-1-m.szyprowski@xxxxxxxxxxx
Hope this helps,
greg k-h
