All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ASoC: compress: Fix compress device direction check
@ 2016-01-07  8:23 ` Vinod Koul
  0 siblings, 0 replies; 4+ messages in thread
From: Vinod Koul @ 2016-01-07  8:23 UTC (permalink / raw)
  To: alsa-devel; +Cc: broonie, liam.r.girdwood, patches.audio, Vinod Koul, stable

The detection of direction for compress takes into account codec
capabilities only and not the CPU ones. Fix this by checking the
CPU side capabilities as well.

Cc: <stable@vger.kernel.org>
Tested-by: Ashish Panwar <ashish.panwar@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/soc/soc-compress.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c
index 12a9820feac1..b5737945bcc3 100644
--- a/sound/soc/soc-compress.c
+++ b/sound/soc/soc-compress.c
@@ -630,6 +630,7 @@ int snd_soc_new_compress(struct snd_soc_pcm_runtime *rtd, int num)
 	struct snd_pcm *be_pcm;
 	char new_name[64];
 	int ret = 0, direction = 0;
+	int playback = 0, capture = 0;
 
 	if (rtd->num_codecs > 1) {
 		dev_err(rtd->card->dev, "Multicodec not supported for compressed stream\n");
@@ -641,11 +642,27 @@ int snd_soc_new_compress(struct snd_soc_pcm_runtime *rtd, int num)
 			rtd->dai_link->stream_name, codec_dai->name, num);
 
 	if (codec_dai->driver->playback.channels_min)
+		playback = 1;
+	if (codec_dai->driver->capture.channels_min)
+		capture = 1;
+
+	capture = capture && cpu_dai->driver->capture.channels_min;
+	playback = playback && cpu_dai->driver->playback.channels_min;
+
+	/*
+	 * Compress devices are unidirectional so only one of the directions
+	 * should be set, check for that (xor)
+	 */
+	if (!(playback || capture)) {
+		dev_err(rtd->card->dev, "Invalid direction for compress P %d, C %d\n",
+				playback, capture);
+		return -EINVAL;
+	}
+
+	if (playback)
 		direction = SND_COMPRESS_PLAYBACK;
-	else if (codec_dai->driver->capture.channels_min)
-		direction = SND_COMPRESS_CAPTURE;
 	else
-		return -EINVAL;
+		direction = SND_COMPRESS_CAPTURE;
 
 	compr = kzalloc(sizeof(*compr), GFP_KERNEL);
 	if (compr == NULL) {
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* [PATCH] ASoC: compress: Fix compress device direction check
@ 2016-01-07  8:23 ` Vinod Koul
  0 siblings, 0 replies; 4+ messages in thread
From: Vinod Koul @ 2016-01-07  8:23 UTC (permalink / raw)
  To: alsa-devel; +Cc: broonie, liam.r.girdwood, patches.audio, Vinod Koul, stable

The detection of direction for compress takes into account codec
capabilities only and not the CPU ones. Fix this by checking the
CPU side capabilities as well.

Cc: <stable@vger.kernel.org>
Tested-by: Ashish Panwar <ashish.panwar@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/soc/soc-compress.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c
index 12a9820feac1..b5737945bcc3 100644
--- a/sound/soc/soc-compress.c
+++ b/sound/soc/soc-compress.c
@@ -630,6 +630,7 @@ int snd_soc_new_compress(struct snd_soc_pcm_runtime *rtd, int num)
 	struct snd_pcm *be_pcm;
 	char new_name[64];
 	int ret = 0, direction = 0;
+	int playback = 0, capture = 0;
 
 	if (rtd->num_codecs > 1) {
 		dev_err(rtd->card->dev, "Multicodec not supported for compressed stream\n");
@@ -641,11 +642,27 @@ int snd_soc_new_compress(struct snd_soc_pcm_runtime *rtd, int num)
 			rtd->dai_link->stream_name, codec_dai->name, num);
 
 	if (codec_dai->driver->playback.channels_min)
+		playback = 1;
+	if (codec_dai->driver->capture.channels_min)
+		capture = 1;
+
+	capture = capture && cpu_dai->driver->capture.channels_min;
+	playback = playback && cpu_dai->driver->playback.channels_min;
+
+	/*
+	 * Compress devices are unidirectional so only one of the directions
+	 * should be set, check for that (xor)
+	 */
+	if (!(playback || capture)) {
+		dev_err(rtd->card->dev, "Invalid direction for compress P %d, C %d\n",
+				playback, capture);
+		return -EINVAL;
+	}
+
+	if (playback)
 		direction = SND_COMPRESS_PLAYBACK;
-	else if (codec_dai->driver->capture.channels_min)
-		direction = SND_COMPRESS_CAPTURE;
 	else
-		return -EINVAL;
+		direction = SND_COMPRESS_CAPTURE;
 
 	compr = kzalloc(sizeof(*compr), GFP_KERNEL);
 	if (compr == NULL) {
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [alsa-devel] [PATCH] ASoC: compress: Fix compress device direction check
  2016-01-07  8:23 ` Vinod Koul
  (?)
@ 2016-01-07 10:19 ` Takashi Iwai
  2016-01-07 16:14   ` Vinod Koul
  -1 siblings, 1 reply; 4+ messages in thread
From: Takashi Iwai @ 2016-01-07 10:19 UTC (permalink / raw)
  To: Vinod Koul; +Cc: alsa-devel, liam.r.girdwood, patches.audio, broonie, stable

On Thu, 07 Jan 2016 09:23:11 +0100,
Vinod Koul wrote:
> 
> The detection of direction for compress takes into account codec
> capabilities only and not the CPU ones. Fix this by checking the
> CPU side capabilities as well.
> 
> Cc: <stable@vger.kernel.org>
> Tested-by: Ashish Panwar <ashish.panwar@intel.com>
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> ---
>  sound/soc/soc-compress.c | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c
> index 12a9820feac1..b5737945bcc3 100644
> --- a/sound/soc/soc-compress.c
> +++ b/sound/soc/soc-compress.c
> @@ -630,6 +630,7 @@ int snd_soc_new_compress(struct snd_soc_pcm_runtime *rtd, int num)
>  	struct snd_pcm *be_pcm;
>  	char new_name[64];
>  	int ret = 0, direction = 0;
> +	int playback = 0, capture = 0;
>  
>  	if (rtd->num_codecs > 1) {
>  		dev_err(rtd->card->dev, "Multicodec not supported for compressed stream\n");
> @@ -641,11 +642,27 @@ int snd_soc_new_compress(struct snd_soc_pcm_runtime *rtd, int num)
>  			rtd->dai_link->stream_name, codec_dai->name, num);
>  
>  	if (codec_dai->driver->playback.channels_min)
> +		playback = 1;
> +	if (codec_dai->driver->capture.channels_min)
> +		capture = 1;
> +
> +	capture = capture && cpu_dai->driver->capture.channels_min;
> +	playback = playback && cpu_dai->driver->playback.channels_min;
> +
> +	/*
> +	 * Compress devices are unidirectional so only one of the directions
> +	 * should be set, check for that (xor)
> +	 */
> +	if (!(playback || capture)) {

I don't think this can catch the case where both playback and capture
are set.  You need really use XOR operator for that.  Or, something
like:
	if (playback + capture != 1) {
		....


Takashi

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [alsa-devel] [PATCH] ASoC: compress: Fix compress device direction check
  2016-01-07 10:19 ` [alsa-devel] " Takashi Iwai
@ 2016-01-07 16:14   ` Vinod Koul
  0 siblings, 0 replies; 4+ messages in thread
From: Vinod Koul @ 2016-01-07 16:14 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, liam.r.girdwood, patches.audio, broonie, stable

On Thu, Jan 07, 2016 at 11:19:17AM +0100, Takashi Iwai wrote:
> On Thu, 07 Jan 2016 09:23:11 +0100,
> Vinod Koul wrote:
> > 
> > The detection of direction for compress takes into account codec
> > capabilities only and not the CPU ones. Fix this by checking the
> > CPU side capabilities as well.
> > 
> > Cc: <stable@vger.kernel.org>
> > Tested-by: Ashish Panwar <ashish.panwar@intel.com>
> > Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> > ---
> >  sound/soc/soc-compress.c | 23 ++++++++++++++++++++---
> >  1 file changed, 20 insertions(+), 3 deletions(-)
> > 
> > diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c
> > index 12a9820feac1..b5737945bcc3 100644
> > --- a/sound/soc/soc-compress.c
> > +++ b/sound/soc/soc-compress.c
> > @@ -630,6 +630,7 @@ int snd_soc_new_compress(struct snd_soc_pcm_runtime *rtd, int num)
> >  	struct snd_pcm *be_pcm;
> >  	char new_name[64];
> >  	int ret = 0, direction = 0;
> > +	int playback = 0, capture = 0;
> >  
> >  	if (rtd->num_codecs > 1) {
> >  		dev_err(rtd->card->dev, "Multicodec not supported for compressed stream\n");
> > @@ -641,11 +642,27 @@ int snd_soc_new_compress(struct snd_soc_pcm_runtime *rtd, int num)
> >  			rtd->dai_link->stream_name, codec_dai->name, num);
> >  
> >  	if (codec_dai->driver->playback.channels_min)
> > +		playback = 1;
> > +	if (codec_dai->driver->capture.channels_min)
> > +		capture = 1;
> > +
> > +	capture = capture && cpu_dai->driver->capture.channels_min;
> > +	playback = playback && cpu_dai->driver->playback.channels_min;
> > +
> > +	/*
> > +	 * Compress devices are unidirectional so only one of the directions
> > +	 * should be set, check for that (xor)
> > +	 */
> > +	if (!(playback || capture)) {
> 
> I don't think this can catch the case where both playback and capture
> are set.  You need really use XOR operator for that.  Or, something
> like:
> 	if (playback + capture != 1) {

Oops, i did mean XOR (see comment) but messed the implementation
and did NOR :(, thanks for pointing, will send updated one..

-- 
~Vinod

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-01-07 16:11 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-07  8:23 [PATCH] ASoC: compress: Fix compress device direction check Vinod Koul
2016-01-07  8:23 ` Vinod Koul
2016-01-07 10:19 ` [alsa-devel] " Takashi Iwai
2016-01-07 16:14   ` Vinod Koul

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.