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: u_audio: Fix high-speed max packet size
Date: Fri, 17 Jan 2020 10:40:22 +0000	[thread overview]
Message-ID: <20200117104022.5bb769f2.john@metanate.com> (raw)
In-Reply-To: <24f0935d-16a7-4301-78f4-fa459e356ca9@ivitera.com>

On Thu, 16 Jan 2020 16:39:50 +0100
Pavel Hofman <pavel.hofman@ivitera.com> wrote:

> > I've taken a look at this and the patch below fixes it in my simple
> > testing.  But note that this doesn't adjust the PCM's min_period_bytes
> > which will be necessary if you want to minimize latency with an adjusted
> > high-speed bInterval setting.
> >   
> 
> Please can I ask you to submit your patch? IMO your perhaps slightly 
> suboptimal solution is much better than the current broken version.

Yes, the patch is definitely an improvement.  I thought it would be
picked up from the earlier mail, but I think Patchwork requires the
subject to match, so I'm including it again here.

Are you able to provide a Tested-by for this change?

-- >8 --
Prior to commit eb9fecb9e69b ("usb: gadget: f_uac2: split out audio
core") the maximum packet size was calculated only from the high-speed
descriptor but now we use the largest of the full-speed and high-speed
descriptors.

This is correct, but the full-speed value is likely to be higher than
that for high-speed and this leads to submitting requests for OUT
transfers (received by the gadget) which are larger than the endpoint's
maximum packet size.  These are rightly rejected by the gadget core.

config_ep_by_speed() already sets up the correct maximum packet size for
the enumerated speed in the usb_ep structure, so we can simply use this
instead of the overall value that has been used to allocate buffers for
requests.

Note that the minimum period for ALSA is still set from the largest
value, and this is unavoidable because it's possible to open the audio
device before the gadget has been enumerated.

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

diff --git a/drivers/usb/gadget/function/u_audio.c b/drivers/usb/gadget/function/u_audio.c
index 6d956f190f5a..e6d32c536781 100644
--- a/drivers/usb/gadget/function/u_audio.c
+++ b/drivers/usb/gadget/function/u_audio.c
@@ -361,7 +361,7 @@ int u_audio_start_capture(struct g_audio *audio_dev)
 	ep = audio_dev->out_ep;
 	prm = &uac->c_prm;
 	config_ep_by_speed(gadget, &audio_dev->func, ep);
-	req_len = prm->max_psize;
+	req_len = ep->maxpacket;
 
 	prm->ep_enabled = true;
 	usb_ep_enable(ep);
@@ -379,7 +379,7 @@ int u_audio_start_capture(struct g_audio *audio_dev)
 			req->context = &prm->ureq[i];
 			req->length = req_len;
 			req->complete = u_audio_iso_complete;
-			req->buf = prm->rbuf + i * prm->max_psize;
+			req->buf = prm->rbuf + i * ep->maxpacket;
 		}
 
 		if (usb_ep_queue(ep, prm->ureq[i].req, GFP_ATOMIC))
@@ -430,9 +430,9 @@ int u_audio_start_playback(struct g_audio *audio_dev)
 	uac->p_pktsize = min_t(unsigned int,
 				uac->p_framesize *
 					(params->p_srate / uac->p_interval),
-				prm->max_psize);
+				ep->maxpacket);
 
-	if (uac->p_pktsize < prm->max_psize)
+	if (uac->p_pktsize < ep->maxpacket)
 		uac->p_pktsize_residue = uac->p_framesize *
 			(params->p_srate % uac->p_interval);
 	else
@@ -457,7 +457,7 @@ int u_audio_start_playback(struct g_audio *audio_dev)
 			req->context = &prm->ureq[i];
 			req->length = req_len;
 			req->complete = u_audio_iso_complete;
-			req->buf = prm->rbuf + i * prm->max_psize;
+			req->buf = prm->rbuf + i * ep->maxpacket;
 		}
 
 		if (usb_ep_queue(ep, prm->ureq[i].req, GFP_ATOMIC))
-- 
2.24.1


  reply	other threads:[~2020-01-17 10:40 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 ` [PATCH] usb: gadget: f_uac2: fix packet size calculation John Keeping
2020-01-11  9:12   ` 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         ` John Keeping [this message]
2020-01-19 14:53           ` [PATCH] usb: gadget: u_audio: Fix high-speed max packet size 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=20200117104022.5bb769f2.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).