linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: John Keeping <john@metanate.com>
To: Pavel Hofman <pavel.hofman@ivitera.com>
Cc: linux-usb@vger.kernel.org, Felipe Balbi <balbi@kernel.org>
Subject: [PATCH] usb: gadget: f_uac2: fix packet size calculation
Date: Fri, 10 Jan 2020 11:28:14 +0000	[thread overview]
Message-ID: <20200110112814.1abf2075.john@metanate.com> (raw)
In-Reply-To: <4f2df2bc-e208-fffb-48e2-3e14cd093103@ivitera.com>

On Fri, 10 Jan 2020 08:29:06 +0100
Pavel Hofman <pavel.hofman@ivitera.com> wrote:

> Together with dwc2 maintainer Minas Harutyunyan we have been
> troubleshooting various issues of dwc2 on RPi4. We hit a problem where
> the g_audio in capture (EP OUT, playback from USB host) requests req->
> length larger than maxpacket*mc.
> 
> As a workaround we removed the check in dwc2/gadget.c, however that is
> not a proper solution. Minas with his team decided to add a patch where 
> dwc2 will rejecting this type of wrong requests in dwc2_hsotg_ep_queue()
> function, to not process these request at all. The f_uac2 + g_audio
> gadget should restrain from sending such requests.
> 
> Steps to reproduce:
> 
> * Changing fs_epout_desc.bInterval in f_uac2.c from 4 (1ms) to 1 (125us) 
> - the goal is to maximize available throughput of the audio gadget
> 
> * Loading the g_audio module with c_srate=48000, c_ssize=2, c_chmask=2 - 
> i.e. standard 48kHz/16bit/2ch USB playback -> alsa capture
> 
> This combination produces mps=24 and mc=1 for EP OUT. Yet the audio 
> function driver sometimes queues request with req->length 192.

This sounded familiar to me, and I realised that I've been running with
a patch for this case that I never submitted upstream.

Does the patch below fix the issue you're seeing?

-- >8 --
The packet size for USB audio must always be a multiple of the frame
size, otherwise we are transmitting a partial frame which omits some
channels (and these end up at the wrong offset in the next packet).
Furthermore, it breaks the residue handling such that we end up trying
to send a packet exceeding the maximum packet size for the endpoint.

Signed-off-by: John Keeping <john@metanate.com>
---
 drivers/usb/gadget/function/u_audio.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/function/u_audio.c b/drivers/usb/gadget/function/u_audio.c
index cf4f2358889b..6d956f190f5a 100644
--- a/drivers/usb/gadget/function/u_audio.c
+++ b/drivers/usb/gadget/function/u_audio.c
@@ -407,7 +407,7 @@ int u_audio_start_playback(struct g_audio *audio_dev)
 	struct usb_ep *ep;
 	struct uac_rtd_params *prm;
 	struct uac_params *params = &audio_dev->params;
-	unsigned int factor, rate;
+	unsigned int factor;
 	const struct usb_endpoint_descriptor *ep_desc;
 	int req_len, i;
 
@@ -426,13 +426,15 @@ int u_audio_start_playback(struct g_audio *audio_dev)
 	/* pre-compute some values for iso_complete() */
 	uac->p_framesize = params->p_ssize *
 			    num_channels(params->p_chmask);
-	rate = params->p_srate * uac->p_framesize;
 	uac->p_interval = factor / (1 << (ep_desc->bInterval - 1));
-	uac->p_pktsize = min_t(unsigned int, rate / uac->p_interval,
+	uac->p_pktsize = min_t(unsigned int,
+				uac->p_framesize *
+					(params->p_srate / uac->p_interval),
 				prm->max_psize);
 
 	if (uac->p_pktsize < prm->max_psize)
-		uac->p_pktsize_residue = rate % uac->p_interval;
+		uac->p_pktsize_residue = uac->p_framesize *
+			(params->p_srate % uac->p_interval);
 	else
 		uac->p_pktsize_residue = 0;
 
-- 
2.24.1


  reply	other threads:[~2020-01-10 11:28 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-10  7:29 USB:UAC2: Incorrect req->length > maxpacket*mc Pavel Hofman
2020-01-10 11:28 ` John Keeping [this message]
2020-01-11  9:12   ` [PATCH] usb: gadget: f_uac2: fix packet size calculation Pavel Hofman
2020-01-11  9:31 ` USB:UAC2: Incorrect req->length > maxpacket*mc - cause likely found Pavel Hofman
2020-01-11  9:49   ` Pavel Hofman
2020-01-14 18:39   ` Pavel Hofman
2020-01-14 20:04     ` John Keeping
2020-01-14 20:21       ` Pavel Hofman
2020-01-16 15:39       ` Pavel Hofman
2020-01-17 10:40         ` [PATCH] usb: gadget: u_audio: Fix high-speed max packet size John Keeping
2020-01-19 14:53           ` Pavel Hofman
2020-01-24 12:16             ` Pavel Hofman
2020-01-31 10:30               ` Pavel Hofman
2020-01-31 11:27                 ` John Keeping
2020-01-31 12:47                   ` Pavel Hofman
2020-01-31 13:09                     ` Felipe Balbi
2020-01-24 12:52           ` Felipe Balbi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200110112814.1abf2075.john@metanate.com \
    --to=john@metanate.com \
    --cc=balbi@kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=pavel.hofman@ivitera.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).