diff mbox series

ASoC: intel: use devm_snd_soc_register_dai()

Message ID 87v8dzpsvm.wl-kuninori.morimoto.gx@renesas.com
State New
Headers show
Series ASoC: intel: use devm_snd_soc_register_dai() | expand

Commit Message

Kuninori Morimoto July 31, 2023, 11:23 p.m. UTC
It is still using snd_soc_{un}register_dai() manually.
Let's use devm_snd_soc_register_dai().

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/intel/avs/pcm.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Amadeusz Sławiński Aug. 1, 2023, 2:14 p.m. UTC | #1
On 8/1/2023 1:23 AM, Kuninori Morimoto wrote:
> It is still using snd_soc_{un}register_dai() manually.
> Let's use devm_snd_soc_register_dai().
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---

There are two other related patches..., should this have been a patch 
series?

And while I like the series, after applying all three patches I get 
failures in module reload tests.

If I remember correctly the reason we did explicit flow of register_ and 
free_, instead of using devm_register_ is that the lifetime may be 
correlated to existence of other device and we need to unregister at 
correct moment. Maybe something to revisit, but I would prefer to keep 
tests green for now.
Mark Brown Aug. 1, 2023, 3:04 p.m. UTC | #2
On Tue, Aug 01, 2023 at 04:14:37PM +0200, Amadeusz Sławiński wrote:
> On 8/1/2023 1:23 AM, Kuninori Morimoto wrote:
> > It is still using snd_soc_{un}register_dai() manually.
> > Let's use devm_snd_soc_register_dai().

> > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

> There are two other related patches..., should this have been a patch
> series?

Given that there's no interrelationship between the patches other than
them doing roughly the same thing there's no need, pulling things into a
series is only needed if the patches depend on each other.  Keeping
things together can make repetitive things clearer, but splitting avoids
anything getting stuck if one driver has an issue for some reason.
Kuninori Morimoto Aug. 1, 2023, 11:56 p.m. UTC | #3
Hi Amadeusz, Mark

> > It is still using snd_soc_{un}register_dai() manually.
> > Let's use devm_snd_soc_register_dai().
> > 
> > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > ---
(snip)
> And while I like the series, after applying all three patches I get 
> failures in module reload tests.
> 
> If I remember correctly the reason we did explicit flow of register_ and 
> free_, instead of using devm_register_ is that the lifetime may be 
> correlated to existence of other device and we need to unregister at 
> correct moment. Maybe something to revisit, but I would prefer to keep 
> tests green for now.

I have sent patch-set to use devm_xxx as much as possible before,
but there are still some drivers which doesn't use it.
It is because of lifetime, like this time.

I don't remember detail and/or which driver was,
and no comment about it on code.
But this 3 patches might be it.

Let's keep all tests green.
Please drop this patch-set > Mark
Sorry for my noise

Thank you for your help !!

Best regards
---
Kuninori Morimoto
diff mbox series

Patch

diff --git a/sound/soc/intel/avs/pcm.c b/sound/soc/intel/avs/pcm.c
index 1fbb2c2fadb5..dd03db31c2d8 100644
--- a/sound/soc/intel/avs/pcm.c
+++ b/sound/soc/intel/avs/pcm.c
@@ -1309,8 +1309,6 @@  static void avs_component_hda_unregister_dais(struct snd_soc_component *componen
 
 		for_each_pcm_streams(stream)
 			snd_soc_dapm_free_widget(snd_soc_dai_get_widget(dai, stream));
-
-		snd_soc_unregister_dai(dai);
 	}
 }
 
@@ -1375,7 +1373,7 @@  static int avs_component_hda_probe(struct snd_soc_component *component)
 			}
 		}
 
-		dai = snd_soc_register_dai(component, &dais[i], false);
+		dai = devm_snd_soc_register_dai(component->dev, component, &dais[i], false);
 		if (!dai) {
 			dev_err(component->dev, "register dai for %s failed\n",
 				pcm->name);