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

[Felipe Balbi <balbi@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3054 bytes --]


Hi,

Jerome Brunet <jbrunet@baylibre.com> writes:
>> 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@gmail.com/
>>
>> 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@baylibre.com/
>>
>> 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."
>>
>
> Maybe I am lacking USB expertize, but is there any reason why a 192bytes
> maximum packet size should be considered invalid ? Just from your
> comment, I can't figure it out.

it sounds to me like one part of the descriptor claims 192 while another
claims 196, then there is a mismatch and Windows is ignoring the
interface. A quick dump of the descriptors would prove this.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 511 bytes --]