Re: retire_capture_urb: Corrected urb data len

[Sean Anderson <sean.anderson@seco.com>]

Hi,

On 9/2/22 01:52, Takashi Iwai wrote:
> On Thu, 01 Sep 2022 17:25:41 +0200,
> Sean Anderson wrote:
>> 
>> 
>> 
>> On 8/28/22 3:49 AM, Takashi Iwai wrote:
>> > On Fri, 26 Aug 2022 20:57:53 +0200,
>> > Sean Anderson wrote:
>> >> 
>> >> On 8/26/22 12:36 PM, Takashi Iwai wrote:
>> >> > On Fri, 26 Aug 2022 18:22:24 +0200,
>> >> > Sean Anderson wrote:
>> >> >> 
>> >> >> Hi all,
>> >> >> 
>> >> >> I have a "FiiO DigiHug USB Audio" sound card (1852:7022) [3]. I have had
>> >> >> no problems with the audio, but I did notice a large number of message
>> >> >> like 
>> >> >> 
>> >> >> retire_capture_urb: 4992 callbacks suppressed
>> >> >> 
>> >> >> in my dmesg [1]. This is caused by the "Corrected urb data len."
>> >> >> warning.
>> >> > 
>> >> > What exact values are shown there?
>> >> 
>> >> Unfortunately, as detailed below, I was unable to turn off ratelimiting.
>> >> 
>> >> > The problem is that your hardware
>> >> > (likely a buggy firmware) returns the unaligned size of bytes as the
>> >> > data.  Maybe it's worth to replace dev_warn_ratelimited() there with
>> >> > dev_warn() and take all warnings once.  Then we can see what kind of
>> >> > values are delivered from the hardware.
>> >> 
>> >> I'll have an attempt at that next week
>> >> 
>> >> >> The patch adding this warning [2] makes it seem like
>> >> >> this warning should be an uncommon occurance. However, based on the
>> >> >> number of suppressed callbacks, this seems to be happening at a rate of
>> >> >> around 500 Hz.
>> >> >> 
>> >> >> Is this buggy hardware? Or is this a bug in the driver? Does there need
>> >> >> to be a quirk? Or perhaps the warning above should be a debug instead?
>> >> > 
>> >> > There is no quirk for that.  As long as the device works with that
>> >> > workaround (except for messages), we can simply add a quirk to not
>> >> > warn but always apply the workaround silently for such devices.
>> >> 
>> >> OK. I wasn't sure what the correct resolution would be.
>> > 
>> > Actually I was wrong: the existing quirk QUIRK_FLAG_ALIGN_TRANSFER
>> > should cover that.
>> > 
>> > Could you try to pass quirk_flags=0x04 for the corresponding card slot
>> > (the option takes an array) to snd-usb-audio module?  Alternatively,
>> > try to pass quirk_alias=18557022:0e510408 to snd-usb-audio?
>> 
>> I tried both options, but neither worked.
> 
> I have no further idea.  You should try the latest kernel without
> modification before checking further.
> 
> And, looking at the code again, it's really strange that you get those
> messages.  Actually the transfer size *is* aligned to the audio frames
> as default *unless* QUIRK_FLAG_ALIGN_TRANSFER is passed.  And the
> check is done rather the audio sample size alignment -- which must fit
> within the audio frame alignment.
> 
> So, QUIRK_FLAG_ALIGN_TRANSFER is already set for your device by some
> reason incorrectly, or the code is doing wrong on your kernel.
> We need to check what values are shown there actually, then check
> whether the problem happens with the latest vanilla kernel.

Sorry for the very long hiatus. I have reproduced this issue on kernel
6.15.8.

From closer inspection this message seems to be from the first print and
not the second one:

if (urb->iso_frame_desc[i].status && printk_ratelimit()) {
	dev_dbg(&subs->dev->dev, "frame %d active: %d\n",
		i, urb->iso_frame_desc[i].status);
	// continue;
}

This probably be a dev_dbg_ratelimited. Indeed, that suppresses these
messages.

The actual message that's being ratelimited is

usb 3-11.2: frame 0 active: -71

which is EPROTO. Looks like that comes from xhci_hcd. With debug enabled
I get a lot of

xhci_hcd 0000:80:14.0: Transfer error for slot 36 ep 4 on endpoint

The spec says this is 

| Asserted in the case where the host did not receive a valid response
| from the device (Timeout, CRC, Bad PID, unexpected NYET, etc.).

and I don't really know what to make of that. Should I send a patch to
convert that dev_dbg to dev_dbg_ratelimited? Or do you want to debug
this further?

--Sean