[alsa-devel] ASoC: How to write new (atmel) driver?

[Ladislav Michl]

Hi there!

I've got new at91sam9g20 board with max9867 codec (*), so I wanted to get
inspiration from other drivers in sound/soc/atmel. Indeed, there are three
of them:
atmel_wm8904.c
sam9g20_wm8731.c
sam9x5_wm8731.c
Now, looking at sound/soc/tegra, sound/soc/mediatek, etc. most drivers seems
to be created by copy, modify (mostly find&replace), commit. There are so many
of them, that it is probably impossible to fix them all (consider for
example patch bellow to be done several times for different drivers) (**)

So is sound/soc/generic preferred solution these days? Btw, generic drivers
are calling of_node_put before they are done dealing with that node.

And if above is preferred solution what about all those already created
drivers? Fix them all or covert to generic one by introducing DT fixups
for all those over the time created DT properties? At least for atmel
drivers that seems to be doable as atmel_ssc_dai needs to be modified
to get it work under generic framework and keeping old interface would
needlesly complicate things - unless I miss something obvious of course :)

(*) for now I wrote driver using copy&modify, but that does not seem to be
    the right solution anymore
(**) almost two decades ago I rather wrote OSS driver for the very same
     reasons, but now OSS is gone...

diff --git a/sound/soc/atmel/sam9x5_wm8731.c b/sound/soc/atmel/sam9x5_wm8731.c
index 9db08826b32a..11e2c37a2e42 100644
--- a/sound/soc/atmel/sam9x5_wm8731.c
+++ b/sound/soc/atmel/sam9x5_wm8731.c
@@ -78,7 +78,6 @@ static const struct snd_soc_dapm_widget sam9x5_dapm_widgets[] = {
 static int sam9x5_wm8731_driver_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
-	struct device_node *codec_np, *cpu_np;
 	struct snd_soc_card *card;
 	struct snd_soc_dai_link *dai;
 	struct sam9x5_drvdata *priv;
@@ -122,23 +121,20 @@ static int sam9x5_wm8731_driver_probe(struct platform_device *pdev)
 		goto out;
 	}
 
-	codec_np = of_parse_phandle(np, "atmel,audio-codec", 0);
-	if (!codec_np) {
+	dai->codec_of_node = of_parse_phandle(np, "atmel,audio-codec", 0);
+	if (!dai->codec_of_node) {
 		dev_err(&pdev->dev, "atmel,audio-codec node missing\n");
 		ret = -EINVAL;
 		goto out;
 	}
 
-	dai->codec_of_node = codec_np;
-
-	cpu_np = of_parse_phandle(np, "atmel,ssc-controller", 0);
-	if (!cpu_np) {
+	dai->cpu_of_node = of_parse_phandle(np, "atmel,ssc-controller", 0);
+	if (!dai->cpu_of_node) {
 		dev_err(&pdev->dev, "atmel,ssc-controller node missing\n");
 		ret = -EINVAL;
-		goto out;
+		goto out_put_codec;
 	}
-	dai->cpu_of_node = cpu_np;
-	dai->platform_of_node = cpu_np;
+	dai->platform_of_node = dai->cpu_of_node;
 
 	priv->ssc_id = of_alias_get_id(cpu_np, "ssc");
 
@@ -146,12 +142,9 @@ static int sam9x5_wm8731_driver_probe(struct platform_device *pdev)
 	if (ret != 0) {
 		dev_err(&pdev->dev, "Failed to set SSC %d for audio: %d\n",
 			ret, priv->ssc_id);
-		goto out;
+		goto out_put_cpu;
 	}
 
-	of_node_put(codec_np);
-	of_node_put(cpu_np);
-
 	ret = snd_soc_register_card(card);
 	if (ret) {
 		dev_err(&pdev->dev, "Platform device allocation failed\n");
@@ -164,6 +157,10 @@ static int sam9x5_wm8731_driver_probe(struct platform_device *pdev)
 
 out_put_audio:
 	atmel_ssc_put_audio(priv->ssc_id);
+out_put_cpu:
+	of_node_put(dai->cpu_of_node);
+out_put_codec:
+	of_node_put(dai->codec_of_node);
 out:
 	return ret;
 }