From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from svenfoo.org ([82.94.215.22]:47003 "EHLO mail.zonque.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752132AbbGNUE3 (ORCPT ); Tue, 14 Jul 2015 16:04:29 -0400 Subject: Re: [PATCH 1/1] usb: gadget: f_uac2: finalize wMaxPacketSize according to bandwidth To: Peter Chen , balbi@ti.com References: <1436862578-31625-1-git-send-email-peter.chen@freescale.com> Cc: linux-usb@vger.kernel.org, andrzej.p@samsung.com, zonque@gmail.com, tiwai@suse.de, stable@vger.kernel.org, Alan Stern From: Daniel Mack Message-ID: <55A56B47.2090608@zonque.org> Date: Tue, 14 Jul 2015 16:04:23 -0400 MIME-Version: 1.0 In-Reply-To: <1436862578-31625-1-git-send-email-peter.chen@freescale.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Sender: stable-owner@vger.kernel.org List-ID: Hi, On 07/14/2015 04:29 AM, Peter Chen wrote: > According to USB Audio Device 2.0 Spec, Ch4.10.1.1: > wMaxPacketSize is defined as follows: > Maximum packet size this endpoint is capable of sending or receiving > when this configuration is selected. > This is determined by the audio bandwidth constraints of the endpoint. > > In current code, the wMaxPacketSize is defined as the maximum packet size > for ISO endpoint, and it will let the host reserve much more space than > it really needs, so that we can't let more endpoints work together at > one frame. ... > diff --git a/drivers/usb/gadget/function/f_uac2.c b/drivers/usb/gadget/function/f_uac2.c > index 6d3eb8b..0ed6f0e 100644 > --- a/drivers/usb/gadget/function/f_uac2.c > +++ b/drivers/usb/gadget/function/f_uac2.c > @@ -987,6 +987,7 @@ afunc_bind(struct usb_configuration *cfg, struct usb_function *fn) > struct f_uac2_opts *uac2_opts; > struct usb_string *us; > int ret; > + u16 c_max_packet_size, p_max_packet_size; > > uac2_opts = container_of(fn->fi, struct f_uac2_opts, func_inst); > > @@ -1070,6 +1071,29 @@ afunc_bind(struct usb_configuration *cfg, struct usb_function *fn) > uac2->p_prm.uac2 = uac2; > uac2->c_prm.uac2 = uac2; > > + /* Calculate wMaxPacketSize according to audio bandwidth */ > + c_max_packet_size = uac2_opts->c_chmask * uac2_opts->c_ssize > + * uac2_opts->c_srate / 1000; > + p_max_packet_size = uac2_opts->p_chmask * uac2_opts->p_ssize > + * uac2_opts->p_srate / 1000; For high-speed endpoints, we in fact have 8 microframes per frame, and the factor also depends on the endpoint's polling interval. See what afunc_set_alt() does here. Did you test this in HS or FS setups or both? > + if ((c_max_packet_size > fs_epout_desc.wMaxPacketSize) || > + (p_max_packet_size > fs_epin_desc.wMaxPacketSize)) { > + dev_err(dev, "parameters are incorrect\n"); > + goto err; > + } > + /* > + * Keep max packet size is multiplier of 4, and > + * a little larger than bandwidth. > + * Eg, for frame rate 44100, 1 channel, and 16 bits data > + * we need to reserve more than 44 * 2 bytes. > + */ > + c_max_packet_size += (4 - (c_max_packet_size % 4)); I guess DIV_ROUND_UP() allows you to account for the packets that are sent when the devision residue accumulator overflows. See agdev_iso_complete(). > + fs_epin_desc.wMaxPacketSize = min(cpu_to_le16(c_max_packet_size), > + fs_epin_desc.wMaxPacketSize); > + p_max_packet_size += (4 - (p_max_packet_size % 4)); > + fs_epout_desc.wMaxPacketSize = min(cpu_to_le16(p_max_packet_size), > + fs_epout_desc.wMaxPacketSize); > + > hs_epout_desc.bEndpointAddress = fs_epout_desc.bEndpointAddress; > hs_epout_desc.wMaxPacketSize = fs_epout_desc.wMaxPacketSize; We probably have to pass different values for the two settings, as they can't be the same once they contain pre-calculated values. Thanks, Daniel