| Message ID | 1677129510-10283-3-git-send-email-quic_prashk@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fix vbus draw of dwc3 gadget | expand |
Hi Prashanth, On Thu, Feb 23, 2023 at 10:48:30AM +0530, Prashanth K wrote: > Currently we don't change the current value if device isn't in > configured state. But the battery charging specification says, > the device can draw upto 100mA of current if its in unconfigured Here you say spec says "up to" (BTW you have a typo) 100mA... > state. Hence add a Vbus_draw work in composite_resume to draw > 100mA if the device isn't configured. But here and in the patch you are calling the function to draw exactly 100mA. Consider the possibility that a gadget could be configured to draw less current than that or not anything at all, we should make sure to honor that as an absolute maximum. > Signed-off-by: Prashanth K <quic_prashk@quicinc.com> > --- > drivers/usb/gadget/composite.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c > index fa7dd6c..147d278 100644 > --- a/drivers/usb/gadget/composite.c > +++ b/drivers/usb/gadget/composite.c > @@ -2531,6 +2531,8 @@ void composite_resume(struct usb_gadget *gadget) > usb_gadget_clear_selfpowered(gadget); > > usb_gadget_vbus_draw(gadget, maxpower); > + } else { > + usb_gadget_vbus_draw(gadget, 100); Similar to the configured case, maybe you can perform a min() calculation against either or both the config->MaxPower or CONFIG_USB_GADGET_VBUS_DRAW. Thanks, Jack
On 23-02-23 01:03 pm, Jack Pham wrote: > Hi Prashanth, > > On Thu, Feb 23, 2023 at 10:48:30AM +0530, Prashanth K wrote: >> Currently we don't change the current value if device isn't in >> configured state. But the battery charging specification says, >> the device can draw upto 100mA of current if its in unconfigured > > Here you say spec says "up to" (BTW you have a typo) 100mA... > Will fix it in v2 >> state. Hence add a Vbus_draw work in composite_resume to draw >> 100mA if the device isn't configured. > > But here and in the patch you are calling the function to draw exactly > 100mA. Consider the possibility that a gadget could be configured to > draw less current than that or not anything at all, we should make sure > to honor that as an absolute maximum. That's right > >> Signed-off-by: Prashanth K <quic_prashk@quicinc.com> >> --- >> drivers/usb/gadget/composite.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c >> index fa7dd6c..147d278 100644 >> --- a/drivers/usb/gadget/composite.c >> +++ b/drivers/usb/gadget/composite.c >> @@ -2531,6 +2531,8 @@ void composite_resume(struct usb_gadget *gadget) >> usb_gadget_clear_selfpowered(gadget); >> >> usb_gadget_vbus_draw(gadget, maxpower); >> + } else { >> + usb_gadget_vbus_draw(gadget, 100); > > Similar to the configured case, maybe you can perform a min() > calculation against either or both the config->MaxPower or > CONFIG_USB_GADGET_VBUS_DRAW. > Thanks for the suggestion, will update it in v2 patch > Thanks, > Jack
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c index fa7dd6c..147d278 100644 --- a/drivers/usb/gadget/composite.c +++ b/drivers/usb/gadget/composite.c @@ -2531,6 +2531,8 @@ void composite_resume(struct usb_gadget *gadget) usb_gadget_clear_selfpowered(gadget); usb_gadget_vbus_draw(gadget, maxpower); + } else { + usb_gadget_vbus_draw(gadget, 100); } cdev->suspended = 0;
Currently we don't change the current value if device isn't in configured state. But the battery charging specification says, the device can draw upto 100mA of current if its in unconfigured state. Hence add a Vbus_draw work in composite_resume to draw 100mA if the device isn't configured. Signed-off-by: Prashanth K <quic_prashk@quicinc.com> --- drivers/usb/gadget/composite.c | 2 ++ 1 file changed, 2 insertions(+)