Re: [PATCH v3 3/4] ALSA: usb-audio: add module param device_quirk_flags

[Takashi Iwai <tiwai@suse.de>]

On Wed, 17 Sep 2025 14:46:42 +0200,
Cryolitia PukNgae via B4 Relay wrote:
> 
> From: Cryolitia PukNgae <cryolitia@uniontech.com>
> 
> For apply and unapply quirk flags more flexibly though param and sysfs
> 
> Co-developed-by: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Cryolitia PukNgae <cryolitia@uniontech.com>

I think an easier approach would be to rather parse the string value
at each time when probing a device and seeking for options.

That is, let's code without a mutex at first (for the permission
0444):

void snd_usb_init_dynamic_quirks(int idx, struct snd_usb_audio *chip)
{
	....
	/* old style option found: the position-based integer value */
	if (quirk_flags[idx] &&
	    !kstrtou32(quirk_flags[idx], 0, &chip->quirk_flags)) {
		usb_audio_dbg(....);
		return;
	}

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

	for (i = 0; i < ARRAY_SIZE(quirk_flags); i++) {
		if (quirk_flags[i] && *quirk_flags[i]) {
			err = parse_quirk_option(chip, quirk_flags[i]);
			if (err < 0)
				return;
		}
	}
}

and the parser would be something like:

static int parse_quirk_option(struct snd_usb_audio *chip, const char *str)
{
	char *val __free(kfree) = NULL;
	char *field;
	int pid, vid;

	if (!strchr(str, ':'))
		return 0;

	val = kstrdup(str, GFP_KERNEL);
	if (!val)
		return -ENOMEM;

	/* Each entry consists of VID:PID:flags */
	field = strsep(&p, ":");
	if (!field)
		return 0;
	if (strcmp(field, "*") == 0)
		vid = 0;
	else if (kstrtou16(field, 16, &vid))
		return 0; // can spew warning message, too

	field = strsep(&p, ":");
	if (!field)
		return 0;
	if (strcmp(field, "*") == 0)
		pid = 0;
	else if (kstrtou16(field, 16, &pid))
		return 0; // can spew warning message, too

	.... // evaluate the tokens, set or clear chip->quirk_flags accordingly
	return 0;
}

So you can just use the normal charp type for the parameters.

Once after this working, we may want to allow the dynamic module
parameter change.  Then make the parameter a special type just to take
the device_quirk_mutex at set callback, while the set callback simply
calls parm_set_charp() for the rest.  For get and free callbacks, we
can use param_get_charp() and param_free_charp() as is.
Finally, snd_usb_init_dynamic_quirks() takes the device_quirk_mutex,
and that's all.  No need for heavy cleanups or linked list handling.

Of course, it's a bit inefficient from the performance POV, but the
device probing is rather a very rare event, so the speed doesn't
matter at all.


thanks,

Takashi