Re: [PATCH 2/3 v2] ALSA: usb-audio: avoid setting of sample rate multiple times on bus

[Takashi Iwai <tiwai@suse.de>]

On Wed, 30 Nov 2016 08:59:22 +0100,
Jiada Wang wrote:
> 
> From: Daniel Girnus <dgirnus@de.adit-jv.com>
> 
> ALSA usually calls the prepare function twice before starting the playback:
> 1. On hw_params call from userland and
> 2. internally when starting the stream.
> Some device are not able to manage this and they will stop playback
> if the sample rate will be configured several times over USB protocol.
> 
> Signed-off-by: Jens Lorenz <jlorenz@de.adit-jv.com>
> Signed-off-by: Jiada Wang <jiada_wang@mentor.com>

The sign-off from Daniel seems missing?

The code change looks OK, but it'd be nice to mention in the changelog
that, after this patch, snd_usb_init_sample_rate() is still called
properly whenever the parameter is changed since ep->need_setup_ep is
set in snd_hsb_hw_params().


thanks,

Takashi

> ---
>  sound/usb/pcm.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
> index 44d178e..a522c9a 100644
> --- a/sound/usb/pcm.c
> +++ b/sound/usb/pcm.c
> @@ -806,17 +806,18 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream)
>  	if (ret < 0)
>  		goto unlock;
>  
> -	iface = usb_ifnum_to_if(subs->dev, subs->cur_audiofmt->iface);
> -	alts = &iface->altsetting[subs->cur_audiofmt->altset_idx];
> -	ret = snd_usb_init_sample_rate(subs->stream->chip,
> -				       subs->cur_audiofmt->iface,
> -				       alts,
> -				       subs->cur_audiofmt,
> -				       subs->cur_rate);
> -	if (ret < 0)
> -		goto unlock;
> -
>  	if (subs->need_setup_ep) {
> +
> +		iface = usb_ifnum_to_if(subs->dev, subs->cur_audiofmt->iface);
> +		alts = &iface->altsetting[subs->cur_audiofmt->altset_idx];
> +		ret = snd_usb_init_sample_rate(subs->stream->chip,
> +					       subs->cur_audiofmt->iface,
> +					       alts,
> +					       subs->cur_audiofmt,
> +					       subs->cur_rate);
> +		if (ret < 0)
> +			goto unlock;
> +
>  		ret = configure_endpoint(subs);
>  		if (ret < 0)
>  			goto unlock;
> -- 
> 2.9.3
> 
>