| Message ID | 20180114115400.GA5867@lenoch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, Jan 14, 2018 at 12:54:00PM +0100, Ladislav Michl wrote: > 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) (**) Okay, at least for atmel drivers, I'll just sent few cleanups/fixes, not sure about the rest. Perhaps some coccinelle scripting would help. > 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 :) ...and the obvious is drivers/misc/atmel-ssc.c and then simple-audio-card should do the trick. Well, remaining question is how is SND_SOC_MAX9768 expected to be selected? > (*) 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; > } > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
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; }
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...