* USB:UAC2: Incorrect req->length > maxpacket*mc @ 2020-01-10 7:29 Pavel Hofman 2020-01-10 11:28 ` [PATCH] usb: gadget: f_uac2: fix packet size calculation John Keeping 2020-01-11 9:31 ` USB:UAC2: Incorrect req->length > maxpacket*mc - cause likely found Pavel Hofman 0 siblings, 2 replies; 17+ messages in thread From: Pavel Hofman @ 2020-01-10 7:29 UTC (permalink / raw) To: linux-usb, Felipe Balbi Hi, 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. Please may I ask for fixing the audio function in this respect? Unfortunately that is way above my knowledge of that code/requirements. However, I can send debug dump for u_audio.c, f_uac2.c, gadget.c if needed. Thanks a lot in advance. Best regards, Pavel. ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] usb: gadget: f_uac2: fix packet size calculation 2020-01-10 7:29 USB:UAC2: Incorrect req->length > maxpacket*mc Pavel Hofman @ 2020-01-10 11:28 ` 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 1 sibling, 1 reply; 17+ messages in thread From: John Keeping @ 2020-01-10 11:28 UTC (permalink / raw) To: Pavel Hofman; +Cc: linux-usb, Felipe Balbi 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] usb: gadget: f_uac2: fix packet size calculation 2020-01-10 11:28 ` [PATCH] usb: gadget: f_uac2: fix packet size calculation John Keeping @ 2020-01-11 9:12 ` Pavel Hofman 0 siblings, 0 replies; 17+ messages in thread From: Pavel Hofman @ 2020-01-11 9:12 UTC (permalink / raw) To: John Keeping; +Cc: linux-usb, Felipe Balbi Hi, Dne 10. 01. 20 v 12:28 John Keeping napsal(a): > 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? > @@ -407,7 +407,7 @@ int u_audio_start_playback(struct g_audio *audio_dev) Thanks for your hint. Your patch handles u_audio playback which is the other direction than I am trying to fix (usb host playback -> u_audio capture). With regards, Pavel. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: USB:UAC2: Incorrect req->length > maxpacket*mc - cause likely found 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:31 ` Pavel Hofman 2020-01-11 9:49 ` Pavel Hofman 2020-01-14 18:39 ` Pavel Hofman 1 sibling, 2 replies; 17+ messages in thread From: Pavel Hofman @ 2020-01-11 9:31 UTC (permalink / raw) To: linux-usb, Felipe Balbi Hi, Dne 10. 01. 20 v 8:29 Pavel Hofman napsal(a): > Hi, > > 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. > IMO the problem is here https://github.com/torvalds/linux/blob/master/drivers/usb/gadget/function/f_uac2.c#L675 : First wMaxPacketSize is correctly calculated for HS and FS. In my setup it yields (confirmed by an added debug in set_ep_max_packet_size): FS: ep_desc->wMaxPacketSize 192 HS: ep_desc->wMaxPacketSize 24 However, a few lines later the agdev->out_ep_maxpsize is set as maximum from these two values https://github.com/torvalds/linux/blob/master/drivers/usb/gadget/function/f_uac2.c#L700 : agdev->out_ep_maxpsize = max_t(u16, le16_to_cpu(fs_epout_desc.wMaxPacketSize), le16_to_cpu(hs_epout_desc.wMaxPacketSize)); In g_audio_setup this value is used for max_psize https://github.com/torvalds/linux/blob/master/drivers/usb/gadget/function/u_audio.c#L519 : prm->max_psize = g_audio->out_ep_maxpsize; And max_psize is used as req->length in u_audio_start_capture https://github.com/torvalds/linux/blob/master/drivers/usb/gadget/function/u_audio.c#L379 : req_len = prm->max_psize; ... req->length = req_len; 192bytes (the value for FS) will be assigned here, instead of the correct 24 bytes for HS. The reason it has worked so far is the original bInterval=4 for HS which yields the same 192bytes as the original for FS. But lowering the fs_epout_desc.bInterval reduces the packets, yet the maximum of the two values is being used. IMO the very same problem will be on u_audio playback side, the preceeding line: agdev->in_ep_maxpsize = max_t(u16, le16_to_cpu(fs_epin_desc.wMaxPacketSize), le16_to_cpu(hs_epin_desc.wMaxPacketSize)); Unfortunately I do not know the reason for selection of the maximum value from FS and HS, I cannot create a patch. Very likely there is more hidden know-how which I do not know. Thanks a lot for looking at the issue. With regards, Pavel. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: USB:UAC2: Incorrect req->length > maxpacket*mc - cause likely found 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 1 sibling, 0 replies; 17+ messages in thread From: Pavel Hofman @ 2020-01-11 9:49 UTC (permalink / raw) To: linux-usb, Felipe Balbi Dne 11. 01. 20 v 10:31 Pavel Hofman napsal(a): > Unfortunately I do not know the reason for selection of the maximum > value from FS and HS, I cannot create a patch. Very likely there is more > hidden know-how which I do not know. > The change was introduced by https://github.com/torvalds/linux/commit/eb9fecb9e69b0be8c267c55b0bb52a08e8fb6bee#diff-db92e74adcaaf2659ecf1a2135fd5901R572 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: USB:UAC2: Incorrect req->length > maxpacket*mc - cause likely found 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 1 sibling, 1 reply; 17+ messages in thread From: Pavel Hofman @ 2020-01-14 18:39 UTC (permalink / raw) To: linux-usb, Felipe Balbi Hi, Dne 11. 01. 20 v 10:31 Pavel Hofman napsal(a): > Hi, > > Dne 10. 01. 20 v 8:29 Pavel Hofman napsal(a): >> Hi, >> >> 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. My question relates to the recent patch https://marc.info/?l=linux-usb&m=157901102706577&w=2 > > IMO the problem is here > https://github.com/torvalds/linux/blob/master/drivers/usb/gadget/function/f_uac2.c#L675 > : > > However, a few lines later the agdev->out_ep_maxpsize is set as maximum > from these two values > https://github.com/torvalds/linux/blob/master/drivers/usb/gadget/function/f_uac2.c#L700 > : > > agdev->out_ep_maxpsize = max_t(u16, > le16_to_cpu(fs_epout_desc.wMaxPacketSize), > le16_to_cpu(hs_epout_desc.wMaxPacketSize)); > > > Unfortunately I do not know the reason for selection of the maximum > value from FS and HS, I cannot create a patch. Very likely there is more > hidden know-how which I do not know. > Please can we solve this issue so that the gadget can work for any bInterval value? Thanks a lot, Pavel. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: USB:UAC2: Incorrect req->length > maxpacket*mc - cause likely found 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 0 siblings, 2 replies; 17+ messages in thread From: John Keeping @ 2020-01-14 20:04 UTC (permalink / raw) To: Pavel Hofman; +Cc: linux-usb, Felipe Balbi Hi Pavel, On Tue, 14 Jan 2020 19:39:05 +0100 Pavel Hofman <pavel.hofman@ivitera.com> wrote: > Dne 11. 01. 20 v 10:31 Pavel Hofman napsal(a): > > Hi, > > > > Dne 10. 01. 20 v 8:29 Pavel Hofman napsal(a): > >> Hi, > >> > >> 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. > > My question relates to the recent patch > https://marc.info/?l=linux-usb&m=157901102706577&w=2 > > > > IMO the problem is here > > https://github.com/torvalds/linux/blob/master/drivers/usb/gadget/function/f_uac2.c#L675 > > : > > > > However, a few lines later the agdev->out_ep_maxpsize is set as maximum > > from these two values > > https://github.com/torvalds/linux/blob/master/drivers/usb/gadget/function/f_uac2.c#L700 > > : > > > > agdev->out_ep_maxpsize = max_t(u16, > > le16_to_cpu(fs_epout_desc.wMaxPacketSize), > > le16_to_cpu(hs_epout_desc.wMaxPacketSize)); > > > > > > Unfortunately I do not know the reason for selection of the maximum > > value from FS and HS, I cannot create a patch. Very likely there is more > > hidden know-how which I do not know. > > > > Please can we solve this issue so that the gadget can work for any > bInterval value? 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. I'm not sure what the right answer is for that; we could update min_period_bytes if the PCM is opened after the gadget attaches, but then if it is re-attached at a slower speed the PCM configuration will be wrong. -- >8 -- From 1809582b8acfa4127fb507a97206e598fa47f55a Mon Sep 17 00:00:00 2001 From: John Keeping <john@metanate.com> Date: Tue, 14 Jan 2020 19:16:10 +0000 Subject: [PATCH] usb: gadget: u_audio: Fix high-speed max packet size 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: USB:UAC2: Incorrect req->length > maxpacket*mc - cause likely found 2020-01-14 20:04 ` John Keeping @ 2020-01-14 20:21 ` Pavel Hofman 2020-01-16 15:39 ` Pavel Hofman 1 sibling, 0 replies; 17+ messages in thread From: Pavel Hofman @ 2020-01-14 20:21 UTC (permalink / raw) To: John Keeping; +Cc: linux-usb, Felipe Balbi Hi John, Dne 14. 01. 20 v 21:04 John Keeping napsal(a): > > I've taken a look at this and the patch below fixes it in my simple > testing. I like your solution, simple and understandable. > 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. My motivation for the smaller bInterval is higher attainable throughput. In fact to reach stable operation (avoiding random xruns) I have to use larger period on RPi4 - hence larger latency anyway. > > I'm not sure what the right answer is for that; we could update > min_period_bytes if the PCM is opened after the gadget attaches, but > then if it is re-attached at a slower speed the PCM configuration will > be wrong. I would suggest to keep the minimum period setting as is. Thanks a lot for your help. Pavel. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: USB:UAC2: Incorrect req->length > maxpacket*mc - cause likely found 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 1 sibling, 1 reply; 17+ messages in thread From: Pavel Hofman @ 2020-01-16 15:39 UTC (permalink / raw) To: John Keeping; +Cc: linux-usb, Felipe Balbi Hi John, > 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. Thanks a lot, Pavel. ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] usb: gadget: u_audio: Fix high-speed max packet size 2020-01-16 15:39 ` Pavel Hofman @ 2020-01-17 10:40 ` John Keeping 2020-01-19 14:53 ` Pavel Hofman 2020-01-24 12:52 ` Felipe Balbi 0 siblings, 2 replies; 17+ messages in thread From: John Keeping @ 2020-01-17 10:40 UTC (permalink / raw) To: Pavel Hofman; +Cc: linux-usb, Felipe Balbi 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] usb: gadget: u_audio: Fix high-speed max packet size 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-24 12:52 ` Felipe Balbi 1 sibling, 1 reply; 17+ messages in thread From: Pavel Hofman @ 2020-01-19 14:53 UTC (permalink / raw) To: John Keeping; +Cc: linux-usb, Felipe Balbi Dne 17. 01. 20 v 11:40 John Keeping napsal(a): > 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? Testing looks OK, thanks a lot! Tested-by: Pavel Hofman <pavel.hofman@ivitera.com> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] usb: gadget: u_audio: Fix high-speed max packet size 2020-01-19 14:53 ` Pavel Hofman @ 2020-01-24 12:16 ` Pavel Hofman 2020-01-31 10:30 ` Pavel Hofman 0 siblings, 1 reply; 17+ messages in thread From: Pavel Hofman @ 2020-01-24 12:16 UTC (permalink / raw) To: John Keeping; +Cc: linux-usb, Felipe Balbi Dne 19. 01. 20 v 15:53 Pavel Hofman napsal(a): > > Dne 17. 01. 20 v 11:40 John Keeping napsal(a): >> 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? > > > Testing looks OK, thanks a lot! > > Tested-by: Pavel Hofman <pavel.hofman@ivitera.com> > Please may I ask for finishing the patch submittal procedure, when it is already prepared and useful? Thanks a lot, Pavel. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] usb: gadget: u_audio: Fix high-speed max packet size 2020-01-24 12:16 ` Pavel Hofman @ 2020-01-31 10:30 ` Pavel Hofman 2020-01-31 11:27 ` John Keeping 0 siblings, 1 reply; 17+ messages in thread From: Pavel Hofman @ 2020-01-31 10:30 UTC (permalink / raw) To: John Keeping; +Cc: linux-usb, Felipe Balbi Dne 24. 01. 20 v 13:16 Pavel Hofman napsal(a): > > Dne 19. 01. 20 v 15:53 Pavel Hofman napsal(a): >> >> Dne 17. 01. 20 v 11:40 John Keeping napsal(a): >>> 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? >> >> >> Testing looks OK, thanks a lot! >> >> Tested-by: Pavel Hofman <pavel.hofman@ivitera.com> >> > I apologize for a basic question - please which official repository to check status of a gadget patch after being accepted? Thanks a lot for the information. Best regards, Pavel. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] usb: gadget: u_audio: Fix high-speed max packet size 2020-01-31 10:30 ` Pavel Hofman @ 2020-01-31 11:27 ` John Keeping 2020-01-31 12:47 ` Pavel Hofman 0 siblings, 1 reply; 17+ messages in thread From: John Keeping @ 2020-01-31 11:27 UTC (permalink / raw) To: Pavel Hofman; +Cc: linux-usb, Felipe Balbi On Fri, 31 Jan 2020 11:30:11 +0100 Pavel Hofman <pavel.hofman@ivitera.com> wrote: > Dne 24. 01. 20 v 13:16 Pavel Hofman napsal(a): > > > > Dne 19. 01. 20 v 15:53 Pavel Hofman napsal(a): > >> > >> Dne 17. 01. 20 v 11:40 John Keeping napsal(a): > >>> 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? > >> > >> > >> Testing looks OK, thanks a lot! > >> > >> Tested-by: Pavel Hofman <pavel.hofman@ivitera.com> > >> > > > > I apologize for a basic question - please which official repository to > check status of a gadget patch after being accepted? Thanks a lot for > the information. If you have a kernel tree, you can ask the MAINTAINERS file: ./scripts/get_maintainer.pl --scm -f drivers/usb/gadget/function/u_audio.c I'd expect this to appear in Felipe's tree first at: https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git but I don't see it yet. I guess it won't be picked up until after the merge window. Regards, John ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] usb: gadget: u_audio: Fix high-speed max packet size 2020-01-31 11:27 ` John Keeping @ 2020-01-31 12:47 ` Pavel Hofman 2020-01-31 13:09 ` Felipe Balbi 0 siblings, 1 reply; 17+ messages in thread From: Pavel Hofman @ 2020-01-31 12:47 UTC (permalink / raw) To: John Keeping; +Cc: linux-usb, Felipe Balbi Hi John, >> I apologize for a basic question - please which official repository to >> check status of a gadget patch after being accepted? Thanks a lot for >> the information. > > If you have a kernel tree, you can ask the MAINTAINERS file: > > ./scripts/get_maintainer.pl --scm -f drivers/usb/gadget/function/u_audio.c > > I'd expect this to appear in Felipe's tree first at: > > https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git > > but I don't see it yet. I guess it won't be picked up until after the > merge window. > Thanks a lot for you info. How does the maintainer pick a patch from the flood of messages? Some extra headers (Tested:by, Acked-by:) are sent separately by different people, does the maintainer have to keep track of all of that manually? Thanks for your enlightenment. Pavel. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] usb: gadget: u_audio: Fix high-speed max packet size 2020-01-31 12:47 ` Pavel Hofman @ 2020-01-31 13:09 ` Felipe Balbi 0 siblings, 0 replies; 17+ messages in thread From: Felipe Balbi @ 2020-01-31 13:09 UTC (permalink / raw) To: Pavel Hofman, John Keeping; +Cc: linux-usb [-- Attachment #1: Type: text/plain, Size: 1140 bytes --] Pavel Hofman <pavel.hofman@ivitera.com> writes: > Hi John, > >>> I apologize for a basic question - please which official repository to >>> check status of a gadget patch after being accepted? Thanks a lot for >>> the information. >> >> If you have a kernel tree, you can ask the MAINTAINERS file: >> >> ./scripts/get_maintainer.pl --scm -f drivers/usb/gadget/function/u_audio.c >> >> I'd expect this to appear in Felipe's tree first at: >> >> https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git >> >> but I don't see it yet. I guess it won't be picked up until after the >> merge window. >> > > Thanks a lot for you info. How does the maintainer pick a patch from the > flood of messages? Some extra headers (Tested:by, Acked-by:) are sent > separately by different people, does the maintainer have to keep track > of all of that manually? I had acked it, so I was under the expectation that Greg would pick it up. Unfortunately, it seems like it has slipped through the cracks. I have now queued it in my testing/fixes branch. It'll be sent forward after -rc1 is tagged. -- balbi [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] usb: gadget: u_audio: Fix high-speed max packet size 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:52 ` Felipe Balbi 1 sibling, 0 replies; 17+ messages in thread From: Felipe Balbi @ 2020-01-24 12:52 UTC (permalink / raw) To: John Keeping, Pavel Hofman, Greg Kroah-Hartman; +Cc: linux-usb [-- Attachment #1: Type: text/plain, Size: 1921 bytes --] Hi, John Keeping <john@metanate.com> writes: > 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> Acked-by: Felipe Balbi <balbi@kernel.org> -- balbi [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2020-01-31 13:09 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 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 ` [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
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).