Hi Xiubo,
On Fri, Nov 01, 2013 at 03:04:53PM +0800, Xiubo Li wrote:
> This is the SGTL5000 codec based audio driver supported with both
> playback and capture dai link implemention.
>
> This implementation is only compatible with device tree definition.
>
> Signed-off-by: Alison Wang <b18965@xxxxxxxxxxxxx
> Signed-off-by: Xiubo Li <Li.Xiubo@xxxxxxxxxxxxx>
>
> Conflicts:
> sound/soc/fsl/Makefile
> ---
> sound/soc/fsl/Kconfig | 10 ++
> sound/soc/fsl/Makefile | 2 +
> sound/soc/fsl/fsl-sgtl5000-vf610.c | 208 +++++++++++++++++++++++++++++++++++++
I just doubt if this file naming is appropriate. Even if we might not have
rigor rule for the file names, according to existing ones, they are all in
a same pattern: [SoC name]-[codec name].c
"imx-sgtl5000.c" for example
I think it would make user less confused about what this file exactly is if
this machine driver also follow the pattern: vf610-sgtl5000.c
@Shawn
What do you think about the file name?
> 3 files changed, 220 insertions(+)
> create mode 100644 sound/soc/fsl/fsl-sgtl5000-vf610.c
>
> diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
> index 9a8851e..1b835ba 100644
> --- a/sound/soc/fsl/Kconfig
> +++ b/sound/soc/fsl/Kconfig
> @@ -228,4 +228,14 @@ config SND_SOC_FSL_SAI
> tristate
> select SND_SOC_GENERIC_DMAENGINE_PCM
>
> +config SND_SOC_FSL_SGTL5000_VF610
Same problem with the this define.
> + tristate "SoC Audio support for FSL boards with sgtl5000"
And 'FSL' here confuses me a lot. Because those boards based on i.MX series
also could be called FSL boards.
> + depends on OF && I2C
> + select SND_SOC_FSL_SAI
> + select SND_SOC_FSL_PCM
> + select SND_SOC_SGTL5000
> + help
> + Say Y if you want to add support for SoC audio on an FSL board with
> + a sgtl5000 codec.
> +
> endif # SND_FSL_SOC
> diff --git a/sound/soc/fsl/Makefile b/sound/soc/fsl/Makefile
> index e5acc03..26fc551 100644
> --- a/sound/soc/fsl/Makefile
> +++ b/sound/soc/fsl/Makefile
> @@ -59,5 +59,7 @@ obj-$(CONFIG_SND_SOC_IMX_MC13783) += snd-soc-imx-mc13783.o
>
> # FSL ARM SAI/SGT15000 Platform Support
> snd-soc-fsl-sai-objs := fsl-sai.o
> +snd-soc-fsl-sgtl5000-vf610-objs := fsl-sgtl5000-vf610.o
>
> obj-$(CONFIG_SND_SOC_FSL_SAI) += snd-soc-fsl-sai.o
> +obj-$(CONFIG_SND_SOC_FSL_SGTL5000_VF610) += snd-soc-fsl-sgtl5000-vf610.o
> diff --git a/sound/soc/fsl/fsl-sgtl5000-vf610.c b/sound/soc/fsl/fsl-sgtl5000-vf610.c
> new file mode 100644
> index 0000000..f535b42
> --- /dev/null
> +++ b/sound/soc/fsl/fsl-sgtl5000-vf610.c
> @@ -0,0 +1,208 @@
> +/*
> + * Freeacale ALSA SoC Audio using SGT1500 as codec.
> + *
> + * Copyright 2012-2013 Freescale Semiconductor, Inc.
> + *
> + * The code contained herein is licensed under the GNU General Public
> + * License. You may obtain a copy of the GNU General Public License
> + * Version 2 or later at the following locations:
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/i2c.h>
> +#include <linux/clk.h>
> +
> +#include "../codecs/sgtl5000.h"
> +#include "fsl-sai.h"
> +
> +static unsigned int sysclk_rate;
> +
> +static int fsl_sgtl5000_dai_init(struct snd_soc_pcm_runtime *rtd)
Naming issue here again.
At least from my point of view, if you actually merged imx-sgtl5000 with
vf610-sgtl5000 and made it also compatible to other freescale SoCs, you
could then fairly call it fsl_sgtl5000_xxxx.
Well, I might be a little picky here because it's a static function and
won't conflict others. Just the name here doesn't look so explicit to me.
Please reconsider about this whole file's naming.
Best regards,
Nicolin Chen
> +{
> + int ret;
> + struct device *dev = rtd->card->dev;
> +
> + ret = snd_soc_dai_set_sysclk(rtd->codec_dai, SGTL5000_SYSCLK,
> + sysclk_rate, SND_SOC_CLOCK_IN);
> + if (ret) {
> + dev_err(dev, "could not set codec driver clock params :%d\n",
> + ret);
> + return ret;
> + }
> +
> + ret = snd_soc_dai_set_sysclk(rtd->cpu_dai, FSL_SAI_CLK_BUS,
> + sysclk_rate, SND_SOC_CLOCK_OUT);
> + if (ret) {
> + dev_err(dev, "could not set cpu dai driver clock params :%d\n",
> + ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int sgtl5000_params(struct snd_pcm_substream *substream,
> + struct snd_pcm_hw_params *params)
> +{
> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
> + struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
> + unsigned int channels = params_channels(params);
> +
> + /* TODO: The SAI driver should figure this out for us */
> + switch (channels) {
> + case 2:
> + snd_soc_dai_set_tdm_slot(cpu_dai, 0xfffffffc, 0xfffffffc, 2, 0);
> + break;
> + case 1:
> + snd_soc_dai_set_tdm_slot(cpu_dai, 0xfffffffe, 0xfffffffe, 1, 0);
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static struct snd_soc_ops fsl_sgtl5000_hifi_ops = {
> + .hw_params = sgtl5000_params,
> +};
> +
> +static struct snd_soc_dai_link fsl_sgtl5000_dai = {
> + .name = "HiFi",
> + .stream_name = "HiFi",
> + .codec_dai_name = "sgtl5000",
> + .init = &fsl_sgtl5000_dai_init,
> + .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
> + SND_SOC_DAIFMT_CBM_CFM,
> + .ops = &fsl_sgtl5000_hifi_ops,
> +};
> +
> +static const struct snd_soc_dapm_widget fsl_sgtl5000_dapm_widgets[] = {
> + SND_SOC_DAPM_MIC("Mic Jack", NULL),
> + SND_SOC_DAPM_LINE("Line In Jack", NULL),
> + SND_SOC_DAPM_HP("Headphone Jack", NULL),
> + SND_SOC_DAPM_SPK("Line Out Jack", NULL),
> + SND_SOC_DAPM_SPK("Ext Spk", NULL),
> +};
> +
> +static struct snd_soc_card fsl_sgt1500_card = {
> + .owner = THIS_MODULE,
> + .num_links = 1,
> + .dai_link = &fsl_sgtl5000_dai,
> + .dapm_widgets = fsl_sgtl5000_dapm_widgets,
> + .num_dapm_widgets = ARRAY_SIZE(fsl_sgtl5000_dapm_widgets),
> +};
> +
> +static int fsl_sgtl5000_parse_dt(struct platform_device *pdev)
> +{
> + int ret;
> + struct device_node *sai_np, *codec_np;
> + struct clk *codec_clk;
> + struct i2c_client *codec_dev;
> + struct device_node *np = pdev->dev.of_node;
> +
> + ret = snd_soc_of_parse_card_name(&fsl_sgt1500_card, "model");
> + if (ret)
> + return ret;
> +
> + ret = snd_soc_of_parse_audio_routing(&fsl_sgt1500_card,
> + "audio-routing");
> + if (ret)
> + return ret;
> +
> + sai_np = of_parse_phandle(np, "saif-controller", 0);
> + if (!sai_np) {
> + dev_err(&pdev->dev, "\"saif-controller\" phandle missing or "
> + "invalid\n");
> + return -EINVAL;
> + }
> + fsl_sgtl5000_dai.cpu_of_node = sai_np;
> + fsl_sgtl5000_dai.platform_of_node = sai_np;
> +
> + codec_np = of_parse_phandle(np, "audio-codec", 0);
> + if (!codec_np) {
> + dev_err(&pdev->dev, "\"audio-codec\" phandle missing or "
> + "invalid\n");
> + ret = -EINVAL;
> + goto sai_np_fail;
> + }
> + fsl_sgtl5000_dai.codec_of_node = codec_np;
> +
> + codec_dev = of_find_i2c_device_by_node(codec_np);
> + if (!codec_dev) {
> + dev_err(&pdev->dev, "failed to find codec platform device\n");
> + ret = PTR_ERR(codec_dev);
> + goto codec_np_fail;
> + }
> +
> + codec_clk = devm_clk_get(&codec_dev->dev, NULL);
> + if (IS_ERR(codec_clk)) {
> + dev_err(&pdev->dev, "failed to get codec clock\n");
> + ret = PTR_ERR(codec_clk);
> + goto codec_np_fail;
> + }
> +
> + sysclk_rate = clk_get_rate(codec_clk);
> +
> +codec_np_fail:
> + of_node_put(codec_np);
> +sai_np_fail:
> + of_node_put(sai_np);
> +
> + return ret;
> +}
> +
> +static int fsl_sgtl5000_probe(struct platform_device *pdev)
> +{
> + int ret;
> +
> + fsl_sgt1500_card.dev = &pdev->dev;
> +
> + ret = fsl_sgtl5000_parse_dt(pdev);
> + if (ret) {
> + dev_err(&pdev->dev,
> + "parse sgtl5000 device tree failed :%d\n",
> + ret);
> + return ret;
> + }
> +
> + ret = devm_snd_soc_register_card(&pdev->dev, &fsl_sgt1500_card);
> + if (ret) {
> + dev_err(&pdev->dev, "register soc sound card failed :%d\n",
> + ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int fsl_sgtl5000_remove(struct platform_device *pdev)
> +{
> + snd_soc_unregister_card(&fsl_sgt1500_card);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id fsl_sgtl5000_dt_ids[] = {
> + { .compatible = "fsl,vf610-sgtl5000", },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, fsl_sgtl5000_dt_ids);
> +
> +static struct platform_driver fsl_sgtl5000_driver = {
> + .driver = {
> + .name = "fsl-sgtl5000",
> + .owner = THIS_MODULE,
> + .of_match_table = fsl_sgtl5000_dt_ids,
> + },
> + .probe = fsl_sgtl5000_probe,
> + .remove = fsl_sgtl5000_remove,
> +};
> +module_platform_driver(fsl_sgtl5000_driver);
> +
> +MODULE_AUTHOR("Xiubo Li <Li.Xiubo@xxxxxxxxxxxxx>");
> +MODULE_DESCRIPTION("Freescale SGTL5000 ASoC driver");
> +MODULE_LICENSE("GPL");
> --
> 1.8.4
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
