[PATCH v2 0/3] ALSA: usb-audio: add module param device_quirk_flags

[PATCH v2 0/3] ALSA: usb-audio: add module param device_quirk_flags

[Cryolitia PukNgae via B4 Relay]

As an implementation of what has been discussed previously[1].

> An open question is whether we may want yet a new module option or
> rather extend the existing quirk option to accept the strings
> instead.  Basically, when the given argument has a colon, it's a new
> syntax.  If it's only a number, it's an old syntax, and parse like
> before.  But, I'm open for either way (a new option or extend the
> existing one).

I would like to add a new param. The existed param
`static unsigned int quirk_flags[SNDRV_CARDS]` seems to related to
some sequence the card probed. To be honest, I havn't fully understood
it. And it seems hard to improve it while keeping compatibility.

1. https://lore.kernel.org/all/87h5xm5g7f.wl-tiwai@suse.de/

Signed-off-by: Cryolitia PukNgae <cryolitia@uniontech.com>
---
Changes in v2:
- Cleaned up some internal rebase confusion, sorry for that
- Link to v1: https://lore.kernel.org/r/20250912-sound-v1-0-cc9cfd9f2d01@uniontech.com

---
Cryolitia PukNgae (3):
      ALSA: usb-audio: add two-way convert between name and bit for QUIRK_FLAG_*
      ALSA: usb-audio: add module param device_quirk_flags
      ALSA: doc: add docs about device_device_quirk_flags in snd-usb-audio

 Documentation/sound/alsa-configuration.rst | 105 ++++++++++++------
 sound/usb/card.c                           | 165 ++++++++++++++++++++++++++++-
 sound/usb/quirks.c                         | 110 ++++++++++++++++++-
 sound/usb/quirks.h                         |   5 +
 sound/usb/usbaudio.h                       |  14 +++
 5 files changed, 362 insertions(+), 37 deletions(-)
---
base-commit: 82ad508a85dc64cb4bd648edec5a4ce741648426
change-id: 20250910-sound-a91c86c92dba

Best regards,
-- 
Cryolitia PukNgae <cryolitia@uniontech.com>

[PATCH v2 1/3] ALSA: usb-audio: add two-way convert between name and bit for QUIRK_FLAG_*

[Cryolitia PukNgae via B4 Relay]

From: Cryolitia PukNgae <cryolitia@uniontech.com>

Also improve debug logs for applied quirks

Signed-off-by: Cryolitia PukNgae <cryolitia@uniontech.com>
---
 sound/usb/quirks.c   | 82 +++++++++++++++++++++++++++++++++++++++++++++++++---
 sound/usb/quirks.h   |  3 ++
 sound/usb/usbaudio.h |  1 +
 3 files changed, 82 insertions(+), 4 deletions(-)

diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c
index d736a4750356597bfb0f9d5ab01cdaeaac0f907c..94854f352b1702b491e1bf3c8b769f7088e03976 100644
--- a/sound/usb/quirks.c
+++ b/sound/usb/quirks.c
@@ -2446,6 +2446,62 @@ static const struct usb_audio_quirk_flags_table quirk_flags_table[] = {
 	{} /* terminator */
 };
 
+static const char *const snd_usb_audio_quirk_flag_names[] = {
+	"get_sample_rate",
+	"share_media_device",
+	"align_transfer",
+	"tx_length",
+	"playback_first",
+	"skip_clock_selector",
+	"ignore_clock_source",
+	"itf_usb_dsd_dac",
+	"ctl_msg_delay",
+	"ctl_msg_delay_1m",
+	"ctl_msg_delay_5m",
+	"iface_delay",
+	"validate_rates",
+	"disable_autosuspend",
+	"ignore_ctl_error",
+	"dsd_raw",
+	"set_iface_first",
+	"generic_implicit_fb",
+	"skip_implicit_fb",
+	"iface_skip_close",
+	"force_iface_reset",
+	"fixed_rate",
+	"mic_res_16",
+	"mic_res_384",
+	"mixer_playback_min_mute",
+	"mixer_capture_min_mute",
+	NULL
+};
+
+const char *snd_usb_quirk_flag_find_name(unsigned long index)
+{
+	if (index >= ARRAY_SIZE(snd_usb_audio_quirk_flag_names))
+		return NULL;
+
+	return snd_usb_audio_quirk_flag_names[index];
+}
+
+u32 snd_usb_quirk_flags_from_name(char *name)
+{
+	u32 flag = 0;
+	u32 i;
+
+	if (!name || !*name)
+		return 0;
+
+	for (i = 0; snd_usb_audio_quirk_flag_names[i]; i++) {
+		if (strcmp(name, snd_usb_audio_quirk_flag_names[i]) == 0) {
+			flag = (1U << i);
+			break;
+		}
+	}
+
+	return flag;
+}
+
 void snd_usb_init_quirk_flags(struct snd_usb_audio *chip)
 {
 	const struct usb_audio_quirk_flags_table *p;
@@ -2454,10 +2510,28 @@ void snd_usb_init_quirk_flags(struct snd_usb_audio *chip)
 		if (chip->usb_id == p->id ||
 		    (!USB_ID_PRODUCT(p->id) &&
 		     USB_ID_VENDOR(chip->usb_id) == USB_ID_VENDOR(p->id))) {
-			usb_audio_dbg(chip,
-				      "Set quirk_flags 0x%x for device %04x:%04x\n",
-				      p->flags, USB_ID_VENDOR(chip->usb_id),
-				      USB_ID_PRODUCT(chip->usb_id));
+			unsigned long flags = p->flags;
+			unsigned long bit;
+
+			for_each_set_bit(bit, &flags,
+					 BYTES_TO_BITS(sizeof(p->flags))) {
+				const char *name =
+					snd_usb_audio_quirk_flag_names[bit];
+
+				if (name)
+					usb_audio_dbg(chip,
+						      "Set quirk flag %s for device %04x:%04x\n",
+						      name,
+						      USB_ID_VENDOR(chip->usb_id),
+						      USB_ID_PRODUCT(chip->usb_id));
+				else
+					usb_audio_warn(chip,
+						       "Set unknown quirk flag 0x%lx for device %04x:%04x\n",
+						       bit,
+						       USB_ID_VENDOR(chip->usb_id),
+						       USB_ID_PRODUCT(chip->usb_id));
+			}
+
 			chip->quirk_flags |= p->flags;
 			return;
 		}
diff --git a/sound/usb/quirks.h b/sound/usb/quirks.h
index f9bfd5ac7bab01717de3a76227482a128bf73165..bd5baf2b193a1985f3a0e52bf4a77ca741364769 100644
--- a/sound/usb/quirks.h
+++ b/sound/usb/quirks.h
@@ -50,4 +50,7 @@ void snd_usb_audioformat_attributes_quirk(struct snd_usb_audio *chip,
 
 void snd_usb_init_quirk_flags(struct snd_usb_audio *chip);
 
+const char *snd_usb_quirk_flag_find_name(unsigned long flag);
+u32 snd_usb_quirk_flags_from_name(char *name);
+
 #endif /* __USBAUDIO_QUIRKS_H */
diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h
index 30b5102e3caed01eeb86d0075c41338104c58950..0a22cb4a02344b2dcf4009c560a759f2da25ca67 100644
--- a/sound/usb/usbaudio.h
+++ b/sound/usb/usbaudio.h
@@ -252,5 +252,6 @@ extern bool snd_usb_skip_validation;
 #define QUIRK_FLAG_MIC_RES_384		(1U << 23)
 #define QUIRK_FLAG_MIXER_PLAYBACK_MIN_MUTE	(1U << 24)
 #define QUIRK_FLAG_MIXER_CAPTURE_MIN_MUTE	(1U << 25)
+/* Please also edit snd_usb_audio_quirk_flag_names */
 
 #endif /* __USBAUDIO_H */

-- 
2.51.0

[PATCH v2 2/3] ALSA: usb-audio: add module param device_quirk_flags

[Cryolitia PukNgae via B4 Relay]

From: Cryolitia PukNgae <cryolitia@uniontech.com>

For apply and unapply quirk flags more flexibly though param and sysfs

Signed-off-by: Cryolitia PukNgae <cryolitia@uniontech.com>
---
 sound/usb/card.c     | 165 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 sound/usb/quirks.c   |  28 +++++++++
 sound/usb/quirks.h   |   2 +
 sound/usb/usbaudio.h |  13 ++++
 4 files changed, 207 insertions(+), 1 deletion(-)

diff --git a/sound/usb/card.c b/sound/usb/card.c
index 0265206a8e8cf31133e8463c98fe0497d8ace89e..e53fd868f34e93d42b1447958fc136658a6f9dd2 100644
--- a/sound/usb/card.c
+++ b/sound/usb/card.c
@@ -77,6 +77,7 @@ static unsigned int quirk_flags[SNDRV_CARDS];
 
 bool snd_usb_use_vmalloc = true;
 bool snd_usb_skip_validation;
+char device_quirk_flags[MAX_QUIRK_PARAM_LEN];
 
 module_param_array(index, int, NULL, 0444);
 MODULE_PARM_DESC(index, "Index value for the USB audio adapter.");
@@ -110,6 +111,149 @@ MODULE_PARM_DESC(use_vmalloc, "Use vmalloc for PCM intermediate buffers (default
 module_param_named(skip_validation, snd_usb_skip_validation, bool, 0444);
 MODULE_PARM_DESC(skip_validation, "Skip unit descriptor validation (default: no).");
 
+DEFINE_MUTEX(device_quirk_mutex); /* protects device_quirk_list */
+
+struct device_quirk_entry *device_quirk_list;
+unsigned int device_quirk_count;
+
+static int device_quirks_param_set(const char *value,
+				   const struct kernel_param *kp)
+{
+	char *val, *p, *field, *flag_field;
+	u32 mask_flags, unmask_flags, bit;
+	bool is_unmask;
+	u16 vid, pid;
+	size_t i;
+	int err;
+
+	val = kstrdup(value, GFP_KERNEL);
+	if (!val)
+		return -ENOMEM;
+
+	err = param_set_copystring(val, kp);
+	if (err) {
+		kfree(val);
+		return err;
+	}
+
+	mutex_lock(&device_quirk_mutex);
+
+	if (!*val) {
+		device_quirk_count = 0;
+		kfree(device_quirk_list);
+		device_quirk_list = NULL;
+		goto unlock;
+	}
+
+	for (device_quirk_count = 1, i = 0; val[i]; i++)
+		if (val[i] == ';')
+			device_quirk_count++;
+
+	kfree(device_quirk_list);
+
+	device_quirk_list = kcalloc(device_quirk_count,
+				    sizeof(struct device_quirk_entry),
+				    GFP_KERNEL);
+	if (!device_quirk_list) {
+		device_quirk_count = 0;
+		mutex_unlock(&device_quirk_mutex);
+		kfree(val);
+		return -ENOMEM;
+	}
+
+	for (i = 0, p = val; p && *p;) {
+		/* Each entry consists of VID:PID:flags */
+		field = strsep(&p, ":");
+		if (!field)
+			break;
+
+		if (strcmp(field, "*") == 0)
+			vid = 0;
+		else if (kstrtou16(field, 16, &vid))
+			break;
+
+		field = strsep(&p, ":");
+		if (!field)
+			break;
+
+		if (strcmp(field, "*") == 0)
+			pid = 0;
+		else if (kstrtou16(field, 16, &pid))
+			break;
+
+		field = strsep(&p, ";");
+		if (!field || !*field)
+			break;
+
+		/* Collect the flags */
+		mask_flags = 0;
+		unmask_flags = 0;
+		while (field && *field) {
+			flag_field = strsep(&field, ",");
+
+			if (!flag_field)
+				break;
+
+			if (*flag_field == '!') {
+				is_unmask = true;
+				flag_field++;
+			} else {
+				is_unmask = false;
+			}
+
+			if (!kstrtou32(flag_field, 16, &bit)) {
+				if (is_unmask)
+					unmask_flags |= bit;
+				else
+					mask_flags |= bit;
+
+				break;
+			}
+
+			bit = snd_usb_quirk_flags_from_name(flag_field);
+
+			if (bit) {
+				if (is_unmask)
+					unmask_flags |= bit;
+				else
+					mask_flags |= bit;
+			} else {
+				pr_warn("snd_usb_audio: unknown flag %s while parsing param device_quirk_flags\n",
+					flag_field);
+			}
+		}
+		device_quirk_list[i++] = (struct device_quirk_entry){
+			.vid = vid,
+			.pid = pid,
+			.mask_flags = mask_flags,
+			.unmask_flags = unmask_flags,
+		};
+	}
+
+	if (i < device_quirk_count)
+		device_quirk_count = i;
+
+unlock:
+	mutex_unlock(&device_quirk_mutex);
+	kfree(val);
+
+	return 0;
+}
+
+static const struct kernel_param_ops device_quirks_param_ops = {
+	.set = device_quirks_param_set,
+	.get = param_get_string,
+};
+
+static struct kparam_string device_quirks_param_string = {
+	.maxlen = sizeof(device_quirk_flags),
+	.string = device_quirk_flags,
+};
+
+device_param_cb(device_quirk_flags, &device_quirks_param_ops,
+		&device_quirks_param_string, 0644);
+MODULE_PARM_DESC(device_quirk_flags, "Add/modify USB audio quirks");
+
 /*
  * we keep the snd_usb_audio_t instances by ourselves for merging
  * the all interfaces on the same card as one sound device.
@@ -755,6 +899,8 @@ static int snd_usb_audio_create(struct usb_interface *intf,
 	else
 		snd_usb_init_quirk_flags(chip);
 
+	snd_usb_init_dynamic_quirks(chip);
+
 	card->private_free = snd_usb_audio_free;
 
 	strscpy(card->driver, "USB-Audio");
@@ -1290,4 +1436,21 @@ static struct usb_driver usb_audio_driver = {
 	.supports_autosuspend = 1,
 };
 
-module_usb_driver(usb_audio_driver);
+static int __init usb_audio_init(void)
+{
+	return usb_register_driver(&usb_audio_driver, THIS_MODULE,
+							   KBUILD_MODNAME);
+}
+
+static void __exit usb_audio_exit(void)
+{
+	mutex_lock(&device_quirk_mutex);
+	kfree(device_quirk_list);
+	device_quirk_list = NULL;
+	mutex_unlock(&device_quirk_mutex);
+
+	usb_deregister(&usb_audio_driver);
+}
+
+module_init(usb_audio_init);
+module_exit(usb_audio_exit);
diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c
index 94854f352b1702b491e1bf3c8b769f7088e03976..bb40c747c1d25ddedeaf9df7df99edc79eacbb84 100644
--- a/sound/usb/quirks.c
+++ b/sound/usb/quirks.c
@@ -2537,3 +2537,31 @@ void snd_usb_init_quirk_flags(struct snd_usb_audio *chip)
 		}
 	}
 }
+
+void snd_usb_init_dynamic_quirks(struct snd_usb_audio *chip)
+{
+	u16 vid = USB_ID_VENDOR(chip->usb_id);
+	u16 pid = USB_ID_PRODUCT(chip->usb_id);
+	int i;
+
+	mutex_lock(&device_quirk_mutex);
+
+	for (i = 0; i < device_quirk_count; i++) {
+		if (device_quirk_list[i].vid == 0 ||
+		    (vid == device_quirk_list[i].vid &&
+		     device_quirk_list[i].pid == 0) ||
+		    (vid == device_quirk_list[i].vid &&
+		     pid == device_quirk_list[i].pid)) {
+			chip->quirk_flags |= device_quirk_list[i].mask_flags;
+			chip->quirk_flags &= ~device_quirk_list[i].unmask_flags;
+			usb_audio_dbg(chip,
+				      "Set mask quirk flag 0x%x and unmask quirk flag 0x%x from param for device %04x:%04x\n",
+				      device_quirk_list[i].mask_flags,
+				      device_quirk_list[i].unmask_flags,
+				      USB_ID_VENDOR(chip->usb_id),
+				      USB_ID_PRODUCT(chip->usb_id));
+		}
+	}
+
+	mutex_unlock(&device_quirk_mutex);
+}
diff --git a/sound/usb/quirks.h b/sound/usb/quirks.h
index bd5baf2b193a1985f3a0e52bf4a77ca741364769..1257d81bf5fd9f6b83bfdfb190f18fe08f83f12a 100644
--- a/sound/usb/quirks.h
+++ b/sound/usb/quirks.h
@@ -53,4 +53,6 @@ void snd_usb_init_quirk_flags(struct snd_usb_audio *chip);
 const char *snd_usb_quirk_flag_find_name(unsigned long flag);
 u32 snd_usb_quirk_flags_from_name(char *name);
 
+void snd_usb_init_dynamic_quirks(struct snd_usb_audio *chip);
+
 #endif /* __USBAUDIO_QUIRKS_H */
diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h
index 0a22cb4a02344b2dcf4009c560a759f2da25ca67..cb6cab37d749a258258394816e74c939cdd471fe 100644
--- a/sound/usb/usbaudio.h
+++ b/sound/usb/usbaudio.h
@@ -20,6 +20,7 @@ struct media_device;
 struct media_intf_devnode;
 
 #define MAX_CARD_INTERFACES	16
+#define MAX_QUIRK_PARAM_LEN	128
 
 /*
  * Structure holding assosiation between Audio Control Interface
@@ -165,6 +166,11 @@ DEFINE_CLASS(snd_usb_lock, struct __snd_usb_lock,
 extern bool snd_usb_use_vmalloc;
 extern bool snd_usb_skip_validation;
 
+extern struct mutex device_quirk_mutex;
+extern char device_quirk_flags[MAX_QUIRK_PARAM_LEN];
+extern unsigned int device_quirk_count;
+extern struct device_quirk_entry *device_quirk_list;
+
 /*
  * Driver behavior quirk flags, stored in chip->quirk_flags
  *
@@ -254,4 +260,11 @@ extern bool snd_usb_skip_validation;
 #define QUIRK_FLAG_MIXER_CAPTURE_MIN_MUTE	(1U << 25)
 /* Please also edit snd_usb_audio_quirk_flag_names */
 
+struct device_quirk_entry {
+	u16 vid;
+	u16 pid;
+	u32 mask_flags;
+	u32 unmask_flags;
+};
+
 #endif /* __USBAUDIO_H */

-- 
2.51.0

[PATCH v2 3/3] ALSA: doc: add docs about device_device_quirk_flags in snd-usb-audio

[Cryolitia PukNgae via B4 Relay]

From: Cryolitia PukNgae <cryolitia@uniontech.com>

Just briefly described about the new option.

Signed-off-by: Cryolitia PukNgae <cryolitia@uniontech.com>
---
 Documentation/sound/alsa-configuration.rst | 105 ++++++++++++++++++++---------
 1 file changed, 73 insertions(+), 32 deletions(-)

diff --git a/Documentation/sound/alsa-configuration.rst b/Documentation/sound/alsa-configuration.rst
index a2fb8ed251dd0294e7a62209ca15d5c32c6adfae..060dfbd4197d3134f20d3d86300d97b14071eee9 100644
--- a/Documentation/sound/alsa-configuration.rst
+++ b/Documentation/sound/alsa-configuration.rst
@@ -2296,40 +2296,81 @@ skip_validation
     The option is used to ignore the validation errors with the hexdump
     of the unit descriptor instead of a driver probe error, so that we
     can check its details.
+device_device_quirk_flags
+    The option povides a refined and flexible control for applying quirk
+    flags.  It allows to specify the quirk flags for each device, and could
+    be modified dynamically via sysfs.
+    The option accepts a string in the format of ``VID1:PID1:FLAGS1;VID2:PID2:
+    FLAGS2;...``, where ``VIDx`` and ``PIDx`` specify the device, and
+    ``FLAGSx`` specify the flags to be applied.  ``VIDx`` and ``PIDx`` are
+    4-digit hexadecimal numbers, and could be specified as ``*`` to match any
+    value.  ``FLAGSx`` could be a set of flags given by name, separated by
+    comma, or a hexadecimal number representing the bit flags.  The available
+    flag names are listed above.  An exclamation mark could be prefixed to a
+    flag name to negate the flag. For example,
+    ``1234:abcd:mixer_playback_min_mute,!ignore_ctl_error;*:*:0x01;``
+    applies the ``mixer_playback_min_mute`` flag and clears the
+    ``ignore_ctl_error`` flag for the device 1234:abcd, and applies
+    the ``skip_sample_rate`` flag for all devices.
 quirk_flags
     Contains the bit flags for various device specific workarounds.
     Applied to the corresponding card index.
 
-        * bit 0: Skip reading sample rate for devices
-        * bit 1: Create Media Controller API entries
-        * bit 2: Allow alignment on audio sub-slot at transfer
-        * bit 3: Add length specifier to transfers
-        * bit 4: Start playback stream at first in implement feedback mode
-        * bit 5: Skip clock selector setup
-        * bit 6: Ignore errors from clock source search
-        * bit 7: Indicates ITF-USB DSD based DACs
-        * bit 8: Add a delay of 20ms at each control message handling
-        * bit 9: Add a delay of 1-2ms at each control message handling
-        * bit 10: Add a delay of 5-6ms at each control message handling
-        * bit 11: Add a delay of 50ms at each interface setup
-        * bit 12: Perform sample rate validations at probe
-        * bit 13: Disable runtime PM autosuspend
-        * bit 14: Ignore errors for mixer access
-        * bit 15: Support generic DSD raw U32_BE format
-        * bit 16: Set up the interface at first like UAC1
-        * bit 17: Apply the generic implicit feedback sync mode
-        * bit 18: Don't apply implicit feedback sync mode
-        * bit 19: Don't closed interface during setting sample rate
-        * bit 20: Force an interface reset whenever stopping & restarting
-          a stream
-        * bit 21: Do not set PCM rate (frequency) when only one rate is
-          available for the given endpoint.
-        * bit 22: Set the fixed resolution 16 for Mic Capture Volume
-        * bit 23: Set the fixed resolution 384 for Mic Capture Volume
-        * bit 24: Set minimum volume control value as mute for devices
-          where the lowest playback value represents muted state instead
-          of minimum audible volume
-        * bit 25: Be similar to bit 24 but for capture streams
+        * bit 0: ``get_sample_rate``
+          Skip reading sample rate for devices
+        * bit 1: ``share_media_device``
+          Create Media Controller API entries
+        * bit 2: ``align_transfer``
+          Allow alignment on audio sub-slot at transfer
+        * bit 3: ``tx_length``
+          Add length specifier to transfers
+        * bit 4: ``playback_first``
+          Start playback stream at first in implement feedback mode
+        * bit 5: ``skip_clock_selector``
+          Skip clock selector setup
+        * bit 6: ``ignore_clock_source``
+          Ignore errors from clock source search
+        * bit 7: ``itf_usb_dsd_dac``
+          Indicates ITF-USB DSD based DACs
+        * bit 8: ``ctl_msg_delay``
+          Add a delay of 20ms at each control message handling
+        * bit 9: ``ctl_msg_delay_1m``
+          Add a delay of 1-2ms at each control message handling
+        * bit 10: ``ctl_msg_delay_5m``
+          Add a delay of 5-6ms at each control message handling
+        * bit 11: ``iface_delay``
+          Add a delay of 50ms at each interface setup
+        * bit 12: ``validate_rates``
+          Perform sample rate validations at probe
+        * bit 13: ``disable_autosuspend``
+          Disable runtime PM autosuspend
+        * bit 14: ``ignore_ctl_error``
+          Ignore errors for mixer access
+        * bit 15: ``dsd_raw``
+          Support generic DSD raw U32_BE format
+        * bit 16: ``set_iface_first``
+          Set up the interface at first like UAC1
+        * bit 17: ``generic_implicit_fb``
+          Apply the generic implicit feedback sync mode
+        * bit 18: ``skip_implicit_fb``
+          Don't apply implicit feedback sync mode
+        * bit 19: ``iface_skip_close``
+          Don't closed interface during setting sample rate
+        * bit 20: ``force_iface_reset``
+          Force an interface reset whenever stopping & restarting a stream
+        * bit 21: ``fixed_rate``
+          Do not set PCM rate (frequency) when only one rate is available
+          for the given endpoint
+        * bit 22: ``mic_res_16``
+          Set the fixed resolution 16 for Mic Capture Volume
+        * bit 23: ``mic_res_384``
+          Set the fixed resolution 384 for Mic Capture Volume
+        * bit 24: ``mixer_playback_min_mute``
+          Set minimum volume control value as mute for devices where the
+          lowest playback value represents muted state instead of minimum
+          audible volume
+        * bit 25: ``mixer_capture_min_mute``
+          Be similar to bit 24 but for capture streams
 
 This module supports multiple devices, autoprobe and hotplugging.
 
@@ -2344,8 +2385,8 @@ report it to the upstream.
 NB: ``quirk_alias`` option is provided only for testing / development.
 If you want to have a proper support, contact to upstream for
 adding the matching quirk in the driver code statically.
-Ditto for ``quirk_flags``.  If a device is known to require specific
-workarounds, please report to the upstream.
+Ditto for ``quirk_flags`` and ``device_device_quirk_flags``.  If a device
+is known to require specific workarounds, please report to the upstream.
 
 Module snd-usb-caiaq
 --------------------

-- 
2.51.0

Re: [PATCH v2 0/3] ALSA: usb-audio: add module param device_quirk_flags

[Takashi Iwai]

On Fri, 12 Sep 2025 08:48:57 +0200,
Cryolitia PukNgae via B4 Relay wrote:
> 
> As an implementation of what has been discussed previously[1].
> 
> > An open question is whether we may want yet a new module option or
> > rather extend the existing quirk option to accept the strings
> > instead.  Basically, when the given argument has a colon, it's a new
> > syntax.  If it's only a number, it's an old syntax, and parse like
> > before.  But, I'm open for either way (a new option or extend the
> > existing one).
> 
> I would like to add a new param. The existed param
> `static unsigned int quirk_flags[SNDRV_CARDS]` seems to related to
> some sequence the card probed. To be honest, I havn't fully understood
> it. And it seems hard to improve it while keeping compatibility.
> 
> 1. https://lore.kernel.org/all/87h5xm5g7f.wl-tiwai@suse.de/
> 
> Signed-off-by: Cryolitia PukNgae <cryolitia@uniontech.com>
> ---
> Changes in v2:
> - Cleaned up some internal rebase confusion, sorry for that
> - Link to v1: https://lore.kernel.org/r/20250912-sound-v1-0-cc9cfd9f2d01@uniontech.com
> 
> ---
> Cryolitia PukNgae (3):
>       ALSA: usb-audio: add two-way convert between name and bit for QUIRK_FLAG_*
>       ALSA: usb-audio: add module param device_quirk_flags
>       ALSA: doc: add docs about device_device_quirk_flags in snd-usb-audio

Well, what I had in mind is something like:

--- a/sound/usb/card.c
+++ b/sound/usb/card.c
@@ -73,7 +73,7 @@ static bool lowlatency = true;
 static char *quirk_alias[SNDRV_CARDS];
 static char *delayed_register[SNDRV_CARDS];
 static bool implicit_fb[SNDRV_CARDS];
-static unsigned int quirk_flags[SNDRV_CARDS];
+static char *quirk_flags[SNDRV_CARDS];
 
 bool snd_usb_use_vmalloc = true;
 bool snd_usb_skip_validation;
@@ -103,8 +103,8 @@ module_param_array(delayed_register, charp, NULL, 0444);
 MODULE_PARM_DESC(delayed_register, "Quirk for delayed registration, given by id:iface, e.g. 0123abcd:4.");
 module_param_array(implicit_fb, bool, NULL, 0444);
 MODULE_PARM_DESC(implicit_fb, "Apply generic implicit feedback sync mode.");
-module_param_array(quirk_flags, uint, NULL, 0444);
-MODULE_PARM_DESC(quirk_flags, "Driver quirk bit flags.");
+module_param_array(quirk_flags, charp, NULL, 0444);
+MODULE_PARM_DESC(quirk_flags, "Driver quirk overrides.");
 module_param_named(use_vmalloc, snd_usb_use_vmalloc, bool, 0444);
 MODULE_PARM_DESC(use_vmalloc, "Use vmalloc for PCM intermediate buffers (default: yes).");
 module_param_named(skip_validation, snd_usb_skip_validation, bool, 0444);
@@ -692,6 +692,22 @@ static void usb_audio_make_longname(struct usb_device *dev,
 	}
 }
 
+static void set_quirk_flags(struct snd_usb_audio *chip, int idx)
+{
+	int i;
+
+	/* old style option found: the position-based integer value */
+	if (quirk_flags[idx] &&
+	    !kstrtou32(quirk_flags[idx], 0, &chip->quirk_flags))
+		return;
+
+	/* take the default quirk from the quirk table */
+	snd_usb_init_quirk_flags(chip);
+	/* add or correct quirk bits from options */
+	for (i = 0; i < ARRAY_SIZE(quirk_flags); i++)
+		snd_usb_apply_quirk_option(chip, quirk_flags[i]);
+}
+
 /*
  * create a chip instance and set its names.
  */
@@ -750,10 +766,7 @@ static int snd_usb_audio_create(struct usb_interface *intf,
 	INIT_LIST_HEAD(&chip->midi_v2_list);
 	INIT_LIST_HEAD(&chip->mixer_list);
 
-	if (quirk_flags[idx])
-		chip->quirk_flags = quirk_flags[idx];
-	else
-		snd_usb_init_quirk_flags(chip);
+	set_quirk_flags(chip, idx);
 
 	card->private_free = snd_usb_audio_free;
 
.... and snd_usb_apply_quirk_option() adds or corrects the quirk bits
based on the string value if it matches with the probed device.
This function will be similar like your parser.

In that way, the old quirk_flags options work as-is, while you can use
a new style by passing values with "X:Y:Z" style.


Takashi

Re: [PATCH v2 0/3] ALSA: usb-audio: add module param device_quirk_flags

[Cryolitia PukNgae]

On 12/09/2025 23.09, Takashi Iwai wrote:
> On Fri, 12 Sep 2025 08:48:57 +0200,
> Cryolitia PukNgae via B4 Relay wrote:
>>
>> As an implementation of what has been discussed previously[1].
>>
>>> An open question is whether we may want yet a new module option or
>>> rather extend the existing quirk option to accept the strings
>>> instead.  Basically, when the given argument has a colon, it's a new
>>> syntax.  If it's only a number, it's an old syntax, and parse like
>>> before.  But, I'm open for either way (a new option or extend the
>>> existing one).
>>
>> I would like to add a new param. The existed param
>> `static unsigned int quirk_flags[SNDRV_CARDS]` seems to related to
>> some sequence the card probed. To be honest, I havn't fully understood
>> it. And it seems hard to improve it while keeping compatibility.
>>
>> 1. https://lore.kernel.org/all/87h5xm5g7f.wl-tiwai@suse.de/
>>
>> Signed-off-by: Cryolitia PukNgae <cryolitia@uniontech.com>
>> ---
>> Changes in v2:
>> - Cleaned up some internal rebase confusion, sorry for that
>> - Link to v1: https://lore.kernel.org/r/20250912-sound-v1-0-cc9cfd9f2d01@uniontech.com
>>
>> ---
>> Cryolitia PukNgae (3):
>>       ALSA: usb-audio: add two-way convert between name and bit for QUIRK_FLAG_*
>>       ALSA: usb-audio: add module param device_quirk_flags
>>       ALSA: doc: add docs about device_device_quirk_flags in snd-usb-audio
> 
> Well, what I had in mind is something like:
> 
> --- a/sound/usb/card.c
> +++ b/sound/usb/card.c
> @@ -73,7 +73,7 @@ static bool lowlatency = true;
>  static char *quirk_alias[SNDRV_CARDS];
>  static char *delayed_register[SNDRV_CARDS];
>  static bool implicit_fb[SNDRV_CARDS];
> -static unsigned int quirk_flags[SNDRV_CARDS];
> +static char *quirk_flags[SNDRV_CARDS];
>  
>  bool snd_usb_use_vmalloc = true;
>  bool snd_usb_skip_validation;
> @@ -103,8 +103,8 @@ module_param_array(delayed_register, charp, NULL, 0444);
>  MODULE_PARM_DESC(delayed_register, "Quirk for delayed registration, given by id:iface, e.g. 0123abcd:4.");
>  module_param_array(implicit_fb, bool, NULL, 0444);
>  MODULE_PARM_DESC(implicit_fb, "Apply generic implicit feedback sync mode.");
> -module_param_array(quirk_flags, uint, NULL, 0444);
> -MODULE_PARM_DESC(quirk_flags, "Driver quirk bit flags.");
> +module_param_array(quirk_flags, charp, NULL, 0444);
> +MODULE_PARM_DESC(quirk_flags, "Driver quirk overrides.");
>  module_param_named(use_vmalloc, snd_usb_use_vmalloc, bool, 0444);
>  MODULE_PARM_DESC(use_vmalloc, "Use vmalloc for PCM intermediate buffers (default: yes).");
>  module_param_named(skip_validation, snd_usb_skip_validation, bool, 0444);
> @@ -692,6 +692,22 @@ static void usb_audio_make_longname(struct usb_device *dev,
>  	}
>  }
>  
> +static void set_quirk_flags(struct snd_usb_audio *chip, int idx)
> +{
> +	int i;
> +
> +	/* old style option found: the position-based integer value */
> +	if (quirk_flags[idx] &&
> +	    !kstrtou32(quirk_flags[idx], 0, &chip->quirk_flags))
> +		return;
> +
> +	/* take the default quirk from the quirk table */
> +	snd_usb_init_quirk_flags(chip);
> +	/* add or correct quirk bits from options */
> +	for (i = 0; i < ARRAY_SIZE(quirk_flags); i++)
> +		snd_usb_apply_quirk_option(chip, quirk_flags[i]);
> +}
> +
>  /*
>   * create a chip instance and set its names.
>   */
> @@ -750,10 +766,7 @@ static int snd_usb_audio_create(struct usb_interface *intf,
>  	INIT_LIST_HEAD(&chip->midi_v2_list);
>  	INIT_LIST_HEAD(&chip->mixer_list);
>  
> -	if (quirk_flags[idx])
> -		chip->quirk_flags = quirk_flags[idx];
> -	else
> -		snd_usb_init_quirk_flags(chip);
> +	set_quirk_flags(chip, idx);
>  
>  	card->private_free = snd_usb_audio_free;
>  
> .... and snd_usb_apply_quirk_option() adds or corrects the quirk bits
> based on the string value if it matches with the probed device.
> This function will be similar like your parser.
> 
> In that way, the old quirk_flags options work as-is, while you can use
> a new style by passing values with "X:Y:Z" style.
> 

Thanks for your review. To be honest, I haven't understand how
`static unsigned int quirk_flags[SNDRV_CARDS]` works. e.g., based on the
current array form, how to pass a flag, and what does the index of the
array means.

Could you please explain it for me. thx.

> Takashi
> 

Cryolitia PukNgae

Re: [PATCH v2 0/3] ALSA: usb-audio: add module param device_quirk_flags

[Takashi Iwai]

On Mon, 15 Sep 2025 09:43:05 +0200,
Cryolitia PukNgae wrote:
> 
> 
> 
> On 12/09/2025 23.09, Takashi Iwai wrote:
> > On Fri, 12 Sep 2025 08:48:57 +0200,
> > Cryolitia PukNgae via B4 Relay wrote:
> >>
> >> As an implementation of what has been discussed previously[1].
> >>
> >>> An open question is whether we may want yet a new module option or
> >>> rather extend the existing quirk option to accept the strings
> >>> instead.  Basically, when the given argument has a colon, it's a new
> >>> syntax.  If it's only a number, it's an old syntax, and parse like
> >>> before.  But, I'm open for either way (a new option or extend the
> >>> existing one).
> >>
> >> I would like to add a new param. The existed param
> >> `static unsigned int quirk_flags[SNDRV_CARDS]` seems to related to
> >> some sequence the card probed. To be honest, I havn't fully understood
> >> it. And it seems hard to improve it while keeping compatibility.
> >>
> >> 1. https://lore.kernel.org/all/87h5xm5g7f.wl-tiwai@suse.de/
> >>
> >> Signed-off-by: Cryolitia PukNgae <cryolitia@uniontech.com>
> >> ---
> >> Changes in v2:
> >> - Cleaned up some internal rebase confusion, sorry for that
> >> - Link to v1: https://lore.kernel.org/r/20250912-sound-v1-0-cc9cfd9f2d01@uniontech.com
> >>
> >> ---
> >> Cryolitia PukNgae (3):
> >>       ALSA: usb-audio: add two-way convert between name and bit for QUIRK_FLAG_*
> >>       ALSA: usb-audio: add module param device_quirk_flags
> >>       ALSA: doc: add docs about device_device_quirk_flags in snd-usb-audio
> > 
> > Well, what I had in mind is something like:
> > 
> > --- a/sound/usb/card.c
> > +++ b/sound/usb/card.c
> > @@ -73,7 +73,7 @@ static bool lowlatency = true;
> >  static char *quirk_alias[SNDRV_CARDS];
> >  static char *delayed_register[SNDRV_CARDS];
> >  static bool implicit_fb[SNDRV_CARDS];
> > -static unsigned int quirk_flags[SNDRV_CARDS];
> > +static char *quirk_flags[SNDRV_CARDS];
> >  
> >  bool snd_usb_use_vmalloc = true;
> >  bool snd_usb_skip_validation;
> > @@ -103,8 +103,8 @@ module_param_array(delayed_register, charp, NULL, 0444);
> >  MODULE_PARM_DESC(delayed_register, "Quirk for delayed registration, given by id:iface, e.g. 0123abcd:4.");
> >  module_param_array(implicit_fb, bool, NULL, 0444);
> >  MODULE_PARM_DESC(implicit_fb, "Apply generic implicit feedback sync mode.");
> > -module_param_array(quirk_flags, uint, NULL, 0444);
> > -MODULE_PARM_DESC(quirk_flags, "Driver quirk bit flags.");
> > +module_param_array(quirk_flags, charp, NULL, 0444);
> > +MODULE_PARM_DESC(quirk_flags, "Driver quirk overrides.");
> >  module_param_named(use_vmalloc, snd_usb_use_vmalloc, bool, 0444);
> >  MODULE_PARM_DESC(use_vmalloc, "Use vmalloc for PCM intermediate buffers (default: yes).");
> >  module_param_named(skip_validation, snd_usb_skip_validation, bool, 0444);
> > @@ -692,6 +692,22 @@ static void usb_audio_make_longname(struct usb_device *dev,
> >  	}
> >  }
> >  
> > +static void set_quirk_flags(struct snd_usb_audio *chip, int idx)
> > +{
> > +	int i;
> > +
> > +	/* old style option found: the position-based integer value */
> > +	if (quirk_flags[idx] &&
> > +	    !kstrtou32(quirk_flags[idx], 0, &chip->quirk_flags))
> > +		return;
> > +
> > +	/* take the default quirk from the quirk table */
> > +	snd_usb_init_quirk_flags(chip);
> > +	/* add or correct quirk bits from options */
> > +	for (i = 0; i < ARRAY_SIZE(quirk_flags); i++)
> > +		snd_usb_apply_quirk_option(chip, quirk_flags[i]);
> > +}
> > +
> >  /*
> >   * create a chip instance and set its names.
> >   */
> > @@ -750,10 +766,7 @@ static int snd_usb_audio_create(struct usb_interface *intf,
> >  	INIT_LIST_HEAD(&chip->midi_v2_list);
> >  	INIT_LIST_HEAD(&chip->mixer_list);
> >  
> > -	if (quirk_flags[idx])
> > -		chip->quirk_flags = quirk_flags[idx];
> > -	else
> > -		snd_usb_init_quirk_flags(chip);
> > +	set_quirk_flags(chip, idx);
> >  
> >  	card->private_free = snd_usb_audio_free;
> >  
> > .... and snd_usb_apply_quirk_option() adds or corrects the quirk bits
> > based on the string value if it matches with the probed device.
> > This function will be similar like your parser.
> > 
> > In that way, the old quirk_flags options work as-is, while you can use
> > a new style by passing values with "X:Y:Z" style.
> > 
> 
> Thanks for your review. To be honest, I haven't understand how
> `static unsigned int quirk_flags[SNDRV_CARDS]` works. e.g., based on the
> current array form, how to pass a flag, and what does the index of the
> array means.
> 
> Could you please explain it for me. thx.

That option works just like other options of the card arrays -- each
value is passed sequentially to the device of the given probe slot.
That is, the first probed device takes quirk_flags[0], the second
probed device takes quirk_flats[1], and so on.
Admittedly, although this works fine for the static probe
configuration like PCI devices or such, it's not ideal with
USB-audio.  So the new format is requested.


Takashi

Re: [PATCH v2 0/3] ALSA: usb-audio: add module param device_quirk_flags

[Cryolitia PukNgae]

On 15/09/2025 16.08, Takashi Iwai wrote:
> On Mon, 15 Sep 2025 09:43:05 +0200,
> Cryolitia PukNgae wrote:
>>
>>
>>
>> On 12/09/2025 23.09, Takashi Iwai wrote:
>>> On Fri, 12 Sep 2025 08:48:57 +0200,
>>> Cryolitia PukNgae via B4 Relay wrote:
>>>>
>>>> As an implementation of what has been discussed previously[1].
>>>>
>>>>> An open question is whether we may want yet a new module option or
>>>>> rather extend the existing quirk option to accept the strings
>>>>> instead.  Basically, when the given argument has a colon, it's a new
>>>>> syntax.  If it's only a number, it's an old syntax, and parse like
>>>>> before.  But, I'm open for either way (a new option or extend the
>>>>> existing one).
>>>>
>>>> I would like to add a new param. The existed param
>>>> `static unsigned int quirk_flags[SNDRV_CARDS]` seems to related to
>>>> some sequence the card probed. To be honest, I havn't fully understood
>>>> it. And it seems hard to improve it while keeping compatibility.
>>>>
>>>> 1. https://lore.kernel.org/all/87h5xm5g7f.wl-tiwai@suse.de/
>>>>
>>>> Signed-off-by: Cryolitia PukNgae <cryolitia@uniontech.com>
>>>> ---
>>>> Changes in v2:
>>>> - Cleaned up some internal rebase confusion, sorry for that
>>>> - Link to v1: https://lore.kernel.org/r/20250912-sound-v1-0-cc9cfd9f2d01@uniontech.com
>>>>
>>>> ---
>>>> Cryolitia PukNgae (3):
>>>>       ALSA: usb-audio: add two-way convert between name and bit for QUIRK_FLAG_*
>>>>       ALSA: usb-audio: add module param device_quirk_flags
>>>>       ALSA: doc: add docs about device_device_quirk_flags in snd-usb-audio
>>>
>>> Well, what I had in mind is something like:
>>>
>>> --- a/sound/usb/card.c
>>> +++ b/sound/usb/card.c
>>> @@ -73,7 +73,7 @@ static bool lowlatency = true;
>>>  static char *quirk_alias[SNDRV_CARDS];
>>>  static char *delayed_register[SNDRV_CARDS];
>>>  static bool implicit_fb[SNDRV_CARDS];
>>> -static unsigned int quirk_flags[SNDRV_CARDS];
>>> +static char *quirk_flags[SNDRV_CARDS];
>>>  
>>>  bool snd_usb_use_vmalloc = true;
>>>  bool snd_usb_skip_validation;
>>> @@ -103,8 +103,8 @@ module_param_array(delayed_register, charp, NULL, 0444);
>>>  MODULE_PARM_DESC(delayed_register, "Quirk for delayed registration, given by id:iface, e.g. 0123abcd:4.");
>>>  module_param_array(implicit_fb, bool, NULL, 0444);
>>>  MODULE_PARM_DESC(implicit_fb, "Apply generic implicit feedback sync mode.");
>>> -module_param_array(quirk_flags, uint, NULL, 0444);
>>> -MODULE_PARM_DESC(quirk_flags, "Driver quirk bit flags.");
>>> +module_param_array(quirk_flags, charp, NULL, 0444);
>>> +MODULE_PARM_DESC(quirk_flags, "Driver quirk overrides.");
>>>  module_param_named(use_vmalloc, snd_usb_use_vmalloc, bool, 0444);
>>>  MODULE_PARM_DESC(use_vmalloc, "Use vmalloc for PCM intermediate buffers (default: yes).");
>>>  module_param_named(skip_validation, snd_usb_skip_validation, bool, 0444);
>>> @@ -692,6 +692,22 @@ static void usb_audio_make_longname(struct usb_device *dev,
>>>  	}
>>>  }
>>>  
>>> +static void set_quirk_flags(struct snd_usb_audio *chip, int idx)
>>> +{
>>> +	int i;
>>> +
>>> +	/* old style option found: the position-based integer value */
>>> +	if (quirk_flags[idx] &&
>>> +	    !kstrtou32(quirk_flags[idx], 0, &chip->quirk_flags))
>>> +		return;
>>> +
>>> +	/* take the default quirk from the quirk table */
>>> +	snd_usb_init_quirk_flags(chip);
>>> +	/* add or correct quirk bits from options */
>>> +	for (i = 0; i < ARRAY_SIZE(quirk_flags); i++)
>>> +		snd_usb_apply_quirk_option(chip, quirk_flags[i]);
>>> +}
>>> +
>>>  /*
>>>   * create a chip instance and set its names.
>>>   */
>>> @@ -750,10 +766,7 @@ static int snd_usb_audio_create(struct usb_interface *intf,
>>>  	INIT_LIST_HEAD(&chip->midi_v2_list);
>>>  	INIT_LIST_HEAD(&chip->mixer_list);
>>>  
>>> -	if (quirk_flags[idx])
>>> -		chip->quirk_flags = quirk_flags[idx];
>>> -	else
>>> -		snd_usb_init_quirk_flags(chip);
>>> +	set_quirk_flags(chip, idx);
>>>  
>>>  	card->private_free = snd_usb_audio_free;
>>>  
>>> .... and snd_usb_apply_quirk_option() adds or corrects the quirk bits
>>> based on the string value if it matches with the probed device.
>>> This function will be similar like your parser.
>>>
>>> In that way, the old quirk_flags options work as-is, while you can use
>>> a new style by passing values with "X:Y:Z" style.
>>>
>>
>> Thanks for your review. To be honest, I haven't understand how
>> `static unsigned int quirk_flags[SNDRV_CARDS]` works. e.g., based on the
>> current array form, how to pass a flag, and what does the index of the
>> array means.
>>
>> Could you please explain it for me. thx.
> 
> That option works just like other options of the card arrays -- each
> value is passed sequentially to the device of the given probe slot.
> That is, the first probed device takes quirk_flags[0], the second
> probed device takes quirk_flats[1], and so on.
> Admittedly, although this works fine for the static probe
> configuration like PCI devices or such, it's not ideal with
> USB-audio.  So the new format is requested.
> 

Thanks for your clear explain. I would try to implement it.

Cryolitia PukNgae