| Message ID | 1310483085-31442-4-git-send-email-b29396@freescale.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jul 12, 2011 at 11:04:38PM +0800, Dong Aisheng wrote: > The driver only supports playback firstly. > For recording, as we have to use two saif instances to implement full > duplex (playback & recording) due to hardware limitation, we need to > figure out a good design to fit in ASoC. > > Signed-off-by: Dong Aisheng <b29396@freescale.com> Which version of the codec do you have? I have: [ 0.370000] sgtl5000 0-000a: sgtl5000 revision 17 [ 0.370000] sgtl5000 0-000a: asoc: failed to probe CODEC sgtl5000.0-000a: -22 [ 0.380000] asoc: failed to instantiate card mxs_sgtl5000: -22 The failure is because CONFIG_REGULATOR is not set and it thus fails in the codec-driver: #else /* CONFIG_REGULATOR */ static int ldo_regulator_register(struct snd_soc_codec *codec, struct regulator_init_data *init_data, int voltage) { return -EINVAL; } (I can patch it to load properly, but well...) Proper regulator support for mxs is not in mainline yet. Is that planned? How do you do it? Regards, Wolfram
On Wed, Jul 13, 2011 at 01:57:11PM +0200, Wolfram Sang wrote: > Proper regulator support for mxs is not in mainline yet. Is that > planned? How do you do it? Regulator support is not a feature of the platform, it's a generic Linux feature.
On Wed, Jul 13, 2011 at 09:46:52PM +0900, Mark Brown wrote: > On Wed, Jul 13, 2011 at 01:57:11PM +0200, Wolfram Sang wrote: > > > Proper regulator support for mxs is not in mainline yet. Is that > > planned? How do you do it? > > Regulator support is not a feature of the platform, it's a generic Linux > feature. Yup, sorry, meant s/mxs/mx28evk/
On Tue, Jul 12, 2011 at 11:04:38PM +0800, Dong Aisheng wrote: > The driver only supports playback firstly. > For recording, as we have to use two saif instances to implement full > duplex (playback & recording) due to hardware limitation, we need to > figure out a good design to fit in ASoC. > > Signed-off-by: Dong Aisheng <b29396@freescale.com> > Cc: Mark Brown <broonie@opensource.wolfsonmicro.com> > Cc: Liam Girdwood <lrg@ti.com> > Cc: Sascha Hauer <s.hauer@pengutronix.de> > --- > sound/soc/mxs/mxs-sgtl5000.c | 170 ++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 170 insertions(+), 0 deletions(-) > > diff --git a/sound/soc/mxs/mxs-sgtl5000.c b/sound/soc/mxs/mxs-sgtl5000.c > new file mode 100644 > index 0000000..be9b5d5 > --- /dev/null > +++ b/sound/soc/mxs/mxs-sgtl5000.c > @@ -0,0 +1,170 @@ > +/* > + * Copyright 2011 Freescale Semiconductor, Inc. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License along > + * with this program; if not, write to the Free Software Foundation, Inc., > + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. > + */ > + > +#include <linux/module.h> > +#include <linux/moduleparam.h> No parameters here, drop it. > +#include <linux/device.h> > +#include <linux/i2c.h> > +#include <linux/fsl_devices.h> > +#include <linux/gpio.h> The last three, too. > +#include <sound/core.h> > +#include <sound/pcm.h> > +#include <sound/soc.h> > +#include <sound/jack.h> > +#include <sound/soc-dapm.h> > +#include <asm/mach-types.h> > + > +#include "../codecs/sgtl5000.h" > +#include "mxs-saif.h" > + > +static int mxs_sgtl5000_hw_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 *codec_dai = rtd->codec_dai; > + struct snd_soc_dai *cpu_dai = rtd->cpu_dai; > + unsigned int rate = params_rate(params); Here you initialize 'rate'... > + u32 dai_format, mclk; > + int ret; > + > + /* sgtl5000 does not support 512*rate when in 96000 fs */ > + rate = params_rate(params); ...so this line can go. Thanks, Wolfram
> -----Original Message----- > From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel- > bounces@alsa-project.org] On Behalf Of Wolfram Sang > Sent: Wednesday, July 13, 2011 11:58 PM > To: Dong Aisheng-B29396 > Cc: alsa-devel@alsa-project.org; s.hauer@pengutronix.de; > broonie@opensource.wolfsonmicro.com; u.kleine-koenig@pengutronix.de; > lrg@ti.com; linux-arm-kernel@lists.infradead.org > Subject: Re: [alsa-devel] [PATCH V2 03/10] ASoc: mxs: add mxs-sgtl5000 > machine driver > > > +#include <linux/module.h> > > +#include <linux/moduleparam.h> > > No parameters here, drop it. > > > +#include <linux/device.h> > > +#include <linux/i2c.h> > > +#include <linux/fsl_devices.h> > > +#include <linux/gpio.h> > > The last three, too. > > > + /* sgtl5000 does not support 512*rate when in 96000 fs */ > > + rate = params_rate(params); > > ...so this line can go. > Updated. Thanks. Regards Dong Aisheng
On Tue, Jul 12, 2011 at 11:04:38PM +0800, Dong Aisheng wrote: > The driver only supports playback firstly. > For recording, as we have to use two saif instances to implement full > duplex (playback & recording) due to hardware limitation, we need to > figure out a good design to fit in ASoC. > > Signed-off-by: Dong Aisheng <b29396@freescale.com> > Cc: Mark Brown <broonie@opensource.wolfsonmicro.com> > Cc: Liam Girdwood <lrg@ti.com> > Cc: Sascha Hauer <s.hauer@pengutronix.de> > --- > sound/soc/mxs/mxs-sgtl5000.c | 170 ++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 170 insertions(+), 0 deletions(-) > > diff --git a/sound/soc/mxs/mxs-sgtl5000.c b/sound/soc/mxs/mxs-sgtl5000.c > new file mode 100644 > index 0000000..be9b5d5 > --- /dev/null > +++ b/sound/soc/mxs/mxs-sgtl5000.c > @@ -0,0 +1,170 @@ > +/* > + * Copyright 2011 Freescale Semiconductor, Inc. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License along > + * with this program; if not, write to the Free Software Foundation, Inc., > + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. > + */ > + > +#include <linux/module.h> > +#include <linux/moduleparam.h> > +#include <linux/device.h> > +#include <linux/i2c.h> > +#include <linux/fsl_devices.h> > +#include <linux/gpio.h> > +#include <sound/core.h> > +#include <sound/pcm.h> > +#include <sound/soc.h> > +#include <sound/jack.h> > +#include <sound/soc-dapm.h> > +#include <asm/mach-types.h> > + > +#include "../codecs/sgtl5000.h" > +#include "mxs-saif.h" > + > +static int mxs_sgtl5000_hw_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 *codec_dai = rtd->codec_dai; > + struct snd_soc_dai *cpu_dai = rtd->cpu_dai; > + unsigned int rate = params_rate(params); > + u32 dai_format, mclk; > + int ret; > + > + /* sgtl5000 does not support 512*rate when in 96000 fs */ > + rate = params_rate(params); > + switch (rate) { > + case 96000: > + mclk = 256 * rate; > + break; > + default: > + mclk = 512 * rate; > + break; > + } > + > + /* Sgtl5000 sysclk should be >= 8MHz and <= 27M */ > + if (mclk < 8000000 || mclk > 27000000) > + return -EINVAL; > + > + /* Set SGTL5000's SYSCLK (provided by SAIF MCLK) */ > + ret = snd_soc_dai_set_sysclk(codec_dai, SGTL5000_SYSCLK, mclk, 0); > + if (ret) > + return -EINVAL; > + > + /* The SAIF MCLK should be the same as SGTL5000_SYSCLK */ > + ret = snd_soc_dai_set_sysclk(cpu_dai, MXS_SAIF_MCLK, mclk, 0); > + if (ret) > + return -EINVAL; > + > + /* set codec to slave mode */ > + dai_format = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | > + SND_SOC_DAIFMT_CBS_CFS; > + > + /* set codec DAI configuration */ > + ret = snd_soc_dai_set_fmt(codec_dai, dai_format); > + if (ret) > + return -EINVAL; > + > + /* set cpu DAI configuration */ > + ret = snd_soc_dai_set_fmt(cpu_dai, dai_format); > + if (ret) > + return -EINVAL; > + > + return 0; > +} > + > +static struct snd_soc_ops mxs_sgtl5000_hifi_ops = { > + .hw_params = mxs_sgtl5000_hw_params, > +}; > + > +static struct snd_soc_dai_link mxs_sgtl5000_dai[] = { > + { > + .name = "HiFi", > + .stream_name = "HiFi Playback", > + .codec_dai_name = "sgtl5000", > + .codec_name = "sgtl5000.0-000a", > + .cpu_dai_name = "mxs-saif.0", > + .platform_name = "mxs-pcm-audio.0", > + .ops = &mxs_sgtl5000_hifi_ops, > + }, > +}; > + It's not related this patch itself, and I'm not against this patch anyhow. I just use it as an example to start a device tree related discussion here. [Cc-ed Grant and devicetree-discuss list] Currently, soc-core requires machine driver register snd_soc_dai_link with proper codec, cpu_dai and platform device name coded, so that it can match/bind dai link. It works for existing non-dt devices probe, because they can have static device id assigned by platform code to map the actual hardware instance. For codec device example above, if the codec sgtl5000 connects to i2c.0, the .codec_name has to be sgtl5000.0-000a, and if i2c.1 is used, it has to be sgtl5000.1-000a. And it can scale a little bit by checking machine_is_xxx() and fix it up before calling snd_soc_register_card(), if a board uses i2c instance different than the default one. But when I recently migrate imx audio driver to be device tree aware, I feel that matching/binding codec using device name in soc-core does not fit there. The devices in device tree probe case either have an unused device id (-1) or a dynamically assigned one which is not necessarily mapped to actually hardware id. This will make device name of codec, cpu_dai, and platform too dynamic for soc-core to match/bind them. Actually, I found matching cpu_dai using name does not scale in imx-ssi case even for non-dt probe. imx provides some flexibility on the connection between codec and ssi. With proper audmux configuration, codec can be connected to either ssi.0 or ssi.1 for given board design. That said even for given board, .cpu_dai_name can be dynamic. This is something the current soc-core matching/binding approach hates to see. I'm not sure if this is something that we can solve in soc-core (with an improved matching/binding approach), or we have to sort it out in every single machine driver when they move to device tree. Mark, Grant, you can tell that's why I start discussing this thing with you guys :) Regards, Shawn > +static struct snd_soc_card mxs_sgtl5000 = { > + .name = "mxs_sgtl5000", > + .dai_link = mxs_sgtl5000_dai, > + .num_links = ARRAY_SIZE(mxs_sgtl5000_dai), > +}; > + > +static int __devinit mxs_sgtl5000_probe(struct platform_device *pdev) > +{ > + struct snd_soc_card *card = &mxs_sgtl5000; > + int ret; > + > + /* > + * Set an init clock(11.28Mhz) for sgtl5000 initialization(i2c r/w). > + * The Sgtl5000 sysclk is derived from saif0 mclk and it's range > + * should be >= 8MHz and <= 27M. > + */ > + ret = mxs_saif_get_mclk(0, 44100 * 256, 44100); > + if (ret) > + return -EINVAL; > + > + card->dev = &pdev->dev; > + platform_set_drvdata(pdev, card); > + > + ret = snd_soc_register_card(card); > + if (ret) { > + dev_err(&pdev->dev, "snd_soc_register_card failed (%d)\n", > + ret); > + return ret; > + } > + > + return 0; > +} > + > +static int __devexit mxs_sgtl5000_remove(struct platform_device *pdev) > +{ > + struct snd_soc_card *card = platform_get_drvdata(pdev); > + > + mxs_saif_put_mclk(0); > + > + snd_soc_unregister_card(card); > + > + return 0; > +} > + > +static struct platform_driver mxs_sgtl5000_audio_driver = { > + .driver = { > + .name = "mxs-sgtl5000", > + .owner = THIS_MODULE, > + }, > + .probe = mxs_sgtl5000_probe, > + .remove = __devexit_p(mxs_sgtl5000_remove), > +}; > + > +static int __init mxs_sgtl5000_init(void) > +{ > + return platform_driver_register(&mxs_sgtl5000_audio_driver); > +} > +module_init(mxs_sgtl5000_init); > + > +static void __exit mxs_sgtl5000_exit(void) > +{ > + platform_driver_unregister(&mxs_sgtl5000_audio_driver); > +} > +module_exit(mxs_sgtl5000_exit); > + > +MODULE_AUTHOR("Freescale Semiconductor, Inc."); > +MODULE_DESCRIPTION("MXS ALSA SoC Machine driver"); > +MODULE_LICENSE("GPL"); > -- > 1.7.0.4 > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
On Fri, Jul 15, 2011 at 11:11:06PM +0800, Shawn Guo wrote: > Currently, soc-core requires machine driver register snd_soc_dai_link > with proper codec, cpu_dai and platform device name coded, so that it > can match/bind dai link. It works for existing non-dt devices probe, > because they can have static device id assigned by platform code to > map the actual hardware instance. There's existing device tree code for PowerPC which seems to figure this out by doing a DT lookup in the machine driver which appears to be sane enough. If we can't look up a struct device via the device tree we'd seem to have a fairly serious problem as struct devices are how we talk about devices at runtime in general. To be honest I've not paid too much attention to the device tree code people are currently using since discussion around it has always involved repeatedly going over all the reasons why we actually need machine drivers and that gets tedious after a while. > Actually, I found matching cpu_dai using name does not scale in imx-ssi > case even for non-dt probe. imx provides some flexibility on the > connection between codec and ssi. With proper audmux configuration, > codec can be connected to either ssi.0 or ssi.1 for given board design. > That said even for given board, .cpu_dai_name can be dynamic. This is > something the current soc-core matching/binding approach hates to see. This isn't a scalability thing and it's not related to binding, it's because the AUXMUX isn't properly represented as a driver. It probably wants to be moved over to use the soc-dsp stuff Liam's doing at the minute. Currently we don't tell the core about the AUDMUX at all so it's unreasonable to expect it to do anything except a static route.
diff --git a/sound/soc/mxs/mxs-sgtl5000.c b/sound/soc/mxs/mxs-sgtl5000.c new file mode 100644 index 0000000..be9b5d5 --- /dev/null +++ b/sound/soc/mxs/mxs-sgtl5000.c @@ -0,0 +1,170 @@ +/* + * Copyright 2011 Freescale Semiconductor, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include <linux/module.h> +#include <linux/moduleparam.h> +#include <linux/device.h> +#include <linux/i2c.h> +#include <linux/fsl_devices.h> +#include <linux/gpio.h> +#include <sound/core.h> +#include <sound/pcm.h> +#include <sound/soc.h> +#include <sound/jack.h> +#include <sound/soc-dapm.h> +#include <asm/mach-types.h> + +#include "../codecs/sgtl5000.h" +#include "mxs-saif.h" + +static int mxs_sgtl5000_hw_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 *codec_dai = rtd->codec_dai; + struct snd_soc_dai *cpu_dai = rtd->cpu_dai; + unsigned int rate = params_rate(params); + u32 dai_format, mclk; + int ret; + + /* sgtl5000 does not support 512*rate when in 96000 fs */ + rate = params_rate(params); + switch (rate) { + case 96000: + mclk = 256 * rate; + break; + default: + mclk = 512 * rate; + break; + } + + /* Sgtl5000 sysclk should be >= 8MHz and <= 27M */ + if (mclk < 8000000 || mclk > 27000000) + return -EINVAL; + + /* Set SGTL5000's SYSCLK (provided by SAIF MCLK) */ + ret = snd_soc_dai_set_sysclk(codec_dai, SGTL5000_SYSCLK, mclk, 0); + if (ret) + return -EINVAL; + + /* The SAIF MCLK should be the same as SGTL5000_SYSCLK */ + ret = snd_soc_dai_set_sysclk(cpu_dai, MXS_SAIF_MCLK, mclk, 0); + if (ret) + return -EINVAL; + + /* set codec to slave mode */ + dai_format = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | + SND_SOC_DAIFMT_CBS_CFS; + + /* set codec DAI configuration */ + ret = snd_soc_dai_set_fmt(codec_dai, dai_format); + if (ret) + return -EINVAL; + + /* set cpu DAI configuration */ + ret = snd_soc_dai_set_fmt(cpu_dai, dai_format); + if (ret) + return -EINVAL; + + return 0; +} + +static struct snd_soc_ops mxs_sgtl5000_hifi_ops = { + .hw_params = mxs_sgtl5000_hw_params, +}; + +static struct snd_soc_dai_link mxs_sgtl5000_dai[] = { + { + .name = "HiFi", + .stream_name = "HiFi Playback", + .codec_dai_name = "sgtl5000", + .codec_name = "sgtl5000.0-000a", + .cpu_dai_name = "mxs-saif.0", + .platform_name = "mxs-pcm-audio.0", + .ops = &mxs_sgtl5000_hifi_ops, + }, +}; + +static struct snd_soc_card mxs_sgtl5000 = { + .name = "mxs_sgtl5000", + .dai_link = mxs_sgtl5000_dai, + .num_links = ARRAY_SIZE(mxs_sgtl5000_dai), +}; + +static int __devinit mxs_sgtl5000_probe(struct platform_device *pdev) +{ + struct snd_soc_card *card = &mxs_sgtl5000; + int ret; + + /* + * Set an init clock(11.28Mhz) for sgtl5000 initialization(i2c r/w). + * The Sgtl5000 sysclk is derived from saif0 mclk and it's range + * should be >= 8MHz and <= 27M. + */ + ret = mxs_saif_get_mclk(0, 44100 * 256, 44100); + if (ret) + return -EINVAL; + + card->dev = &pdev->dev; + platform_set_drvdata(pdev, card); + + ret = snd_soc_register_card(card); + if (ret) { + dev_err(&pdev->dev, "snd_soc_register_card failed (%d)\n", + ret); + return ret; + } + + return 0; +} + +static int __devexit mxs_sgtl5000_remove(struct platform_device *pdev) +{ + struct snd_soc_card *card = platform_get_drvdata(pdev); + + mxs_saif_put_mclk(0); + + snd_soc_unregister_card(card); + + return 0; +} + +static struct platform_driver mxs_sgtl5000_audio_driver = { + .driver = { + .name = "mxs-sgtl5000", + .owner = THIS_MODULE, + }, + .probe = mxs_sgtl5000_probe, + .remove = __devexit_p(mxs_sgtl5000_remove), +}; + +static int __init mxs_sgtl5000_init(void) +{ + return platform_driver_register(&mxs_sgtl5000_audio_driver); +} +module_init(mxs_sgtl5000_init); + +static void __exit mxs_sgtl5000_exit(void) +{ + platform_driver_unregister(&mxs_sgtl5000_audio_driver); +} +module_exit(mxs_sgtl5000_exit); + +MODULE_AUTHOR("Freescale Semiconductor, Inc."); +MODULE_DESCRIPTION("MXS ALSA SoC Machine driver"); +MODULE_LICENSE("GPL");
The driver only supports playback firstly. For recording, as we have to use two saif instances to implement full duplex (playback & recording) due to hardware limitation, we need to figure out a good design to fit in ASoC. Signed-off-by: Dong Aisheng <b29396@freescale.com> Cc: Mark Brown <broonie@opensource.wolfsonmicro.com> Cc: Liam Girdwood <lrg@ti.com> Cc: Sascha Hauer <s.hauer@pengutronix.de> --- sound/soc/mxs/mxs-sgtl5000.c | 170 ++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 170 insertions(+), 0 deletions(-)