[alsa-devel] [RFC PATCH 0/5] mpc5200 asoc fixups
The mpc5200 asoc code did not run after the multi-codec patches. This series fixes the existing code and adds a generic platform driver that allows the audio layout to be described in the device tree.
Eric Millbrandt (5): ASoC: fsl: mpc5200 multi-codec fixups ASoC: fsl: mpc5200 combine psc_dma platform data ASoC: fsl: mpc5200-soc-audio driver arch/powerpc/boot/dts pcm030 add mpc5200-soc-audio node ASoC: fsl: mpc5200 remove pcm030 and efika audio fabric
.../devicetree/bindings/powerpc/fsl/mpc5200.txt | 17 ++ arch/powerpc/boot/dts/pcm030.dts | 23 ++- arch/powerpc/platforms/52xx/Kconfig | 1 + sound/soc/fsl/Kconfig | 20 +-- sound/soc/fsl/Makefile | 3 +- sound/soc/fsl/efika-audio-fabric.c | 91 -------- sound/soc/fsl/mpc5200_dma.c | 28 +-- sound/soc/fsl/mpc5200_dma.h | 3 + sound/soc/fsl/mpc5200_psc_ac97.c | 13 +- sound/soc/fsl/mpc5200_psc_i2s.c | 8 + sound/soc/fsl/mpc5200_soc_audio.c | 232 ++++++++++++++++++++ sound/soc/fsl/pcm030-audio-fabric.c | 91 -------- 12 files changed, 306 insertions(+), 224 deletions(-) delete mode 100644 sound/soc/fsl/efika-audio-fabric.c create mode 100644 sound/soc/fsl/mpc5200_soc_audio.c delete mode 100644 sound/soc/fsl/pcm030-audio-fabric.c
Add platform DAI information that is needed to successfully register the mpc5200 platform.
Signed-off-by: Eric Millbrandt emillbrandt@dekaresearch.com --- arch/powerpc/platforms/52xx/Kconfig | 1 + sound/soc/fsl/mpc5200_dma.c | 4 ++-- sound/soc/fsl/mpc5200_psc_ac97.c | 8 ++++++-- sound/soc/fsl/mpc5200_psc_i2s.c | 3 +++ 4 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/platforms/52xx/Kconfig b/arch/powerpc/platforms/52xx/Kconfig index 90f4496..fb35944 100644 --- a/arch/powerpc/platforms/52xx/Kconfig +++ b/arch/powerpc/platforms/52xx/Kconfig @@ -1,6 +1,7 @@ config PPC_MPC52xx bool "52xx-based boards" depends on 6xx + select FSL_SOC select PPC_CLOCK select PPC_PCI_CHOICE
diff --git a/sound/soc/fsl/mpc5200_dma.c b/sound/soc/fsl/mpc5200_dma.c index 9a3f7c5..46fb518 100644 --- a/sound/soc/fsl/mpc5200_dma.c +++ b/sound/soc/fsl/mpc5200_dma.c @@ -366,8 +366,8 @@ static void psc_dma_free(struct snd_pcm *pcm)
static struct snd_soc_platform_driver mpc5200_audio_dma_platform = { .ops = &psc_dma_ops, - .pcm_new = &psc_dma_new, - .pcm_free = &psc_dma_free, + .pcm_new = psc_dma_new, + .pcm_free = psc_dma_free, };
static int mpc5200_hpcd_probe(struct platform_device *op) diff --git a/sound/soc/fsl/mpc5200_psc_ac97.c b/sound/soc/fsl/mpc5200_psc_ac97.c index ffa00a2..2f70729 100644 --- a/sound/soc/fsl/mpc5200_psc_ac97.c +++ b/sound/soc/fsl/mpc5200_psc_ac97.c @@ -28,7 +28,7 @@
#define DRV_NAME "mpc5200-psc-ac97"
-/* ALSA only supports a single AC97 device so static is recommend here */ +/* ALSA only supports a single AC97 bus device so static is recommend here */ static struct psc_dma *psc_dma;
static unsigned short psc_ac97_read(struct snd_ac97 *ac97, unsigned short reg) @@ -237,15 +237,18 @@ static const struct snd_soc_dai_ops psc_ac97_digital_ops = {
static struct snd_soc_dai_driver psc_ac97_dai[] = { { + .name = "mpc5200_psc_ac97.0", .ac97_control = 1, .probe = psc_ac97_probe, .playback = { + .stream_name = "AC97 Playback", .channels_min = 1, .channels_max = 6, .rates = SNDRV_PCM_RATE_8000_48000, .formats = SNDRV_PCM_FMTBIT_S32_BE, }, .capture = { + .stream_name = "AC97 Capture", .channels_min = 1, .channels_max = 2, .rates = SNDRV_PCM_RATE_8000_48000, @@ -254,8 +257,10 @@ static struct snd_soc_dai_driver psc_ac97_dai[] = { .ops = &psc_ac97_analog_ops, }, { + .name = "mpc5200_psc_ac97.1", .ac97_control = 1, .playback = { + .stream_name = "AC97 SPDIF", .channels_min = 1, .channels_max = 2, .rates = SNDRV_PCM_RATE_32000 | \ @@ -330,4 +335,3 @@ module_platform_driver(psc_ac97_driver); MODULE_AUTHOR("Jon Smirl jonsmirl@gmail.com"); MODULE_DESCRIPTION("mpc5200 AC97 module"); MODULE_LICENSE("GPL"); - diff --git a/sound/soc/fsl/mpc5200_psc_i2s.c b/sound/soc/fsl/mpc5200_psc_i2s.c index 7b53032..3cf21df 100644 --- a/sound/soc/fsl/mpc5200_psc_i2s.c +++ b/sound/soc/fsl/mpc5200_psc_i2s.c @@ -130,13 +130,16 @@ static const struct snd_soc_dai_ops psc_i2s_dai_ops = { };
static struct snd_soc_dai_driver psc_i2s_dai[] = {{ + .name = "mpc5200_psc_i2s.0", .playback = { + .stream_name = "I2S Playback", .channels_min = 2, .channels_max = 2, .rates = PSC_I2S_RATES, .formats = PSC_I2S_FORMATS, }, .capture = { + .stream_name = "I2S Capture", .channels_min = 2, .channels_max = 2, .rates = PSC_I2S_RATES,
On Tue, Sep 11, 2012 at 10:14:45PM -0400, Eric Millbrandt wrote:
Add platform DAI information that is needed to successfully register the mpc5200 platform.
I'm really not clear what this patch is supposed to be doing. What are the problems you are trying to fix and how does your patch fix them? This should almost certainly be split into a bunch of smaller patches with focused changelogs.
--- a/arch/powerpc/platforms/52xx/Kconfig +++ b/arch/powerpc/platforms/52xx/Kconfig @@ -1,6 +1,7 @@ config PPC_MPC52xx bool "52xx-based boards" depends on 6xx
- select FSL_SOC select PPC_CLOCK select PPC_PCI_CHOICE
This doesnt seem to have much to do with the commit message...
diff --git a/sound/soc/fsl/mpc5200_dma.c b/sound/soc/fsl/mpc5200_dma.c index 9a3f7c5..46fb518 100644 --- a/sound/soc/fsl/mpc5200_dma.c +++ b/sound/soc/fsl/mpc5200_dma.c @@ -366,8 +366,8 @@ static void psc_dma_free(struct snd_pcm *pcm)
static struct snd_soc_platform_driver mpc5200_audio_dma_platform = { .ops = &psc_dma_ops,
- .pcm_new = &psc_dma_new,
- .pcm_free = &psc_dma_free,
- .pcm_new = psc_dma_new,
- .pcm_free = psc_dma_free,
};
Nor does this.
-/* ALSA only supports a single AC97 device so static is recommend here */ +/* ALSA only supports a single AC97 bus device so static is recommend here */ static struct psc_dma *psc_dma;
Or this.
Hi Mark,
On 2012-09-11 Mark Brown wrote:
--- a/arch/powerpc/platforms/52xx/Kconfig +++ b/arch/powerpc/platforms/52xx/Kconfig @@ -1,6 +1,7 @@ config PPC_MPC52xx bool "52xx-based boards" depends on 6xx + select FSL_SOC select PPC_CLOCK select PPC_PCI_CHOICE
This doesnt seem to have much to do with the commit message...
I'll spit this into another patch. SND_POWERPC_SOC, which wraps all of the Powerpc socs, depends on FSL_SOC. Would it be better to have SND_POWERPC_SOC depend on FSL_SOC or PPC_MPC52xx?
-/* ALSA only supports a single AC97 device so static is recommend here */ +/* ALSA only supports a single AC97 bus device so static is recommend +here */ static struct psc_dma *psc_dma;
Or this.
I'll also be split this out. The change was to clarify that now you can have multiple ac97 codec, but soc-core can still only have on ac97 bus.
Eric
This e-mail and the information, including any attachments, it contains are intended to be a confidential communication only to the person or entity to whom it is addressed and may contain information that is privileged. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, distribution or copying of this communication is strictly prohibited. If you have received this communication in error, please immediately notify the sender and destroy the original message.
Thank you.
Please consider the environment before printing this email.
The mpc5200_psc_ac97 and mpc5200_psc_i2s modules rely on shared platform data with mpc5200_dma.
Signed-off-by: Eric Millbrandt emillbrandt@dekaresearch.com --- sound/soc/fsl/mpc5200_dma.c | 24 ++++-------------------- sound/soc/fsl/mpc5200_dma.h | 3 +++ sound/soc/fsl/mpc5200_psc_ac97.c | 5 +++++ sound/soc/fsl/mpc5200_psc_i2s.c | 5 +++++ 4 files changed, 17 insertions(+), 20 deletions(-)
diff --git a/sound/soc/fsl/mpc5200_dma.c b/sound/soc/fsl/mpc5200_dma.c index 46fb518..4fabc85 100644 --- a/sound/soc/fsl/mpc5200_dma.c +++ b/sound/soc/fsl/mpc5200_dma.c @@ -370,7 +370,7 @@ static struct snd_soc_platform_driver mpc5200_audio_dma_platform = { .pcm_free = psc_dma_free, };
-static int mpc5200_hpcd_probe(struct platform_device *op) +int mpc5200_audio_dma_create(struct platform_device *op) { phys_addr_t fifo; struct psc_dma *psc_dma; @@ -487,8 +487,9 @@ out_unmap: iounmap(regs); return ret; } +EXPORT_SYMBOL_GPL(mpc5200_audio_dma_create);
-static int mpc5200_hpcd_remove(struct platform_device *op) +int mpc5200_audio_dma_destroy(struct platform_device *op) { struct psc_dma *psc_dma = dev_get_drvdata(&op->dev);
@@ -510,24 +511,7 @@ static int mpc5200_hpcd_remove(struct platform_device *op)
return 0; } - -static struct of_device_id mpc5200_hpcd_match[] = { - { .compatible = "fsl,mpc5200-pcm", }, - {} -}; -MODULE_DEVICE_TABLE(of, mpc5200_hpcd_match); - -static struct platform_driver mpc5200_hpcd_of_driver = { - .probe = mpc5200_hpcd_probe, - .remove = mpc5200_hpcd_remove, - .driver = { - .owner = THIS_MODULE, - .name = "mpc5200-pcm-audio", - .of_match_table = mpc5200_hpcd_match, - } -}; - -module_platform_driver(mpc5200_hpcd_of_driver); +EXPORT_SYMBOL_GPL(mpc5200_audio_dma_destroy);
MODULE_AUTHOR("Grant Likely grant.likely@secretlab.ca"); MODULE_DESCRIPTION("Freescale MPC5200 PSC in DMA mode ASoC Driver"); diff --git a/sound/soc/fsl/mpc5200_dma.h b/sound/soc/fsl/mpc5200_dma.h index a3c0cd5..dff253f 100644 --- a/sound/soc/fsl/mpc5200_dma.h +++ b/sound/soc/fsl/mpc5200_dma.h @@ -81,4 +81,7 @@ to_psc_dma_stream(struct snd_pcm_substream *substream, struct psc_dma *psc_dma) return &psc_dma->playback; }
+int mpc5200_audio_dma_create(struct platform_device *op); +int mpc5200_audio_dma_destroy(struct platform_device *op); + #endif /* __SOUND_SOC_FSL_MPC5200_DMA_H__ */ diff --git a/sound/soc/fsl/mpc5200_psc_ac97.c b/sound/soc/fsl/mpc5200_psc_ac97.c index 2f70729..17b17b8 100644 --- a/sound/soc/fsl/mpc5200_psc_ac97.c +++ b/sound/soc/fsl/mpc5200_psc_ac97.c @@ -283,6 +283,10 @@ static int __devinit psc_ac97_of_probe(struct platform_device *op) struct snd_ac97 ac97; struct mpc52xx_psc __iomem *regs;
+ rc = mpc5200_audio_dma_create(op); + if (rc != 0) + return rc; + rc = snd_soc_register_dais(&op->dev, psc_ac97_dai, ARRAY_SIZE(psc_ac97_dai)); if (rc != 0) { dev_err(&op->dev, "Failed to register DAI\n"); @@ -308,6 +312,7 @@ static int __devinit psc_ac97_of_probe(struct platform_device *op)
static int __devexit psc_ac97_of_remove(struct platform_device *op) { + mpc5200_audio_dma_destroy(op); snd_soc_unregister_dais(&op->dev, ARRAY_SIZE(psc_ac97_dai)); return 0; } diff --git a/sound/soc/fsl/mpc5200_psc_i2s.c b/sound/soc/fsl/mpc5200_psc_i2s.c index 3cf21df..5710dc8 100644 --- a/sound/soc/fsl/mpc5200_psc_i2s.c +++ b/sound/soc/fsl/mpc5200_psc_i2s.c @@ -159,6 +159,10 @@ static int __devinit psc_i2s_of_probe(struct platform_device *op) struct psc_dma *psc_dma; struct mpc52xx_psc __iomem *regs;
+ rc = mpc5200_audio_dma_create(op); + if (rc != 0) + return rc; + rc = snd_soc_register_dais(&op->dev, psc_i2s_dai, ARRAY_SIZE(psc_i2s_dai)); if (rc != 0) { pr_err("Failed to register DAI\n"); @@ -203,6 +207,7 @@ static int __devinit psc_i2s_of_probe(struct platform_device *op)
static int __devexit psc_i2s_of_remove(struct platform_device *op) { + mpc5200_audio_dma_destroy(op); snd_soc_unregister_dais(&op->dev, ARRAY_SIZE(psc_i2s_dai)); return 0; }
On Tue, Sep 11, 2012 at 10:14:46PM -0400, Eric Millbrandt wrote:
The mpc5200_psc_ac97 and mpc5200_psc_i2s modules rely on shared platform data with mpc5200_dma.
This looks good but depends on patch 1 so I can't apply it - if you can rebase things so this is patch 1 I'll apply it.
Hi Mark,
On 2012-09-12 Mark Brown wrote:
On Tue, Sep 11, 2012 at 10:14:46PM -0400, Eric Millbrandt wrote:
The mpc5200_psc_ac97 and mpc5200_psc_i2s modules rely on shared platform data with mpc5200_dma.
This looks good but depends on patch 1 so I can't apply it - if you can rebase things so this is patch 1 I'll apply it.
After mulling your comments I see now that not being able to define a snd_soc_ops for an ASoC machine driver would only work for very few boards. I'll think on it some more and respin the series to be pcm030 specific.
Eric
-DISCLAIMER: an automatically appended disclaimer may follow. By posting- -to a public e-mail mailing list I hereby grant permission to distribute- -and copy this message.-
This e-mail and the information, including any attachments, it contains are intended to be a confidential communication only to the person or entity to whom it is addressed and may contain information that is privileged. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, distribution or copying of this communication is strictly prohibited. If you have received this communication in error, please immediately notify the sender and destroy the original message.
Thank you.
Please consider the environment before printing this email.
Add a generic mpc5200 driver that allows asoc cards to be defined in the device tree.
Signed-off-by: Eric Millbrandt emillbrandt@dekaresearch.com --- .../devicetree/bindings/powerpc/fsl/mpc5200.txt | 17 ++ sound/soc/fsl/Kconfig | 7 + sound/soc/fsl/Makefile | 1 + sound/soc/fsl/mpc5200_soc_audio.c | 232 ++++++++++++++++++++ 4 files changed, 257 insertions(+), 0 deletions(-) create mode 100644 sound/soc/fsl/mpc5200_soc_audio.c
diff --git a/Documentation/devicetree/bindings/powerpc/fsl/mpc5200.txt b/Documentation/devicetree/bindings/powerpc/fsl/mpc5200.txt index 4ccb2cd..399d159 100644 --- a/Documentation/devicetree/bindings/powerpc/fsl/mpc5200.txt +++ b/Documentation/devicetree/bindings/powerpc/fsl/mpc5200.txt @@ -196,3 +196,20 @@ External interrupts: fsl,mpc5200-mscan nodes ----------------------- See file can.txt in this directory. + +fsl,mpc5200-soc-audio +--------------------- +The mpc5200 soc-audio driver allows the snd_soc_dai_link to be filled +in based on the node properties described below. + +A sound node is defined for each asoc platform. A sound node must +have at least one child DAI node. +- card-name - The card name to register in asoc +- audio-platform - Contains a phandle to a ac97 or i2s node +- number-of-dais - The number of DAIs defined + +Multiple DAI nodes may be attached to a sound node +- stream-name - The asoc name of the platform DAI stream +- codec-name - The name of codec to bind to +- codec-dai-name - The codec DAI to bind to +- cpu-dai-name - The cpu DAI to bind to diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig index d701330..b3eee63 100644 --- a/sound/soc/fsl/Kconfig +++ b/sound/soc/fsl/Kconfig @@ -63,6 +63,13 @@ config SND_SOC_MPC5200_AC97 help Say Y here to support the MPC5200 PSCs in AC97 mode.
+config SND_MPC52xx_SOC_AUDIO + tristate "SoC Generic Audio support for MPC5200" + depends on (SND_SOC_MPC5200_AC97 || SND_SOC_MPC5200_I2S) + help + Say Y if you want to generic device-tree support for sound on the + Freescale MPC5200 + config SND_MPC52xx_SOC_PCM030 tristate "SoC AC97 Audio support for Phytec pcm030 and WM9712" depends on PPC_MPC5200_SIMPLE diff --git a/sound/soc/fsl/Makefile b/sound/soc/fsl/Makefile index 5f3cf3f..d2e2e68 100644 --- a/sound/soc/fsl/Makefile +++ b/sound/soc/fsl/Makefile @@ -20,6 +20,7 @@ obj-$(CONFIG_SND_SOC_MPC5200_I2S) += mpc5200_psc_i2s.o obj-$(CONFIG_SND_SOC_MPC5200_AC97) += mpc5200_psc_ac97.o
# MPC5200 Machine Support +obj-$(CONFIG_SND_MPC52xx_SOC_AUDIO) += mpc5200_soc_audio.o obj-$(CONFIG_SND_MPC52xx_SOC_PCM030) += pcm030-audio-fabric.o obj-$(CONFIG_SND_MPC52xx_SOC_EFIKA) += efika-audio-fabric.o
diff --git a/sound/soc/fsl/mpc5200_soc_audio.c b/sound/soc/fsl/mpc5200_soc_audio.c new file mode 100644 index 0000000..8004563 --- /dev/null +++ b/sound/soc/fsl/mpc5200_soc_audio.c @@ -0,0 +1,232 @@ +/* + * Freescale MPC5200 audio bindings + * + * Copyright 2012 DEKA R&D + * Author: Eric Millbrandt emillbrandt@dekaresearch.com + * + * This file is licensed under the terms of the GNU General Public License + * version 2. This program is licensed "as is" without any warranty of any + * kind, whether express or implied. + */ +#define DEBUG +#include <linux/module.h> +#include <linux/device.h> +#include <linux/of_device.h> +#include <linux/of_platform.h> +#include <linux/slab.h> +#include <sound/soc.h> + +#include "mpc5200_dma.h" + +#define DRV_NAME "mpc5200-soc-audio" +struct mpc5200_soc_audio_data { + struct snd_soc_card *card; + struct platform_device *codec_device[AC97_BUS_MAX_DEVICES]; +}; + +static void of_register_ac97_devices(struct platform_device *op, + struct device_node *np, + struct mpc5200_soc_audio_data *pdata) +{ + struct device_node *nc; + const u32 *addr; + char modalias[PLATFORM_NAME_SIZE]; + char codec[PLATFORM_NAME_SIZE]; + int len; + int rc; + int i = 0; + + if (!np) { + dev_err(&op->dev, "No device node found!!!\n"); + return; + } + + for_each_child_of_node(np, nc) { + + if (nc->full_name) + dev_dbg(&op->dev, "loading %s\n", nc->full_name); + + /* Select codec */ + if (of_modalias_node(nc, modalias, + sizeof(modalias)) < 0) { + dev_err(&op->dev, "cannot find modalias for %s\n", + nc->full_name); + continue; + } + strlcpy(codec, modalias, sizeof(codec)); + strlcat(codec, "-codec", sizeof(codec)); + + /* Device address */ + addr = of_get_property(nc, "reg", &len); + if (!addr || len < sizeof(*addr)) { + dev_err(&op->dev, "%s has no 'reg' property\n", + nc->full_name); + continue; + } + + /* Allocate a platform device for the attached codec */ + pdata->codec_device[i] = platform_device_alloc(codec, *addr); + if (!pdata->codec_device[i]) { + dev_err(&op->dev, "device allocation failed\n"); + continue; + } + + rc = platform_device_add(pdata->codec_device[i]); + if (rc) { + dev_err(&op->dev, "device assignment failed\n"); + continue; + } + + /* Register the new codec */ + dev_dbg(&op->dev, "Registering snd-soc-%s\n", modalias); + rc = request_module("snd-soc-%s", modalias); + if (rc) + dev_err(&op->dev, "request_module returned: %d\n", rc); + + i++; + } +} + +static int __init mpc5200_soc_audio_probe(struct platform_device *op) +{ + struct device_node *np = op->dev.of_node; + struct device_node *nc; + struct device_node *cpu_platform_np; + struct snd_soc_card *card; + struct mpc5200_soc_audio_data *pdata; + int ret; + int i = 0; + + card = kzalloc(sizeof(struct snd_soc_card), GFP_KERNEL); + if (!card) { + ret = -ENOMEM; + goto out; + } + + pdata = kzalloc(sizeof(struct mpc5200_soc_audio_data), GFP_KERNEL); + if (!pdata) { + ret = -ENOMEM; + goto no_mem; + } + + pdata->card = card; + + card->owner = THIS_MODULE; + ret = of_property_read_u32(np, "number-of-dais", &card->num_links); + if (ret || card->num_links < 1) { + dev_err(&op->dev, "number-of-dais not setup\n"); + ret = -EINVAL; + goto no_dais; + } + + card->dai_link = kzalloc(card->num_links * + sizeof(struct snd_soc_dai_link), GFP_KERNEL); + if (!card->dai_link) { + ret = -ENOMEM; + goto no_dais; + } + + cpu_platform_np = of_parse_phandle(np, "audio-platform", 0); + if (!cpu_platform_np) + dev_err(&op->dev, "ac97 not enabled!\n"); + + /* Register attached codecs */ + dev_dbg(&op->dev, "Registering attached codecs\n"); + of_register_ac97_devices(op, cpu_platform_np, pdata); + + card->dev = &op->dev; + + /* Set card name */ + snd_soc_of_parse_card_name(card, "card-name"); + + /* Add devices to dai_link */ + for_each_child_of_node(np, nc) { + card->dai_link[i].name = nc->name; + card->dai_link[i].platform_of_node = cpu_platform_np; + of_property_read_string(nc, "stream-name", + &card->dai_link[i].stream_name); + of_property_read_string(nc, "codec-name", + &card->dai_link[i].codec_name); + of_property_read_string(nc, "codec-dai-name", + &card->dai_link[i].codec_dai_name); + of_property_read_string(nc, "cpu-dai-name", + &card->dai_link[i].cpu_dai_name); + + dev_dbg(&op->dev, "%d: name: %s\n", i, + card->dai_link[i].name); + dev_dbg(&op->dev, "%d: stream-name: %s\n", i, + card->dai_link[i].stream_name); + dev_dbg(&op->dev, "%d: codec-name: %s\n", i, + card->dai_link[i].codec_name); + dev_dbg(&op->dev, "%d: codec-dai-name: %s\n", i, + card->dai_link[i].codec_dai_name); + dev_dbg(&op->dev, "%d: cpu-dai-name: %s\n", i, + card->dai_link[i].cpu_dai_name); + + i++; + } + + ret = snd_soc_register_card(card); + if (ret) { + dev_err(&op->dev, "snd_soc_register_card() failed: %d\n", ret); + goto no_card; + } + + platform_set_drvdata(op, pdata); + + return ret; +no_card: + kfree(card->dai_link); +no_dais: + kfree(pdata); +no_mem: + kfree(card); +out: + return ret; +} + +static int mpc5200_soc_audio_remove(struct platform_device *op) +{ + struct mpc5200_soc_audio_data *pdata = platform_get_drvdata(op); + struct snd_soc_card *card = pdata->card; + int i; + int ret; + + ret = snd_soc_unregister_card(card); + + if (ret == 0) { + kfree(card->dai_link); + kfree(card); + + for (i = 0; i < AC97_BUS_MAX_DEVICES; i++) + if (pdata->codec_device[i]) + platform_device_unregister( + pdata->codec_device[i]); + kfree(pdata); + } + + return ret; +} + +static struct of_device_id mpc5200_audio_match[] = { + { .compatible = "fsl,mpc5200b-soc-audio", }, + { .compatible = "fsl,mpc5200-soc-audio", }, + {} +}; +MODULE_DEVICE_TABLE(of, mpc5200_audio_match); + +static struct platform_driver mpc5200_soc_audio_driver = { + .probe = mpc5200_soc_audio_probe, + .remove = mpc5200_soc_audio_remove, + .driver = { + .name = DRV_NAME, + .owner = THIS_MODULE, + .of_match_table = mpc5200_audio_match, + }, +}; + +module_platform_driver(mpc5200_soc_audio_driver); + +MODULE_AUTHOR("Eric Millbrandt emillbrandt@dekaresearch.com"); +MODULE_DESCRIPTION(DRV_NAME ": mpc5200 audio fabric driver"); +MODULE_LICENSE("GPL");
On 09/11/2012 08:14 PM, Eric Millbrandt wrote:
Add a generic mpc5200 driver that allows asoc cards to be defined in the device tree.
+++ b/Documentation/devicetree/bindings/powerpc/fsl/mpc5200.txt
+A sound node is defined for each asoc platform. A sound node must +have at least one child DAI node. +- card-name - The card name to register in asoc +- audio-platform - Contains a phandle to a ac97 or i2s node +- number-of-dais - The number of DAIs defined
Can't you get that value simply by counting all the child nodes? It seems redundant.
On Tue, Sep 11, 2012 at 10:14:47PM -0400, Eric Millbrandt wrote:
Add a generic mpc5200 driver that allows asoc cards to be defined in the device tree.
ASoC - you've misspelt this throughout.
This changelog should discuss the subset of devices supported by your binding, it's only possible to define bindings like this for a subset of possible cards. Looking at the code it seems like this driver can only work for CODECs which require no runtime configuration (which is a very small subset of CODEC drivers).
--- a/Documentation/devicetree/bindings/powerpc/fsl/mpc5200.txt +++ b/Documentation/devicetree/bindings/powerpc/fsl/mpc5200.txt
Audio bindings generally go under sound.
+Multiple DAI nodes may be attached to a sound node +- stream-name - The asoc name of the platform DAI stream +- codec-name - The name of codec to bind to +- codec-dai-name - The codec DAI to bind to +- cpu-dai-name - The cpu DAI to bind to
What are the allowable values for these strings?
+config SND_MPC52xx_SOC_AUDIO
- tristate "SoC Generic Audio support for MPC5200"
- depends on (SND_SOC_MPC5200_AC97 || SND_SOC_MPC5200_I2S)
It seems wrong that this depends on both AC'97 and I2S - users should be able to use one or the other. Given how AC'97 works we really should be defining it as a bus in the device tree anyway, and ideally trying to enumerate the CODECs at runtime, so it should probably be a separate binding.
+#define DEBUG
Remove this for mainline.
- card = kzalloc(sizeof(struct snd_soc_card), GFP_KERNEL);
devm_kzalloc()
Describe the audio codec on the pcm030 baseboard.
Signed-off-by: Eric Millbrandt emillbrandt@dekaresearch.com --- arch/powerpc/boot/dts/pcm030.dts | 23 ++++++++++++++++++++++- 1 files changed, 22 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/boot/dts/pcm030.dts b/arch/powerpc/boot/dts/pcm030.dts index 9e35499..e9475e9 100644 --- a/arch/powerpc/boot/dts/pcm030.dts +++ b/arch/powerpc/boot/dts/pcm030.dts @@ -59,7 +59,7 @@ #gpio-cells = <2>; };
- psc@2000 { /* PSC1 in ac97 mode */ + audio_platform: psc@2000 { /* PSC1 in ac97 mode */ compatible = "mpc5200b-psc-ac97","fsl,mpc5200b-psc-ac97"; cell-index = <0>; }; @@ -134,4 +134,25 @@ localbus { status = "disabled"; }; + + sound { + compatible = "fsl,mpc5200b-soc-audio","fsl,mpc5200-soc-audio"; + card-name = "pcm030"; + audio-platform = <&audio_platform>; + number-of-dais = <2>; + + analog@0 { + stream-name = "AC97 Analog"; + codec-name = "wm9712-codec.0"; + codec-dai-name = "wm9712-hifi"; + cpu-dai-name = "mpc5200-psc-ac97.0"; + }; + + digital@1 { + stream-name = "AC97 IEC958"; + codec-name = "wm9712-codec.0"; + codec-dai-name = "wm9712-aux"; + cpu-dai-name = "mpc5200-psc-ac97.0"; + }; + }; };
On 09/11/2012 08:14 PM, Eric Millbrandt wrote:
Describe the audio codec on the pcm030 baseboard.
+++ b/arch/powerpc/boot/dts/pcm030.dts
- sound {
compatible = "fsl,mpc5200b-soc-audio","fsl,mpc5200-soc-audio";card-name = "pcm030";audio-platform = <&audio_platform>;number-of-dais = <2>;analog@0 {
Purely from a DT perspective (i.e. I didn't look at the code that parses this), you don't need to include the "@0" and "@1" in the node names, because the two node names "analog" and "digital" are already unique.
However, in general I can see that you might want multiple analog DAIs. There are a couple choices for differentiating the node names in that case:
1) If you want to use the "@0" unit address syntax in the node names to differentiate them, each child node needs a reg property containing the same value, and the parent node needs properties #address-cells=<1>, #size-cells=<0>;
2) Or, since this is within an individual device binding rather than something within a standardized bus, you can simply choose to make the node names unique in some other way, such as "analog0", "analog1", i.e. without the "@"; I believe the "@" syntax would be explicitly reserved for representing a unit address as in (1).
Of course, I could be wrong about this assertion that the "@n" is reserved for the unit address even with the privacy of an individual binding; it'd be best to validate it by posting to the devicetree-discuss mailing list.
stream-name = "AC97 Analog";codec-name = "wm9712-codec.0";codec-dai-name = "wm9712-hifi";cpu-dai-name = "mpc5200-psc-ac97.0";};digital@1 {stream-name = "AC97 IEC958";codec-name = "wm9712-codec.0";codec-dai-name = "wm9712-aux";cpu-dai-name = "mpc5200-psc-ac97.0";};- };
};
On Tue, Sep 11, 2012 at 10:14:48PM -0400, Eric Millbrandt wrote:
analog@0 {stream-name = "AC97 Analog";codec-name = "wm9712-codec.0";
This name is fairly clearly an internal implementation detail of how Linux does audio drivers, we shouldn't be using it in device tree bindings. We should instead be doing something like referencing a device tree node for the CODEC. Look at how drivers like the nVidia Tegra ones handle this, though for AC'97 we should really be able to figure out a standard hookup like this automatically at runtime, it's all enumerable.
MPC5200 ASoC setup can now be done in the device tree.
Signed-off-by: Eric Millbrandt emillbrandt@dekaresearch.com --- sound/soc/fsl/Kconfig | 17 ------- sound/soc/fsl/Makefile | 2 - sound/soc/fsl/efika-audio-fabric.c | 91 ----------------------------------- sound/soc/fsl/pcm030-audio-fabric.c | 91 ----------------------------------- 4 files changed, 0 insertions(+), 201 deletions(-) delete mode 100644 sound/soc/fsl/efika-audio-fabric.c delete mode 100644 sound/soc/fsl/pcm030-audio-fabric.c
diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig index b3eee63..54ba798 100644 --- a/sound/soc/fsl/Kconfig +++ b/sound/soc/fsl/Kconfig @@ -70,23 +70,6 @@ config SND_MPC52xx_SOC_AUDIO Say Y if you want to generic device-tree support for sound on the Freescale MPC5200
-config SND_MPC52xx_SOC_PCM030 - tristate "SoC AC97 Audio support for Phytec pcm030 and WM9712" - depends on PPC_MPC5200_SIMPLE - select SND_SOC_MPC5200_AC97 - select SND_SOC_WM9712 - help - Say Y if you want to add support for sound on the Phytec pcm030 - baseboard. - -config SND_MPC52xx_SOC_EFIKA - tristate "SoC AC97 Audio support for bbplan Efika and STAC9766" - depends on PPC_EFIKA - select SND_SOC_MPC5200_AC97 - select SND_SOC_STAC9766 - help - Say Y if you want to add support for sound on the Efika. - endif # SND_POWERPC_SOC
menuconfig SND_IMX_SOC diff --git a/sound/soc/fsl/Makefile b/sound/soc/fsl/Makefile index d2e2e68..bd5f511 100644 --- a/sound/soc/fsl/Makefile +++ b/sound/soc/fsl/Makefile @@ -21,8 +21,6 @@ obj-$(CONFIG_SND_SOC_MPC5200_AC97) += mpc5200_psc_ac97.o
# MPC5200 Machine Support obj-$(CONFIG_SND_MPC52xx_SOC_AUDIO) += mpc5200_soc_audio.o -obj-$(CONFIG_SND_MPC52xx_SOC_PCM030) += pcm030-audio-fabric.o -obj-$(CONFIG_SND_MPC52xx_SOC_EFIKA) += efika-audio-fabric.o
# i.MX Platform Support snd-soc-imx-ssi-objs := imx-ssi.o diff --git a/sound/soc/fsl/efika-audio-fabric.c b/sound/soc/fsl/efika-audio-fabric.c deleted file mode 100644 index b2acd329..0000000 --- a/sound/soc/fsl/efika-audio-fabric.c +++ /dev/null @@ -1,91 +0,0 @@ -/* - * Efika driver for the PSC of the Freescale MPC52xx - * configured as AC97 interface - * - * Copyright 2008 Jon Smirl, Digispeaker - * Author: Jon Smirl jonsmirl@gmail.com - * - * This file is licensed under the terms of the GNU General Public License - * version 2. This program is licensed "as is" without any warranty of any - * kind, whether express or implied. - */ - -#include <linux/init.h> -#include <linux/module.h> -#include <linux/interrupt.h> -#include <linux/device.h> -#include <linux/delay.h> -#include <linux/of_device.h> -#include <linux/of_platform.h> -#include <linux/dma-mapping.h> - -#include <sound/core.h> -#include <sound/pcm.h> -#include <sound/pcm_params.h> -#include <sound/initval.h> -#include <sound/soc.h> - -#include "mpc5200_dma.h" -#include "mpc5200_psc_ac97.h" -#include "../codecs/stac9766.h" - -#define DRV_NAME "efika-audio-fabric" - -static struct snd_soc_dai_link efika_fabric_dai[] = { -{ - .name = "AC97", - .stream_name = "AC97 Analog", - .codec_dai_name = "stac9766-hifi-analog", - .cpu_dai_name = "mpc5200-psc-ac97.0", - .platform_name = "mpc5200-pcm-audio", - .codec_name = "stac9766-codec", -}, -{ - .name = "AC97", - .stream_name = "AC97 IEC958", - .codec_dai_name = "stac9766-hifi-IEC958", - .cpu_dai_name = "mpc5200-psc-ac97.1", - .platform_name = "mpc5200-pcm-audio", - .codec_name = "stac9766-codec", -}, -}; - -static struct snd_soc_card card = { - .name = "Efika", - .owner = THIS_MODULE, - .dai_link = efika_fabric_dai, - .num_links = ARRAY_SIZE(efika_fabric_dai), -}; - -static __init int efika_fabric_init(void) -{ - struct platform_device *pdev; - int rc; - - if (!of_machine_is_compatible("bplan,efika")) - return -ENODEV; - - pdev = platform_device_alloc("soc-audio", 1); - if (!pdev) { - pr_err("efika_fabric_init: platform_device_alloc() failed\n"); - return -ENODEV; - } - - platform_set_drvdata(pdev, &card); - - rc = platform_device_add(pdev); - if (rc) { - pr_err("efika_fabric_init: platform_device_add() failed\n"); - platform_device_put(pdev); - return -ENODEV; - } - return 0; -} - -module_init(efika_fabric_init); - - -MODULE_AUTHOR("Jon Smirl jonsmirl@gmail.com"); -MODULE_DESCRIPTION(DRV_NAME ": mpc5200 Efika fabric driver"); -MODULE_LICENSE("GPL"); - diff --git a/sound/soc/fsl/pcm030-audio-fabric.c b/sound/soc/fsl/pcm030-audio-fabric.c deleted file mode 100644 index b3af55d..0000000 --- a/sound/soc/fsl/pcm030-audio-fabric.c +++ /dev/null @@ -1,91 +0,0 @@ -/* - * Phytec pcm030 driver for the PSC of the Freescale MPC52xx - * configured as AC97 interface - * - * Copyright 2008 Jon Smirl, Digispeaker - * Author: Jon Smirl jonsmirl@gmail.com - * - * This file is licensed under the terms of the GNU General Public License - * version 2. This program is licensed "as is" without any warranty of any - * kind, whether express or implied. - */ - -#include <linux/init.h> -#include <linux/module.h> -#include <linux/interrupt.h> -#include <linux/device.h> -#include <linux/delay.h> -#include <linux/of_device.h> -#include <linux/of_platform.h> -#include <linux/dma-mapping.h> - -#include <sound/core.h> -#include <sound/pcm.h> -#include <sound/pcm_params.h> -#include <sound/initval.h> -#include <sound/soc.h> - -#include "mpc5200_dma.h" -#include "mpc5200_psc_ac97.h" -#include "../codecs/wm9712.h" - -#define DRV_NAME "pcm030-audio-fabric" - -static struct snd_soc_dai_link pcm030_fabric_dai[] = { -{ - .name = "AC97", - .stream_name = "AC97 Analog", - .codec_dai_name = "wm9712-hifi", - .cpu_dai_name = "mpc5200-psc-ac97.0", - .platform_name = "mpc5200-pcm-audio", - .codec_name = "wm9712-codec", -}, -{ - .name = "AC97", - .stream_name = "AC97 IEC958", - .codec_dai_name = "wm9712-aux", - .cpu_dai_name = "mpc5200-psc-ac97.1", - .platform_name = "mpc5200-pcm-audio", - .codec_name = "wm9712-codec", -}, -}; - -static struct snd_soc_card card = { - .name = "pcm030", - .owner = THIS_MODULE, - .dai_link = pcm030_fabric_dai, - .num_links = ARRAY_SIZE(pcm030_fabric_dai), -}; - -static __init int pcm030_fabric_init(void) -{ - struct platform_device *pdev; - int rc; - - if (!of_machine_is_compatible("phytec,pcm030")) - return -ENODEV; - - pdev = platform_device_alloc("soc-audio", 1); - if (!pdev) { - pr_err("pcm030_fabric_init: platform_device_alloc() failed\n"); - return -ENODEV; - } - - platform_set_drvdata(pdev, &card); - - rc = platform_device_add(pdev); - if (rc) { - pr_err("pcm030_fabric_init: platform_device_add() failed\n"); - platform_device_put(pdev); - return -ENODEV; - } - return 0; -} - -module_init(pcm030_fabric_init); - - -MODULE_AUTHOR("Jon Smirl jonsmirl@gmail.com"); -MODULE_DESCRIPTION(DRV_NAME ": mpc5200 pcm030 fabric driver"); -MODULE_LICENSE("GPL"); -
Hi Mark,
On 2012-09-11 Mark Brown wrote:
On Tue, Sep 11, 2012 at 10:14:49PM -0400, Eric Millbrandt wrote:
MPC5200 ASoC setup can now be done in the device tree.
I only noticed DT bindings being added for pcm030, not for efika?
When I looked I didn't see the Efika (PPC 5200B) DT in-tree. It only appears to exist out-of-tree, http://www.powerdeveloper.org/platforms/efika/devicetree.
Eric
-DISCLAIMER: an automatically appended disclaimer may follow. By posting- -to a public e-mail mailing list I hereby grant permission to distribute- -and copy this message.-
This e-mail and the information, including any attachments, it contains are intended to be a confidential communication only to the person or entity to whom it is addressed and may contain information that is privileged. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, distribution or copying of this communication is strictly prohibited. If you have received this communication in error, please immediately notify the sender and destroy the original message.
Thank you.
Please consider the environment before printing this email.
On Wed, Sep 12, 2012 at 10:05:33AM -0400, Eric Millbrandt wrote:
Please fix your mailer to word wrap within paragraphs.
On 2012-09-11 Mark Brown wrote:
I only noticed DT bindings being added for pcm030, not for efika?
When I looked I didn't see the Efika (PPC 5200B) DT in-tree. It only appears to exist out-of-tree,
Hrm, well - that's not terribly clever. It'll mean a regression on existing systems and since they're clearly taking advantage of the ability to distribute the device tree separately... we'd really want some sort of backwards compatibility to avoid breaking upgrades. Not sure what the normal way of doing that is.
participants (3)
-
Eric Millbrandt -
Mark Brown -
Stephen Warren