linux-kernel - Re: [PATCH] ALSA: usb-audio: fix incorrect clock source setting

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2db5d44b88ace1c25af4a45b3469a425fe1d81a5.camel@mediatek.com>
Date:   Mon, 26 Jul 2021 16:42:14 +0800
From:   chihhao chen <chihhao.chen@...iatek.com>
To:     Geraldo Nascimento <geraldogabriel@...il.com>,
        Takashi Iwai <tiwai@...e.de>
CC:     <alsa-devel@...a-project.org>, <wsd_upstream@...iatek.com>,
        <damien@...audio.com>, <tiwai@...e.com>,
        <linux-kernel@...r.kernel.org>,
        <linux-mediatek@...ts.infradead.org>, <matthias.bgg@...il.com>,
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH] ALSA: usb-audio: fix incorrect clock source setting

Hello,

Attach USB descriptor of clock source and selectior for this earphone.

AC Clock Source Descriptor:
------------------------------

Value	Valuename
0x08	bLength
0x24	bDescriptorType
0x0A	bDescriptorSubtype
0x09	bClockID
0x03	bmAttributes
0x07	bmControls
0x00	bAssocTerminal
0x00	iClockSource
Hex dump: 
0x08 0x24 0x0A 0x09 0x03 0x07 0x00 0x00 

AC Clock Selector Descriptor:
------------------------------

Value	Valuename
0x09	bLength
0x24	bDescriptorType
0x0B	bDescriptorSubtype
0x0B	bClockID
0x02	bNrInPins
0x09	baCSourceID(1)
0x0A	baCSourceID(2)
0x03	bmControls
0x00	iClockSelector
Hex dump: 
0x09 0x24 0x0B 0x0B 0x02 0x09 0x0A 0x03 0x00 

AC Clock Source Descriptor:
------------------------------

Value	Valuename
0x08	bLength
0x24	bDescriptorType
0x0A	bDescriptorSubtype
0x0A	bClockID
0x03	bmAttributes
0x07	bmControls
0x00	bAssocTerminal
0x00	iClockSource
Hex dump: 
0x08 0x24 0x0A 0x0A 0x03 0x07 0x00 0x00 

AC Clock Selector Descriptor:
------------------------------

Value	Valuename
0x09	bLength
0x24	bDescriptorType
0x0B	bDescriptorSubtype
0x0C	bClockID
0x02	bNrInPins
0x09	baCSourceID(1)
0x0A	baCSourceID(2)
0x03	bmControls
0x00	iClockSelector
Hex dump: 
0x09 0x24 0x0B 0x0C 0x02 0x09 0x0A 0x03 0x00

Thanks
Chihhao

On Mon, 2021-07-26 at 02:16 +0000, Geraldo Nascimento wrote:
> On Sun, Jul 25, 2021 at 7:44 AM Takashi Iwai <tiwai@...e.de> wrote:
> > 
> > On Sat, 24 Jul 2021 17:04:13 +0200,
> > Geraldo Nascimento wrote:
> > > 
> > > On Sat, Jul 24, 2021 at 8:05 AM Takashi Iwai <tiwai@...e.de>
> > > wrote:
> > > > 
> > > > This looks like a regression introduced by the recent commit
> > > > d2e8f641257d ("ALSA: usb-audio: Explicitly set up the clock
> > > > selector"), which is a fix for certain devices.  Too bad that
> > > > the
> > > > behavior really depends on the device...
> > > 
> > > Dr. Iwai, perhaps we could restrict the generalized fix for the
> > > Behringer UFX1604 / UFX1204 with some simple logic to devices
> > > that
> > > only have *one* clock source.
> > > 
> > > In that case the clock selector must be set to the only clock
> > > source.
> > > 
> > > This way we keep the generalization without breaking devices with
> > > more
> > > than one clock source.
> > > 
> > > Just an idea.
> > 
> > I don't think it's easy to generalize.  All those bugs are more or
> > less BIOS bugs, and a logic doesn't apply always, just because it's
> > a
> > bug :)  For example, setting the clock selector itself should be a
> > valid operation from the specification POV, while this leads to
> > breakage on some devices.  So, even if we add a more generic
> > workaround, we need to see which side effect is more commonly seen
> > at
> > first.
> > 
> > 
> > Takashi
> 
> Hello,
> 
> Like I said in one of the other emails in this thread, it's hard to
> pinpoint a cause for the breakage of Samsung USBC Headset (AKG) with
> VID/PID (0x04e8/0xa051) without the lsusb -v of the device in
> question.
> 
> But from the description Chihhao Chen gave in the original message,
> I'm *guessing* the Clock Source for the Samsung USB Headset (AKG)
> runs
> at 48000hz and that we'd see a 2x Clock Multiplier in the lsusb -v
> 
> This is all a wild guess, without the lsusb -v it's impossible to be
> sure, but if I'm right then the valid setting for the Microphone's
> Clock Selector is the Clock Multiplier, not the Clock Source, which,
> remember, runs at half the clock, hence why Chihhao Chen sees half
> the
> data rate for USB IN.
> 
> Unfortunately our kernel code presently *does* always set the Clock
> Selector to the Clock Source, which is a bad assumption to make in my
> humble opinion.
> 
> The only valid case for setting the Clock Selector to the Clock
> Source
> is when there's precisely one Clock Selector, precisely one Clock
> Source and no Clock Multipliers.
> 
> In that special case we may be able to touch the setting of the only
> Clock Selector to match the only Clock Source.
> 
> And, frankly, the only reason we're forced to do that explicitly is
> because some Behringer gear (Archwave AG DACs) gets confused and
> seems
> to somehow keep the old rate on the Clock Selector upon sample rate
> change.
> 
> Thank you,
> Geraldo Nascimento