lkml.org 
[lkml]   [2014]   [Sep]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCHv2 2/2] ASoC: simple-card: Fix bug of wrong decrement DT node's refcount
On Mon, 1 Sep 2014 09:35:27 +0800
Xiubo Li <Li.Xiubo@freescale.com> wrote:

> DAI links's cpu_of_node's and codec_of_node's refcounts shouldn't
> be decremented immediately at the end of the probe() fucntion.
> Because we will still use them before the audio card is removed.

Right, but your patch seems a bit complicated. See below.

>
> Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
> ---
> sound/soc/generic/simple-card.c | 52 ++++++++++++++++++++++-------------------
> 1 file changed, 28 insertions(+), 24 deletions(-)
>
> diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
> index 9e170fe..08c5d7d 100644
> --- a/sound/soc/generic/simple-card.c
> +++ b/sound/soc/generic/simple-card.c
> @@ -307,6 +307,22 @@ dai_link_of_err:
> return ret;
> }
>
> +static inline void
> +asoc_simple_card_unref(const struct snd_soc_dai_link *dai_link,
> + int num_links)
> +{
> + struct device_node *np;
> +
> + while (num_links--) {
> + np = dai_link[num_links].cpu_of_node;
> + if (np)
> + of_node_put(np);
> + np = dai_link[num_links].codec_of_node;
> + if (np)
> + of_node_put(np);
> + }
> +}
> +
> static int asoc_simple_card_parse_of(struct device_node *node,
> struct simple_card_data *priv,
> struct device *dev,
> @@ -355,6 +371,7 @@ static int asoc_simple_card_parse_of(struct device_node *node,
> dai_props + i,
> false);
> if (ret < 0) {
> + asoc_simple_card_unref(dai_link, i + 1);
> of_node_put(np);
> return ret;
> }
> @@ -374,27 +391,6 @@ static int asoc_simple_card_parse_of(struct device_node *node,
> return 0;
> }
>
> -/* update the reference count of the devices nodes at end of probe */
> -static int asoc_simple_card_unref(struct platform_device *pdev)
> -{
> - struct snd_soc_card *card = platform_get_drvdata(pdev);
> - struct snd_soc_dai_link *dai_link;
> - struct device_node *np;
> - int num_links;
> -
> - for (num_links = 0, dai_link = card->dai_link;
> - num_links < card->num_links;
> - num_links++, dai_link++) {
> - np = (struct device_node *) dai_link->cpu_of_node;
> - if (np)
> - of_node_put(np);
> - np = (struct device_node *) dai_link->codec_of_node;
> - if (np)
> - of_node_put(np);
> - }
> - return 0;
> -}
> -
> static int asoc_simple_card_probe(struct platform_device *pdev)
> {
> struct simple_card_data *priv;
> @@ -441,7 +437,7 @@ static int asoc_simple_card_probe(struct platform_device *pdev)
> if (ret < 0) {
> if (ret != -EPROBE_DEFER)
> dev_err(dev, "parse error %d\n", ret);
> - goto err;
> + return ret;
> }
>
> } else {
> @@ -483,11 +479,18 @@ static int asoc_simple_card_probe(struct platform_device *pdev)
>
> ret = devm_snd_soc_register_card(&pdev->dev, &priv->snd_card);
>
> -err:
> - asoc_simple_card_unref(pdev);
> return ret;
> }

The main problem is there: don't unref the nodes is no error.
Why not simply:

ret = devm_snd_soc_register_card(&pdev->dev, &priv->snd_card);
+ if (ret >= 0)
+ return ret; /* success */

err:
asoc_simple_card_unref(pdev);
return ret;
}

? Then, the above stuff is not needed.

> +static int asoc_simple_card_remove(struct platform_device *pdev)
> +{
> + struct snd_soc_card *card = platform_get_drvdata(pdev);
> +
> + asoc_simple_card_unref(card->dai_link, card->num_links);
> +
> + return 0;
> +}
> +
> static const struct of_device_id asoc_simple_of_match[] = {
> { .compatible = "simple-audio-card", },
> {},
> @@ -501,6 +504,7 @@ static struct platform_driver asoc_simple_card = {
> .of_match_table = asoc_simple_of_match,
> },
> .probe = asoc_simple_card_probe,
> + .remove = asoc_simple_card_remove,
> };
>
> module_platform_driver(asoc_simple_card);

OK.

--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2014-09-01 09:21    [W:0.100 / U:0.116 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site