Re: [PATCH v1 2/2] ASoC: tegra: Add RT5631 machine driver

From: Dmitry Osipenko <digetx@gmail.com>
To: Ion Agorria <AG0RRIA@yahoo.com>,
	Thierry Reding <thierry.reding@gmail.com>,
	Jonathan Hunter <jonathanh@nvidia.com>,
	Stephen Warren <swarren@nvidia.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>, Takashi Iwai <tiwai@suse.com>,
	Jaroslav Kysela <perex@perex.cz>,
	Rob Herring <robh+dt@kernel.org>,
	Svyatoslav Ryhel <clamor95@gmail.com>,
	Ion Agorria <ion@agorria.com>
Cc: alsa-devel@alsa-project.org, devicetree@vger.kernel.org,
	linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1 2/2] ASoC: tegra: Add RT5631 machine driver
Date: Tue, 2 Feb 2021 18:54:03 +0300	[thread overview]
Message-ID: <5ee67a6b-ebe0-8565-5315-bc823ece32f2@gmail.com> (raw)
In-Reply-To: <20210131184101.651486-3-AG0RRIA@yahoo.com>

31.01.2021 21:41, Ion Agorria пишет:
> +	np_codec = of_parse_phandle(pdev->dev.of_node, "nvidia,audio-codec", 0);
> +	if (!np_codec) {
> +		dev_err(&pdev->dev,
> +			"Property 'nvidia,audio-codec' missing or invalid\n");
> +		return -EINVAL;
> +	}
> +
> +	np_i2s = of_parse_phandle(pdev->dev.of_node, "nvidia,i2s-controller", 0);
> +	if (!np_i2s) {
> +		dev_err(&pdev->dev,
> +			"Property 'nvidia,i2s-controller' missing or invalid\n");
> +		return -EINVAL;
> +	}

We missed that the np_codec and np_i2s should be put when driver is released.

https://elixir.bootlin.com/linux/v5.11-rc6/source/drivers/of/base.c#L1429

We could fix it with a devm helper in v2.

diff --git a/sound/soc/tegra/tegra_rt5631.c b/sound/soc/tegra/tegra_rt5631.c
index 9034f48bcb26..84f23915bd95 100644
--- a/sound/soc/tegra/tegra_rt5631.c
+++ b/sound/soc/tegra/tegra_rt5631.c
@@ -172,6 +172,30 @@ static struct snd_soc_card snd_soc_tegra_rt5631 = {
 	.fully_routed = true,
 };
 
+static void tegra_rt5631_node_release(void *of_node)
+{
+	of_node_put(of_node);
+}
+
+static struct device_node *
+tegra_rt5631_parse_phandle(struct device *dev, const char *name)
+{
+	struct device_node *np;
+	int err;
+
+	np = of_parse_phandle(dev->of_node, name, 0);
+	if (!np) {
+		dev_err(dev, "Property '%s' missing or invalid\n", name);
+		return ERR_PTR(-EINVAL);
+	}
+
+	err = devm_add_action_or_reset(dev, tegra_rt5631_node_release, np);
+	if (err)
+		return ERR_PTR(err);
+
+	return np;
+}
+
 static int tegra_rt5631_probe(struct platform_device *pdev)
 {
 	struct snd_soc_card *card = &snd_soc_tegra_rt5631;
@@ -209,19 +233,13 @@ static int tegra_rt5631_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	np_codec = of_parse_phandle(pdev->dev.of_node, "nvidia,audio-codec", 0);
-	if (!np_codec) {
-		dev_err(&pdev->dev,
-			"Property 'nvidia,audio-codec' missing or invalid\n");
-		return -EINVAL;
-	}
+	np_codec = tegra_rt5631_parse_phandle(&pdev->dev, "nvidia,audio-codec");
+	if (IS_ERR(np_codec))
+		return PTR_ERR(np_codec);
 
-	np_i2s = of_parse_phandle(pdev->dev.of_node, "nvidia,i2s-controller", 0);
-	if (!np_i2s) {
-		dev_err(&pdev->dev,
-			"Property 'nvidia,i2s-controller' missing or invalid\n");
-		return -EINVAL;
-	}
+	np_i2s = tegra_rt5631_parse_phandle(&pdev->dev, "nvidia,i2s-controller");
+	if (!np_i2s)
+		return PTR_ERR(np_i2s);
 
 	tegra_rt5631_dai.cpus->of_node = np_i2s;
 	tegra_rt5631_dai.codecs->of_node = np_codec;
