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
