Jack Pham wrote:
> 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
>
> This is easily fixed by bailing out of dwc3_gadget_resize_tx_fifos()
> if an endpoint is already resized, avoiding the calculation error
> resulting from accumulating the EP's FIFO depth repeatedly.
>
> Fixes: 9f607a309fbe9 ("usb: dwc3: Resize TX FIFOs to meet EP bursting requirements")
> Signed-off-by: Jack Pham <jackp@xxxxxxxxxxxxxx>
> ---
> drivers/usb/dwc3/gadget.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 804b50548163..c647c76d7361 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -747,6 +747,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 (dwc3_readl(dwc->regs, DWC3_GTXFIFOSIZ(dep->number >> 1)))
> + return 0;
> +
> ram1_depth = DWC3_RAM1_DEPTH(dwc->hwparams.hwparams7);
>
> if ((dep->endpoint.maxburst > 1 &&
>
This seems like a workaround more than a fix. As previously pointed out,
there will be problems when there are multiple alternate setting
interface [2]. If we're doing this way, are we properly allocating the
fifo size for the alternate setting that requires the most fifo size and
not just the first alt-setting 0? Also different alt-setting can have
different endpoints, the logic handling this may not be simple.
There are a few review comments for Wesley. Hopefully they get resolved
eventually.
[1]
https://lore.kernel.org/linux-usb/b65463e9-3a8d-1ee5-3e26-09990aa8ec53@xxxxxxxxxxxx/
[2]
https://lore.kernel.org/linux-usb/0f8384bc-18a6-8ca5-c38b-6b5523c60a68@xxxxxxxxxxxx/
Thanks,
Thinh
