Re: [PATCH v4 1/1] usb: gadget: f_uac2: finalize wMaxPacketSize according to bandwidth
On 07/28/2015 03:38 AM, Peter Chen wrote:
+static void set_ep_max_packet_size (struct f_uac2_opts *uac2_opts,
+ struct usb_endpoint_descriptor *ep_desc, unsigned int factor)
+{
+ int chmask;
+ int srate;
+ int ssize;
+ u16 max_packet_size;
+
+ if (ep_desc == fs_epin_desc || ep_desc == hs_epin_desc) {
+ chmask = uac2_opts-p_chmask;
+ srate = uac2_opts-p_srate;
+ ssize = uac2_opts-p_ssize;
+ } else {
+ WARN_ON (ep_desc != fs_epout_desc ep_desc !=
hs_epout_desc);
+ chmask = uac2_opts-c_chmask;
+ srate = uac2_opts-c_srate;
+ ssize = uac2_opts-c_ssize;
+ }
+
+ max_packet_size = num_channels(chmask) * ssize *
+ DIV_ROUND_UP(srate, factor / (1 (ep_desc-bInterval - 1)));
+ ep_desc-wMaxPacketSize = min(cpu_to_le16(max_packet_size),
+ ep_desc-wMaxPacketSize);
+}
Thinking about it again, it would probably be nicer to make this
function return wMaxPacketSize directly:
fs_epin_desc.wMaxPacketSize =
calc_ep_max_packet_size(uac2_opts-p_chmask,
uac2_opts-p_srate,
uac2_opts-p_ssize,
1000);
hs_epin_desc.wMaxPacketSize =
calc_ep_max_packet_size(uac2_opts-p_chmask,
uac2_opts-p_srate,
uac2_opts-p_ssize,
8000);
...
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/1] usb: gadget: f_uac2: finalize wMaxPacketSize according to bandwidth
On Tue, Jul 28, 2015 at 10:13:48AM +0200, Daniel Mack wrote:
On 07/28/2015 03:38 AM, Peter Chen wrote:
diff --git a/drivers/usb/gadget/function/f_uac2.c
b/drivers/usb/gadget/function/f_uac2.c
index 5318615..6a8e0d2 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -975,6 +975,31 @@ free_ep(struct uac2_rtd_params *prm, struct usb_ep *ep)
%s:%d Error!\n, __func__, __LINE__);
}
+static void set_ep_max_packet_size (struct f_uac2_opts *uac2_opts,
+ struct usb_endpoint_descriptor *ep_desc, unsigned int factor)
scripts/checkpatch.pl will complain about a stray space before '(' and
wrong indentation. Also, uac2_opts can be const.
oh, I forget to run it when send this version.
+{
+ int chmask;
+ int srate;
+ int ssize;
These can be put in one line.
I will change it
+ u16 max_packet_size;
+
+ if (ep_desc == fs_epin_desc || ep_desc == hs_epin_desc) {
+ chmask = uac2_opts-p_chmask;
+ srate = uac2_opts-p_srate;
+ ssize = uac2_opts-p_ssize;
+ } else {
+ WARN_ON (ep_desc != fs_epout_desc ep_desc !=
hs_epout_desc);
I would rather pass a boolean flag called 'is_playback' than checking
the input parameter like this. But I forgot this detail in my proposal,
I admit.
Apart from that, I like the patch.
I will change it, thanks.
Thanks,
Daniel
+ chmask = uac2_opts-c_chmask;
+ srate = uac2_opts-c_srate;
+ ssize = uac2_opts-c_ssize;
+ }
+
+ max_packet_size = num_channels(chmask) * ssize *
+ DIV_ROUND_UP(srate, factor / (1 (ep_desc-bInterval - 1)));
+ ep_desc-wMaxPacketSize = min(cpu_to_le16(max_packet_size),
+ ep_desc-wMaxPacketSize);
+}
+
static int
afunc_bind(struct usb_configuration *cfg, struct usb_function *fn)
{
@@ -1070,10 +1095,14 @@ 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 */
+ set_ep_max_packet_size(uac2_opts, fs_epin_desc, 1000);
+ set_ep_max_packet_size(uac2_opts, fs_epout_desc, 1000);
+ set_ep_max_packet_size(uac2_opts, hs_epin_desc, 8000);
+ set_ep_max_packet_size(uac2_opts, hs_epout_desc, 8000);
+
hs_epout_desc.bEndpointAddress = fs_epout_desc.bEndpointAddress;
- hs_epout_desc.wMaxPacketSize = fs_epout_desc.wMaxPacketSize;
hs_epin_desc.bEndpointAddress = fs_epin_desc.bEndpointAddress;
- hs_epin_desc.wMaxPacketSize = fs_epin_desc.wMaxPacketSize;
ret = usb_assign_descriptors(fn, fs_audio_desc, hs_audio_desc, NULL);
if (ret)
--
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/1] usb: gadget: f_uac2: finalize wMaxPacketSize according to bandwidth
On 07/28/2015 11:30 AM, Daniel Mack wrote:
On 07/28/2015 03:38 AM, Peter Chen wrote:
+static void set_ep_max_packet_size (struct f_uac2_opts *uac2_opts,
+struct usb_endpoint_descriptor *ep_desc, unsigned int factor)
+{
+int chmask;
+int srate;
+int ssize;
+u16 max_packet_size;
+
+if (ep_desc == fs_epin_desc || ep_desc == hs_epin_desc) {
+chmask = uac2_opts-p_chmask;
+srate = uac2_opts-p_srate;
+ssize = uac2_opts-p_ssize;
+} else {
+WARN_ON (ep_desc != fs_epout_desc ep_desc !=
hs_epout_desc);
+chmask = uac2_opts-c_chmask;
+srate = uac2_opts-c_srate;
+ssize = uac2_opts-c_ssize;
+}
+
+max_packet_size = num_channels(chmask) * ssize *
+DIV_ROUND_UP(srate, factor / (1 (ep_desc-bInterval - 1)));
+ep_desc-wMaxPacketSize = min(cpu_to_le16(max_packet_size),
+ep_desc-wMaxPacketSize);
+}
Thinking about it again, it would probably be nicer to make this
function return wMaxPacketSize directly:
Ah, no, that doesn't make sense, sorry. We need bInterval and the
current value of wMaxPacketSize as well. Just stick with your current
approach then :)
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/1] usb: gadget: f_uac2: finalize wMaxPacketSize according to bandwidth
On 07/28/2015 03:38 AM, Peter Chen wrote:
diff --git a/drivers/usb/gadget/function/f_uac2.c
b/drivers/usb/gadget/function/f_uac2.c
index 5318615..6a8e0d2 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -975,6 +975,31 @@ free_ep(struct uac2_rtd_params *prm, struct usb_ep *ep)
%s:%d Error!\n, __func__, __LINE__);
}
+static void set_ep_max_packet_size (struct f_uac2_opts *uac2_opts,
+ struct usb_endpoint_descriptor *ep_desc, unsigned int factor)
scripts/checkpatch.pl will complain about a stray space before '(' and
wrong indentation. Also, uac2_opts can be const.
+{
+ int chmask;
+ int srate;
+ int ssize;
These can be put in one line.
+ u16 max_packet_size;
+
+ if (ep_desc == fs_epin_desc || ep_desc == hs_epin_desc) {
+ chmask = uac2_opts-p_chmask;
+ srate = uac2_opts-p_srate;
+ ssize = uac2_opts-p_ssize;
+ } else {
+ WARN_ON (ep_desc != fs_epout_desc ep_desc !=
hs_epout_desc);
I would rather pass a boolean flag called 'is_playback' than checking
the input parameter like this. But I forgot this detail in my proposal,
I admit.
Apart from that, I like the patch.
Thanks,
Daniel
+ chmask = uac2_opts-c_chmask;
+ srate = uac2_opts-c_srate;
+ ssize = uac2_opts-c_ssize;
+ }
+
+ max_packet_size = num_channels(chmask) * ssize *
+ DIV_ROUND_UP(srate, factor / (1 (ep_desc-bInterval - 1)));
+ ep_desc-wMaxPacketSize = min(cpu_to_le16(max_packet_size),
+ ep_desc-wMaxPacketSize);
+}
+
static int
afunc_bind(struct usb_configuration *cfg, struct usb_function *fn)
{
@@ -1070,10 +1095,14 @@ 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 */
+ set_ep_max_packet_size(uac2_opts, fs_epin_desc, 1000);
+ set_ep_max_packet_size(uac2_opts, fs_epout_desc, 1000);
+ set_ep_max_packet_size(uac2_opts, hs_epin_desc, 8000);
+ set_ep_max_packet_size(uac2_opts, hs_epout_desc, 8000);
+
hs_epout_desc.bEndpointAddress = fs_epout_desc.bEndpointAddress;
- hs_epout_desc.wMaxPacketSize = fs_epout_desc.wMaxPacketSize;
hs_epin_desc.bEndpointAddress = fs_epin_desc.bEndpointAddress;
- hs_epin_desc.wMaxPacketSize = fs_epin_desc.wMaxPacketSize;
ret = usb_assign_descriptors(fn, fs_audio_desc, hs_audio_desc, NULL);
if (ret)
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 1/1] usb: gadget: f_uac2: finalize wMaxPacketSize according to bandwidth
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.
We find this issue when we try to let 4 f_uac2 gadgets work together [1]
at FS connection.
[1]http://www.spinics.net/lists/linux-usb/msg123478.html
Cc: andrze...@samsung.com
Cc: Daniel Mack zon...@gmail.com
Cc: ti...@suse.de
Cc: sta...@vger.kernel.org #v3.18+
Cc: Alan Stern st...@rowland.harvard.edu
Signed-off-by: Peter Chen peter.c...@freescale.com
---
Changes for v4:
- Add helper set_ep_max_packet_size to calculate max packet size
Changes for v3:
- Consider 'bInterval' to calculate wMaxPacketSize
- playback endpoint is IN ep, and capture endpoint is OUT ep
Changes for v2:
- Using DIV_ROUND_UP to calculate max packet size
drivers/usb/gadget/function/f_uac2.c | 33 +++--
1 file changed, 31 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/function/f_uac2.c
b/drivers/usb/gadget/function/f_uac2.c
index 5318615..6a8e0d2 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -975,6 +975,31 @@ free_ep(struct uac2_rtd_params *prm, struct usb_ep *ep)
%s:%d Error!\n, __func__, __LINE__);
}
+static void set_ep_max_packet_size (struct f_uac2_opts *uac2_opts,
+ struct usb_endpoint_descriptor *ep_desc, unsigned int factor)
+{
+ int chmask;
+ int srate;
+ int ssize;
+ u16 max_packet_size;
+
+ if (ep_desc == fs_epin_desc || ep_desc == hs_epin_desc) {
+ chmask = uac2_opts-p_chmask;
+ srate = uac2_opts-p_srate;
+ ssize = uac2_opts-p_ssize;
+ } else {
+ WARN_ON (ep_desc != fs_epout_desc ep_desc !=
hs_epout_desc);
+ chmask = uac2_opts-c_chmask;
+ srate = uac2_opts-c_srate;
+ ssize = uac2_opts-c_ssize;
+ }
+
+ max_packet_size = num_channels(chmask) * ssize *
+ DIV_ROUND_UP(srate, factor / (1 (ep_desc-bInterval - 1)));
+ ep_desc-wMaxPacketSize = min(cpu_to_le16(max_packet_size),
+ ep_desc-wMaxPacketSize);
+}
+
static int
afunc_bind(struct usb_configuration *cfg, struct usb_function *fn)
{
@@ -1070,10 +1095,14 @@ 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 */
+ set_ep_max_packet_size(uac2_opts, fs_epin_desc, 1000);
+ set_ep_max_packet_size(uac2_opts, fs_epout_desc, 1000);
+ set_ep_max_packet_size(uac2_opts, hs_epin_desc, 8000);
+ set_ep_max_packet_size(uac2_opts, hs_epout_desc, 8000);
+
hs_epout_desc.bEndpointAddress = fs_epout_desc.bEndpointAddress;
- hs_epout_desc.wMaxPacketSize = fs_epout_desc.wMaxPacketSize;
hs_epin_desc.bEndpointAddress = fs_epin_desc.bEndpointAddress;
- hs_epin_desc.wMaxPacketSize = fs_epin_desc.wMaxPacketSize;
ret = usb_assign_descriptors(fn, fs_audio_desc, hs_audio_desc, NULL);
if (ret)
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
