* focusrite scarlett 18i20 : Mixer controls with corrupted names for
@ 2019-06-22 20:55 Stefan Sauer
2019-06-22 21:17 ` Stefan Sauer
2019-06-25 7:54 ` Takashi Iwai
0 siblings, 2 replies; 9+ messages in thread
From: Stefan Sauer @ 2019-06-22 20:55 UTC (permalink / raw)
To: alsa-devel
Hi,
when I run
amixer -D hw:4 controls | sort -n -t = -k2
I get the output below
numid=1,iface=MIXER,name='KKKKKKKKKKKKKÃÃÃÃÃÃÃÃÃÃÃ Switch'
numid=2,iface=CARD,name='Internal Validity'
numid=3,iface=CARD,name='S/PDIF Validity'
numid=4,iface=CARD,name='ADAT Validity'
numid=5,iface=MIXER,name='Scarlett 18i20 USB-Sync Clock Source'
numid=6,iface=MIXER,name=' Switch'
numid=7,iface=CARD,name='Keep Interface'
numid=8,iface=MIXER,name='Master Playback Switch'
numid=9,iface=MIXER,name='Master Playback Volume'
numid=10,iface=MIXER,name='Master 1 (Monitor) Playback Switch'
numid=11,iface=MIXER,name='Master 1 (Monitor) Playback Volume'
... lots of extra lines ...
Please note the lines for numid=1 and numid=6. The first one contains some
garbage and the 2nd one look like it should start with another word.
I now added some debug printing here:
https://github.com/torvalds/linux/blob/master/sound/usb/mixer_scarlett.c#L562
and get output in dmesg:
[ 2971.642137] usb 1-2: Product: Scarlett 18i20 USB
[ 2971.642141] usb 1-2: Manufacturer: Focusrite
[ 2971.709773] new ctrl: name='Master Playback Switch', index=10, offset=1,
num=0: numid=8
[ 2971.709781] new ctrl: name='Master Playback Volume', index=10, offset=2,
num=0: numid=9
[ 2971.709788] new ctrl: name='Master 1 (Monitor) Playback Switch', index=10,
offset=1, num=1: numid=10
So the first 8 controls are added somewhere else. Looks like this is from
mixer.c and after
echo -n 'file sound/usb/mixer.c +p' >/sys/kernel/debug/dynamic_debug/control
I get
[ 4405.855432] usb 1-2: [51] PU [KKKKKKKKKKKKKÃÃÃÃÃÃÃÃÃÃÃ Switch] ch = 1, val = 0/1
[ 4405.856423] usb 1-2: [52] PU [ Switch] ch = 1, val = 0/1
I now added more debug prints into
https://github.com/torvalds/linux/blob/master/sound/usb/mixer.c#L2431
and its the code that calls snd_usb_copy_string_desc()
[ 5750.124123] usb 1-2: nameid=90, len=35
[ 5750.124157] usb 1-2: [51] PU [KKKKKKKKKKKKKÃÃÃÃÃÃÃÃÃÃÃ Switch] ch = 1, val = 0/1
[ 5750.125241] usb 1-2: nameid=82, len=1
[ 5750.125260] usb 1-2: [52] PU [ Switch] ch = 1, val = 0/1
In both cases the returned len seems wrong and the function does not seems to
copy any useful string here. snd_usb_copy_string_desc() is just a wrapper around
usb_string().
Is my hardware returning bogus data in usb descriptors? Can this be address
through some quirks table? Any other ideas?
Thanks!
Stefan
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: focusrite scarlett 18i20 : Mixer controls with corrupted names for
2019-06-22 20:55 focusrite scarlett 18i20 : Mixer controls with corrupted names for Stefan Sauer
@ 2019-06-22 21:17 ` Stefan Sauer
2019-06-23 11:28 ` Stefan Sauer
2019-06-25 7:54 ` Takashi Iwai
1 sibling, 1 reply; 9+ messages in thread
From: Stefan Sauer @ 2019-06-22 21:17 UTC (permalink / raw)
To: alsa-devel
Quick followup. Here is the lsusb -v output for the device:
https://gist.github.com/ensonic/2d286a85bbf4aca25e6f36b3569849c3
I noticed two warnings:
Warning: Junk at end of descriptor (17 bytes):
ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff 00 00
Could those be the cause? I don't now how to map the 'nameid' index to anything
in the lsub output though.
Am 22.06.19 um 22:55 schrieb Stefan Sauer:
> Hi,
>
> when I run
> amixer -D hw:4 controls | sort -n -t = -k2
>
> I get the output below
>
> numid=1,iface=MIXER,name='KKKKKKKKKKKKKÃÃÃÃÃÃÃÃÃÃÃ Switch'
> numid=2,iface=CARD,name='Internal Validity'
> numid=3,iface=CARD,name='S/PDIF Validity'
> numid=4,iface=CARD,name='ADAT Validity'
> numid=5,iface=MIXER,name='Scarlett 18i20 USB-Sync Clock Source'
> numid=6,iface=MIXER,name=' Switch'
> numid=7,iface=CARD,name='Keep Interface'
> numid=8,iface=MIXER,name='Master Playback Switch'
> numid=9,iface=MIXER,name='Master Playback Volume'
> numid=10,iface=MIXER,name='Master 1 (Monitor) Playback Switch'
> numid=11,iface=MIXER,name='Master 1 (Monitor) Playback Volume'
> ... lots of extra lines ...
>
> Please note the lines for numid=1 and numid=6. The first one contains some
> garbage and the 2nd one look like it should start with another word.
>
> I now added some debug printing here:
> https://github.com/torvalds/linux/blob/master/sound/usb/mixer_scarlett.c#L562
>
> and get output in dmesg:
> [ 2971.642137] usb 1-2: Product: Scarlett 18i20 USB
> [ 2971.642141] usb 1-2: Manufacturer: Focusrite
> [ 2971.709773] new ctrl: name='Master Playback Switch', index=10, offset=1,
> num=0: numid=8
> [ 2971.709781] new ctrl: name='Master Playback Volume', index=10, offset=2,
> num=0: numid=9
> [ 2971.709788] new ctrl: name='Master 1 (Monitor) Playback Switch', index=10,
> offset=1, num=1: numid=10
>
> So the first 8 controls are added somewhere else. Looks like this is from
> mixer.c and after
> echo -n 'file sound/usb/mixer.c +p' >/sys/kernel/debug/dynamic_debug/control
> I get
> [ 4405.855432] usb 1-2: [51] PU [KKKKKKKKKKKKKÃÃÃÃÃÃÃÃÃÃÃ Switch] ch = 1, val = 0/1
> [ 4405.856423] usb 1-2: [52] PU [ Switch] ch = 1, val = 0/1
>
> I now added more debug prints into
> https://github.com/torvalds/linux/blob/master/sound/usb/mixer.c#L2431
> and its the code that calls snd_usb_copy_string_desc()
> [ 5750.124123] usb 1-2: nameid=90, len=35
> [ 5750.124157] usb 1-2: [51] PU [KKKKKKKKKKKKKÃÃÃÃÃÃÃÃÃÃÃ Switch] ch = 1, val = 0/1
> [ 5750.125241] usb 1-2: nameid=82, len=1
> [ 5750.125260] usb 1-2: [52] PU [ Switch] ch = 1, val = 0/1
>
> In both cases the returned len seems wrong and the function does not seems to
> copy any useful string here. snd_usb_copy_string_desc() is just a wrapper around
> usb_string().
>
> Is my hardware returning bogus data in usb descriptors? Can this be address
> through some quirks table? Any other ideas?
>
> Thanks!
> Stefan
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: focusrite scarlett 18i20 : Mixer controls with corrupted names for
2019-06-22 21:17 ` Stefan Sauer
@ 2019-06-23 11:28 ` Stefan Sauer
0 siblings, 0 replies; 9+ messages in thread
From: Stefan Sauer @ 2019-06-23 11:28 UTC (permalink / raw)
To: alsa-devel
Another followup. I made a little python tool to dump all string descriptors via
pyusb - here is the code and the output for the device. No idea why lsusb hides
string descriptors.
https://gist.github.com/ensonic/00fa344a05c87093058a98a62893e3b4
I just dumped the first 100, since I haven't figured how to check for the number
of strings provided by the device. In any case there aren't any useful strings
beyond index=62, but the mixer.c code is accessing 82, 90. Will now figure how
it gets that number.
If there are ready made useful tools that I should use instead of writing them
myself in python, please let me know. Thanks
Stefan
Am 22.06.19 um 23:17 schrieb Stefan Sauer:
> Quick followup. Here is the lsusb -v output for the device:
> https://gist.github.com/ensonic/2d286a85bbf4aca25e6f36b3569849c3
>
> I noticed two warnings:
> Warning: Junk at end of descriptor (17 bytes):
> ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff 00 00
> Could those be the cause? I don't now how to map the 'nameid' index to anything
> in the lsub output though.
>
> Am 22.06.19 um 22:55 schrieb Stefan Sauer:
>> Hi,
>>
>> when I run
>> amixer -D hw:4 controls | sort -n -t = -k2
>>
>> I get the output below
>>
>> numid=1,iface=MIXER,name='KKKKKKKKKKKKKÃÃÃÃÃÃÃÃÃÃÃ Switch'
>> numid=2,iface=CARD,name='Internal Validity'
>> numid=3,iface=CARD,name='S/PDIF Validity'
>> numid=4,iface=CARD,name='ADAT Validity'
>> numid=5,iface=MIXER,name='Scarlett 18i20 USB-Sync Clock Source'
>> numid=6,iface=MIXER,name=' Switch'
>> numid=7,iface=CARD,name='Keep Interface'
>> numid=8,iface=MIXER,name='Master Playback Switch'
>> numid=9,iface=MIXER,name='Master Playback Volume'
>> numid=10,iface=MIXER,name='Master 1 (Monitor) Playback Switch'
>> numid=11,iface=MIXER,name='Master 1 (Monitor) Playback Volume'
>> ... lots of extra lines ...
>>
>> Please note the lines for numid=1 and numid=6. The first one contains some
>> garbage and the 2nd one look like it should start with another word.
>>
>> I now added some debug printing here:
>> https://github.com/torvalds/linux/blob/master/sound/usb/mixer_scarlett.c#L562
>>
>> and get output in dmesg:
>> [ 2971.642137] usb 1-2: Product: Scarlett 18i20 USB
>> [ 2971.642141] usb 1-2: Manufacturer: Focusrite
>> [ 2971.709773] new ctrl: name='Master Playback Switch', index=10, offset=1,
>> num=0: numid=8
>> [ 2971.709781] new ctrl: name='Master Playback Volume', index=10, offset=2,
>> num=0: numid=9
>> [ 2971.709788] new ctrl: name='Master 1 (Monitor) Playback Switch', index=10,
>> offset=1, num=1: numid=10
>>
>> So the first 8 controls are added somewhere else. Looks like this is from
>> mixer.c and after
>> echo -n 'file sound/usb/mixer.c +p' >/sys/kernel/debug/dynamic_debug/control
>> I get
>> [ 4405.855432] usb 1-2: [51] PU [KKKKKKKKKKKKKÃÃÃÃÃÃÃÃÃÃÃ Switch] ch = 1, val = 0/1
>> [ 4405.856423] usb 1-2: [52] PU [ Switch] ch = 1, val = 0/1
>>
>> I now added more debug prints into
>> https://github.com/torvalds/linux/blob/master/sound/usb/mixer.c#L2431
>> and its the code that calls snd_usb_copy_string_desc()
>> [ 5750.124123] usb 1-2: nameid=90, len=35
>> [ 5750.124157] usb 1-2: [51] PU [KKKKKKKKKKKKKÃÃÃÃÃÃÃÃÃÃÃ Switch] ch = 1, val = 0/1
>> [ 5750.125241] usb 1-2: nameid=82, len=1
>> [ 5750.125260] usb 1-2: [52] PU [ Switch] ch = 1, val = 0/1
>>
>> In both cases the returned len seems wrong and the function does not seems to
>> copy any useful string here. snd_usb_copy_string_desc() is just a wrapper around
>> usb_string().
>>
>> Is my hardware returning bogus data in usb descriptors? Can this be address
>> through some quirks table? Any other ideas?
>>
>> Thanks!
>> Stefan
>> _______________________________________________
>> Alsa-devel mailing list
>> Alsa-devel@alsa-project.org
>> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>>
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: focusrite scarlett 18i20 : Mixer controls with corrupted names for
2019-06-22 20:55 focusrite scarlett 18i20 : Mixer controls with corrupted names for Stefan Sauer
2019-06-22 21:17 ` Stefan Sauer
@ 2019-06-25 7:54 ` Takashi Iwai
2019-06-26 20:06 ` Stefan Sauer
1 sibling, 1 reply; 9+ messages in thread
From: Takashi Iwai @ 2019-06-25 7:54 UTC (permalink / raw)
To: Stefan Sauer; +Cc: alsa-devel
On Sat, 22 Jun 2019 22:55:25 +0200,
Stefan Sauer wrote:
>
> So the first 8 controls are added somewhere else. Looks like this is from
> mixer.c and after
> echo -n 'file sound/usb/mixer.c +p' >/sys/kernel/debug/dynamic_debug/control
> I get
> [ 4405.855432] usb 1-2: [51] PU [KKKKKKKKKKKKKÃÃÃÃÃÃÃÃÃÃÃ Switch] ch = 1, val = 0/1
> [ 4405.856423] usb 1-2: [52] PU [ Switch] ch = 1, val = 0/1
This indicates that these weird names come from the two extension
units id 51 and 52.
Could you put some debug print in build_audio_procunit() like below?
thanks,
Takashi
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -2358,8 +2358,10 @@ static int build_audio_procunit(struct mixer_build *state, int unitid,
for (info = list; info && info->type; info++)
if (info->type == type)
break;
- if (!info || !info->type)
+ if (!info || !info->type) {
+ pr_info("XXX unit %d to default_info\n", unitid);
info = &default_info;
+ }
for (valinfo = info->values; valinfo->control; valinfo++) {
__u8 *controls = uac_processing_unit_bmControls(desc, state->mixer->protocol);
@@ -2375,8 +2377,11 @@ static int build_audio_procunit(struct mixer_build *state, int unitid,
}
map = find_map(state->map, unitid, valinfo->control);
+ if (map)
+ pr_info("XXX map found for unit %d\n", unitid);
if (check_ignored_ctl(map))
continue;
+
cval = kzalloc(sizeof(*cval), GFP_KERNEL);
if (!cval)
return -ENOMEM;
@@ -2444,19 +2449,26 @@ static int build_audio_procunit(struct mixer_build *state, int unitid,
kctl->private_free = snd_usb_mixer_elem_free;
if (check_mapped_name(map, kctl->id.name, sizeof(kctl->id.name))) {
+ pr_info("XXX name copied from map: '%s' for unit %d\n", kctl->id.name, unitid);
/* nothing */ ;
} else if (info->name) {
strlcpy(kctl->id.name, info->name, sizeof(kctl->id.name));
+ pr_info("XXX name copied from info: '%s' for unit %d\n", kctl->id.name, unitid);
} else {
nameid = uac_processing_unit_iProcessing(desc, state->mixer->protocol);
+ pr_info("XXX nameid=%d, unit %d\n", nameid, unitid);
len = 0;
- if (nameid)
+ if (nameid) {
len = snd_usb_copy_string_desc(state->chip,
nameid,
kctl->id.name,
sizeof(kctl->id.name));
- if (!len)
+ pr_info("XXX copy string desc '%s'\n", kctl->id.name);
+ }
+ if (!len) {
strlcpy(kctl->id.name, name, sizeof(kctl->id.name));
+ pr_info("XXX copy default name '%s'\n", kctl->id.name);
+ }
}
append_ctl_name(kctl, " ");
append_ctl_name(kctl, valinfo->suffix);
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: focusrite scarlett 18i20 : Mixer controls with corrupted names for
2019-06-25 7:54 ` Takashi Iwai
@ 2019-06-26 20:06 ` Stefan Sauer
2019-06-26 20:13 ` Stefan Sauer
2019-06-26 21:42 ` Takashi Iwai
0 siblings, 2 replies; 9+ messages in thread
From: Stefan Sauer @ 2019-06-26 20:06 UTC (permalink / raw)
To: alsa-devel
Am 25.06.19 um 09:54 schrieb Takashi Iwai:
> On Sat, 22 Jun 2019 22:55:25 +0200,
> Stefan Sauer wrote:
>>
>> So the first 8 controls are added somewhere else. Looks like this is from
>> mixer.c and after
>> echo -n 'file sound/usb/mixer.c +p' >/sys/kernel/debug/dynamic_debug/control
>> I get
>> [ 4405.855432] usb 1-2: [51] PU [KKKKKKKKKKKKKÃÃÃÃÃÃÃÃÃÃÃ Switch] ch = 1, val = 0/1
>> [ 4405.856423] usb 1-2: [52] PU [ Switch] ch = 1, val = 0/1
>
> This indicates that these weird names come from the two extension
> units id 51 and 52.
> Could you put some debug print in build_audio_procunit() like below?
I traced it down to this part from sound/usb/mixer.c:
nameid = uac_processing_unit_iProcessing(desc, state->mixer->protocol);
usb_audio_dbg(state->chip, "desc->bUnitID=%d, proto=%d, nameid=%d\n",
desc->bUnitID, state->mixer->protocol, nameid);
len = 0;
if (nameid) {
len = snd_usb_copy_string_desc(state->chip,
nameid,
kctl->id.name,
sizeof(kctl->id.name));
usb_audio_dbg(state->chip, "nameid=%d, len=%d, fallback name='%s'\n",
nameid, len, name);
}
[ 6241.045734] usb 1-2: desc->bUnitID=51, proto=32, nameid=90
[ 6241.045861] usb 1-2: nameid=90, len=35, fallback name='Extension Unit'
[ 6241.045868] usb 1-2: [51] PU [KKKKKKKKKKKKKÃÃÃÃÃÃÃÃÃÃÃ Switch] ch = 1, val = 0/1
[ 6241.046745] usb 1-2: desc->bUnitID=52, proto=32, nameid=82
[ 6241.046857] usb 1-2: nameid=82, len=1, fallback name='Extension Unit'
[ 6241.046862] usb 1-2: [52] PU [ Switch] ch = 1, val = 0/1
The device is using UAC_VERSION_2. The code in include/uapi/linux/usb/audio.h is
a bit hard to read since uac_mixer_unit_descriptor has a variable length and the
code is adding several offset, I'll need to add more printfs there to check if
it is correct. I am consulting
https://www.usb.org/sites/default/files/audio10.pdf but I am not sure if this
covers UAC2.
Stefan
>
>
> thanks,
>
> Takashi
>
> --- a/sound/usb/mixer.c
> +++ b/sound/usb/mixer.c
> @@ -2358,8 +2358,10 @@ static int build_audio_procunit(struct mixer_build *state, int unitid,
> for (info = list; info && info->type; info++)
> if (info->type == type)
> break;
> - if (!info || !info->type)
> + if (!info || !info->type) {
> + pr_info("XXX unit %d to default_info\n", unitid);
> info = &default_info;
> + }
>
> for (valinfo = info->values; valinfo->control; valinfo++) {
> __u8 *controls = uac_processing_unit_bmControls(desc, state->mixer->protocol);
> @@ -2375,8 +2377,11 @@ static int build_audio_procunit(struct mixer_build *state, int unitid,
> }
>
> map = find_map(state->map, unitid, valinfo->control);
> + if (map)
> + pr_info("XXX map found for unit %d\n", unitid);
> if (check_ignored_ctl(map))
> continue;
> +
> cval = kzalloc(sizeof(*cval), GFP_KERNEL);
> if (!cval)
> return -ENOMEM;
> @@ -2444,19 +2449,26 @@ static int build_audio_procunit(struct mixer_build *state, int unitid,
> kctl->private_free = snd_usb_mixer_elem_free;
>
> if (check_mapped_name(map, kctl->id.name, sizeof(kctl->id.name))) {
> + pr_info("XXX name copied from map: '%s' for unit %d\n", kctl->id.name, unitid);
> /* nothing */ ;
> } else if (info->name) {
> strlcpy(kctl->id.name, info->name, sizeof(kctl->id.name));
> + pr_info("XXX name copied from info: '%s' for unit %d\n", kctl->id.name, unitid);
> } else {
> nameid = uac_processing_unit_iProcessing(desc, state->mixer->protocol);
> + pr_info("XXX nameid=%d, unit %d\n", nameid, unitid);
> len = 0;
> - if (nameid)
> + if (nameid) {
> len = snd_usb_copy_string_desc(state->chip,
> nameid,
> kctl->id.name,
> sizeof(kctl->id.name));
> - if (!len)
> + pr_info("XXX copy string desc '%s'\n", kctl->id.name);
> + }
> + if (!len) {
> strlcpy(kctl->id.name, name, sizeof(kctl->id.name));
> + pr_info("XXX copy default name '%s'\n", kctl->id.name);
> + }
> }
> append_ctl_name(kctl, " ");
> append_ctl_name(kctl, valinfo->suffix);
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: focusrite scarlett 18i20 : Mixer controls with corrupted names for
2019-06-26 20:06 ` Stefan Sauer
@ 2019-06-26 20:13 ` Stefan Sauer
2019-06-26 21:42 ` Takashi Iwai
1 sibling, 0 replies; 9+ messages in thread
From: Stefan Sauer @ 2019-06-26 20:13 UTC (permalink / raw)
To: alsa-devel
Am 26.06.19 um 22:06 schrieb Stefan Sauer:
> Am 25.06.19 um 09:54 schrieb Takashi Iwai:
>> On Sat, 22 Jun 2019 22:55:25 +0200,
>> Stefan Sauer wrote:
>>>
>>> So the first 8 controls are added somewhere else. Looks like this is from
>>> mixer.c and after
>>> echo -n 'file sound/usb/mixer.c +p' >/sys/kernel/debug/dynamic_debug/control
>>> I get
>>> [ 4405.855432] usb 1-2: [51] PU [KKKKKKKKKKKKKÃÃÃÃÃÃÃÃÃÃÃ Switch] ch = 1, val = 0/1
>>> [ 4405.856423] usb 1-2: [52] PU [ Switch] ch = 1, val = 0/1
>>
>> This indicates that these weird names come from the two extension
>> units id 51 and 52.
>> Could you put some debug print in build_audio_procunit() like below?
>
> I traced it down to this part from sound/usb/mixer.c:
> nameid = uac_processing_unit_iProcessing(desc, state->mixer->protocol);
> usb_audio_dbg(state->chip, "desc->bUnitID=%d, proto=%d, nameid=%d\n",
> desc->bUnitID, state->mixer->protocol, nameid);
> len = 0;
> if (nameid) {
> len = snd_usb_copy_string_desc(state->chip,
> nameid,
> kctl->id.name,
> sizeof(kctl->id.name));
> usb_audio_dbg(state->chip, "nameid=%d, len=%d, fallback name='%s'\n",
> nameid, len, name);
> }
>
> [ 6241.045734] usb 1-2: desc->bUnitID=51, proto=32, nameid=90
> [ 6241.045861] usb 1-2: nameid=90, len=35, fallback name='Extension Unit'
> [ 6241.045868] usb 1-2: [51] PU [KKKKKKKKKKKKKÃÃÃÃÃÃÃÃÃÃÃ Switch] ch = 1, val = 0/1
> [ 6241.046745] usb 1-2: desc->bUnitID=52, proto=32, nameid=82
> [ 6241.046857] usb 1-2: nameid=82, len=1, fallback name='Extension Unit'
> [ 6241.046862] usb 1-2: [52] PU [ Switch] ch = 1, val = 0/1
and with the bUnitID I get to these descriptors from lsusb:
AudioControl Interface Descriptor:
bLength 16
bDescriptorType 36
bDescriptorSubtype 9 (EXTENSION_UNIT)
bUnitID 51
wExtensionCode 0x0000
bNrInPins 1
baSourceID(0) 2
bNrChannels 20
bmChannelConfig 0x00000000
iChannelNames 0
bmControls 0x03
Enable Control (read/write)
iExtension 0
AudioControl Interface Descriptor:
bLength 16
bDescriptorType 36
bDescriptorSubtype 9 (EXTENSION_UNIT)
bUnitID 52
wExtensionCode 0x0000
bNrInPins 1
baSourceID(0) 1
bNrChannels 20
bmChannelConfig 0x00000000
iChannelNames 0
bmControls 0x03
Enable Control (read/write)
iExtension 0
>
> The device is using UAC_VERSION_2. The code in include/uapi/linux/usb/audio.h is
> a bit hard to read since uac_mixer_unit_descriptor has a variable length and the
> code is adding several offset, I'll need to add more printfs there to check if
> it is correct. I am consulting
> https://www.usb.org/sites/default/files/audio10.pdf but I am not sure if this
> covers UAC2.
>
> Stefan
>
>>
>>
>> thanks,
>>
>> Takashi
>>
>> --- a/sound/usb/mixer.c
>> +++ b/sound/usb/mixer.c
>> @@ -2358,8 +2358,10 @@ static int build_audio_procunit(struct mixer_build *state, int unitid,
>> for (info = list; info && info->type; info++)
>> if (info->type == type)
>> break;
>> - if (!info || !info->type)
>> + if (!info || !info->type) {
>> + pr_info("XXX unit %d to default_info\n", unitid);
>> info = &default_info;
>> + }
>>
>> for (valinfo = info->values; valinfo->control; valinfo++) {
>> __u8 *controls = uac_processing_unit_bmControls(desc, state->mixer->protocol);
>> @@ -2375,8 +2377,11 @@ static int build_audio_procunit(struct mixer_build *state, int unitid,
>> }
>>
>> map = find_map(state->map, unitid, valinfo->control);
>> + if (map)
>> + pr_info("XXX map found for unit %d\n", unitid);
>> if (check_ignored_ctl(map))
>> continue;
>> +
>> cval = kzalloc(sizeof(*cval), GFP_KERNEL);
>> if (!cval)
>> return -ENOMEM;
>> @@ -2444,19 +2449,26 @@ static int build_audio_procunit(struct mixer_build *state, int unitid,
>> kctl->private_free = snd_usb_mixer_elem_free;
>>
>> if (check_mapped_name(map, kctl->id.name, sizeof(kctl->id.name))) {
>> + pr_info("XXX name copied from map: '%s' for unit %d\n", kctl->id.name, unitid);
>> /* nothing */ ;
>> } else if (info->name) {
>> strlcpy(kctl->id.name, info->name, sizeof(kctl->id.name));
>> + pr_info("XXX name copied from info: '%s' for unit %d\n", kctl->id.name, unitid);
>> } else {
>> nameid = uac_processing_unit_iProcessing(desc, state->mixer->protocol);
>> + pr_info("XXX nameid=%d, unit %d\n", nameid, unitid);
>> len = 0;
>> - if (nameid)
>> + if (nameid) {
>> len = snd_usb_copy_string_desc(state->chip,
>> nameid,
>> kctl->id.name,
>> sizeof(kctl->id.name));
>> - if (!len)
>> + pr_info("XXX copy string desc '%s'\n", kctl->id.name);
>> + }
>> + if (!len) {
>> strlcpy(kctl->id.name, name, sizeof(kctl->id.name));
>> + pr_info("XXX copy default name '%s'\n", kctl->id.name);
>> + }
>> }
>> append_ctl_name(kctl, " ");
>> append_ctl_name(kctl, valinfo->suffix);
>> _______________________________________________
>> Alsa-devel mailing list
>> Alsa-devel@alsa-project.org
>> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>>
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: focusrite scarlett 18i20 : Mixer controls with corrupted names for
2019-06-26 20:06 ` Stefan Sauer
2019-06-26 20:13 ` Stefan Sauer
@ 2019-06-26 21:42 ` Takashi Iwai
2019-06-29 12:32 ` Stefan Sauer
1 sibling, 1 reply; 9+ messages in thread
From: Takashi Iwai @ 2019-06-26 21:42 UTC (permalink / raw)
To: Stefan Sauer; +Cc: alsa-devel
On Wed, 26 Jun 2019 22:06:32 +0200,
Stefan Sauer wrote:
>
> Am 25.06.19 um 09:54 schrieb Takashi Iwai:
> > On Sat, 22 Jun 2019 22:55:25 +0200,
> > Stefan Sauer wrote:
> >>
> >> So the first 8 controls are added somewhere else. Looks like this is from
> >> mixer.c and after
> >> echo -n 'file sound/usb/mixer.c +p' >/sys/kernel/debug/dynamic_debug/control
> >> I get
> >> [ 4405.855432] usb 1-2: [51] PU [KKKKKKKKKKKKKÃÃÃÃÃÃÃÃÃÃÃ Switch] ch = 1, val = 0/1
> >> [ 4405.856423] usb 1-2: [52] PU [ Switch] ch = 1, val = 0/1
> >
> > This indicates that these weird names come from the two extension
> > units id 51 and 52.
> > Could you put some debug print in build_audio_procunit() like below?
>
> I traced it down to this part from sound/usb/mixer.c:
> nameid = uac_processing_unit_iProcessing(desc, state->mixer->protocol);
> usb_audio_dbg(state->chip, "desc->bUnitID=%d, proto=%d, nameid=%d\n",
> desc->bUnitID, state->mixer->protocol, nameid);
> len = 0;
> if (nameid) {
> len = snd_usb_copy_string_desc(state->chip,
> nameid,
> kctl->id.name,
> sizeof(kctl->id.name));
> usb_audio_dbg(state->chip, "nameid=%d, len=%d, fallback name='%s'\n",
> nameid, len, name);
> }
>
> [ 6241.045734] usb 1-2: desc->bUnitID=51, proto=32, nameid=90
> [ 6241.045861] usb 1-2: nameid=90, len=35, fallback name='Extension Unit'
> [ 6241.045868] usb 1-2: [51] PU [KKKKKKKKKKKKKÃÃÃÃÃÃÃÃÃÃÃ Switch] ch = 1, val = 0/1
> [ 6241.046745] usb 1-2: desc->bUnitID=52, proto=32, nameid=82
> [ 6241.046857] usb 1-2: nameid=82, len=1, fallback name='Extension Unit'
> [ 6241.046862] usb 1-2: [52] PU [ Switch] ch = 1, val = 0/1
>
> The device is using UAC_VERSION_2. The code in include/uapi/linux/usb/audio.h is
> a bit hard to read since uac_mixer_unit_descriptor has a variable length and the
> code is adding several offset, I'll need to add more printfs there to check if
> it is correct. I am consulting
> https://www.usb.org/sites/default/files/audio10.pdf but I am not sure if this
> covers UAC2.
UAC2 is completely different from UAC1, so you can forget that PDF.
I think I found the culprit. The bug was that UAC2 extension unit
descriptor has a very slight difference from UAC2 processing unit
descriptor: namely, the size of bmControls field is 1 while processing
unit has 2. And iExtension follows that, so we're reading a wrong
offset.
That's the reason a bogus nameid like 90 is read.
Could you try the patch below?
thanks,
Takashi
--- a/include/uapi/linux/usb/audio.h
+++ b/include/uapi/linux/usb/audio.h
@@ -450,6 +450,37 @@ static inline __u8 *uac_processing_unit_specific(struct uac_processing_unit_desc
}
}
+static inline __u8 uac_extension_unit_bControlSize(struct uac_processing_unit_descriptor *desc,
+ int protocol)
+{
+ switch (protocol) {
+ case UAC_VERSION_1:
+ return desc->baSourceID[desc->bNrInPins + 4];
+ case UAC_VERSION_2:
+ return 1; /* in UAC2, this value is constant */
+ case UAC_VERSION_3:
+ return 4; /* in UAC3, this value is constant */
+ default:
+ return 1;
+ }
+}
+
+static inline __u8 uac_extension_unit_iExtension(struct uac_processing_unit_descriptor *desc,
+ int protocol)
+{
+ __u8 control_size = uac_extension_unit_bControlSize(desc, protocol);
+
+ switch (protocol) {
+ case UAC_VERSION_1:
+ case UAC_VERSION_2:
+ default:
+ return *(uac_processing_unit_bmControls(desc, protocol)
+ + control_size);
+ case UAC_VERSION_3:
+ return 0; /* UAC3 does not have this field */
+ }
+}
+
/* 4.5.2 Class-Specific AS Interface Descriptor */
struct uac1_as_header_descriptor {
__u8 bLength; /* in bytes: 7 */
diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index e003b5e7b01a..ac121b10c51c 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -2318,7 +2318,7 @@ static struct procunit_info extunits[] = {
*/
static int build_audio_procunit(struct mixer_build *state, int unitid,
void *raw_desc, struct procunit_info *list,
- char *name)
+ bool extension_unit)
{
struct uac_processing_unit_descriptor *desc = raw_desc;
int num_ins;
@@ -2335,6 +2335,8 @@ static int build_audio_procunit(struct mixer_build *state, int unitid,
static struct procunit_info default_info = {
0, NULL, default_value_info
};
+ const char *name = extension_unit ?
+ "Extension Unit" : "Processing Unit";
if (desc->bLength < 13) {
usb_audio_err(state->chip, "invalid %s descriptor (id %d)\n", name, unitid);
@@ -2448,7 +2450,10 @@ static int build_audio_procunit(struct mixer_build *state, int unitid,
} else if (info->name) {
strlcpy(kctl->id.name, info->name, sizeof(kctl->id.name));
} else {
- nameid = uac_processing_unit_iProcessing(desc, state->mixer->protocol);
+ if (extension_unit)
+ nameid = uac_extension_unit_iExtension(desc, state->mixer->protocol);
+ else
+ nameid = uac_processing_unit_iProcessing(desc, state->mixer->protocol);
len = 0;
if (nameid)
len = snd_usb_copy_string_desc(state->chip,
@@ -2481,10 +2486,10 @@ static int parse_audio_processing_unit(struct mixer_build *state, int unitid,
case UAC_VERSION_2:
default:
return build_audio_procunit(state, unitid, raw_desc,
- procunits, "Processing Unit");
+ procunits, false);
case UAC_VERSION_3:
return build_audio_procunit(state, unitid, raw_desc,
- uac3_procunits, "Processing Unit");
+ uac3_procunits, false);
}
}
@@ -2495,8 +2500,7 @@ static int parse_audio_extension_unit(struct mixer_build *state, int unitid,
* Note that we parse extension units with processing unit descriptors.
* That's ok as the layout is the same.
*/
- return build_audio_procunit(state, unitid, raw_desc,
- extunits, "Extension Unit");
+ return build_audio_procunit(state, unitid, raw_desc, extunits, true);
}
/*
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: focusrite scarlett 18i20 : Mixer controls with corrupted names for
2019-06-26 21:42 ` Takashi Iwai
@ 2019-06-29 12:32 ` Stefan Sauer
2019-06-29 15:31 ` Takashi Iwai
0 siblings, 1 reply; 9+ messages in thread
From: Stefan Sauer @ 2019-06-29 12:32 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
Am 26.06.19 um 23:42 schrieb Takashi Iwai:
> On Wed, 26 Jun 2019 22:06:32 +0200,
> Stefan Sauer wrote:
>>
>> Am 25.06.19 um 09:54 schrieb Takashi Iwai:
>>> On Sat, 22 Jun 2019 22:55:25 +0200,
>>> Stefan Sauer wrote:
>>>>
>>>> So the first 8 controls are added somewhere else. Looks like this is from
>>>> mixer.c and after
>>>> echo -n 'file sound/usb/mixer.c +p' >/sys/kernel/debug/dynamic_debug/control
>>>> I get
>>>> [ 4405.855432] usb 1-2: [51] PU [KKKKKKKKKKKKKÃÃÃÃÃÃÃÃÃÃÃ Switch] ch = 1, val = 0/1
>>>> [ 4405.856423] usb 1-2: [52] PU [ Switch] ch = 1, val = 0/1
>>>
>>> This indicates that these weird names come from the two extension
>>> units id 51 and 52.
>>> Could you put some debug print in build_audio_procunit() like below?
Thanks for the patch! With that I get nameid=0 for both PUs:
[ 976.445182] usb 1-2: desc->bUnitID=51, proto=32, nameid=0
[ 976.445186] usb 1-2: fallback name='Extension Unit'
[ 976.445189] usb 1-2: [51] PU [Extension Unit Switch] ch = 1, val = 0/1
[ 976.446052] usb 1-2: [40] SU [Scarlett 18i20 USB-Sync Clock Source] items = 3
[ 976.446059] usb 1-2: desc->bUnitID=52, proto=32, nameid=0
[ 976.446061] usb 1-2: fallback name='Extension Unit'
[ 976.446064] usb 1-2: [52] PU [Extension Unit Switch] ch = 1, val = 0/1
on eocmment for the patch below.
>>
>> I traced it down to this part from sound/usb/mixer.c:
>> nameid = uac_processing_unit_iProcessing(desc, state->mixer->protocol);
>> usb_audio_dbg(state->chip, "desc->bUnitID=%d, proto=%d, nameid=%d\n",
>> desc->bUnitID, state->mixer->protocol, nameid);
>> len = 0;
>> if (nameid) {
>> len = snd_usb_copy_string_desc(state->chip,
>> nameid,
>> kctl->id.name,
>> sizeof(kctl->id.name));
>> usb_audio_dbg(state->chip, "nameid=%d, len=%d, fallback name='%s'\n",
>> nameid, len, name);
>> }
>>
>> [ 6241.045734] usb 1-2: desc->bUnitID=51, proto=32, nameid=90
>> [ 6241.045861] usb 1-2: nameid=90, len=35, fallback name='Extension Unit'
>> [ 6241.045868] usb 1-2: [51] PU [KKKKKKKKKKKKKÃÃÃÃÃÃÃÃÃÃÃ Switch] ch = 1, val = 0/1
>> [ 6241.046745] usb 1-2: desc->bUnitID=52, proto=32, nameid=82
>> [ 6241.046857] usb 1-2: nameid=82, len=1, fallback name='Extension Unit'
>> [ 6241.046862] usb 1-2: [52] PU [ Switch] ch = 1, val = 0/1
>>
>> The device is using UAC_VERSION_2. The code in include/uapi/linux/usb/audio.h is
>> a bit hard to read since uac_mixer_unit_descriptor has a variable length and the
>> code is adding several offset, I'll need to add more printfs there to check if
>> it is correct. I am consulting
>> https://www.usb.org/sites/default/files/audio10.pdf but I am not sure if this
>> covers UAC2.
>
> UAC2 is completely different from UAC1, so you can forget that PDF.
>
> I think I found the culprit. The bug was that UAC2 extension unit
> descriptor has a very slight difference from UAC2 processing unit
> descriptor: namely, the size of bmControls field is 1 while processing
> unit has 2. And iExtension follows that, so we're reading a wrong
> offset.
>
> That's the reason a bogus nameid like 90 is read.
>
> Could you try the patch below?
>
>
> thanks,
>
> Takashi
>
> --- a/include/uapi/linux/usb/audio.h
> +++ b/include/uapi/linux/usb/audio.h
> @@ -450,6 +450,37 @@ static inline __u8 *uac_processing_unit_specific(struct uac_processing_unit_desc
> }
> }
>
> +static inline __u8 uac_extension_unit_bControlSize(struct uac_processing_unit_descriptor *desc,
> + int protocol)
> +{
> + switch (protocol) {
> + case UAC_VERSION_1:
> + return desc->baSourceID[desc->bNrInPins + 4];
> + case UAC_VERSION_2:
> + return 1; /* in UAC2, this value is constant */
> + case UAC_VERSION_3:
> + return 4; /* in UAC3, this value is constant */
> + default:
> + return 1;
> + }
> +}
> +
> +static inline __u8 uac_extension_unit_iExtension(struct uac_processing_unit_descriptor *desc,
> + int protocol)
> +{
> + __u8 control_size = uac_extension_unit_bControlSize(desc, protocol);
> +
> + switch (protocol) {
> + case UAC_VERSION_1:
> + case UAC_VERSION_2:
> + default:
> + return *(uac_processing_unit_bmControls(desc, protocol)
> + + control_size);
if this is for 'extension_units' can we still use the offset helper for
'processing_units'?
Is the UAC2 spec available freely, if so with a link, I can probably figure this
out.
> + case UAC_VERSION_3:
> + return 0; /* UAC3 does not have this field */
> + }
> +}
> +
> /* 4.5.2 Class-Specific AS Interface Descriptor */
> struct uac1_as_header_descriptor {
> __u8 bLength; /* in bytes: 7 */
> diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> index e003b5e7b01a..ac121b10c51c 100644
> --- a/sound/usb/mixer.c
> +++ b/sound/usb/mixer.c
> @@ -2318,7 +2318,7 @@ static struct procunit_info extunits[] = {
> */
> static int build_audio_procunit(struct mixer_build *state, int unitid,
> void *raw_desc, struct procunit_info *list,
> - char *name)
> + bool extension_unit)
> {
> struct uac_processing_unit_descriptor *desc = raw_desc;
> int num_ins;
> @@ -2335,6 +2335,8 @@ static int build_audio_procunit(struct mixer_build *state, int unitid,
> static struct procunit_info default_info = {
> 0, NULL, default_value_info
> };
> + const char *name = extension_unit ?
> + "Extension Unit" : "Processing Unit";
>
> if (desc->bLength < 13) {
> usb_audio_err(state->chip, "invalid %s descriptor (id %d)\n", name, unitid);
> @@ -2448,7 +2450,10 @@ static int build_audio_procunit(struct mixer_build *state, int unitid,
> } else if (info->name) {
> strlcpy(kctl->id.name, info->name, sizeof(kctl->id.name));
> } else {
> - nameid = uac_processing_unit_iProcessing(desc, state->mixer->protocol);
> + if (extension_unit)
> + nameid = uac_extension_unit_iExtension(desc, state->mixer->protocol);
> + else
> + nameid = uac_processing_unit_iProcessing(desc, state->mixer->protocol);
> len = 0;
> if (nameid)
> len = snd_usb_copy_string_desc(state->chip,
> @@ -2481,10 +2486,10 @@ static int parse_audio_processing_unit(struct mixer_build *state, int unitid,
> case UAC_VERSION_2:
> default:
> return build_audio_procunit(state, unitid, raw_desc,
> - procunits, "Processing Unit");
> + procunits, false);
> case UAC_VERSION_3:
> return build_audio_procunit(state, unitid, raw_desc,
> - uac3_procunits, "Processing Unit");
> + uac3_procunits, false);
> }
> }
>
> @@ -2495,8 +2500,7 @@ static int parse_audio_extension_unit(struct mixer_build *state, int unitid,
> * Note that we parse extension units with processing unit descriptors.
> * That's ok as the layout is the same.
> */
> - return build_audio_procunit(state, unitid, raw_desc,
> - extunits, "Extension Unit");
> + return build_audio_procunit(state, unitid, raw_desc, extunits, true);
> }
>
> /*
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: focusrite scarlett 18i20 : Mixer controls with corrupted names for
2019-06-29 12:32 ` Stefan Sauer
@ 2019-06-29 15:31 ` Takashi Iwai
0 siblings, 0 replies; 9+ messages in thread
From: Takashi Iwai @ 2019-06-29 15:31 UTC (permalink / raw)
To: Stefan Sauer; +Cc: alsa-devel
On Sat, 29 Jun 2019 14:32:21 +0200,
Stefan Sauer wrote:
>
> Am 26.06.19 um 23:42 schrieb Takashi Iwai:
> > On Wed, 26 Jun 2019 22:06:32 +0200,
> > Stefan Sauer wrote:
> >>
> >> Am 25.06.19 um 09:54 schrieb Takashi Iwai:
> >>> On Sat, 22 Jun 2019 22:55:25 +0200,
> >>> Stefan Sauer wrote:
> >>>>
> >>>> So the first 8 controls are added somewhere else. Looks like this is from
> >>>> mixer.c and after
> >>>> echo -n 'file sound/usb/mixer.c +p' >/sys/kernel/debug/dynamic_debug/control
> >>>> I get
> >>>> [ 4405.855432] usb 1-2: [51] PU [KKKKKKKKKKKKKÃÃÃÃÃÃÃÃÃÃÃ Switch] ch = 1, val = 0/1
> >>>> [ 4405.856423] usb 1-2: [52] PU [ Switch] ch = 1, val = 0/1
> >>>
> >>> This indicates that these weird names come from the two extension
> >>> units id 51 and 52.
> >>> Could you put some debug print in build_audio_procunit() like below?
>
> Thanks for the patch! With that I get nameid=0 for both PUs:
>
> [ 976.445182] usb 1-2: desc->bUnitID=51, proto=32, nameid=0
> [ 976.445186] usb 1-2: fallback name='Extension Unit'
> [ 976.445189] usb 1-2: [51] PU [Extension Unit Switch] ch = 1, val = 0/1
> [ 976.446052] usb 1-2: [40] SU [Scarlett 18i20 USB-Sync Clock Source] items = 3
> [ 976.446059] usb 1-2: desc->bUnitID=52, proto=32, nameid=0
> [ 976.446061] usb 1-2: fallback name='Extension Unit'
> [ 976.446064] usb 1-2: [52] PU [Extension Unit Switch] ch = 1, val = 0/1
>
> on eocmment for the patch below.
Good, that's the expected behavior. The string index 0 indicates no
specific string, so it falls back.
> >> I traced it down to this part from sound/usb/mixer.c:
> >> nameid = uac_processing_unit_iProcessing(desc, state->mixer->protocol);
> >> usb_audio_dbg(state->chip, "desc->bUnitID=%d, proto=%d, nameid=%d\n",
> >> desc->bUnitID, state->mixer->protocol, nameid);
> >> len = 0;
> >> if (nameid) {
> >> len = snd_usb_copy_string_desc(state->chip,
> >> nameid,
> >> kctl->id.name,
> >> sizeof(kctl->id.name));
> >> usb_audio_dbg(state->chip, "nameid=%d, len=%d, fallback name='%s'\n",
> >> nameid, len, name);
> >> }
> >>
> >> [ 6241.045734] usb 1-2: desc->bUnitID=51, proto=32, nameid=90
> >> [ 6241.045861] usb 1-2: nameid=90, len=35, fallback name='Extension Unit'
> >> [ 6241.045868] usb 1-2: [51] PU [KKKKKKKKKKKKKÃÃÃÃÃÃÃÃÃÃÃ Switch] ch = 1, val = 0/1
> >> [ 6241.046745] usb 1-2: desc->bUnitID=52, proto=32, nameid=82
> >> [ 6241.046857] usb 1-2: nameid=82, len=1, fallback name='Extension Unit'
> >> [ 6241.046862] usb 1-2: [52] PU [ Switch] ch = 1, val = 0/1
> >>
> >> The device is using UAC_VERSION_2. The code in include/uapi/linux/usb/audio.h is
> >> a bit hard to read since uac_mixer_unit_descriptor has a variable length and the
> >> code is adding several offset, I'll need to add more printfs there to check if
> >> it is correct. I am consulting
> >> https://www.usb.org/sites/default/files/audio10.pdf but I am not sure if this
> >> covers UAC2.
> >
> > UAC2 is completely different from UAC1, so you can forget that PDF.
> >
> > I think I found the culprit. The bug was that UAC2 extension unit
> > descriptor has a very slight difference from UAC2 processing unit
> > descriptor: namely, the size of bmControls field is 1 while processing
> > unit has 2. And iExtension follows that, so we're reading a wrong
> > offset.
> >
> > That's the reason a bogus nameid like 90 is read.
> >
> > Could you try the patch below?
> >
> >
> > thanks,
> >
> > Takashi
> >
> > --- a/include/uapi/linux/usb/audio.h
> > +++ b/include/uapi/linux/usb/audio.h
> > @@ -450,6 +450,37 @@ static inline __u8 *uac_processing_unit_specific(struct uac_processing_unit_desc
> > }
> > }
> >
> > +static inline __u8 uac_extension_unit_bControlSize(struct uac_processing_unit_descriptor *desc,
> > + int protocol)
> > +{
> > + switch (protocol) {
> > + case UAC_VERSION_1:
> > + return desc->baSourceID[desc->bNrInPins + 4];
> > + case UAC_VERSION_2:
> > + return 1; /* in UAC2, this value is constant */
> > + case UAC_VERSION_3:
> > + return 4; /* in UAC3, this value is constant */
> > + default:
> > + return 1;
> > + }
> > +}
> > +
> > +static inline __u8 uac_extension_unit_iExtension(struct uac_processing_unit_descriptor *desc,
> > + int protocol)
> > +{
> > + __u8 control_size = uac_extension_unit_bControlSize(desc, protocol);
> > +
> > + switch (protocol) {
> > + case UAC_VERSION_1:
> > + case UAC_VERSION_2:
> > + default:
> > + return *(uac_processing_unit_bmControls(desc, protocol)
> > + + control_size);
>
> if this is for 'extension_units' can we still use the offset helper for
> 'processing_units'?
Yes, until this point, both extension units and processing units are
identical. The difference comes after that.
> Is the UAC2 spec available freely, if so with a link, I can probably figure this
> out.
You should be able to find on the net, I suppose.
In anyway I'm going to prepare the proper patch for merging to
upstream.
thanks,
Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-06-29 15:31 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-06-22 20:55 focusrite scarlett 18i20 : Mixer controls with corrupted names for Stefan Sauer
2019-06-22 21:17 ` Stefan Sauer
2019-06-23 11:28 ` Stefan Sauer
2019-06-25 7:54 ` Takashi Iwai
2019-06-26 20:06 ` Stefan Sauer
2019-06-26 20:13 ` Stefan Sauer
2019-06-26 21:42 ` Takashi Iwai
2019-06-29 12:32 ` Stefan Sauer
2019-06-29 15:31 ` Takashi Iwai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox