diff mbox series

[6/6] ASoC: audio-graph: remove Platform support

Message ID 87sg3n3ubg.wl-kuninori.morimoto.gx@renesas.com
State Accepted
Commit 63f2f9cceb09f8e5f668e36c1cf764eea468ebed
Headers show
Series ASoC: audio-graph: cleanups | expand

Commit Message

Kuninori Morimoto April 19, 2021, 2:02 a.m. UTC
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Platform was one of mandatory component on ASoC before,
and audio-graph-card was assuming that CPU and Platform were
same driver.

But it is no longer mandatory on ASoC.
Current ASoC will just ignore if Platform and CPU were same
or doplicated component.

Of course ASoC is supporting Platform, but current
audio-graph-card doesn't support detecting it from DT.

This means current audio-graph-card operation for Platform so far
is 100% useless. This patch removes it.
We can respawn it when we need it.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/generic/audio-graph-card.c | 6 ------
 1 file changed, 6 deletions(-)

Comments

Olivier Moysan Aug. 25, 2021, 7:18 a.m. UTC | #1
Hello Kuninori,

I have seen that the STM32MP15 audio sound card is no more functional 
with recent kernels (5.13 or 5.14)
The sound card is registered, but the all devices are issuing an error 
at runtime. These devices are using stm32_sai.c or stm32_i2s.c drivers.

I found that the regression is linked to the commit 
63f2f9cceb09f8e5f668e36c1cf764eea468ebed "ASoC: audio-graph: remove 
Platform support", as reverting this commit fixes the issue.

When the platform component is missing the pcm_construct ops in the pcm 
dmaengine, is never called, resulting in an incomplete initialization of 
the sound card.
I can't figure out what is the right way to handle this change, however.
Do I need to update the CPU drivers to work without a platform component
or does the audio-graph card has to be changed in some way ?
What do you mean "We can respawn it when we need it", in the commit 
message ?

Thanks for your help
regards
Olivier


On 4/19/21 4:02 AM, Kuninori Morimoto wrote:
> 
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> Platform was one of mandatory component on ASoC before,
> and audio-graph-card was assuming that CPU and Platform were
> same driver.
> 
> But it is no longer mandatory on ASoC.
> Current ASoC will just ignore if Platform and CPU were same
> or doplicated component.
> 
> Of course ASoC is supporting Platform, but current
> audio-graph-card doesn't support detecting it from DT.
> 
> This means current audio-graph-card operation for Platform so far
> is 100% useless. This patch removes it.
> We can respawn it when we need it.
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>   sound/soc/generic/audio-graph-card.c | 6 ------
>   1 file changed, 6 deletions(-)
> 
> diff --git a/sound/soc/generic/audio-graph-card.c b/sound/soc/generic/audio-graph-card.c
> index 5594eab9902e..3c4915d1e528 100644
> --- a/sound/soc/generic/audio-graph-card.c
> +++ b/sound/soc/generic/audio-graph-card.c
> @@ -223,7 +223,6 @@ static int graph_dai_link_of_dpcm(struct asoc_simple_priv *priv,
>   	struct asoc_simple_dai *dai;
>   	struct snd_soc_dai_link_component *cpus = asoc_link_to_cpu(dai_link, 0);
>   	struct snd_soc_dai_link_component *codecs = asoc_link_to_codec(dai_link, 0);
> -	struct snd_soc_dai_link_component *platforms = asoc_link_to_platform(dai_link, 0);
>   	int ret;
>   
>   	port	= of_get_parent(ep);
> @@ -275,7 +274,6 @@ static int graph_dai_link_of_dpcm(struct asoc_simple_priv *priv,
>   
>   		/* card->num_links includes Codec */
>   		asoc_simple_canonicalize_cpu(cpus, is_single_links);
> -		asoc_simple_canonicalize_platform(platforms, cpus);
>   	} else {
>   		struct snd_soc_codec_conf *cconf;
>   
> @@ -354,7 +352,6 @@ static int graph_dai_link_of(struct asoc_simple_priv *priv,
>   	struct asoc_simple_dai *codec_dai = simple_props_to_dai_codec(dai_props, 0);
>   	struct snd_soc_dai_link_component *cpus = asoc_link_to_cpu(dai_link, 0);
>   	struct snd_soc_dai_link_component *codecs = asoc_link_to_codec(dai_link, 0);
> -	struct snd_soc_dai_link_component *platforms = asoc_link_to_platform(dai_link, 0);
>   	int ret, single_cpu = 0;
>   
>   	dev_dbg(dev, "link_of (%pOF)\n", cpu_ep);
> @@ -405,7 +402,6 @@ static int graph_dai_link_of(struct asoc_simple_priv *priv,
>   	dai_link->init = asoc_simple_dai_init;
>   
>   	asoc_simple_canonicalize_cpu(cpus, single_cpu);
> -	asoc_simple_canonicalize_platform(platforms, cpus);
>   
>   	return 0;
>   }
> @@ -621,7 +617,6 @@ static int graph_count_noml(struct asoc_simple_priv *priv,
>   
>   	li->num[li->link].cpus		= 1;
>   	li->num[li->link].codecs	= 1;
> -	li->num[li->link].platforms	= 1;
>   
>   	li->link += 1; /* 1xCPU-Codec */
>   
> @@ -644,7 +639,6 @@ static int graph_count_dpcm(struct asoc_simple_priv *priv,
>   
>   	if (li->cpu) {
>   		li->num[li->link].cpus		= 1;
> -		li->num[li->link].platforms	= 1;
>   
>   		li->link++; /* 1xCPU-dummy */
>   	} else {
>
Kuninori Morimoto Aug. 25, 2021, 10:48 p.m. UTC | #2
Hi Olivier

Thank you for conntacting me

> I have seen that the STM32MP15 audio sound card is no more functional
> with recent kernels (5.13 or 5.14)
> The sound card is registered, but the all devices are issuing an error
> at runtime. These devices are using stm32_sai.c or stm32_i2s.c
> drivers.
> 
> I found that the regression is linked to the commit
> 63f2f9cceb09f8e5f668e36c1cf764eea468ebed "ASoC: audio-graph: remove
> Platform support", as reverting this commit fixes the issue.
> 
> When the platform component is missing the pcm_construct ops in the
> pcm dmaengine, is never called, resulting in an incomplete
> initialization of the sound card.
> I can't figure out what is the right way to handle this change, however.
> Do I need to update the CPU drivers to work without a platform component
> or does the audio-graph card has to be changed in some way ?

Ahh, OK, I see.
Indeed the dev which is used for CPU is used at soc-generic-dmaengine as Platform,
without indicating it at DT (= simple-card has "plat" support for platform at DT,
but audio-graph doesn't ).

I think key funciton is asoc_simple_canonicalize_platform().

> What do you mean "We can respawn it when we need it", in the commit
> message ?

This means we can revert this patch if needed, and yes it is needed :)
Could you please respawn the feature ? or I can do it if you want.

Thank you for your help !!

Best regards
---
Kuninori Morimoto
Olivier Moysan Aug. 27, 2021, 3:28 p.m. UTC | #3
Hi Kuninori,

Thanks for your feedback

On 8/26/21 12:48 AM, Kuninori Morimoto wrote:
> 
> Hi Olivier
> 
> Thank you for conntacting me
> 
>> I have seen that the STM32MP15 audio sound card is no more functional
>> with recent kernels (5.13 or 5.14)
>> The sound card is registered, but the all devices are issuing an error
>> at runtime. These devices are using stm32_sai.c or stm32_i2s.c
>> drivers.
>>
>> I found that the regression is linked to the commit
>> 63f2f9cceb09f8e5f668e36c1cf764eea468ebed "ASoC: audio-graph: remove
>> Platform support", as reverting this commit fixes the issue.
>>
>> When the platform component is missing the pcm_construct ops in the
>> pcm dmaengine, is never called, resulting in an incomplete
>> initialization of the sound card.
>> I can't figure out what is the right way to handle this change, however.
>> Do I need to update the CPU drivers to work without a platform component
>> or does the audio-graph card has to be changed in some way ?
> 
> Ahh, OK, I see.
> Indeed the dev which is used for CPU is used at soc-generic-dmaengine as Platform,
> without indicating it at DT (= simple-card has "plat" support for platform at DT,
> but audio-graph doesn't ).
> 

Yes, it seems that there is no way to force CPU to be used as platform 
with audio-graph. so, asoc_simple_canonicalize_platform() is necessary 
to do the job in this case.

> I think key funciton is asoc_simple_canonicalize_platform().
> 
>> What do you mean "We can respawn it when we need it", in the commit
>> message ?
> 
> This means we can revert this patch if needed, and yes it is needed :)
> Could you please respawn the feature ? or I can do it if you want.
> 

I feel more confortable if you revert the commit, as you are the author 
of the patch.
Thanks.

BRs
Olivier

> Thank you for your help !!
> 
> Best regards
> ---
> Kuninori Morimoto
>
diff mbox series

Patch

diff --git a/sound/soc/generic/audio-graph-card.c b/sound/soc/generic/audio-graph-card.c
index 5594eab9902e..3c4915d1e528 100644
--- a/sound/soc/generic/audio-graph-card.c
+++ b/sound/soc/generic/audio-graph-card.c
@@ -223,7 +223,6 @@  static int graph_dai_link_of_dpcm(struct asoc_simple_priv *priv,
 	struct asoc_simple_dai *dai;
 	struct snd_soc_dai_link_component *cpus = asoc_link_to_cpu(dai_link, 0);
 	struct snd_soc_dai_link_component *codecs = asoc_link_to_codec(dai_link, 0);
-	struct snd_soc_dai_link_component *platforms = asoc_link_to_platform(dai_link, 0);
 	int ret;
 
 	port	= of_get_parent(ep);
@@ -275,7 +274,6 @@  static int graph_dai_link_of_dpcm(struct asoc_simple_priv *priv,
 
 		/* card->num_links includes Codec */
 		asoc_simple_canonicalize_cpu(cpus, is_single_links);
-		asoc_simple_canonicalize_platform(platforms, cpus);
 	} else {
 		struct snd_soc_codec_conf *cconf;
 
@@ -354,7 +352,6 @@  static int graph_dai_link_of(struct asoc_simple_priv *priv,
 	struct asoc_simple_dai *codec_dai = simple_props_to_dai_codec(dai_props, 0);
 	struct snd_soc_dai_link_component *cpus = asoc_link_to_cpu(dai_link, 0);
 	struct snd_soc_dai_link_component *codecs = asoc_link_to_codec(dai_link, 0);
-	struct snd_soc_dai_link_component *platforms = asoc_link_to_platform(dai_link, 0);
 	int ret, single_cpu = 0;
 
 	dev_dbg(dev, "link_of (%pOF)\n", cpu_ep);
@@ -405,7 +402,6 @@  static int graph_dai_link_of(struct asoc_simple_priv *priv,
 	dai_link->init = asoc_simple_dai_init;
 
 	asoc_simple_canonicalize_cpu(cpus, single_cpu);
-	asoc_simple_canonicalize_platform(platforms, cpus);
 
 	return 0;
 }
@@ -621,7 +617,6 @@  static int graph_count_noml(struct asoc_simple_priv *priv,
 
 	li->num[li->link].cpus		= 1;
 	li->num[li->link].codecs	= 1;
-	li->num[li->link].platforms	= 1;
 
 	li->link += 1; /* 1xCPU-Codec */
 
@@ -644,7 +639,6 @@  static int graph_count_dpcm(struct asoc_simple_priv *priv,
 
 	if (li->cpu) {
 		li->num[li->link].cpus		= 1;
-		li->num[li->link].platforms	= 1;
 
 		li->link++; /* 1xCPU-dummy */
 	} else {