usb: dwc3: gadget: Skip resizing EP's TX FIFO if already resized
Subsystems (3)
PATCH 1/2 usb: dwc3: gadget: Skip resizing EP's TX FIFO if already resized
Some functions may dynamically enable and disable their endpoints regularly throughout their operation, particularly when Set Interface is employed to switch between Alternate Settings. For instance the UAC2 function has its respective endpoints for playback & capture associated with AltSetting 1, in which case those endpoints would not get enabled until the host activates the AltSetting. And they conversely become disabled when the interfaces' AltSetting 0 is chosen.
With the DWC3 FIFO resizing algorithm recently added, every usb_ep_enable() call results in a call to resize that EP's TXFIFO, but if the same endpoint is enabled again and again, this incorrectly leads to FIFO RAM allocation exhaustion as the mechanism did not account for the possibility that endpoints can be re-enabled many times.
Example log splat:
dwc3 a600000.dwc3: Fifosize(3717) > RAM size(3462) ep3in depth:217973127 configfs-gadget gadget: u_audio_start_capture:521 Error! dwc3 a600000.dwc3: request 000000000be13e18 was not queued to ep3in
Add another bit DWC3_EP_TXFIFO_RESIZED to dep->flags to keep track of whether an EP had already been resized in the current configuration. If so, bail out of dwc3_gadget_resize_tx_fifos() to avoid the calculation error resulting from accumulating the EP's FIFO depth repeatedly. This flag is retained across multiple ep_disable() and ep_enable() calls and is cleared when GTXFIFOSIZn is reset in dwc3_gadget_clear_tx_fifos() upon receiving the next Set Config.
---
v3: Fixed macro definition alignment; added Thinh's reviewed-by.
v2: Added explicit flag to dep->flags and check that instead of directly
reading the GTXFIFOSIZn register.
drivers/usb/dwc3/core.h | 1 + drivers/usb/dwc3/gadget.c | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-)
drivers/usb/dwc3/core.h +1 -0
@@ -723,6 +723,7 @@ struct dwc3_ep {#define DWC3_EP_FORCE_RESTART_STREAM BIT(9)#define DWC3_EP_FIRST_STREAM_PRIMED BIT(10)#define DWC3_EP_PENDING_CLEAR_STALL BIT(11) +#define DWC3_EP_TXFIFO_RESIZED BIT(12)/* This last one is specific to EP0 */ #define DWC3_EP0_DIR_IN BIT(31)
drivers/usb/dwc3/gadget.c +7 -1
@@ -702,6 +702,7 @@ void dwc3_gadget_clear_tx_fifos(struct dwc3 *dwc)DWC31_GTXFIFOSIZ_TXFRAMNUM ;dwc3_writel (dwc->regs,DWC3_GTXFIFOSIZ (num >>1 ), size); + dep->flags &= ~DWC3_EP_TXFIFO_RESIZED ; } dwc->num_ep_resized =0 ; }
@@ -747,6 +748,10 @@ static int dwc3_gadget_resize_tx_fifos(struct dwc3_ep *dep)if (!usb_endpoint_dir_in (dep->endpoint.desc) || dep->number <=1 )return 0 ; +/* bail if already resized */ +if (dep->flags &DWC3_EP_TXFIFO_RESIZED ) +return 0 ; + ram1_depth =DWC3_RAM1_DEPTH (dwc->hwparams.hwparams7); if ((dep->endpoint.maxburst > 1 &&
@@ -807,6 +812,7 @@ static int dwc3_gadget_resize_tx_fifos(struct dwc3_ep *dep) }dwc3_writel (dwc->regs,DWC3_GTXFIFOSIZ (dep->number >>1 ), fifo_size); + dep->flags |=DWC3_EP_TXFIFO_RESIZED ; dwc->num_ep_resized++;return 0 ;
@@ -995,7 +1001,7 @@ static int __dwc3_gadget_ep_disable(struct dwc3_ep *dep) dep->stream_capable = false; dep->type =0 ; - dep->flags =0 ; + dep->flags &=DWC3_EP_TXFIFO_RESIZED ;return 0 ; }
Signature · 1 line
2.24.0
Sorry, disregard this. Forgot to update the patch subject with V3.
Resending...
Quoted 98 lines · Jack Pham
Some functions may dynamically enable and disable their endpoints regularly throughout their operation, particularly when Set Interface is employed to switch between Alternate Settings. For instance the UAC2 function has its respective endpoints for playback & capture associated with AltSetting 1, in which case those endpoints would not get enabled until the host activates the AltSetting. And they conversely become disabled when the interfaces' AltSetting 0 is chosen.
With the DWC3 FIFO resizing algorithm recently added, every
usb_ep_enable()call results in a call to resize that EP's TXFIFO, but if the same endpoint is enabled again and again, this incorrectly leads to FIFO RAM allocation exhaustion as the mechanism did not account for the possibility that endpoints can be re-enabled many times.Example log splat:
dwc3 a600000.dwc3: Fifosize(3717) > RAM size(3462) ep3in depth:217973127 configfs-gadget gadget: u_audio_start_capture:521 Error! dwc3 a600000.dwc3: request 000000000be13e18 was not queued to ep3inAdd another bit
DWC3_EP_TXFIFO_RESIZEDtodep->flagsto keep track of whether an EP had already been resized in the current configuration. If so, bail out ofdwc3_gadget_resize_tx_fifos()to avoid the calculation error resulting from accumulating the EP's FIFO depth repeatedly. This flag is retained across multipleep_disable()andep_enable()calls and is cleared when GTXFIFOSIZn is reset indwc3_gadget_clear_tx_fifos()upon receiving the next Set Config.Fixes 9f607a309fbe9 ("usb: dwc3: Resize TX FIFOs to meet EP bursting requirements")Reviewed-by Thinh Nguyen <[email protected]>Signed-off-by Jack Pham <[email protected]>---
v3: Fixed macro definition alignment; added Thinh's reviewed-by.v2: Added explicit flag to
dep->flagsand check that instead of directly
reading the GTXFIFOSIZn register.drivers/usb/dwc3/core.h | 1 + drivers/usb/dwc3/gadget.c | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-)drivers/usb/dwc3/core.h +1
-0@@ -723,6 +723,7 @@ struct dwc3_ep {#define DWC3_EP_FORCE_RESTART_STREAM BIT(9)#define DWC3_EP_FIRST_STREAM_PRIMED BIT(10)#define DWC3_EP_PENDING_CLEAR_STALL BIT(11) +#define DWC3_EP_TXFIFO_RESIZED BIT(12)/* This last one is specific to EP0 */ #define DWC3_EP0_DIR_IN BIT(31)drivers/usb/dwc3/gadget.c +7
-1@@ -702,6 +702,7 @@ void dwc3_gadget_clear_tx_fifos(struct dwc3 *dwc)DWC31_GTXFIFOSIZ_TXFRAMNUM ;dwc3_writel (dwc->regs,DWC3_GTXFIFOSIZ (num >>1 ), size); + dep->flags &= ~DWC3_EP_TXFIFO_RESIZED ; } dwc->num_ep_resized =0 ; }@@ -747,6 +748,10 @@ static int dwc3_gadget_resize_tx_fifos(struct dwc3_ep *dep)if (!usb_endpoint_dir_in (dep->endpoint.desc) || dep->number <=1 )return 0 ; +/* bail if already resized */ +if (dep->flags &DWC3_EP_TXFIFO_RESIZED ) +return 0 ; + ram1_depth =DWC3_RAM1_DEPTH (dwc->hwparams.hwparams7); if ((dep->endpoint.maxburst > 1 &&@@ -807,6 +812,7 @@ static int dwc3_gadget_resize_tx_fifos(struct dwc3_ep *dep) }dwc3_writel (dwc->regs,DWC3_GTXFIFOSIZ (dep->number >>1 ), fifo_size); + dep->flags |=DWC3_EP_TXFIFO_RESIZED ; dwc->num_ep_resized++;return 0 ;@@ -995,7 +1001,7 @@ static int __dwc3_gadget_ep_disable(struct dwc3_ep *dep) dep->stream_capable = false; dep->type =0 ; - dep->flags =0 ; + dep->flags &=DWC3_EP_TXFIFO_RESIZED ;return 0 ; }Signature · 1 line
2.24.0
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project