[RFC,v2,4/4] ASoC: add warning if the function is not support multi cpu yet.
diff mbox series

Message ID 20200114175152.3291-5-yung-chuan.liao@linux.intel.com
State New
Headers show
Series
  • ASoC: Add Multi CPU DAI support
Related show

Commit Message

Bard Liao Jan. 14, 2020, 5:51 p.m. UTC
Multi cpu is not supported by all functions yet. Add a warning message
to warn it.

Suggested-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Signed-off-by: Bard liao <yung-chuan.liao@linux.intel.com>
---
 sound/soc/soc-compress.c              |  7 ++++---
 sound/soc/soc-generic-dmaengine-pcm.c | 12 ++++++++++++
 sound/soc/soc-pcm.c                   | 12 ++++++++++++
 3 files changed, 28 insertions(+), 3 deletions(-)

Comments

Pierre-Louis Bossart Jan. 16, 2020, 1:49 a.m. UTC | #1
On 1/14/20 11:51 AM, Bard liao wrote:
> Multi cpu is not supported by all functions yet. Add a warning message
> to warn it.

Shouldn't we be consistent and return an error when we have an 
unexpected number of cpu dais? see below

> 
> Suggested-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> Signed-off-by: Bard liao <yung-chuan.liao@linux.intel.com>
> ---
>   sound/soc/soc-compress.c              |  7 ++++---
>   sound/soc/soc-generic-dmaengine-pcm.c | 12 ++++++++++++
>   sound/soc/soc-pcm.c                   | 12 ++++++++++++
>   3 files changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c
> index 223cd045719e..bf9d6a21bcd9 100644
> --- a/sound/soc/soc-compress.c
> +++ b/sound/soc/soc-compress.c
> @@ -810,10 +810,11 @@ int snd_soc_new_compress(struct snd_soc_pcm_runtime *rtd, int num)
>   	int playback = 0, capture = 0;
>   	int i;
>   
> -	if (rtd->num_codecs > 1) {
> +	if (rtd->num_cpus > 1 ||
> +	    rtd->num_codecs) {
>   		dev_err(rtd->card->dev,
> -			"Compress ASoC: Multicodec not supported\n");
> -		return -EINVAL;
> +			"Compress ASoC: Multi CPU/Codec not supported\n");
> +		return -ENOTSUPP;

so this is an error...

>   	}
>   
>   	/* check client and interface hw capabilities */
> diff --git a/sound/soc/soc-generic-dmaengine-pcm.c b/sound/soc/soc-generic-dmaengine-pcm.c
> index df57ec47ad60..a6c8623d03be 100644
> --- a/sound/soc/soc-generic-dmaengine-pcm.c
> +++ b/sound/soc/soc-generic-dmaengine-pcm.c
> @@ -62,6 +62,10 @@ int snd_dmaengine_pcm_prepare_slave_config(struct snd_pcm_substream *substream,
>   	struct snd_dmaengine_dai_dma_data *dma_data;
>   	int ret;
>   
> +	if (rtd->num_cpus > 1)
> +		dev_warn(rtd->dev,
> +			 "%s doesn't support Multi CPU yet\n", __func__);
> +

but here a warning...

>   	dma_data = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream);
>   
>   	ret = snd_hwparams_to_dma_slave_config(substream, params, slave_config);
> @@ -119,6 +123,10 @@ dmaengine_pcm_set_runtime_hwparams(struct snd_soc_component *component,
>   	struct snd_pcm_hardware hw;
>   	int ret;
>   
> +	if (rtd->num_cpus > 1)
> +		dev_warn(rtd->dev,
> +			 "%s doesn't support Multi CPU yet\n", __func__);
> +

and a warning here as well

>   	if (pcm->config && pcm->config->pcm_hardware)
>   		return snd_soc_set_runtime_hwparams(substream,
>   				pcm->config->pcm_hardware);
> @@ -183,6 +191,10 @@ static struct dma_chan *dmaengine_pcm_compat_request_channel(
>   	struct snd_dmaengine_dai_dma_data *dma_data;
>   	dma_filter_fn fn = NULL;
>   
> +	if (rtd->num_cpus > 1)
> +		dev_warn(rtd->dev,
> +			 "%s doesn't support Multi CPU yet\n", __func__);
> +

warning again...

>   	dma_data = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream);
>   
>   	if ((pcm->flags & SND_DMAENGINE_PCM_FLAG_HALF_DUPLEX) && pcm->chan[0])
> diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
> index 52c6f3c3ab63..e16af8effffa 100644
> --- a/sound/soc/soc-pcm.c
> +++ b/sound/soc/soc-pcm.c
> @@ -1469,6 +1469,10 @@ int dpcm_path_get(struct snd_soc_pcm_runtime *fe,
>   	struct snd_soc_dai *cpu_dai = fe->cpu_dai;
>   	int paths;
>   
> +	if (fe->num_cpus > 1)
> +		dev_warn(fe->dev,
> +			 "%s doesn't support Multi CPU yet\n", __func__);
> +

and warning again...

>   	/* get number of valid DAI paths and their widgets */
>   	paths = snd_soc_dapm_dai_get_connected_widgets(cpu_dai, stream, list,
>   			dpcm_end_walk_at_be);
> @@ -2812,6 +2816,10 @@ static int soc_dpcm_fe_runtime_update(struct snd_soc_pcm_runtime *fe, int new)
>   	struct snd_soc_dapm_widget_list *list;
>   	int count, paths;
>   
> +	if (fe->num_cpus > 1)
> +		dev_warn(fe->dev,
> +			 "%s doesn't support Multi CPU yet\n", __func__ > +

warning

>   	if (!fe->dai_link->dynamic)
>   		return 0;
>   
> @@ -3368,6 +3376,10 @@ static ssize_t dpcm_state_read_file(struct file *file, char __user *user_buf,
>   	ssize_t out_count = PAGE_SIZE, offset = 0, ret = 0;
>   	char *buf;
>   
> +	if (fe->num_cpus > 1)
> +		dev_warn(fe->dev,
> +			 "%s doesn't support Multi CPU yet\n", __func__);
> +

warning

>   	buf = kmalloc(out_count, GFP_KERNEL);
>   	if (!buf)
>   		return -ENOMEM;
>
Liao, Bard Jan. 16, 2020, 7:42 a.m. UTC | #2
> -----Original Message-----
> From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@linux.intel.com]
> Sent: Thursday, January 16, 2020 9:50 AM
> To: Bard liao <yung-chuan.liao@linux.intel.com>; broonie@kernel.org;
> tiwai@suse.de
> Cc: liam.r.girdwood@linux.intel.com; alsa-devel@alsa-project.org; Liao, Bard
> <bard.liao@intel.com>; kuninori.morimoto.gx@renesas.com
> Subject: Re: [alsa-devel] [PATCH RFC v2 4/4] ASoC: add warning if the function is
> not support multi cpu yet.
> 
> 
> 
> On 1/14/20 11:51 AM, Bard liao wrote:
> > Multi cpu is not supported by all functions yet. Add a warning message
> > to warn it.
> 
> Shouldn't we be consistent and return an error when we have an unexpected
> number of cpu dais? see below
> 
> >
> > Suggested-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > Signed-off-by: Bard liao <yung-chuan.liao@linux.intel.com>
> > ---
> >   sound/soc/soc-compress.c              |  7 ++++---
> >   sound/soc/soc-generic-dmaengine-pcm.c | 12 ++++++++++++
> >   sound/soc/soc-pcm.c                   | 12 ++++++++++++
> >   3 files changed, 28 insertions(+), 3 deletions(-)
> >
> > diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c index
> > 223cd045719e..bf9d6a21bcd9 100644
> > --- a/sound/soc/soc-compress.c
> > +++ b/sound/soc/soc-compress.c
> > @@ -810,10 +810,11 @@ int snd_soc_new_compress(struct
> snd_soc_pcm_runtime *rtd, int num)
> >   	int playback = 0, capture = 0;
> >   	int i;
> >
> > -	if (rtd->num_codecs > 1) {
> > +	if (rtd->num_cpus > 1 ||
> > +	    rtd->num_codecs) {
> >   		dev_err(rtd->card->dev,
> > -			"Compress ASoC: Multicodec not supported\n");
> > -		return -EINVAL;
> > +			"Compress ASoC: Multi CPU/Codec not supported\n");
> > +		return -ENOTSUPP;
> 
> so this is an error...

Changing to -ENOTSUPP will prevent below to return error in
soc_init_pcm_runtime().

/* create compress_device if possible */
ret = snd_soc_dai_compress_new(cpu_dai, rtd, num);
if (ret != -ENOTSUPP) {
	if (ret < 0)
		dev_err(card->dev, "ASoC: can't create compress %s\n",
			dai_link->stream_name);
	return ret;
}

So it is to prevent an error actually. But I agree it is better to return
an error when we have an unexpected number of cpu dais. What do you
think? @Morimoto-san
Kuninori Morimoto Jan. 17, 2020, 12:45 a.m. UTC | #3
Hi Liao

> > > 223cd045719e..bf9d6a21bcd9 100644
> > > --- a/sound/soc/soc-compress.c
> > > +++ b/sound/soc/soc-compress.c
> > > @@ -810,10 +810,11 @@ int snd_soc_new_compress(struct
> > snd_soc_pcm_runtime *rtd, int num)
> > >   	int playback = 0, capture = 0;
> > >   	int i;
> > >
> > > -	if (rtd->num_codecs > 1) {
> > > +	if (rtd->num_cpus > 1 ||
> > > +	    rtd->num_codecs) {
> > >   		dev_err(rtd->card->dev,
> > > -			"Compress ASoC: Multicodec not supported\n");
> > > -		return -EINVAL;
> > > +			"Compress ASoC: Multi CPU/Codec not supported\n");
> > > +		return -ENOTSUPP;
> > 
> > so this is an error...
> 
> Changing to -ENOTSUPP will prevent below to return error in
> soc_init_pcm_runtime().
> 
> /* create compress_device if possible */
> ret = snd_soc_dai_compress_new(cpu_dai, rtd, num);
> if (ret != -ENOTSUPP) {
> 	if (ret < 0)
> 		dev_err(card->dev, "ASoC: can't create compress %s\n",
> 			dai_link->stream_name);
> 	return ret;
> }
> 
> So it is to prevent an error actually. But I agree it is better to return
> an error when we have an unexpected number of cpu dais. What do you
> think? @Morimoto-san

Yes, it should return error (= should not prevent error) I think

Thank you for your help !!
Best regards
---
Kuninori Morimoto

Patch
diff mbox series

diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c
index 223cd045719e..bf9d6a21bcd9 100644
--- a/sound/soc/soc-compress.c
+++ b/sound/soc/soc-compress.c
@@ -810,10 +810,11 @@  int snd_soc_new_compress(struct snd_soc_pcm_runtime *rtd, int num)
 	int playback = 0, capture = 0;
 	int i;
 
-	if (rtd->num_codecs > 1) {
+	if (rtd->num_cpus > 1 ||
+	    rtd->num_codecs) {
 		dev_err(rtd->card->dev,
-			"Compress ASoC: Multicodec not supported\n");
-		return -EINVAL;
+			"Compress ASoC: Multi CPU/Codec not supported\n");
+		return -ENOTSUPP;
 	}
 
 	/* check client and interface hw capabilities */
diff --git a/sound/soc/soc-generic-dmaengine-pcm.c b/sound/soc/soc-generic-dmaengine-pcm.c
index df57ec47ad60..a6c8623d03be 100644
--- a/sound/soc/soc-generic-dmaengine-pcm.c
+++ b/sound/soc/soc-generic-dmaengine-pcm.c
@@ -62,6 +62,10 @@  int snd_dmaengine_pcm_prepare_slave_config(struct snd_pcm_substream *substream,
 	struct snd_dmaengine_dai_dma_data *dma_data;
 	int ret;
 
+	if (rtd->num_cpus > 1)
+		dev_warn(rtd->dev,
+			 "%s doesn't support Multi CPU yet\n", __func__);
+
 	dma_data = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream);
 
 	ret = snd_hwparams_to_dma_slave_config(substream, params, slave_config);
@@ -119,6 +123,10 @@  dmaengine_pcm_set_runtime_hwparams(struct snd_soc_component *component,
 	struct snd_pcm_hardware hw;
 	int ret;
 
+	if (rtd->num_cpus > 1)
+		dev_warn(rtd->dev,
+			 "%s doesn't support Multi CPU yet\n", __func__);
+
 	if (pcm->config && pcm->config->pcm_hardware)
 		return snd_soc_set_runtime_hwparams(substream,
 				pcm->config->pcm_hardware);
@@ -183,6 +191,10 @@  static struct dma_chan *dmaengine_pcm_compat_request_channel(
 	struct snd_dmaengine_dai_dma_data *dma_data;
 	dma_filter_fn fn = NULL;
 
+	if (rtd->num_cpus > 1)
+		dev_warn(rtd->dev,
+			 "%s doesn't support Multi CPU yet\n", __func__);
+
 	dma_data = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream);
 
 	if ((pcm->flags & SND_DMAENGINE_PCM_FLAG_HALF_DUPLEX) && pcm->chan[0])
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 52c6f3c3ab63..e16af8effffa 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -1469,6 +1469,10 @@  int dpcm_path_get(struct snd_soc_pcm_runtime *fe,
 	struct snd_soc_dai *cpu_dai = fe->cpu_dai;
 	int paths;
 
+	if (fe->num_cpus > 1)
+		dev_warn(fe->dev,
+			 "%s doesn't support Multi CPU yet\n", __func__);
+
 	/* get number of valid DAI paths and their widgets */
 	paths = snd_soc_dapm_dai_get_connected_widgets(cpu_dai, stream, list,
 			dpcm_end_walk_at_be);
@@ -2812,6 +2816,10 @@  static int soc_dpcm_fe_runtime_update(struct snd_soc_pcm_runtime *fe, int new)
 	struct snd_soc_dapm_widget_list *list;
 	int count, paths;
 
+	if (fe->num_cpus > 1)
+		dev_warn(fe->dev,
+			 "%s doesn't support Multi CPU yet\n", __func__);
+
 	if (!fe->dai_link->dynamic)
 		return 0;
 
@@ -3368,6 +3376,10 @@  static ssize_t dpcm_state_read_file(struct file *file, char __user *user_buf,
 	ssize_t out_count = PAGE_SIZE, offset = 0, ret = 0;
 	char *buf;
 
+	if (fe->num_cpus > 1)
+		dev_warn(fe->dev,
+			 "%s doesn't support Multi CPU yet\n", __func__);
+
 	buf = kmalloc(out_count, GFP_KERNEL);
 	if (!buf)
 		return -ENOMEM;