usb:gadget:u_audio: Regression in [v3,3/3] usb: gadget: u_audio: add real feedback implementation - wMaxPacketSize calculation

[Pavel Hofman <pavel.hofman@xxxxxxxxxxx>]

Hi,

I am testing the three Ruslan's async feedback patches as modified by 
Jerome and I hit a regression in windows 10 enumeration.

Ruslan posted an already accepted patch 
https://github.com/torvalds/linux/commit/789ea77310f0200c84002884ffd628e2baf3ad8a#diff-876615ece7fb56ce8d45fc70623bef9caa2548e810426f217fb785ffa10b33d5 
which allowed win10 enumeration.

Ruslan's async feedback patchset kept the change: 
https://patchwork.kernel.org/project/linux-usb/patch/1614603943-11668-5-git-send-email-ruslan.bilovol@xxxxxxxxx/

diff --git a/drivers/usb/gadget/function/f_uac2.c 
b/drivers/usb/gadget/function/f_uac2.c
index 72b42f8..91b22fb 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -506,6 +506,10 @@  static int set_ep_max_packet_size(const struct 
f_uac2_opts *uac2_opts,

 	max_size_bw = num_channels(chmask) * ssize *
 		((srate / (factor / (1 << (ep_desc->bInterval - 1)))) + 1);
+
+	if (!is_playback && (uac2_opts->c_sync == USB_ENDPOINT_SYNC_ASYNC))
+		max_size_bw = max_size_bw * FBACK_FREQ_MAX / 100;
+
 	ep_desc->wMaxPacketSize = cpu_to_le16(min_t(u16, max_size_bw,
 						    max_size_ep));


Jerome's rebase patch which was accepted recently changed the 
functionality to the original code:
https://patchwork.kernel.org/project/linux-usb/patch/20210603220104.1216001-4-jbrunet@xxxxxxxxxxxx/

diff --git a/drivers/usb/gadget/function/f_uac2.c 
b/drivers/usb/gadget/function/f_uac2.c
index 321e6c05ba93..ae29ff2b2b68 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -584,8 +584,11 @@  static int set_ep_max_packet_size(const struct 
f_uac2_opts *uac2_opts,
 		ssize = uac2_opts->c_ssize;
 	}

+	if (!is_playback && (uac2_opts->c_sync == USB_ENDPOINT_SYNC_ASYNC))
+		srate = srate * (1000 + uac2_opts->fb_max) / 1000;
+
 	max_size_bw = num_channels(chmask) * ssize *
-		((srate / (factor / (1 << (ep_desc->bInterval - 1)))) + 1);
+		DIV_ROUND_UP(srate, factor / (1 << (ep_desc->bInterval - 1)));
 	ep_desc->wMaxPacketSize = cpu_to_le16(min_t(u16, max_size_bw,
 						    max_size_ep));

With this version my Win10 does not enumerate the USB AUDIO device, even 
if it has only EP-IN capability (i.e. is_playback = true). For my setup 
the EP-IN wMaxPacketSize is 192bytes vs. 196bytes in Ruslan's version, 
causing win10 reporting "The specified range could not be found in the 
range list."

A simple change makes Win10 enumerate both playback-only as well as 
duplex playback/capture async device:

diff --git a/drivers/usb/gadget/function/f_uac2.c 
b/drivers/usb/gadget/function/f_uac2.c
index ae29ff2b2b68..5ba778814796 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -588,7 +588,7 @@ static int set_ep_max_packet_size(const struct 
f_uac2_opts *uac2_opts,
                srate = srate * (1000 + uac2_opts->fb_max) / 1000;

        max_size_bw = num_channels(chmask) * ssize *
-               DIV_ROUND_UP(srate, factor / (1 << (ep_desc->bInterval - 
1)));
+               (DIV_ROUND_UP(srate, factor / (1 << (ep_desc->bInterval 
- 1))) + 1);
        ep_desc->wMaxPacketSize = cpu_to_le16(min_t(u16, max_size_bw,
                                                    max_size_ep));


I do not know if this is the most correct solution but IMO a similar 
patch should be applied. I can send a proper patch mail if this solution 
is acceptable.

Thanks a lot,

Pavel.