Re: uac2/hid gadget issues on Win10 hosts

[Jack Pham <jackp@codeaurora.org>]

Hi Pavel,

On Mon, Aug 23, 2021 at 03:17:11PM +0200, Pavel Hofman wrote:
> There is a problem with max packet size calculation for EP-IN. It has been
> discussed here recently
> https://www.spinics.net/lists/linux-usb/msg214615.html
> 
> The simple change in the post above fixed Win10 enumeration for me and
> another tester.

I faced the same error on Win10 and also tried the above patch and it
worked for me as well.  Are you planning to send a formal patch for it?
If so, you can add my

Tested-by: Jack Pham <jackp@codeaurora.org>

> Also, there is a problem with feedback value calculation which Win10 ignores
> and keeps sending the same amount of samples. The fix is to send number of
> samples per the actual packet, not per microframe for USB2. I have not
> posted the attached patch as the whole patchset will most likely be reverted
> for 5.15 https://www.spinics.net/lists/linux-usb/msg216042.html and I wanted
> to wait till the situation works out to avoid confusion. In the attached
> patch just change the ->c_srate_active to ->c_srate (the patch is on top of
> more changes for switching between multiple samplerates).

It doesn't look like any of the feedback EP changes got reverted for
5.14 / 5.15-rc1 so it looks like the dust has settled.  Are you going to
send the below patch formally as well?

Thanks!
Jack

> From 91dda032dd0958006a4e0a6f1f51526cefd95a1e Mon Sep 17 00:00:00 2001
> From: Pavel Hofman <pavel.hofman@ivitera.com>
> Date: Thu, 12 Aug 2021 18:03:36 +0200
> Subject: [PATCH 9/9] usb: gadget: u_audio: EP-OUT bInterval in fback frequency
> 
> Tests have revealed that Win10 and OSX UAC2 drivers require
> the feedback frequency to be based on the actual packet
> interval instead of on the USB2 microframe. Otherwise they
> ignore the feedback value. Linux snd-usb-audio driver
> detects the applied bitshift automatically.
> 
> The patch increases the bitshift in feedback frequency
> calculation with EP-OUT bInterval value.
> 
> Signed-off-by: Pavel Hofman <pavel.hofman@ivitera.com>
> Tested-by: Henrik Enquist <henrik.enquist@gmail.com>
> ---
>  drivers/usb/gadget/function/u_audio.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/u_audio.c b/drivers/usb/gadget/function/u_audio.c
> index a7896fbbab36..c70bdb565153 100644
> --- a/drivers/usb/gadget/function/u_audio.c
> +++ b/drivers/usb/gadget/function/u_audio.c
> @@ -98,11 +98,13 @@ static const struct snd_pcm_hardware uac_pcm_hardware = {
>  };
>  
>  static void u_audio_set_fback_frequency(enum usb_device_speed speed,
> +          struct usb_ep *out_ep,
>  					unsigned long long freq,
>  					unsigned int pitch,
>  					void *buf)
>  {
>  	u32 ff = 0;
> +	const struct usb_endpoint_descriptor *ep_desc;
>  
>  	/*
>  	 * Because the pitch base is 1000000, the final divider here
> @@ -130,8 +132,13 @@ static void u_audio_set_fback_frequency(enum usb_device_speed speed,
>  		 * byte fromat (that is Q16.16)
>  		 *
>  		 * ff = (freq << 16) / 8000
> +		 *
> +		 * Win10 and OSX UAC2 drivers require number of samples per packet
> +		 * in order to honor the feedback value.
> +		 * Linux snd-usb-audio detects the used bit shift automatically.
>  		 */
> -		freq <<= 4;
> +		ep_desc = out_ep->desc;
> +		freq <<= 4 + (ep_desc->bInterval - 1);
>  	}
>  
>  	ff = DIV_ROUND_CLOSEST_ULL((freq * pitch), 1953125);
> @@ -263,7 +270,7 @@ static void u_audio_iso_fback_complete(struct usb_ep *ep,
>  		pr_debug("%s: iso_complete status(%d) %d/%d\n",
>  			__func__, status, req->actual, req->length);
>  
> -	u_audio_set_fback_frequency(audio_dev->gadget->speed,
> +	u_audio_set_fback_frequency(audio_dev->gadget->speed, audio_dev->out_ep,
>  				    params->c_srate_active, prm->pitch,
>  				    req->buf);
>  
> @@ -578,7 +585,7 @@ int u_audio_start_capture(struct g_audio *audio_dev)
>  	 * be meauserd at start of playback
>  	 */
>  	prm->pitch = 1000000;
> -	u_audio_set_fback_frequency(audio_dev->gadget->speed,
> +	u_audio_set_fback_frequency(audio_dev->gadget->speed, ep,
>  				    params->c_srate_active, prm->pitch,
>  				    req_fback->buf);
>  
> -- 
> 2.25.1
> 


-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project