From mboxrd@z Thu Jan 1 00:00:00 1970 From: Shawn Guo Subject: Re: [PATCH V2 03/10] ASoc: mxs: add mxs-sgtl5000 machine driver Date: Fri, 15 Jul 2011 23:11:06 +0800 Message-ID: <20110715151104.GE1840@S2100-06.ap.freescale.net> References: <1310483085-31442-1-git-send-email-b29396@freescale.com> <1310483085-31442-4-git-send-email-b29396@freescale.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <1310483085-31442-4-git-send-email-b29396-KZfg59tc24xl57MIdRCFDg@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: devicetree-discuss-bounces+gldd-devicetree-discuss=m.gmane.org-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org Sender: devicetree-discuss-bounces+gldd-devicetree-discuss=m.gmane.org-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org To: Dong Aisheng Cc: alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw@public.gmane.org, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org, s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org, broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org, u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org, lrg-l0cyMroinI0@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org List-Id: devicetree@vger.kernel.org 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 > Cc: Mark Brown > Cc: Liam Girdwood > Cc: Sascha Hauer > --- > 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 > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#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-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > From mboxrd@z Thu Jan 1 00:00:00 1970 From: shawn.guo@freescale.com (Shawn Guo) Date: Fri, 15 Jul 2011 23:11:06 +0800 Subject: [PATCH V2 03/10] ASoc: mxs: add mxs-sgtl5000 machine driver In-Reply-To: <1310483085-31442-4-git-send-email-b29396@freescale.com> References: <1310483085-31442-1-git-send-email-b29396@freescale.com> <1310483085-31442-4-git-send-email-b29396@freescale.com> Message-ID: <20110715151104.GE1840@S2100-06.ap.freescale.net> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org 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 > Cc: Mark Brown > Cc: Liam Girdwood > Cc: Sascha Hauer > --- > 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 > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#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 at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >