[PATCH v2] ALSA: usb-audio: Skip broken playback feature unit on XiiSound chips

[PATCH v2] ALSA: usb-audio: Skip broken playback feature unit on XiiSound chips

[Cristiano Souza]

Weltrend XiiSound-based devices (040b:0897, e.g. Redragon H878
wireless headset) expose a UAC1 playback feature unit whose volume
and mute controls are unusable in practice: the reported per-channel
dB range is only 0..+0.39dB, yet setting any volume below the
maximum mutes one or even both channels, and the mapping between
logical and physical channels is unreliable.  Any attempt by
userspace to drive the hardware volume therefore results in muted
or one-sided audio.

Ignore the playback feature unit via a usbmix_name_map so that
userspace falls back to software volume, and pin the unit's master
and per-channel volumes to the reported maximum with mutes cleared
when the mixer is created, so the signal path always stays fully
open.  The capture path (microphone feature unit) is not affected
and keeps working.

Tested with a Redragon H878 wireless headset on PipeWire/PulseAudio:
stereo output works on both channels at any volume, software volume
applies smoothly, and mute/unmute works.

Cc: stable@vger.kernel.org
Signed-off-by: Cristiano Souza <cristianocps@gmail.com>
---
Changes in v2:
- Skip the broken playback feature unit via a usbmix_name_map in
  mixer_maps.c instead of open-coding the skip in the FU parser, as
  suggested by Takashi Iwai.
- The pin-to-maximum/unmute initialization in mixer_quirks.c is
  unchanged.
 sound/usb/mixer_maps.c   | 19 ++++++++++++++
 sound/usb/mixer_quirks.c | 56 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+)

diff --git a/sound/usb/mixer_maps.c b/sound/usb/mixer_maps.c
index 98b4146..9218ca6 100644
--- a/sound/usb/mixer_maps.c
+++ b/sound/usb/mixer_maps.c
@@ -374,6 +374,20 @@ static const struct usbmix_name_map dell_alc4020_map[] = {
 	{ 0 }
 };
 
+/*
+ * Weltrend XiiSound chips (040b:0897, e.g. Redragon H878):
+ * the playback feature unit is broken; its volume controls act like
+ * mute switches below the maximum value and the channel mapping is
+ * unreliable, so any hardware volume change mutes one or both
+ * channels.  Skip the unit entirely and let userspace fall back to
+ * software volume; the unit is pinned to maximum/unmuted by the
+ * mixer quirk in mixer_quirks.c.
+ */
+static const struct usbmix_name_map xiisound_map[] = {
+	{ 5, NULL },	/* playback feature unit */
+	{ 0 }
+};
+
 /*
  * Corsair Virtuoso calls everything "Headset" without this, leading to
  * applications moving the sidetone control instead of the main one.
@@ -790,6 +804,11 @@ static const struct usbmix_ctl_map usbmix_ctl_maps[] = {
 		.id = USB_ID(0x1395, 0x0025),
 		.map = sennheiser_pc8_map,
 	},
+	{
+		/* Weltrend XiiSound (Redragon H878) */
+		.id = USB_ID(0x040b, 0x0897),
+		.map = xiisound_map,
+	},
 	{
 		/* Microsoft USB Link headset */
 		.id = USB_ID(0x045e, 0x083c),
diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c
index 5839719..ee12bc7 100644
--- a/sound/usb/mixer_quirks.c
+++ b/sound/usb/mixer_quirks.c
@@ -4603,6 +4603,59 @@ static int snd_audiofuse_controls_create(struct usb_mixer_interface *mixer)
 	return err;
 }
 
+/*
+ * Weltrend XiiSound chips (040b:0897, e.g. Redragon H878):
+ * the playback feature unit (id 5) is broken; its volume controls act like
+ * mute switches below the maximum value and the channel mapping is
+ * unreliable, so the unit is hidden from userspace entirely (see the
+ * quirk in mixer_maps.c) and userspace falls back to software volume.
+ * Pin the unit's master and channel volumes to the reported maximum
+ * and unmute them, so the audio path stays fully open.
+ */
+static int snd_xiisound_init_fu_pins(struct usb_mixer_interface *mixer)
+{
+	struct snd_usb_audio *chip = mixer->chip;
+	unsigned char buf[2];
+	int ifnum, ch, err;
+
+	if (mixer->protocol != UAC_VERSION_1)
+		return 0;
+
+	ifnum = snd_usb_ctrl_intf(chip->ctrl_intf);
+
+	/* unit 5 is the playback feature unit on these chips */
+	for (ch = 0; ch <= 2; ch++) {
+		err = snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0),
+				      UAC_GET_MAX,
+				      USB_DIR_IN | USB_TYPE_CLASS |
+				      USB_RECIP_INTERFACE,
+				      (UAC_FU_VOLUME << 8) | ch,
+				      (5 << 8) | ifnum, buf, 2);
+		if (err < 0)
+			return 0; /* no such unit/channels, nothing to pin */
+
+		err = snd_usb_ctl_msg(chip->dev, usb_sndctrlpipe(chip->dev, 0),
+				      UAC_SET_CUR,
+				      USB_DIR_OUT | USB_TYPE_CLASS |
+				      USB_RECIP_INTERFACE,
+				      (UAC_FU_VOLUME << 8) | ch,
+				      (5 << 8) | ifnum, buf, 2);
+		if (err < 0)
+			return err;
+
+		buf[0] = 0; /* unmute */
+		err = snd_usb_ctl_msg(chip->dev, usb_sndctrlpipe(chip->dev, 0),
+				      UAC_SET_CUR,
+				      USB_DIR_OUT | USB_TYPE_CLASS |
+				      USB_RECIP_INTERFACE,
+				      (UAC_FU_MUTE << 8) | ch,
+				      (5 << 8) | ifnum, buf, 1);
+		if (err < 0)
+			return err;
+	}
+	return 0;
+}
+
 int snd_usb_mixer_apply_create_quirk(struct usb_mixer_interface *mixer)
 {
 	int err = 0;
@@ -4616,6 +4669,9 @@ int snd_usb_mixer_apply_create_quirk(struct usb_mixer_interface *mixer)
 	case USB_ID(0x0644, 0x8047):
 		err = snd_us16x08_controls_create(mixer);
 		break;
+	case USB_ID(0x040b, 0x0897):
+		err = snd_xiisound_init_fu_pins(mixer);
+		break;
 	case USB_ID(0x041e, 0x3020):
 	case USB_ID(0x041e, 0x3040):
 	case USB_ID(0x041e, 0x3042):
-- 
2.43.0

Re: [PATCH v2] ALSA: usb-audio: Skip broken playback feature unit on XiiSound chips

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] ALSA: usb-audio: missing unit 5 re-pinning in
  snd_usb_mixer_resume_quirk()
- [Medium] ALSA: usb-audio: missing short-read check in
  snd_xiisound_init_fu_pins()
--

commit 599ffb39dcedddf5b2afe433d25ebe88e2031e78
Author: Cristiano Souza <cristianocps@gmail.com>

ALSA: usb-audio: Skip broken playback feature unit on XiiSound chips

This commit hides the broken playback feature unit (unit 5) on Weltrend
XiiSound devices (040b:0897) via usbmix_name_map and pins its volume to
maximum with mute cleared during mixer creation.

> diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c
> index 5839719444f8b..ee12bc7d1cacc 100644
> --- a/sound/usb/mixer_quirks.c
> +++ b/sound/usb/mixer_quirks.c
> @@ -4603,6 +4603,59 @@ static int snd_audiofuse_controls_create(struct usb_mixer_interface *mixer)
>  	return err;
>  }
>  
> +/*
> + * Weltrend XiiSound chips (040b:0897, e.g. Redragon H878):
> + * the playback feature unit (id 5) is broken; its volume controls act like
> + * mute switches below the maximum value and the channel mapping is
> + * unreliable, so the unit is hidden from userspace entirely (see the
> + * quirk in mixer_maps.c) and userspace falls back to software volume.
> + * Pin the unit's master and channel volumes to the reported maximum
> + * and unmute them, so the audio path stays fully open.
> + */
> +static int snd_xiisound_init_fu_pins(struct usb_mixer_interface *mixer)
> +{
> +	struct snd_usb_audio *chip = mixer->chip;
> +	unsigned char buf[2];
> +	int ifnum, ch, err;
> +
> +	if (mixer->protocol != UAC_VERSION_1)
> +		return 0;
> +
> +	ifnum = snd_usb_ctrl_intf(chip->ctrl_intf);
> +
> +	/* unit 5 is the playback feature unit on these chips */
> +	for (ch = 0; ch <= 2; ch++) {
> +		err = snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0),
> +				      UAC_GET_MAX,
> +				      USB_DIR_IN | USB_TYPE_CLASS |
> +				      USB_RECIP_INTERFACE,
> +				      (UAC_FU_VOLUME << 8) | ch,
> +				      (5 << 8) | ifnum, buf, 2);
> +		if (err < 0)
> +			return 0; /* no such unit/channels, nothing to pin */
> +
> +		err = snd_usb_ctl_msg(chip->dev, usb_sndctrlpipe(chip->dev, 0),
> +				      UAC_SET_CUR,
> +				      USB_DIR_OUT | USB_TYPE_CLASS |
> +				      USB_RECIP_INTERFACE,
> +				      (UAC_FU_VOLUME << 8) | ch,
> +				      (5 << 8) | ifnum, buf, 2);

[Severity: Medium]
Can a short read from UAC_GET_MAX cause UAC_SET_CUR to send uninitialized
stack bytes from buf[]?

snd_usb_ctl_msg() returns the transferred byte count from usb_control_msg().
If UAC_GET_MAX returns 0 or 1 bytes, checking only err < 0 lets execution
proceed to UAC_SET_CUR, writing uninitialized stack memory on ch == 0 (or
buf[0] overwritten with 0 by UAC_FU_MUTE on ch > 0) to the hardware volume
control.  Should this check err < 2 instead?

> +		if (err < 0)
> +			return err;
> +
> +		buf[0] = 0; /* unmute */
> +		err = snd_usb_ctl_msg(chip->dev, usb_sndctrlpipe(chip->dev, 0),
> +				      UAC_SET_CUR,
> +				      USB_DIR_OUT | USB_TYPE_CLASS |
> +				      USB_RECIP_INTERFACE,
> +				      (UAC_FU_MUTE << 8) | ch,
> +				      (5 << 8) | ifnum, buf, 1);
> +		if (err < 0)
> +			return err;
> +	}
> +	return 0;
> +}
> +
>  int snd_usb_mixer_apply_create_quirk(struct usb_mixer_interface *mixer)
>  {
>  	int err = 0;
> @@ -4616,6 +4669,9 @@ int snd_usb_mixer_apply_create_quirk(struct usb_mixer_interface *mixer)
>  	case USB_ID(0x0644, 0x8047):
>  		err = snd_us16x08_controls_create(mixer);
>  		break;
> +	case USB_ID(0x040b, 0x0897):
> +		err = snd_xiisound_init_fu_pins(mixer);
> +		break;

[Severity: Medium]
Should snd_xiisound_init_fu_pins() also be called from
snd_usb_mixer_resume_quirk(), similar to dell_dock_mixer_init() for
USB_ID(0x0bda, 0x4014)?

Because xiisound_map ignores unit 5, __build_feature_ctl() never registers an
element in mixer->id_elems[5], so snd_usb_mixer_resume() ->
restore_mixer_value() will not restore unit 5 after a suspend/resume cycle
that resets the device, leaving playback audio muted or broken with no userspace
controls to recover it.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261107020622.119529-1-cristianocps@gmail.com?part=1