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@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
