diff mbox series

[06/13] ASoC: soc-core: merge soc_free_pcm_runtime() and soc_rtd_free()

Message ID 87tv9klxca.wl-kuninori.morimoto.gx@renesas.com (mailing list archive)
State New, archived
Headers show
Series ASoC: soc-core cleanup - step 3 | expand

Commit Message

Kuninori Morimoto Sept. 10, 2019, 2:05 a.m. UTC 
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

"rtd"      is handled by soc_xxx_pcm_runtime(), and
"rtd->dev" is handled by soc_rtd_xxx().

There is no reason to separate these, and it makes code complex.
We can free these in the same time.

Here soc_rtd_free() (A) which frees rtd->dev is called from
soc_remove_link_dais() many times (1).
Then, it is using dev_registered flags to avoid multi kfree() (2).
This is no longer needed if we can merge these functions.

	static void soc_remove_link_dais(...)
	{
		...
(1)		for_each_comp_order(order) {
(1)			for_each_card_rtds(card, rtd) {

(A)				soc_rtd_free(rtd);
				...
			}
		}
	}

(A)	static void soc_rtd_free(...)
	{
(2)		if (rtd->dev_registered) {
			/* we don't need to call kfree() for rtd->dev */
			device_unregister(rtd->dev);
(2)			rtd->dev_registered = 0;
		}
	}

This patch merges soc_rtd_free() into soc_free_pcm_runtime().

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 include/sound/soc.h  |  1 -
 sound/soc/soc-core.c | 18 ++----------------
 2 files changed, 2 insertions(+), 17 deletions(-)

Comments

Sridharan, Ranjani Sept. 10, 2019, 3:33 p.m. UTC | #1 
On Mon, Sep 9, 2019 at 7:10 PM Kuninori Morimoto <
kuninori.morimoto.gx@renesas.com> wrote:

> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>
> "rtd"      is handled by soc_xxx_pcm_runtime(), and
> "rtd->dev" is handled by soc_rtd_xxx().
>
> There is no reason to separate these, and it makes code complex.
> We can free these in the same time.
>
> Here soc_rtd_free() (A) which frees rtd->dev is called from
> soc_remove_link_dais() many times (1).
> Then, it is using dev_registered flags to avoid multi kfree() (2).
> This is no longer needed if we can merge these functions.
>
>         static void soc_remove_link_dais(...)
>         {
>                 ...
> (1)             for_each_comp_order(order) {
> (1)                     for_each_card_rtds(card, rtd) {
>
> (A)                             soc_rtd_free(rtd);
>                                 ...
>                         }
>                 }
>         }
>
> (A)     static void soc_rtd_free(...)
>         {
> (2)             if (rtd->dev_registered) {
>                         /* we don't need to call kfree() for rtd->dev */
>                         device_unregister(rtd->dev);
> (2)                     rtd->dev_registered = 0;
>                 }
>         }
>
> This patch merges soc_rtd_free() into soc_free_pcm_runtime().
>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>  include/sound/soc.h  |  1 -
>  sound/soc/soc-core.c | 18 ++----------------
>  2 files changed, 2 insertions(+), 17 deletions(-)
>
> diff --git a/include/sound/soc.h b/include/sound/soc.h
> index f264c65..d93cb46 100644
> --- a/include/sound/soc.h
> +++ b/include/sound/soc.h
> @@ -1148,7 +1148,6 @@ struct snd_soc_pcm_runtime {
>         struct list_head component_list; /* list of connected components */
>
>         /* bit field */
> -       unsigned int dev_registered:1;
>         unsigned int pop_wait:1;
>         unsigned int fe_compr:1; /* for Dynamic PCM */
>  };
> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> index b550fa9..552f460 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -368,6 +368,8 @@ static void soc_free_pcm_runtime(struct
> snd_soc_pcm_runtime *rtd)
>
>         kfree(rtd->codec_dais);
>         snd_soc_rtdcom_del_all(rtd);
> +       if (rtd->dev)
> +               device_unregister(rtd->dev); /* soc_release_rtd_dev */
>         list_del(&rtd->list);
>         kfree(rtd);
>  }
> @@ -434,8 +436,6 @@ static struct snd_soc_pcm_runtime *soc_new_pcm_runtime(
>         rtd->num = card->num_rtd;
>         card->num_rtd++;
>
> -       rtd->dev_registered = 1;
> -
>         return rtd;
>
>  free_rtd:
> @@ -1169,7 +1169,6 @@ static int soc_probe_dai(struct snd_soc_dai *dai,
> int order)
>         return 0;
>  }
>
> -static void soc_rtd_free(struct snd_soc_pcm_runtime *rtd); /* remove me */
>  static void soc_remove_link_dais(struct snd_soc_card *card)
>  {
>         int i;
> @@ -1179,10 +1178,6 @@ static void soc_remove_link_dais(struct
> snd_soc_card *card)
>
>         for_each_comp_order(order) {
>                 for_each_card_rtds(card, rtd) {
> -
> -                       /* finalize rtd device */
> -                       soc_rtd_free(rtd);
> -
>                         /* remove the CODEC DAI */
>                         for_each_rtd_codec_dai(rtd, i, codec_dai)
>                                 soc_remove_dai(codec_dai, order);
> @@ -1460,15 +1455,6 @@ void snd_soc_remove_dai_link(struct snd_soc_card
> *card,
>  }
>  EXPORT_SYMBOL_GPL(snd_soc_remove_dai_link);
>
> -static void soc_rtd_free(struct snd_soc_pcm_runtime *rtd)
> -{
> -       if (rtd->dev_registered) {
> -               /* we don't need to call kfree() for rtd->dev */
>
Morimoto-san,

I think it is useful to keep this comment when you move soc_rtd_free() to
soc_free_pcm_runtime().

Thanks,
Ranjani

> -               device_unregister(rtd->dev);
> -               rtd->dev_registered = 0;
> -       }
> -}
> -
>  static int soc_link_dai_pcm_new(struct snd_soc_dai **dais, int num_dais,
>                                 struct snd_soc_pcm_runtime *rtd)
>  {
> --
> 2.7.4
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>
Kuninori Morimoto Sept. 10, 2019, 11:57 p.m. UTC | #2 
Hi Sridharan

Thank you for your review.


>     -static void soc_rtd_free(struct snd_soc_pcm_runtime *rtd)
>     -{
>     -       if (rtd->dev_registered) {
>     -               /* we don't need to call kfree() for rtd->dev */
> 
> Morimoto-san,
> 
> I think it is useful to keep this comment when you move soc_rtd_free() to  soc_free_pcm_runtime().

Yeah, indeed.
In my mind, the comment /* soc_release_rtd_dev */ was almost for it.
But, yes, Let's keep above comment too.
will fix in v2

>     +       if (rtd->dev)
>     +               device_unregister(rtd->dev); /* soc_release_rtd_dev */
diff mbox series

Patch

diff --git a/include/sound/soc.h b/include/sound/soc.h
index f264c65..d93cb46 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -1148,7 +1148,6 @@  struct snd_soc_pcm_runtime {
 	struct list_head component_list; /* list of connected components */
 
 	/* bit field */
-	unsigned int dev_registered:1;
 	unsigned int pop_wait:1;
 	unsigned int fe_compr:1; /* for Dynamic PCM */
 };
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index b550fa9..552f460 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -368,6 +368,8 @@  static void soc_free_pcm_runtime(struct snd_soc_pcm_runtime *rtd)
 
 	kfree(rtd->codec_dais);
 	snd_soc_rtdcom_del_all(rtd);
+	if (rtd->dev)
+		device_unregister(rtd->dev); /* soc_release_rtd_dev */
 	list_del(&rtd->list);
 	kfree(rtd);
 }
@@ -434,8 +436,6 @@  static struct snd_soc_pcm_runtime *soc_new_pcm_runtime(
 	rtd->num = card->num_rtd;
 	card->num_rtd++;
 
-	rtd->dev_registered = 1;
-
 	return rtd;
 
 free_rtd:
@@ -1169,7 +1169,6 @@  static int soc_probe_dai(struct snd_soc_dai *dai, int order)
 	return 0;
 }
 
-static void soc_rtd_free(struct snd_soc_pcm_runtime *rtd); /* remove me */
 static void soc_remove_link_dais(struct snd_soc_card *card)
 {
 	int i;
@@ -1179,10 +1178,6 @@  static void soc_remove_link_dais(struct snd_soc_card *card)
 
 	for_each_comp_order(order) {
 		for_each_card_rtds(card, rtd) {
-
-			/* finalize rtd device */
-			soc_rtd_free(rtd);
-
 			/* remove the CODEC DAI */
 			for_each_rtd_codec_dai(rtd, i, codec_dai)
 				soc_remove_dai(codec_dai, order);
@@ -1460,15 +1455,6 @@  void snd_soc_remove_dai_link(struct snd_soc_card *card,
 }
 EXPORT_SYMBOL_GPL(snd_soc_remove_dai_link);
 
-static void soc_rtd_free(struct snd_soc_pcm_runtime *rtd)
-{
-	if (rtd->dev_registered) {
-		/* we don't need to call kfree() for rtd->dev */
-		device_unregister(rtd->dev);
-		rtd->dev_registered = 0;
-	}
-}
-
 static int soc_link_dai_pcm_new(struct snd_soc_dai **dais, int num_dais,
 				struct snd_soc_pcm_runtime *rtd)
 {