| Message ID | 87pnk8lxbu.wl-kuninori.morimoto.gx@renesas.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: soc-core cleanup - step 3 | expand |
On Mon, Sep 9, 2019 at 7:12 PM Kuninori Morimoto < kuninori.morimoto.gx@renesas.com> wrote: > > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > Current rtd, rtd->dev, rtd->codec_dais are created by normal kzalloc(), > but we want to use devm_kzalloc() as much as possible. > > Created rtd->dev is registered by device_register() at > soc_new_pcm_runtime(), and it will be freed at > soc_free_pcm_runtime() by device_unregister(). > > This means, if we can use devm_kzalloc(rtd->dev, xxx) for > rtd / rtd->codec_dais, all these are automatically freed > via soc_free_pcm_runtime(). > This patch uses devm_kzalloc(rtd->dev, xxx) for rtd / rtd->codec_dais. > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > --- > sound/soc/soc-core.c | 9 +++------ > 1 file changed, 3 insertions(+), 6 deletions(-) > > diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c > index 8802287..968cf5c 100644 > --- a/sound/soc/soc-core.c > +++ b/sound/soc/soc-core.c > @@ -370,11 +370,8 @@ static void soc_free_pcm_runtime(struct > snd_soc_pcm_runtime *rtd) > if (!rtd) > return; > > - kfree(rtd->codec_dais); > - if (rtd->dev) > - device_unregister(rtd->dev); /* soc_release_rtd_dev */ > list_del(&rtd->list); > - kfree(rtd); > Morimoto-san, Just curious, why did you remove the check for if(rtd->dev) here before calling device_unregister()? Thanks, Ranjani > + device_unregister(rtd->dev); /* soc_release_rtd_dev */ > } > >
Hi Sridharan Thank you for your review > --- a/sound/soc/soc-core.c > +++ b/sound/soc/soc-core.c > @@ -370,11 +370,8 @@ static void soc_free_pcm_runtime(struct snd_soc_pcm_runtime *rtd) > if (!rtd) > return; > > - kfree(rtd->codec_dais); > - if (rtd->dev) > - device_unregister(rtd->dev); /* soc_release_rtd_dev */ > list_del(&rtd->list); > - kfree(rtd); > > Morimoto-san, > > Just curious, why did you remove the check for if(rtd->dev) here before calling device_unregister()? > > Thanks, > Ranjani > > + device_unregister(rtd->dev); /* soc_release_rtd_dev */ > } Can you check [07/13] patch ? It allocs rtd->dev (as dev) first, and allocs rtd next. This means, if it has rtd, it has rtd->dev. Here, this function checks rtd pointer. If rtd is not NULL, rtd->dev is also not NULL. But, indeed it is a littile bit tricky ? Some comment is needed. Thank you for your help !! Best regards --- Kuninori Morimoto
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 8802287..968cf5c 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -370,11 +370,8 @@ static void soc_free_pcm_runtime(struct snd_soc_pcm_runtime *rtd) if (!rtd) return; - kfree(rtd->codec_dais); - if (rtd->dev) - device_unregister(rtd->dev); /* soc_release_rtd_dev */ list_del(&rtd->list); - kfree(rtd); + device_unregister(rtd->dev); /* soc_release_rtd_dev */ } static struct snd_soc_pcm_runtime *soc_new_pcm_runtime( @@ -406,7 +403,7 @@ static struct snd_soc_pcm_runtime *soc_new_pcm_runtime( /* * for rtd */ - rtd = kzalloc(sizeof(struct snd_soc_pcm_runtime), GFP_KERNEL); + rtd = devm_kzalloc(dev, sizeof(*rtd), GFP_KERNEL); if (!rtd) goto free_rtd; @@ -416,7 +413,7 @@ static struct snd_soc_pcm_runtime *soc_new_pcm_runtime( /* * for rtd->codec_dais */ - rtd->codec_dais = kcalloc(dai_link->num_codecs, + rtd->codec_dais = devm_kcalloc(dev, dai_link->num_codecs, sizeof(struct snd_soc_dai *), GFP_KERNEL); if (!rtd->codec_dais)