| Message ID | 20200117104022.5bb769f2.john@metanate.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 904967c60d87393a3708fed2324b684cdb79b1ee |
| Headers | show |
| Series | usb: gadget: u_audio: Fix high-speed max packet size | expand |
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>
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.
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>
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.
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
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.
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.
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))