Messages in this thread | | | Date | Wed, 7 Feb 2024 16:04:20 -0800 | Subject | Re: [PATCH v13 32/53] ALSA: usb-audio: Check for support for requested audio format | From | Wesley Cheng <> |
| |
On 2/6/2024 6:53 AM, Takashi Iwai wrote: > On Tue, 06 Feb 2024 15:50:21 +0100, > Greg KH wrote: >> >> On Tue, Feb 06, 2024 at 02:12:44PM +0100, Takashi Iwai wrote: >>> On Sat, 03 Feb 2024 03:36:24 +0100, >>> Wesley Cheng wrote: >>>> >>>> Allow for checks on a specific USB audio device to see if a requested PCM >>>> format is supported. This is needed for support when playback is >>>> initiated by the ASoC USB backend path. >>>> >>>> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com> >>> >>> Just cosmetic: >>> >>>> +struct snd_usb_stream *snd_usb_find_suppported_substream(int card_idx, >>>> + struct snd_pcm_hw_params *params, int direction) >>>> +{ >>>> + struct snd_usb_audio *chip; >>>> + struct snd_usb_substream *subs; >>>> + struct snd_usb_stream *as; >>>> + const struct audioformat *fmt; >>>> + >>>> + /* >>>> + * Register mutex is held when populating and clearing usb_chip >>>> + * array. >>>> + */ >>>> + mutex_lock(®ister_mutex); >>>> + chip = usb_chip[card_idx]; >>>> + if (!chip) { >>>> + mutex_unlock(®ister_mutex); >>>> + return NULL; >>>> + } >>>> + >>>> + if (enable[card_idx]) { >>>> + list_for_each_entry(as, &chip->pcm_list, list) { >>>> + subs = &as->substream[direction]; >>>> + fmt = snd_usb_find_substream_format(subs, params); >>>> + if (fmt) { >>>> + mutex_unlock(®ister_mutex); >>>> + return as; >>>> + } >>>> + } >>>> + } >>>> + mutex_unlock(®ister_mutex); >>> >>> I prefer having the single lock/unlock call pair, e.g. >>> >>> struct snd_usb_stream *as, *ret; >>> >>> ret = NULL; >>> mutex_lock(®ister_mutex); >>> chip = usb_chip[card_idx]; >>> if (chip && enable[card_idx]) { >>> list_for_each_entry(as, &chip->pcm_list, list) { >>> subs = &as->substream[direction]; >>> if (snd_usb_find_substream_format(subs, params)) { >>> ret = as; >>> break; >>> } >>> } >>> } >>> mutex_unlock(®ister_mutex); >>> return ret; >>> } >>> >>> In this case, we shouldn't reuse "as" for the return value since it >>> can be non-NULL after the loop end. >> >> Why not just use guard(mutex) for this, making it all not an issue? > > Heh, it's too new ;) > That should work gracefully, yes. >
Thanks Greg/Takashi. That is nifty.
Thanks Wesley Cheng
| |