Re: [alsa-devel] [PATCH 5.6 09/71] ALSA: firewire: Use managed buffer allocation

From: Takashi Iwai <tiwai@suse.de>
To: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Cc: alsa-devel@alsa-project.org
Subject: Re: [alsa-devel] [PATCH 5.6 09/71] ALSA: firewire: Use managed buffer allocation
Date: Mon, 09 Dec 2019 20:08:12 +0100	[thread overview]
Message-ID: <s5hy2vl9v1f.wl-tiwai@suse.de> (raw)
In-Reply-To: <s5hwob5cwz9.wl-tiwai@suse.de>

On Mon, 09 Dec 2019 16:58:02 +0100,
Takashi Iwai wrote:
> 
> On Mon, 09 Dec 2019 16:16:56 +0100,
> Takashi Sakamoto wrote:
> > 
> > Hi,
> > 
> > I'm not opposed to this patchset. The direction is preferable.
> > 
> > However changes in this patchset brings bugs in error path of
> > each implementation for pcm.hw_params.
> > 
> > On Mon, Dec 09, 2019 at 10:48:41AM +0100, Takashi Iwai wrote:
> > > diff --git a/sound/firewire/bebob/bebob_pcm.c b/sound/firewire/bebob/bebob_pcm.c
> > > index d4edd06d32cf..2ee91ff6891e 100644
> > > --- a/sound/firewire/bebob/bebob_pcm.c
> > > +++ b/sound/firewire/bebob/bebob_pcm.c
> > > @@ -214,10 +214,6 @@ static int pcm_hw_params(struct snd_pcm_substream *substream,
> > >  	struct snd_bebob *bebob = substream->private_data;
> > >  	int err;
> > >  
> > > -	err = snd_pcm_lib_malloc_pages(substream, params_buffer_bytes(hw_params));
> > > -	if (err < 0)
> > > -		return err;
> > > -
> > >  	if (substream->runtime->status->state == SNDRV_PCM_STATE_OPEN) {
> > >  		unsigned int rate = params_rate(hw_params);
> > >  		unsigned int frames_per_period = params_period_size(hw_params);
> > > @@ -231,7 +227,7 @@ static int pcm_hw_params(struct snd_pcm_substream *substream,
> > >  		mutex_unlock(&bebob->mutex);
> > >  	}
> > >  
> > > -	return err;
> > > +	return 0;
> > >  }
> >   
> > This should return the err variable to report failure of isochronous
> > resources reservation.
> > 
> > However I found that ALSA oxfw/fireface drivers returns 0 even if it
> > fails. This is bug since v5.3 kernel.
> > 
> > Would you please apply below two patches into your three, then modify
> > your patch for all firewire drivers to return error correctly?
> 
> Yes, that sounds like a correct fix.  Thank you for spotting out.
> 
> So, please go ahead, submit these two patches, then I'll update the
> fix containing your correction above and submit as v2 (maybe only for
> this, as the whole series is too big to re-submit).

Never mind, the patches there were already good enough, so I applied
these two now.  Will resend v2 fix for this patch.

thanks,

Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel