[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2024020617-limb-name-f852@gregkh>
Date: Tue, 6 Feb 2024 14:50:21 +0000
From: Greg KH <gregkh@...uxfoundation.org>
To: Takashi Iwai <tiwai@...e.de>
Cc: Wesley Cheng <quic_wcheng@...cinc.com>, srinivas.kandagatla@...aro.org,
mathias.nyman@...el.com, perex@...ex.cz, conor+dt@...nel.org,
corbet@....net, lgirdwood@...il.com, andersson@...nel.org,
krzysztof.kozlowski+dt@...aro.org, Thinh.Nguyen@...opsys.com,
broonie@...nel.org, bgoswami@...cinc.com, tiwai@...e.com,
robh+dt@...nel.org, konrad.dybcio@...aro.org,
linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
linux-sound@...r.kernel.org, linux-usb@...r.kernel.org,
linux-arm-msm@...r.kernel.org, linux-doc@...r.kernel.org,
alsa-devel@...a-project.org
Subject: Re: [PATCH v13 32/53] ALSA: usb-audio: Check for support for
requested audio format
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@...cinc.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?
thanks,
greg k-h
Powered by blists - more mailing lists