Hi Takashi,
I tested the patch and USB audio function works well.
(Start and stop playback more than 20 times with Samsung USB C
Earphone)
Thanks
On Thu, 2022-09-01 at 10:28 +0200, Takashi Iwai wrote:
> On Thu, 01 Sep 2022 09:51:37 +0200,
> Takashi Iwai wrote:
> >
> > On Thu, 01 Sep 2022 07:50:40 +0200,
> > chihhao chen wrote:
> > >
> > > Hi Takashi,
> > >
> > > The patch fixes this problem.
> > > I tried it with "Product: Samsung USB C Earphone" and missing
> > > sound
> > > problem cannot be reproduced.
> >
> > OK, it's a good news. I'm going to add more information to the
> > patch
> > description about the regression and submit the patch.
>
> Could you check whether the below cleanup patch on top of the
> previous
> one doesn't break things? I lightly tested on my devices and don't
> believe it would change the actual behavior, but just to be sure.
>
>
> Takashi
>
> -- 8< --
> From: Takashi Iwai <tiwai@xxxxxxx>
> Subject: [PATCH] ALSA: usb-audio: Clean up endpoint setups at PCM
> prepare
>
> This patch cleans up the superfluous checks and calls for setting up
> the endpoints at PCM prepare callback:
>
> - Drop stop_endpoints() and sync_pending_stops() calls; the stream is
> guaranteed to have been already stopped and synced at each PCM
> prepare call by ALSA PCM core
>
> - Call snd_usb_endpoint_prepare() unconditionally;
> the check for endpoint->need_setup is done in
> snd_pcm_hw_endpoint_prepare() itself
>
> - Apply snd_usb_set_format_quirk() only when the endpoint is actually
> set up (i.e. the return code from snd_usb_endpoint_prepare() > 0)
>
> - Move a few lines back into snd_usb_pcm_prepare();
> it's even easier to follow than a small useless function
>
> Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
> ---
> sound/usb/pcm.c | 40 ++++++++++------------------------------
> 1 file changed, 10 insertions(+), 30 deletions(-)
>
> diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
> index b604f7e95e82..4ed53a3dc922 100644
> --- a/sound/usb/pcm.c
> +++ b/sound/usb/pcm.c
> @@ -433,35 +433,6 @@ static void close_endpoints(struct snd_usb_audio
> *chip,
> }
> }
>
> -static int configure_endpoints(struct snd_usb_audio *chip,
> - struct snd_usb_substream *subs)
> -{
> - int err;
> -
> - if (subs->data_endpoint->need_setup) {
> - /* stop any running stream beforehand */
> - if (stop_endpoints(subs, false))
> - sync_pending_stops(subs);
> - if (subs->sync_endpoint) {
> - err = snd_usb_endpoint_prepare(chip, subs-
> >sync_endpoint);
> - if (err < 0)
> - return err;
> - }
> - err = snd_usb_endpoint_prepare(chip, subs-
> >data_endpoint);
> - if (err < 0)
> - return err;
> - snd_usb_set_format_quirk(subs, subs->cur_audiofmt);
> - } else {
> - if (subs->sync_endpoint) {
> - err = snd_usb_endpoint_prepare(chip, subs-
> >sync_endpoint);
> - if (err < 0)
> - return err;
> - }
> - }
> -
> - return 0;
> -}
> -
> /*
> * hw_params callback
> *
> @@ -640,9 +611,18 @@ static int snd_usb_pcm_prepare(struct
> snd_pcm_substream *substream)
> goto unlock;
> }
>
> - ret = configure_endpoints(chip, subs);
> + if (subs->sync_endpoint) {
> + ret = snd_usb_endpoint_prepare(chip, subs-
> >sync_endpoint);
> + if (ret < 0)
> + goto unlock;
> + }
> +
> + ret = snd_usb_endpoint_prepare(chip, subs->data_endpoint);
> if (ret < 0)
> goto unlock;
> + else if (ret > 0)
> + snd_usb_set_format_quirk(subs, subs->cur_audiofmt);
> + ret = 0;
>
> /* reset the pointer */
> subs->buffer_bytes = frames_to_bytes(runtime, runtime-
> >buffer_size);
