* 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-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
* 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
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.