lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <s5hef0hzhfl.wl-tiwai@suse.de>
Date:   Sun, 15 Sep 2019 17:51:26 +0200
From:   Takashi Iwai <tiwai@...e.de>
To:     Stefan Wahren <wahrenst@....net>
Cc:     Eric Anholt <eric@...olt.net>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-rpi-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] staging: bcm2835-audio: Fix draining behavior regression

On Sun, 15 Sep 2019 15:54:16 +0200,
Stefan Wahren wrote:
> 
> 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?

Yeah, a bit more explanation would be better.  Unfortunately I've been
traveling in the last week (and will be traveling again in the next
weeks), so had little time for caring and testing with any actual
hardware...

> 
> Nevertheless:
> 
> Acked-by: Stefan Wahren <wahrenst@....net>

Thanks!


Takashi


> > Cc: stable@...r.kernel.org
> > Signed-off-by: Takashi Iwai <tiwai@...e.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 = {
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ