Hi Takashi, Am 14.09.19 um 17:24 schrieb Takashi Iwai: > The PCM draining behavior got broken since the recent refactoring, and > this turned out to be the incorrect expectation of the firmware > behavior regarding "draining". While I expected the "drain" flag at > the stop operation would do processing the queued samples, it seems > rather dropping the samples. > > As a quick fix, just drop the SNDRV_PCM_INFO_DRAIN_TRIGGER flag, so > that the driver uses the normal PCM draining procedure. Also, put > some caution comment to the function for future readers not to fall > into the same pitfall. > > Fixes: d7ca3a71545b ("staging: bcm2835-audio: Operate non-atomic PCM ops") > BugLink: https://github.com/raspberrypi/linux/issues/2983
thanks for taking care of this. Wouldn't it be better to add the link to the new comment to provide more context of the unexpected behavior? Nevertheless: Acked-by: Stefan Wahren <wahre...@gmx.net> > Cc: sta...@vger.kernel.org > Signed-off-by: Takashi Iwai <ti...@suse.de> > --- > drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c | 4 ++-- > drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c | 1 + > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c > b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c > index bc1eaa3a0773..826016c3431a 100644 > --- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c > +++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c > @@ -12,7 +12,7 @@ > static const struct snd_pcm_hardware snd_bcm2835_playback_hw = { > .info = (SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_BLOCK_TRANSFER | > SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID | > - SNDRV_PCM_INFO_DRAIN_TRIGGER | SNDRV_PCM_INFO_SYNC_APPLPTR), > + SNDRV_PCM_INFO_SYNC_APPLPTR), > .formats = SNDRV_PCM_FMTBIT_U8 | SNDRV_PCM_FMTBIT_S16_LE, > .rates = SNDRV_PCM_RATE_CONTINUOUS | SNDRV_PCM_RATE_8000_48000, > .rate_min = 8000, > @@ -29,7 +29,7 @@ static const struct snd_pcm_hardware > snd_bcm2835_playback_hw = { > static const struct snd_pcm_hardware snd_bcm2835_playback_spdif_hw = { > .info = (SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_BLOCK_TRANSFER | > SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID | > - SNDRV_PCM_INFO_DRAIN_TRIGGER | SNDRV_PCM_INFO_SYNC_APPLPTR), > + SNDRV_PCM_INFO_SYNC_APPLPTR), > .formats = SNDRV_PCM_FMTBIT_S16_LE, > .rates = SNDRV_PCM_RATE_CONTINUOUS | SNDRV_PCM_RATE_44100 | > SNDRV_PCM_RATE_48000, > diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c > b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c > index 23fba01107b9..c6f9cf1913d2 100644 > --- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c > +++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c > @@ -289,6 +289,7 @@ int bcm2835_audio_stop(struct bcm2835_alsa_stream > *alsa_stream) > VC_AUDIO_MSG_TYPE_STOP, false); > } > > +/* FIXME: this doesn't seem working as expected for "draining" */ > int bcm2835_audio_drain(struct bcm2835_alsa_stream *alsa_stream) > { > struct vc_audio_msg m = {