Re: [PATCH v2] usb: gadget: Set self-powered based on MaxPower and bmAttributes

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

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@vger.kernel.org
> > Fixes: 5e5caf4fa8d3 ("usb: gadget: composite: Inform controller driver of self-powered")
> > Signed-off-by: Prashanth K <prashanth.k@oss.qualcomm.com>
> > ---
> > Changes in v2:
> > - Didn't change anything from RFC.
> > - Link to RFC: https://lore.kernel.org/all/20250204105908.2255686-1-prashanth.k@oss.qualcomm.com/
> > 
> >  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@samsung.com

Hope this helps,

greg k-h