| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <E8E9626E8F00D397+401f622f-247f-4b0e-b82b-faca3a1055d0@uniontech.com>
Date: Mon, 15 Sep 2025 16:39:52 +0800
From: Cryolitia PukNgae <cryolitia@...ontech.com>
To: Takashi Iwai <tiwai@...e.de>
Cc: Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>,
Jonathan Corbet <corbet@....net>, linux-sound@...r.kernel.org,
linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-doc@...r.kernel.org, Mingcong Bai <jeffbai@...c.io>,
Kexy Biscuit <kexybiscuit@...c.io>, Nie Cheng <niecheng1@...ontech.com>,
Zhan Jun <zhanjun@...ontech.com>, Feng Yuan <fengyuan@...ontech.com>,
qaqland <anguoli@...ontech.com>, kernel@...ontech.com
Subject: Re: [PATCH v2 0/3] ALSA: usb-audio: add module param
device_quirk_flags
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@...ontech.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
Powered by blists - more mailing lists