linux-modules.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/5] ALSA: usb-audio: add module param device_quirk_flags
@ 2025-09-18  9:24 Cryolitia PukNgae via B4 Relay
  2025-09-18  9:24 ` [PATCH v4 1/5] ALSA: usb-audio: add two-way convert between name and bit for QUIRK_FLAG_* Cryolitia PukNgae via B4 Relay
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Cryolitia PukNgae via B4 Relay @ 2025-09-18  9:24 UTC (permalink / raw)
  To: Jaroslav Kysela, Takashi Iwai, Jonathan Corbet, Luis Chamberlain,
	Petr Pavlu, Daniel Gomez, Sami Tolvanen
  Cc: linux-sound, linux-usb, linux-kernel, linux-doc, Mingcong Bai,
	Kexy Biscuit, Nie Cheng, Zhan Jun, Feng Yuan, qaqland, kernel,
	linux-modules, Cryolitia PukNgae, Takashi Iwai

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

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

Signed-off-by: Cryolitia PukNgae <cryolitia@uniontech.com>
---
Changes in v4:
- Split basic parse and dynamic change
- Drop usage of linked list
- Link to v3: https://lore.kernel.org/r/20250917-sound-v3-0-92ebe9472a0a@uniontech.com

Changes in v3:
- Instead of a new param, improve the existed one.
- Link to v2: https://lore.kernel.org/r/20250912-sound-v2-0-01ea3d279f4b@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 (5):
      ALSA: usb-audio: add two-way convert between name and bit for QUIRK_FLAG_*
      param: export param_array related functions
      ALSA: usb-audio: improve module param quirk_flags
      ALSA: usb-audio: make param quirk_flags change-able in runtime
      ALSA: doc: add docs about improved quirk_flags in snd-usb-audio

 Documentation/sound/alsa-configuration.rst | 108 ++++++++++-----
 include/linux/moduleparam.h                |   3 +
 kernel/params.c                            |   9 +-
 sound/usb/card.c                           |  63 ++++++++-
 sound/usb/quirks.c                         | 206 ++++++++++++++++++++++++++++-
 sound/usb/quirks.h                         |   6 +-
 sound/usb/usbaudio.h                       |   6 +
 7 files changed, 352 insertions(+), 49 deletions(-)
---
base-commit: 4c421c40c8b30ab7aae1edc7f7e294fcd33fc186
change-id: 20250910-sound-a91c86c92dba

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



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v4 1/5] ALSA: usb-audio: add two-way convert between name and bit for QUIRK_FLAG_*
  2025-09-18  9:24 [PATCH v4 0/5] ALSA: usb-audio: add module param device_quirk_flags Cryolitia PukNgae via B4 Relay
@ 2025-09-18  9:24 ` Cryolitia PukNgae via B4 Relay
  2025-09-19 12:32   ` Takashi Iwai
  2025-09-18  9:24 ` [PATCH v4 2/5] param: export param_array related functions Cryolitia PukNgae via B4 Relay
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Cryolitia PukNgae via B4 Relay @ 2025-09-18  9:24 UTC (permalink / raw)
  To: Jaroslav Kysela, Takashi Iwai, Jonathan Corbet, Luis Chamberlain,
	Petr Pavlu, Daniel Gomez, Sami Tolvanen
  Cc: linux-sound, linux-usb, linux-kernel, linux-doc, Mingcong Bai,
	Kexy Biscuit, Nie Cheng, Zhan Jun, Feng Yuan, qaqland, kernel,
	linux-modules, Cryolitia PukNgae

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



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 2/5] param: export param_array related functions
  2025-09-18  9:24 [PATCH v4 0/5] ALSA: usb-audio: add module param device_quirk_flags Cryolitia PukNgae via B4 Relay
  2025-09-18  9:24 ` [PATCH v4 1/5] ALSA: usb-audio: add two-way convert between name and bit for QUIRK_FLAG_* Cryolitia PukNgae via B4 Relay
@ 2025-09-18  9:24 ` Cryolitia PukNgae via B4 Relay
  2025-09-19 12:37   ` Takashi Iwai
  2025-09-18  9:24 ` [PATCH v4 3/5] ALSA: usb-audio: improve module param quirk_flags Cryolitia PukNgae via B4 Relay
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Cryolitia PukNgae via B4 Relay @ 2025-09-18  9:24 UTC (permalink / raw)
  To: Jaroslav Kysela, Takashi Iwai, Jonathan Corbet, Luis Chamberlain,
	Petr Pavlu, Daniel Gomez, Sami Tolvanen
  Cc: linux-sound, linux-usb, linux-kernel, linux-doc, Mingcong Bai,
	Kexy Biscuit, Nie Cheng, Zhan Jun, Feng Yuan, qaqland, kernel,
	linux-modules, Cryolitia PukNgae

From: Cryolitia PukNgae <cryolitia@uniontech.com>

- int param_array_set(const char *val, const struct kernel_param *kp);
- int param_array_get(char *buffer, const struct kernel_param *kp);
- void param_array_free(void *arg);

It would be helpful for the new module param we designed in
snd_usb_audio, in order to run additional custom codes when params
are set in runtime, and re-use the extisted codes in param.c

Signed-off-by: Cryolitia PukNgae <cryolitia@uniontech.com>
---
 include/linux/moduleparam.h | 3 +++
 kernel/params.c             | 9 ++++++---
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 3a25122d83e2802e6e6a1475a52816251498b26a..4ef09ad2004789855bd21783029c653fac94b9dd 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -593,6 +593,9 @@ enum hwparam_type {
 
 
 extern const struct kernel_param_ops param_array_ops;
+extern int param_array_set(const char *val, const struct kernel_param *kp);
+extern int param_array_get(char *buffer, const struct kernel_param *kp);
+extern void param_array_free(void *arg);
 
 extern const struct kernel_param_ops param_ops_string;
 extern int param_set_copystring(const char *val, const struct kernel_param *);
diff --git a/kernel/params.c b/kernel/params.c
index b96cfd693c9968012d42acb85611fee1acd47790..a936e018a1c6d0bf2b6b4566f80751840366f652 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -462,7 +462,7 @@ static int param_array(struct module *mod,
 	return 0;
 }
 
-static int param_array_set(const char *val, const struct kernel_param *kp)
+int param_array_set(const char *val, const struct kernel_param *kp)
 {
 	const struct kparam_array *arr = kp->arr;
 	unsigned int temp_num;
@@ -471,8 +471,9 @@ static int param_array_set(const char *val, const struct kernel_param *kp)
 			   arr->elemsize, arr->ops->set, kp->level,
 			   arr->num ?: &temp_num);
 }
+EXPORT_SYMBOL(param_array_set);
 
-static int param_array_get(char *buffer, const struct kernel_param *kp)
+int param_array_get(char *buffer, const struct kernel_param *kp)
 {
 	int i, off, ret;
 	const struct kparam_array *arr = kp->arr;
@@ -492,8 +493,9 @@ static int param_array_get(char *buffer, const struct kernel_param *kp)
 	buffer[off] = '\0';
 	return off;
 }
+EXPORT_SYMBOL(param_array_get);
 
-static void param_array_free(void *arg)
+void param_array_free(void *arg)
 {
 	unsigned int i;
 	const struct kparam_array *arr = arg;
@@ -502,6 +504,7 @@ static void param_array_free(void *arg)
 		for (i = 0; i < (arr->num ? *arr->num : arr->max); i++)
 			arr->ops->free(arr->elem + arr->elemsize * i);
 }
+EXPORT_SYMBOL(param_array_free);
 
 const struct kernel_param_ops param_array_ops = {
 	.set = param_array_set,

-- 
2.51.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 3/5] ALSA: usb-audio: improve module param quirk_flags
  2025-09-18  9:24 [PATCH v4 0/5] ALSA: usb-audio: add module param device_quirk_flags Cryolitia PukNgae via B4 Relay
  2025-09-18  9:24 ` [PATCH v4 1/5] ALSA: usb-audio: add two-way convert between name and bit for QUIRK_FLAG_* Cryolitia PukNgae via B4 Relay
  2025-09-18  9:24 ` [PATCH v4 2/5] param: export param_array related functions Cryolitia PukNgae via B4 Relay
@ 2025-09-18  9:24 ` Cryolitia PukNgae via B4 Relay
  2025-09-19 12:47   ` Takashi Iwai
  2025-09-18  9:24 ` [PATCH v4 4/5] ALSA: usb-audio: make param quirk_flags change-able in runtime Cryolitia PukNgae via B4 Relay
  2025-09-18  9:24 ` [PATCH v4 5/5] ALSA: doc: add docs about improved quirk_flags in snd-usb-audio Cryolitia PukNgae via B4 Relay
  4 siblings, 1 reply; 11+ messages in thread
From: Cryolitia PukNgae via B4 Relay @ 2025-09-18  9:24 UTC (permalink / raw)
  To: Jaroslav Kysela, Takashi Iwai, Jonathan Corbet, Luis Chamberlain,
	Petr Pavlu, Daniel Gomez, Sami Tolvanen
  Cc: linux-sound, linux-usb, linux-kernel, linux-doc, Mingcong Bai,
	Kexy Biscuit, Nie Cheng, Zhan Jun, Feng Yuan, qaqland, kernel,
	linux-modules, Cryolitia PukNgae, Takashi Iwai

From: Cryolitia PukNgae <cryolitia@uniontech.com>

For apply and unapply quirk flags more flexibly though param

Co-developed-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Cryolitia PukNgae <cryolitia@uniontech.com>
---
 sound/usb/card.c     |   9 ++--
 sound/usb/quirks.c   | 119 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 sound/usb/quirks.h   |   3 +-
 sound/usb/usbaudio.h |   3 ++
 4 files changed, 126 insertions(+), 8 deletions(-)

diff --git a/sound/usb/card.c b/sound/usb/card.c
index 0265206a8e8cf31133e8463c98fe0497d8ace89e..5837677effa1787ef9d7d02714c3ae43b1a8bc79 100644
--- a/sound/usb/card.c
+++ b/sound/usb/card.c
@@ -73,8 +73,8 @@ 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];
 
+char *quirk_flags[SNDRV_CARDS];
 bool snd_usb_use_vmalloc = true;
 bool snd_usb_skip_validation;
 
@@ -103,7 +103,7 @@ 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_param_array(quirk_flags, charp, NULL, 0444);
 MODULE_PARM_DESC(quirk_flags, "Driver quirk bit flags.");
 module_param_named(use_vmalloc, snd_usb_use_vmalloc, bool, 0444);
 MODULE_PARM_DESC(use_vmalloc, "Use vmalloc for PCM intermediate buffers (default: yes).");
@@ -750,10 +750,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);
+	snd_usb_init_quirk_flags(idx, chip);
 
 	card->private_free = snd_usb_audio_free;
 
diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c
index 94854f352b1702b491e1bf3c8b769f7088e03976..4dc2464133e934e48b1fa613884a8a0ebdaff91d 100644
--- a/sound/usb/quirks.c
+++ b/sound/usb/quirks.c
@@ -2502,7 +2502,7 @@ u32 snd_usb_quirk_flags_from_name(char *name)
 	return flag;
 }
 
-void snd_usb_init_quirk_flags(struct snd_usb_audio *chip)
+void snd_usb_init_quirk_flags_table(struct snd_usb_audio *chip)
 {
 	const struct usb_audio_quirk_flags_table *p;
 
@@ -2537,3 +2537,120 @@ void snd_usb_init_quirk_flags(struct snd_usb_audio *chip)
 		}
 	}
 }
+
+void snd_usb_init_quirk_flags(int idx, struct snd_usb_audio *chip)
+{
+	u16 chip_vid = USB_ID_VENDOR(chip->usb_id);
+	u16 chip_pid = USB_ID_PRODUCT(chip->usb_id);
+	u32 mask_flags, unmask_flags, bit;
+	char *val, *p, *field, *flag;
+	bool is_unmask;
+	u16 vid, pid;
+	size_t i;
+
+	/* old style option found: the position-based integer value */
+	if (quirk_flags[idx] &&
+	    !kstrtou32(quirk_flags[idx], 0, &chip->quirk_flags)) {
+		usb_audio_dbg(chip,
+			      "Set quirk flags 0x%x from param based on position %d for device %04x:%04x\n",
+			      chip->quirk_flags, idx,
+			      USB_ID_VENDOR(chip->usb_id),
+			      USB_ID_PRODUCT(chip->usb_id));
+		return;
+	}
+
+	/* take the default quirk from the quirk table */
+	snd_usb_init_quirk_flags_table(chip);
+
+	/* add or correct quirk bits from options */
+	for (i = 0; i < ARRAY_SIZE(quirk_flags); i++) {
+		if (!quirk_flags[i] || !*quirk_flags[i])
+			break;
+
+		val = kstrdup(quirk_flags[i], GFP_KERNEL);
+
+		if (!val) {
+			pr_err("snd_usb_audio: Error allocating memory while parsing quirk_flags\n");
+			return;
+		}
+
+		for (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;
+
+			if ((vid != 0 && vid != chip_vid) ||
+			    (pid != 0 && pid != chip_pid))
+				continue;
+
+			/* Collect the flags */
+			mask_flags = 0;
+			unmask_flags = 0;
+			while (field && *field) {
+				flag = strsep(&field, "|");
+
+				if (!flag)
+					break;
+
+				if (*flag == '!') {
+					is_unmask = true;
+					flag++;
+				} else {
+					is_unmask = false;
+				}
+
+				if (!kstrtou32(flag, 16, &bit)) {
+					if (is_unmask)
+						unmask_flags |= bit;
+					else
+						mask_flags |= bit;
+
+					break;
+				}
+
+				bit = snd_usb_quirk_flags_from_name(flag);
+
+				if (bit) {
+					if (is_unmask)
+						unmask_flags |= bit;
+					else
+						mask_flags |= bit;
+				} else {
+					pr_warn("snd_usb_audio: unknown flag %s while parsing param quirk_flags\n",
+						field);
+				}
+			}
+
+			chip->quirk_flags &= ~unmask_flags;
+			chip->quirk_flags |= mask_flags;
+			usb_audio_dbg(chip,
+				      "Set quirk flags (mask 0x%x, unmask 0x%x, finally 0x%x) from param for device %04x:%04x\n",
+				      mask_flags,
+				      unmask_flags,
+				      chip->quirk_flags,
+				      USB_ID_VENDOR(chip->usb_id),
+				      USB_ID_PRODUCT(chip->usb_id));
+		}
+
+		kfree(val);
+	}
+}
diff --git a/sound/usb/quirks.h b/sound/usb/quirks.h
index bd5baf2b193a1985f3a0e52bf4a77ca741364769..0dc8c04afeffe76e1219c71a8fe2e4b66624b48f 100644
--- a/sound/usb/quirks.h
+++ b/sound/usb/quirks.h
@@ -48,7 +48,8 @@ void snd_usb_audioformat_attributes_quirk(struct snd_usb_audio *chip,
 					  struct audioformat *fp,
 					  int stream);
 
-void snd_usb_init_quirk_flags(struct snd_usb_audio *chip);
+void snd_usb_init_quirk_flags_table(struct snd_usb_audio *chip);
+void snd_usb_init_quirk_flags(int idx, 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);
diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h
index 0a22cb4a02344b2dcf4009c560a759f2da25ca67..73564cd68ebf101291440d0171eb81c220c6f5e2 100644
--- a/sound/usb/usbaudio.h
+++ b/sound/usb/usbaudio.h
@@ -7,6 +7,8 @@
  *   Copyright (c) 2002 by Takashi Iwai <tiwai@suse.de>
  */
 
+ #include <sound/core.h>
+
 /* handling of USB vendor/product ID pairs as 32-bit numbers */
 #define USB_ID(vendor, product) (((unsigned int)(vendor) << 16) | (product))
 #define USB_ID_VENDOR(id) ((id) >> 16)
@@ -162,6 +164,7 @@ DEFINE_CLASS(snd_usb_lock, struct __snd_usb_lock,
 	     __snd_usb_unlock_shutdown(&(_T)), __snd_usb_lock_shutdown(chip),
 	     struct snd_usb_audio *chip)
 
+extern char *quirk_flags[SNDRV_CARDS];
 extern bool snd_usb_use_vmalloc;
 extern bool snd_usb_skip_validation;
 

-- 
2.51.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 4/5] ALSA: usb-audio: make param quirk_flags change-able in runtime
  2025-09-18  9:24 [PATCH v4 0/5] ALSA: usb-audio: add module param device_quirk_flags Cryolitia PukNgae via B4 Relay
                   ` (2 preceding siblings ...)
  2025-09-18  9:24 ` [PATCH v4 3/5] ALSA: usb-audio: improve module param quirk_flags Cryolitia PukNgae via B4 Relay
@ 2025-09-18  9:24 ` Cryolitia PukNgae via B4 Relay
  2025-09-18  9:24 ` [PATCH v4 5/5] ALSA: doc: add docs about improved quirk_flags in snd-usb-audio Cryolitia PukNgae via B4 Relay
  4 siblings, 0 replies; 11+ messages in thread
From: Cryolitia PukNgae via B4 Relay @ 2025-09-18  9:24 UTC (permalink / raw)
  To: Jaroslav Kysela, Takashi Iwai, Jonathan Corbet, Luis Chamberlain,
	Petr Pavlu, Daniel Gomez, Sami Tolvanen
  Cc: linux-sound, linux-usb, linux-kernel, linux-doc, Mingcong Bai,
	Kexy Biscuit, Nie Cheng, Zhan Jun, Feng Yuan, qaqland, kernel,
	linux-modules, Cryolitia PukNgae, Takashi Iwai

From: Cryolitia PukNgae <cryolitia@uniontech.com>

Developers now can change it during sysfs, without rebooting, for
debugging new buggy devices.

Co-developed-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Cryolitia PukNgae <cryolitia@uniontech.com>
---
 sound/usb/card.c     | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 sound/usb/quirks.c   |  9 +++++++--
 sound/usb/usbaudio.h |  2 ++
 3 files changed, 63 insertions(+), 4 deletions(-)

diff --git a/sound/usb/card.c b/sound/usb/card.c
index 5837677effa1787ef9d7d02714c3ae43b1a8bc79..fd99df5949df826d97b3d2bc6d3137923ab4295d 100644
--- a/sound/usb/card.c
+++ b/sound/usb/card.c
@@ -103,13 +103,65 @@ 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, charp, NULL, 0444);
-MODULE_PARM_DESC(quirk_flags, "Driver quirk bit flags.");
 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);
 MODULE_PARM_DESC(skip_validation, "Skip unit descriptor validation (default: no).");
 
+/* protects quirk_flags */
+DEFINE_MUTEX(quirk_flags_mutex);
+
+static int quirk_flags_param_set(const char *value,
+				 const struct kernel_param *kp)
+{
+	int err;
+
+	mutex_lock(&quirk_flags_mutex);
+
+	memset(quirk_flags, 0, sizeof(quirk_flags));
+	err = param_array_set(value, kp);
+
+	mutex_unlock(&quirk_flags_mutex);
+
+	return err;
+}
+
+static int quirk_flags_param_get(char *buffer, const struct kernel_param *kp)
+{
+	int err;
+
+	mutex_lock(&quirk_flags_mutex);
+	err = param_array_get(buffer, kp);
+	mutex_unlock(&quirk_flags_mutex);
+
+	return err;
+}
+
+static void quirk_flags_param_free(void *arg)
+{
+	mutex_lock(&quirk_flags_mutex);
+	param_array_free(arg);
+	mutex_unlock(&quirk_flags_mutex);
+}
+
+static const struct kernel_param_ops quirk_flags_param_ops = {
+	.set = quirk_flags_param_set,
+	.get = quirk_flags_param_get,
+	.free = quirk_flags_param_free,
+};
+
+static struct kparam_array quirk_flags_param_array = {
+	.max = SNDRV_CARDS,
+	.elemsize = sizeof(char *),
+	.num = NULL,
+	.ops = &param_ops_charp,
+	.elem = &quirk_flags,
+};
+
+device_param_cb(quirk_flags, &quirk_flags_param_ops, &quirk_flags_param_array,
+		0644);
+MODULE_PARM_DESC(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.
diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c
index 4dc2464133e934e48b1fa613884a8a0ebdaff91d..ba9ea94399e5a6e7e1711b2a9148c90f991f05cd 100644
--- a/sound/usb/quirks.c
+++ b/sound/usb/quirks.c
@@ -2548,6 +2548,8 @@ void snd_usb_init_quirk_flags(int idx, struct snd_usb_audio *chip)
 	u16 vid, pid;
 	size_t i;
 
+	mutex_lock(&quirk_flags_mutex);
+
 	/* old style option found: the position-based integer value */
 	if (quirk_flags[idx] &&
 	    !kstrtou32(quirk_flags[idx], 0, &chip->quirk_flags)) {
@@ -2556,7 +2558,7 @@ void snd_usb_init_quirk_flags(int idx, struct snd_usb_audio *chip)
 			      chip->quirk_flags, idx,
 			      USB_ID_VENDOR(chip->usb_id),
 			      USB_ID_PRODUCT(chip->usb_id));
-		return;
+		goto unlock;
 	}
 
 	/* take the default quirk from the quirk table */
@@ -2571,7 +2573,7 @@ void snd_usb_init_quirk_flags(int idx, struct snd_usb_audio *chip)
 
 		if (!val) {
 			pr_err("snd_usb_audio: Error allocating memory while parsing quirk_flags\n");
-			return;
+			goto unlock;
 		}
 
 		for (p = val; p && *p;) {
@@ -2653,4 +2655,7 @@ void snd_usb_init_quirk_flags(int idx, struct snd_usb_audio *chip)
 
 		kfree(val);
 	}
+
+unlock:
+	mutex_unlock(&quirk_flags_mutex);
 }
diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h
index 73564cd68ebf101291440d0171eb81c220c6f5e2..40551946c20df8c17d306fddd8295ca3a2bfa702 100644
--- a/sound/usb/usbaudio.h
+++ b/sound/usb/usbaudio.h
@@ -168,6 +168,8 @@ extern char *quirk_flags[SNDRV_CARDS];
 extern bool snd_usb_use_vmalloc;
 extern bool snd_usb_skip_validation;
 
+extern struct mutex quirk_flags_mutex;
+
 /*
  * Driver behavior quirk flags, stored in chip->quirk_flags
  *

-- 
2.51.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 5/5] ALSA: doc: add docs about improved quirk_flags in snd-usb-audio
  2025-09-18  9:24 [PATCH v4 0/5] ALSA: usb-audio: add module param device_quirk_flags Cryolitia PukNgae via B4 Relay
                   ` (3 preceding siblings ...)
  2025-09-18  9:24 ` [PATCH v4 4/5] ALSA: usb-audio: make param quirk_flags change-able in runtime Cryolitia PukNgae via B4 Relay
@ 2025-09-18  9:24 ` Cryolitia PukNgae via B4 Relay
  2025-09-18 20:21   ` Randy Dunlap
  4 siblings, 1 reply; 11+ messages in thread
From: Cryolitia PukNgae via B4 Relay @ 2025-09-18  9:24 UTC (permalink / raw)
  To: Jaroslav Kysela, Takashi Iwai, Jonathan Corbet, Luis Chamberlain,
	Petr Pavlu, Daniel Gomez, Sami Tolvanen
  Cc: linux-sound, linux-usb, linux-kernel, linux-doc, Mingcong Bai,
	Kexy Biscuit, Nie Cheng, Zhan Jun, Feng Yuan, qaqland, kernel,
	linux-modules, Cryolitia PukNgae

From: Cryolitia PukNgae <cryolitia@uniontech.com>

Just briefly described about the option.

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

diff --git a/Documentation/sound/alsa-configuration.rst b/Documentation/sound/alsa-configuration.rst
index a2fb8ed251dd0294e7a62209ca15d5c32c6adfae..efffe3d534beeddcb6a47ac27a24defb6879f534 100644
--- a/Documentation/sound/alsa-configuration.rst
+++ b/Documentation/sound/alsa-configuration.rst
@@ -2297,39 +2297,81 @@ skip_validation
     of the unit descriptor instead of a driver probe error, so that we
     can check its details.
 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
+    The option provides 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 old usage accepts an array of integers, each of which apply quirk
+    flags on the device in the order of probing.
+    e.g. ``quirk_flags=0x01,0x02`` applies get_sample_rate to the first
+    device, and share_media_device to the second device.
+    The new usage 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 ``|``, 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.
+
+        * 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.
 

-- 
2.51.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 5/5] ALSA: doc: add docs about improved quirk_flags in snd-usb-audio
  2025-09-18  9:24 ` [PATCH v4 5/5] ALSA: doc: add docs about improved quirk_flags in snd-usb-audio Cryolitia PukNgae via B4 Relay
@ 2025-09-18 20:21   ` Randy Dunlap
  2025-09-19  1:43     ` Cryolitia PukNgae
  0 siblings, 1 reply; 11+ messages in thread
From: Randy Dunlap @ 2025-09-18 20:21 UTC (permalink / raw)
  To: cryolitia, Jaroslav Kysela, Takashi Iwai, Jonathan Corbet,
	Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen
  Cc: linux-sound, linux-usb, linux-kernel, linux-doc, Mingcong Bai,
	Kexy Biscuit, Nie Cheng, Zhan Jun, Feng Yuan, qaqland, kernel,
	linux-modules

Hi--

On 9/18/25 2:24 AM, Cryolitia PukNgae via B4 Relay wrote:
> From: Cryolitia PukNgae <cryolitia@uniontech.com>
> 
> Just briefly described about the option.
> 
> Signed-off-by: Cryolitia PukNgae <cryolitia@uniontech.com>
> ---
>  Documentation/sound/alsa-configuration.rst | 108 ++++++++++++++++++++---------
>  1 file changed, 75 insertions(+), 33 deletions(-)
> 
> diff --git a/Documentation/sound/alsa-configuration.rst b/Documentation/sound/alsa-configuration.rst
> index a2fb8ed251dd0294e7a62209ca15d5c32c6adfae..efffe3d534beeddcb6a47ac27a24defb6879f534 100644
> --- a/Documentation/sound/alsa-configuration.rst
> +++ b/Documentation/sound/alsa-configuration.rst
> @@ -2297,39 +2297,81 @@ skip_validation
>      of the unit descriptor instead of a driver probe error, so that we
>      can check its details.
>  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
> +    The option provides a refined and flexible control for applying quirk
> +    flags.  It allows to specify the quirk flags for each device, and could

                                                                     and may
or: and can

> +    be modified dynamically via sysfs.
> +    The old usage accepts an array of integers, each of which apply quirk

                                                                 applies

> +    flags on the device in the order of probing.
> +    e.g. ``quirk_flags=0x01,0x02`` applies get_sample_rate to the first

       E.g.,

> +    device, and share_media_device to the second device.
> +    The new usage 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

                                                           s/could/may/

> +    specified as ``*`` to match any value.  ``FLAGSx`` could be a set of

                                                      s/could/may/

> +    flags given by name, separated by ``|``, or a hexadecimal number
> +    representing the bit flags.  The available flag names are listed above.

                                                              s/above/below/ ?

> +    An exclamation mark could be prefixed to a flag name to negate the flag.
                       s/could/may/

> +    For example, ``1234:abcd:mixer_playback_min_mute|!ignore_ctl_error;*:*:0x01;``

What happens if the trailing (ending) ';' is omitted?

> +    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.
> +
> +        * bit 0: ``get_sample_rate``
> +          Skip reading sample rate for devices

get vs Skip is a little confusing.

> +        * 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

                               DSD-based

> +        * 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

                   close

> +        * 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

             Similar to

>  
>  This module supports multiple devices, autoprobe and hotplugging.
>  
> Are all of these quirks used on various devices or are some of these
just implemented just in case they are needed in the future?thanks.
-- 
~Randy


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 5/5] ALSA: doc: add docs about improved quirk_flags in snd-usb-audio
  2025-09-18 20:21   ` Randy Dunlap
@ 2025-09-19  1:43     ` Cryolitia PukNgae
  0 siblings, 0 replies; 11+ messages in thread
From: Cryolitia PukNgae @ 2025-09-19  1:43 UTC (permalink / raw)
  To: Randy Dunlap, Jaroslav Kysela, Takashi Iwai, Jonathan Corbet,
	Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen
  Cc: linux-sound, linux-usb, linux-kernel, linux-doc, Mingcong Bai,
	Kexy Biscuit, Nie Cheng, Zhan Jun, Feng Yuan, qaqland, kernel,
	linux-modules

Thanks for your review. I'll waiting some reviews on other patches and resend
a new version about it.

On 19/09/2025 04.21, Randy Dunlap wrote:
> Hi--
> 
> On 9/18/25 2:24 AM, Cryolitia PukNgae via B4 Relay wrote:
>> From: Cryolitia PukNgae <cryolitia@uniontech.com>
>>
>> Just briefly described about the option.
>>
>> Signed-off-by: Cryolitia PukNgae <cryolitia@uniontech.com>
>> ---
>>  Documentation/sound/alsa-configuration.rst | 108 ++++++++++++++++++++---------
>>  1 file changed, 75 insertions(+), 33 deletions(-)
>>
>> diff --git a/Documentation/sound/alsa-configuration.rst b/Documentation/sound/alsa-configuration.rst
>> index a2fb8ed251dd0294e7a62209ca15d5c32c6adfae..efffe3d534beeddcb6a47ac27a24defb6879f534 100644
>> --- a/Documentation/sound/alsa-configuration.rst
>> +++ b/Documentation/sound/alsa-configuration.rst
>> @@ -2297,39 +2297,81 @@ skip_validation
>>      of the unit descriptor instead of a driver probe error, so that we
>>      can check its details.
>>  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
>> +    The option provides a refined and flexible control for applying quirk
>> +    flags.  It allows to specify the quirk flags for each device, and could
> 
>                                                                      and may
> or: and can
> 
>> +    be modified dynamically via sysfs.
>> +    The old usage accepts an array of integers, each of which apply quirk
> 
>                                                                  applies
> 
>> +    flags on the device in the order of probing.
>> +    e.g. ``quirk_flags=0x01,0x02`` applies get_sample_rate to the first
> 
>        E.g.,
> 
>> +    device, and share_media_device to the second device.
>> +    The new usage 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
> 
>                                                            s/could/may/
> 
>> +    specified as ``*`` to match any value.  ``FLAGSx`` could be a set of
> 
>                                                       s/could/may/
> 
>> +    flags given by name, separated by ``|``, or a hexadecimal number
>> +    representing the bit flags.  The available flag names are listed above.
> 
>                                                               s/above/below/ ?
> 
>> +    An exclamation mark could be prefixed to a flag name to negate the flag.
>                        s/could/may/
> 
>> +    For example, ``1234:abcd:mixer_playback_min_mute|!ignore_ctl_error;*:*:0x01;``
> 
> What happens if the trailing (ending) ';' is omitted?

This is where I found something strange when I was testing it myself. When I use
`echo '*:*:mixer_playback_min_mute` > /sys/module/snd_usb_audio/parameters/quirk_flags`,
the string received by the driver contains a lot of trailing spaces. I am not familiar
with the kernel and don't know how to handle this situation. Simply adding a semicolon
to the input will make the trailing spaces after the semicolon be ignored.

> 
>> +    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.
>> +
>> +        * bit 0: ``get_sample_rate``
>> +          Skip reading sample rate for devices
> 
> get vs Skip is a little confusing.

This part is copied from Takashi Iwai[1]. It does a little confusing.

>> +        * 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
> 
>                                DSD-based
> 
>> +        * 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
> 
>                    close
> 
>> +        * 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
> 
>              Similar to
> 
>>  
>>  This module supports multiple devices, autoprobe and hotplugging.
>>  
>> Are all of these quirks used on various devices or are some of these
> just implemented just in case they are needed in the future?thanks.

I believe in that all of them are used in quirk_flags_table in sound/usb/quirks.c

1. https://lore.kernel.org/all/20210729073855.19043-2-tiwai@suse.de/

Best regards,
Cryolitia


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 1/5] ALSA: usb-audio: add two-way convert between name and bit for QUIRK_FLAG_*
  2025-09-18  9:24 ` [PATCH v4 1/5] ALSA: usb-audio: add two-way convert between name and bit for QUIRK_FLAG_* Cryolitia PukNgae via B4 Relay
@ 2025-09-19 12:32   ` Takashi Iwai
  0 siblings, 0 replies; 11+ messages in thread
From: Takashi Iwai @ 2025-09-19 12:32 UTC (permalink / raw)
  To: cryolitia
  Cc: Jaroslav Kysela, Takashi Iwai, Jonathan Corbet, Luis Chamberlain,
	Petr Pavlu, Daniel Gomez, Sami Tolvanen, linux-sound, linux-usb,
	linux-kernel, linux-doc, Mingcong Bai, Kexy Biscuit, Nie Cheng,
	Zhan Jun, Feng Yuan, qaqland, kernel, linux-modules

On Thu, 18 Sep 2025 11:24:30 +0200,
Cryolitia PukNgae via B4 Relay wrote:
> --- 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",

I think it's worth to add a comment that this each entry corresponds
to QUIRK_FLAG_XXX.  Or another idea would be to define enums like:

enum {
	QUIRK_TYPE_GET_SAMPLE_RATE,
	QUIRK_TYPE_SHARE_MEDIA_DEVICE,
	....
};

then redefine QUIRK_FLAG_* like:

#define QUIRK_FLAG_GET_SAMPLE_RATE	BIT_U32(QUIRK_TYPE_GET_SAMPLE_RATE)
#define QUIRK_FLAG_SHARE_MEDIA_DEVICE	BIT_U32(QUIRK_TYPE_SHARE_MEDIA_DEVICE)
....
 
or

#define QUIRK_FLAG(x)	BIT_U32(QUIRK_TYPE_ ## x)

and use like QUIRK_FLAG(GET_SAMPLE_RATE).

With those changes, the above can be defined more safely like

static const char *const snd_usb_audio_quirk_flag_names[] = {
	[QUIRK_TYPE_GET_SAMPLE_RATE] = "get_sample_rate",
	....

or even more drastically by defining some macro for each entry like:

#define QUIRK_STRING_ENTRY(x) \
	[QUIRK_TYPE_ ## x] = __stringify(x)

and put like:

static const char *const snd_usb_audio_quirk_flag_names[] = {
	QUIRK_STRING_ENTRY(GET_SAMPLE_RATE),
	....
};

In this case, it'll become upper letters, so the parse would need to
deal with the case-insensitive comparison, though.

> +u32 snd_usb_quirk_flags_from_name(char *name)

Use const char *.

> +{
> +	u32 flag = 0;
> +	u32 i;

The iterator can be simple int.

> +	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);

Use BIT_U32(i)

> +			break;

We can return the value directly, so flag variable can be dropped.

>  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));
> +			}

This could be better factored out as a function.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 2/5] param: export param_array related functions
  2025-09-18  9:24 ` [PATCH v4 2/5] param: export param_array related functions Cryolitia PukNgae via B4 Relay
@ 2025-09-19 12:37   ` Takashi Iwai
  0 siblings, 0 replies; 11+ messages in thread
From: Takashi Iwai @ 2025-09-19 12:37 UTC (permalink / raw)
  To: cryolitia
  Cc: Jaroslav Kysela, Takashi Iwai, Jonathan Corbet, Luis Chamberlain,
	Petr Pavlu, Daniel Gomez, Sami Tolvanen, linux-sound, linux-usb,
	linux-kernel, linux-doc, Mingcong Bai, Kexy Biscuit, Nie Cheng,
	Zhan Jun, Feng Yuan, qaqland, kernel, linux-modules

On Thu, 18 Sep 2025 11:24:31 +0200,
Cryolitia PukNgae via B4 Relay wrote:
> 
> From: Cryolitia PukNgae <cryolitia@uniontech.com>
> 
> - int param_array_set(const char *val, const struct kernel_param *kp);
> - int param_array_get(char *buffer, const struct kernel_param *kp);
> - void param_array_free(void *arg);
> 
> It would be helpful for the new module param we designed in
> snd_usb_audio, in order to run additional custom codes when params
> are set in runtime, and re-use the extisted codes in param.c
> 
> Signed-off-by: Cryolitia PukNgae <cryolitia@uniontech.com>

Can we do just like below?

static int param_set_quirkp(const char *val, const struct kernel_param *kp)
{
	guard(mutex)(&quirk_flags_mutex);
	return param_set_charp(val, kp);
}

static const struct kernel_param_ops param_ops_quirkp = {
	.set = param_set_quirkp,
	.get = param_get_charp,
	.free = param_free_charp,
};
#define param_check_quirkp param_check_charp

modules_param_parray(quirk_flags, quirkp, NULL, 0644);

Then mutex is locked at each time a parameter is set.
Optionally, the string value can be verified in param_set_quirkp()
before passing to param_set_charp(), too.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 3/5] ALSA: usb-audio: improve module param quirk_flags
  2025-09-18  9:24 ` [PATCH v4 3/5] ALSA: usb-audio: improve module param quirk_flags Cryolitia PukNgae via B4 Relay
@ 2025-09-19 12:47   ` Takashi Iwai
  0 siblings, 0 replies; 11+ messages in thread
From: Takashi Iwai @ 2025-09-19 12:47 UTC (permalink / raw)
  To: cryolitia
  Cc: Jaroslav Kysela, Takashi Iwai, Jonathan Corbet, Luis Chamberlain,
	Petr Pavlu, Daniel Gomez, Sami Tolvanen, linux-sound, linux-usb,
	linux-kernel, linux-doc, Mingcong Bai, Kexy Biscuit, Nie Cheng,
	Zhan Jun, Feng Yuan, qaqland, kernel, linux-modules, Takashi Iwai

On Thu, 18 Sep 2025 11:24:32 +0200,
Cryolitia PukNgae via B4 Relay wrote:
> 
> From: Cryolitia PukNgae <cryolitia@uniontech.com>
> 
> For apply and unapply quirk flags more flexibly though param
> 
> Co-developed-by: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Cryolitia PukNgae <cryolitia@uniontech.com>
> ---
>  sound/usb/card.c     |   9 ++--
>  sound/usb/quirks.c   | 119 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>  sound/usb/quirks.h   |   3 +-
>  sound/usb/usbaudio.h |   3 ++
>  4 files changed, 126 insertions(+), 8 deletions(-)
> 
> diff --git a/sound/usb/card.c b/sound/usb/card.c
> index 0265206a8e8cf31133e8463c98fe0497d8ace89e..5837677effa1787ef9d7d02714c3ae43b1a8bc79 100644
> --- a/sound/usb/card.c
> +++ b/sound/usb/card.c
> @@ -73,8 +73,8 @@ 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];
>  
> +char *quirk_flags[SNDRV_CARDS];

My preference is to keep this static, but pass the string value to a
function.  That is, define snd_usb_init_quirk_flags() in main.c:

static void snd_usb_init_quirk_flags(struct snd_usb_audio *chip, int indx)
{
	/* old style option found: the position-based integer value */
	if (quirk_flags[idx] &&
	    !kstrtou32(quirk_flags[idx], 0, &chip->quirk_flags)) {
		usb_audio_dbg(chip,
			      "Set quirk flags 0x%x from param based on position %d for device %04x:%04x\n",
			      chip->quirk_flags, idx,
			      USB_ID_VENDOR(chip->usb_id),
			      USB_ID_PRODUCT(chip->usb_id));
		return;
	}

	/* take the default quirk from the quirk table */
	snd_usb_init_quirk_flags_table(chip);

	/* add or correct quirk bits from options */
	for (i = 0; i < ARRAY_SIZE(quirk_flags); i++) {
		char *val __free(kfree) = NULL;

		if (!quirk_flags[i] || !*quirk_flags[i])
			continue;
		
		val = kstrdup(quirk_flags[i], GFP_KERNEL);
		if (!val)
			return;

		snd_usb_parse_quirk_string(chip, val);
	}
}
	
static int snd_usb_audio_create(....)
{
	....
	snd_usb_init_quirk_flags(chip, idx);
	....
}

The function snd_usb_parse_quirk_string() is defined in quirks.c,

void snd_usb_parse_quirk_string(struct snd_usb_audio *chip, char *val)
{
	for (p = val; p && *p;) {
		/* Each entry consists of VID:PID:flags */
		field = strsep(&p, ":");
		if (!field)
			break;
		.....
	}
}


thanks,

Takashi

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2025-09-19 12:47 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-18  9:24 [PATCH v4 0/5] ALSA: usb-audio: add module param device_quirk_flags Cryolitia PukNgae via B4 Relay
2025-09-18  9:24 ` [PATCH v4 1/5] ALSA: usb-audio: add two-way convert between name and bit for QUIRK_FLAG_* Cryolitia PukNgae via B4 Relay
2025-09-19 12:32   ` Takashi Iwai
2025-09-18  9:24 ` [PATCH v4 2/5] param: export param_array related functions Cryolitia PukNgae via B4 Relay
2025-09-19 12:37   ` Takashi Iwai
2025-09-18  9:24 ` [PATCH v4 3/5] ALSA: usb-audio: improve module param quirk_flags Cryolitia PukNgae via B4 Relay
2025-09-19 12:47   ` Takashi Iwai
2025-09-18  9:24 ` [PATCH v4 4/5] ALSA: usb-audio: make param quirk_flags change-able in runtime Cryolitia PukNgae via B4 Relay
2025-09-18  9:24 ` [PATCH v4 5/5] ALSA: doc: add docs about improved quirk_flags in snd-usb-audio Cryolitia PukNgae via B4 Relay
2025-09-18 20:21   ` Randy Dunlap
2025-09-19  1:43     ` Cryolitia PukNgae

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).