Add vbus draw support to DWC3
Subsystems (2)
cover Add vbus draw support to DWC3
Some devices are connected to standard downstream ports (SDP) and draw current from them. The current rates are defined in the BC1.2 specification, and highlights the different charge rates depending on the device state. The DWC3 gadget does not currently have a mechanism to notify external drivers about how much current can be drawn.
The current rates are notified by the USB gadget layer, and the DWC3 gadget will propagate this potentially to external charger drivers. Also, the USB gadget needs to be fixed to only allow 100mA current draw when receiving a bus reset from the host, as the BC1.2 specification states that this is the max current draw possible when in the connected and unconfigured state.
Wesley Cheng (3):
usb: dwc3: gadget: Introduce a DWC3 VBUS draw callback
usb: gadget: composite: Split composite reset and disconnect
usb: gadget: configfs: Add a specific configFS reset callback
drivers/usb/dwc3/gadget.c | 11 +++++++++++ drivers/usb/gadget/composite.c | 21 +++++++++++++++++++-- drivers/usb/gadget/configfs.c | 24 +++++++++++++++++++++++- include/linux/usb/composite.h | 2 ++ 4 files changed, 55 insertions(+), 3 deletions(-)
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH 1/3 usb: dwc3: gadget: Introduce a DWC3 VBUS draw callback
Some devices support charging while in device mode. In these situations, the USB gadget will notify the DWC3 gadget driver to modify the current based on the enumeration and device state. The usb_phy_set_power() API will allow external charger entities to adjust the charge current through the notifier block.
--- drivers/usb/dwc3/gadget.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
drivers/usb/dwc3/gadget.c +11 -0
@@ -2387,6 +2387,16 @@ static void dwc3_gadget_set_speed(struct usb_gadget *g,spin_unlock_irqrestore (&dwc->lock, flags); } +static int dwc3_gadget_vbus_draw (struct usb_gadget *g,unsigned int mA) +{ +struct dwc3 *dwc =gadget_to_dwc (g); + +if (dwc->usb2_phy) +return usb_phy_set_power (dwc->usb2_phy, mA); + +return 0 ; +} +static const struct usb_gadget_ops dwc3_gadget_ops = { .get_frame = dwc3_gadget_get_frame, .wakeup = dwc3_gadget_wakeup,
@@ -2396,6 +2406,7 @@ static const struct usb_gadget_ops dwc3_gadget_ops = { .udc_stop = dwc3_gadget_stop, .udc_set_speed = dwc3_gadget_set_speed, .get_config_params = dwc3_gadget_config_params, + .vbus_draw = dwc3_gadget_vbus_draw, };/* -------------------------------------------------------------------------- */
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH 2/3 usb: gadget: composite: Split composite reset and disconnect
In reply to Wesley ChengAdd a specific composite reset API to differentiate between disconnect and reset events. This is needed for adjusting the current draw accordingly based on the USB battery charging specification. The device is only allowed to draw the 500/900 mA (HS/SS) while in the CONFIGURED state, and only 100 mA in the connected and UNCONFIGURED state.
--- drivers/usb/gadget/composite.c | 21 +++++++++++++++++++-- include/linux/usb/composite.h | 2 ++ 2 files changed, 21 insertions(+), 2 deletions(-)
drivers/usb/gadget/composite.c +19 -2
@@ -2036,7 +2036,7 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)return value; } -void composite_disconnect (struct usb_gadget *gadget) +static void __composite_disconnect (struct usb_gadget *gadget) {struct usb_composite_dev *cdev =get_gadget_data (gadget);unsigned long flags;
@@ -2053,6 +2053,23 @@ void composite_disconnect(struct usb_gadget *gadget)spin_unlock_irqrestore (&cdev->lock, flags); } +void composite_disconnect (struct usb_gadget *gadget) +{ +usb_gadget_vbus_draw (gadget,0 ); +__composite_disconnect (gadget); +} + +void composite_reset (struct usb_gadget *gadget) +{ +/* +* Section 1.4.13 Standard Downstream Port of the USB battery charging +* specification v1.2 states that a device connected on a SDP shall only +* draw at max 100mA while in a connected, but unconfigured state. +*/ +usb_gadget_vbus_draw (gadget,100 ); +__composite_disconnect (gadget); +} +/*-------------------------------------------------------------------------*/ static ssize_t suspended_show(struct device *dev, struct device_attribute *attr,
@@ -2373,7 +2390,7 @@ static const struct usb_gadget_driver composite_driver_template = { .unbind = composite_unbind, .setup = composite_setup, - .reset = composite_disconnect, + .reset = composite_reset, .disconnect = composite_disconnect, .suspend = composite_suspend,
include/linux/usb/composite.h +2 -0
@@ -525,6 +525,8 @@ extern struct usb_string *usb_gstrings_attach(struct usb_composite_dev *cdev,extern int usb_string_ids_n (struct usb_composite_dev *c,unsigned n);extern void composite_disconnect (struct usb_gadget *gadget); +extern void composite_reset (struct usb_gadget *gadget); +extern int composite_setup (struct usb_gadget *gadget,const struct usb_ctrlrequest *ctrl);extern void composite_suspend (struct usb_gadget *gadget);
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH 2/3] usb: gadget: composite: Split composite reset and disconnect
Hi,
drivers/usb/gadget/composite.c void composite_disconnect(struct usb_gadget *gadget) · line 2067
Quoted 6 lines · Wesley Cheng
drivers/usb/gadget/composite.c +6
-0+void composite_reset (struct usb_gadget *gadget) +{ +/* +* Section 1.4.13 Standard Downstream Port of the USB battery charging +* specification v1.2 states that a device connected on a SDP shall only +* draw at max 100mA while in a connected, but unconfigured state.
The requirements are different, though. I think OTG spec has some extra requirements where only 8mA can be drawn max. You need to check for the otg flag. Moreover, USB3+ spec has units of 150mA meaning the device can't draw 100mA (IIRC).
Signature · 1 line
balbi
Thinh Nguyen
maintainerRe: [PATCH 2/3] usb: gadget: composite: Split composite reset and disconnect
Hi Wesley,
Quoted 14 lines · Felipe Balbi
Hi,
drivers/usb/gadget/composite.c void composite_disconnect(struct usb_gadget *gadget) · line 2067
Quoted 6 lines · Wesley Cheng
drivers/usb/gadget/composite.c +6
-0+void composite_reset (struct usb_gadget *gadget) +{ +/* +* Section 1.4.13 Standard Downstream Port of the USB battery charging +* specification v1.2 states that a device connected on a SDP shall only +* draw at max 100mA while in a connected, but unconfigured state. The requirements are different, though. I think OTG spec has some extra requirements where only 8mA can be drawn max. You need to check for the otg flag. Moreover, USB3+ spec has units of 150mA meaning the device can't draw 100mA (IIRC).
We see issue with this patch series. For our device running at SSP, the device couldn't recover from a port reset and remained in eSS.Inactive state.
This patch series is already in Greg's usb-testing. Please review and help fix it.
We can see the failure once the patch "usb: gadget: configfs: Add a specific configFS reset callback" is introduced.
Thanks,
Thinh
Re: [PATCH 2/3] usb: gadget: composite: Split composite reset and disconnect
Hi Thinh,
Quoted 22 lines · Thinh Nguyen
Hi Wesley,
Quoted 14 lines · Felipe Balbi
Hi,
drivers/usb/gadget/composite.c void composite_disconnect(struct usb_gadget *gadget) · line 2067
Quoted 6 lines · Wesley Cheng
drivers/usb/gadget/composite.c +6
-0+void composite_reset (struct usb_gadget *gadget) +{ +/* +* Section 1.4.13 Standard Downstream Port of the USB battery charging +* specification v1.2 states that a device connected on a SDP shall only +* draw at max 100mA while in a connected, but unconfigured state. The requirements are different, though. I think OTG spec has some extra requirements where only 8mA can be drawn max. You need to check for the otg flag. Moreover, USB3+ spec has units of 150mA meaning the device can't draw 100mA (IIRC).
We see issue with this patch series. For our device running at SSP, the device couldn't recover from a port reset and remained in eSS.Inactive state.
Quoted 5 lines
This patch series is already in Greg's usb-testing. Please review and help fix it.
We can see the failure once the patch "usb: gadget: configfs: Add a specific configFS reset callback" is introduced.
Hmm. Does your device use a legacy USB2 PHY (not generic PHY) driver, and does it implement the usb_phy_set_power() callback? Because otherwise this new configfs_composite_reset() callback would not have changed from the original behavior since the newly introduced (in patch 1/3) dwc3_gadget_vbus_draw() callback would simply be a no-op if dwc->usb2_phy is not present.
If it does turn out to be something with your PHY driver's set_power(), it's still puzzling since it's directed to only the usb2_phy, so I'm curious how telling it to draw 100mA could affect SSP operation at all.
Thanks,
Jack
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Thinh Nguyen
maintainerRe: [PATCH 2/3] usb: gadget: composite: Split composite reset and disconnect
Quoted 36 lines · Jack Pham
Hi Thinh,
Quoted 21 lines · Thinh Nguyen
Hi Wesley,
Quoted 14 lines · Felipe Balbi
Hi,
drivers/usb/gadget/composite.c void composite_disconnect(struct usb_gadget *gadget) · line 2067
Quoted 6 lines · Wesley Cheng
drivers/usb/gadget/composite.c +6
-0+void composite_reset (struct usb_gadget *gadget) +{ +/* +* Section 1.4.13 Standard Downstream Port of the USB battery charging +* specification v1.2 states that a device connected on a SDP shall only +* draw at max 100mA while in a connected, but unconfigured state. The requirements are different, though. I think OTG spec has some extra requirements where only 8mA can be drawn max. You need to check for the otg flag. Moreover, USB3+ spec has units of 150mA meaning the device can't draw 100mA (IIRC).
We see issue with this patch series. For our device running at SSP, the device couldn't recover from a port reset and remained in eSS.Inactive state.
Quoted 5 lines
This patch series is already in Greg's usb-testing. Please review and help fix it.
We can see the failure once the patch "usb: gadget: configfs: Add a specific configFS reset callback" is introduced.
Hmm. Does your device use a legacy USB2 PHY (not generic PHY) driver, and does it implement the
usb_phy_set_power()callback? Because otherwise this newconfigfs_composite_reset()callback would not have changed from the original behavior since the newly introduced (in patch 1/3)dwc3_gadget_vbus_draw()callback would simply be a no-op ifdwc->usb2_phyis not present.
Quoted 6 lines
If it does turn out to be something with your PHY driver's
set_power(), it's still puzzling since it's directed to only theusb2_phy, so I'm curious how telling it to draw 100mA could affect SSP operation at all.Thanks,
Jack
So, I ran some more tests. It seems like this new change affects some timing in my setup that triggers this failure. I tried to add some printouts to look into it further, but somehow that reduces the failure rate significantly. This doesn't seem related to power drawing as it doesn't update usb2_phy for SSP as you said.
After switching my HW setup, the problem seems to go away. I guess we can ignore this since the code path looks to be the same as previous.
BR,
Thinh
[email protected]
maintainerRe: [PATCH 2/3] usb: gadget: composite: Split composite reset and disconnect
In reply to Thinh NguyenQuoted 18 lines · Thinh Nguyen
Hi Wesley,
Quoted 14 lines · Felipe Balbi
Hi,
drivers/usb/gadget/composite.c void composite_disconnect(struct usb_gadget *gadget) · line 2067
Quoted 6 lines · Wesley Cheng
drivers/usb/gadget/composite.c +6
-0+void composite_reset (struct usb_gadget *gadget) +{ +/* +* Section 1.4.13 Standard Downstream Port of the USB battery charging +* specification v1.2 states that a device connected on a SDP shall only +* draw at max 100mA while in a connected, but unconfigured state. The requirements are different, though. I think OTG spec has some extra requirements where only 8mA can be drawn max. You need to check for the otg flag. Moreover, USB3+ spec has units of 150mA meaning the device can't draw 100mA (IIRC).
Quoted 6 lines
We see issue with this patch series. For our device running at SSP, the device couldn't recover from a port reset and remained in eSS.Inactive state.
This patch series is already in Greg's usb-testing. Please review and help fix it.
Should I just revert this? I'll be glad to drop it.
thanks,
greg k-h
Thinh Nguyen
maintainerRe: [PATCH 2/3] usb: gadget: composite: Split composite reset and disconnect
Quoted 22 lines · [email protected] wrote:
Quoted 21 lines · Thinh Nguyen
Hi Wesley,
Quoted 14 lines · Felipe Balbi
Hi,
drivers/usb/gadget/composite.c void composite_disconnect(struct usb_gadget *gadget) · line 2067
Quoted 6 lines · Wesley Cheng
drivers/usb/gadget/composite.c +6
-0+void composite_reset (struct usb_gadget *gadget) +{ +/* +* Section 1.4.13 Standard Downstream Port of the USB battery charging +* specification v1.2 states that a device connected on a SDP shall only +* draw at max 100mA while in a connected, but unconfigured state. The requirements are different, though. I think OTG spec has some extra requirements where only 8mA can be drawn max. You need to check for the otg flag. Moreover, USB3+ spec has units of 150mA meaning the device can't draw 100mA (IIRC).
We see issue with this patch series. For our device running at SSP, the device couldn't recover from a port reset and remained in eSS.Inactive state.
Quoted 7 lines
Quoted 2 lines
This patch series is already in Greg's usb-testing. Please review and help fix it.
Should I just revert this? I'll be glad to drop it.
thanks,
greg k-h
Unless there's some other issue, let's not do so as it seems to be related to my HW stability than any SW issue. Sorry for the noise.
BR,
Thinh
PATCH 3/3 usb: gadget: configfs: Add a specific configFS reset callback
In reply to Wesley ChengIn order for configFS based USB gadgets to set the proper charge current for bus reset scenarios, expose a separate reset callback to set the current to 100mA based on the USB battery charging specification.
--- drivers/usb/gadget/configfs.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-)
drivers/usb/gadget/configfs.c +23 -1
@@ -1481,6 +1481,28 @@ static void configfs_composite_disconnect(struct usb_gadget *gadget)spin_unlock_irqrestore (&gi->spinlock, flags); } +static void configfs_composite_reset (struct usb_gadget *gadget) +{ +struct usb_composite_dev *cdev; +struct gadget_info *gi; +unsigned long flags; + + cdev =get_gadget_data (gadget); +if (!cdev) +return ; + + gi = container_of(cdev, struct gadget_info, cdev); +spin_lock_irqsave (&gi->spinlock, flags); + cdev =get_gadget_data (gadget); +if (!cdev || gi->unbind) { +spin_unlock_irqrestore (&gi->spinlock, flags); +return ; + } + +composite_reset (gadget); +spin_unlock_irqrestore (&gi->spinlock, flags); +} +static void configfs_composite_suspend (struct usb_gadget *gadget) {struct usb_composite_dev *cdev;
@@ -1530,7 +1552,7 @@ static const struct usb_gadget_driver configfs_driver_template = { .unbind = configfs_composite_unbind, .setup = configfs_composite_setup, - .reset = configfs_composite_disconnect, + .reset = configfs_composite_reset, .disconnect = configfs_composite_disconnect, .suspend = configfs_composite_suspend,
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Greg KH
maintainerRe: [PATCH 3/3] usb: gadget: configfs: Add a specific configFS reset callback
Quoted 56 lines · Wesley Cheng
In order for configFS based USB gadgets to set the proper charge current for bus reset scenarios, expose a separate reset callback to set the current to 100mA based on the USB battery charging specification.
Reviewed-by Peter Chen <[email protected]>Signed-off-by Wesley Cheng <[email protected]>--- drivers/usb/gadget/configfs.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-)drivers/usb/gadget/configfs.c +23
-1@@ -1481,6 +1481,28 @@ static void configfs_composite_disconnect(struct usb_gadget *gadget)spin_unlock_irqrestore (&gi->spinlock, flags); } +static void configfs_composite_reset (struct usb_gadget *gadget) +{ +struct usb_composite_dev *cdev; +struct gadget_info *gi; +unsigned long flags; + + cdev =get_gadget_data (gadget); +if (!cdev) +return ; + + gi = container_of(cdev, struct gadget_info, cdev); +spin_lock_irqsave (&gi->spinlock, flags); + cdev =get_gadget_data (gadget); +if (!cdev || gi->unbind) { +spin_unlock_irqrestore (&gi->spinlock, flags); +return ; + } + +composite_reset (gadget); +spin_unlock_irqrestore (&gi->spinlock, flags); +} +static void configfs_composite_suspend (struct usb_gadget *gadget) {struct usb_composite_dev *cdev;@@ -1530,7 +1552,7 @@ static const struct usb_gadget_driver configfs_driver_template = { .unbind = configfs_composite_unbind, .setup = configfs_composite_setup, - .reset = configfs_composite_disconnect, + .reset = configfs_composite_reset, .disconnect = configfs_composite_disconnect, .suspend = configfs_composite_suspend,Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
So this changes the existing userspace functionality? What will break because of this now unexpected change?
thanks,
greg k-h
Re: [PATCH 3/3] usb: gadget: configfs: Add a specific configFS reset callback
Quoted 61 lines · Greg KH
Quoted 56 lines · Wesley Cheng
In order for configFS based USB gadgets to set the proper charge current for bus reset scenarios, expose a separate reset callback to set the current to 100mA based on the USB battery charging specification.
Reviewed-by Peter Chen <[email protected]>Signed-off-by Wesley Cheng <[email protected]>--- drivers/usb/gadget/configfs.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-)drivers/usb/gadget/configfs.c +23
-1@@ -1481,6 +1481,28 @@ static void configfs_composite_disconnect(struct usb_gadget *gadget)spin_unlock_irqrestore (&gi->spinlock, flags); } +static void configfs_composite_reset (struct usb_gadget *gadget) +{ +struct usb_composite_dev *cdev; +struct gadget_info *gi; +unsigned long flags; + + cdev =get_gadget_data (gadget); +if (!cdev) +return ; + + gi = container_of(cdev, struct gadget_info, cdev); +spin_lock_irqsave (&gi->spinlock, flags); + cdev =get_gadget_data (gadget); +if (!cdev || gi->unbind) { +spin_unlock_irqrestore (&gi->spinlock, flags); +return ; + } + +composite_reset (gadget); +spin_unlock_irqrestore (&gi->spinlock, flags); +} +static void configfs_composite_suspend (struct usb_gadget *gadget) {struct usb_composite_dev *cdev;@@ -1530,7 +1552,7 @@ static const struct usb_gadget_driver configfs_driver_template = { .unbind = configfs_composite_unbind, .setup = configfs_composite_setup, - .reset = configfs_composite_disconnect, + .reset = configfs_composite_reset, .disconnect = configfs_composite_disconnect, .suspend = configfs_composite_suspend,Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative ProjectSo this changes the existing userspace functionality? What will break because of this now unexpected change?
Quoted 4 lines
thanks,
greg k-h
Hi Greg,
Happy new years! This wouldn't affect the userspace interaction with configFS, as this is modifying the reset callback for the UDC core. The reset callback is only executed during usb_gadget_udc_reset(), which is specifically run when vendor UDC drivers (i.e. DWC3 gadget) receive a USB bus reset interrupt. This is similar to the composite.c patch, because for configFS based gadgets, they do not directly register the USB composite ops and have their own routines.
Thanks
Wesley Cheng
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project