[alsa-devel] [PATCH] ASoC: add ak4554 driver
ak4554 is very simple DA/AD converter which has no setting register. But, playback format is SND_SOC_DAIFMT_RIGHT_J, and, capture format is SND_SOC_DAIFMT_LEFT_J on same bit clock, LR clock. Because of that, snd_soc_dai_driver consists from "playback only" and "capture only" here, and it has software base symmetric_rates check.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/codecs/Kconfig | 3 + sound/soc/codecs/Makefile | 2 + sound/soc/codecs/ak4554.c | 171 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 176 insertions(+) create mode 100644 sound/soc/codecs/ak4554.c
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index 2f45f00..b4ec4ea 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -196,6 +196,9 @@ config SND_SOC_AK4104 config SND_SOC_AK4535 tristate
+config SND_SOC_AK4554 + tristate + config SND_SOC_AK4641 tristate
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile index b9e41c9..e601a47 100644 --- a/sound/soc/codecs/Makefile +++ b/sound/soc/codecs/Makefile @@ -11,6 +11,7 @@ snd-soc-adav80x-objs := adav80x.o snd-soc-ads117x-objs := ads117x.o snd-soc-ak4104-objs := ak4104.o snd-soc-ak4535-objs := ak4535.o +snd-soc-ak4554-objs := ak4554.o snd-soc-ak4641-objs := ak4641.o snd-soc-ak4642-objs := ak4642.o snd-soc-ak4671-objs := ak4671.o @@ -136,6 +137,7 @@ obj-$(CONFIG_SND_SOC_ADAV80X) += snd-soc-adav80x.o obj-$(CONFIG_SND_SOC_ADS117X) += snd-soc-ads117x.o obj-$(CONFIG_SND_SOC_AK4104) += snd-soc-ak4104.o obj-$(CONFIG_SND_SOC_AK4535) += snd-soc-ak4535.o +obj-$(CONFIG_SND_SOC_AK4554) += snd-soc-ak4554.o obj-$(CONFIG_SND_SOC_AK4641) += snd-soc-ak4641.o obj-$(CONFIG_SND_SOC_AK4642) += snd-soc-ak4642.o obj-$(CONFIG_SND_SOC_AK4671) += snd-soc-ak4671.o diff --git a/sound/soc/codecs/ak4554.c b/sound/soc/codecs/ak4554.c new file mode 100644 index 0000000..277f216 --- /dev/null +++ b/sound/soc/codecs/ak4554.c @@ -0,0 +1,171 @@ +/* + * ak4554.c + * + * Copyright (C) 2013 Renesas Solutions Corp. + * Kuninori Morimoto kuninori.morimoto.gx@renesas.com + * + * 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. + */ + +#include <linux/module.h> +#include <sound/soc.h> + +/* + * ak4554 is very simple DA/AD converter which has no setting register. + * But, playback format is SND_SOC_DAIFMT_RIGHT_J, + * and, capture format is SND_SOC_DAIFMT_LEFT_J + * on same bit clock, LR clock. + * Because of that, snd_soc_dai_driver consists from + * "playback only" and "capture only" here. + */ + +struct ak4554_priv { + int rate; + int usrcnt; +}; + +static int ak4554_dai_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) +{ + int dai_is_play = (dai->id == 0); + int fmt_is_play = 0; + + switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) { + case SND_SOC_DAIFMT_CBM_CFM: + case SND_SOC_DAIFMT_CBS_CFM: + case SND_SOC_DAIFMT_CBM_CFS: + dev_err(dai->dev, "can't be clock master\n"); + return -EINVAL; + } + + switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) { + case SND_SOC_DAIFMT_RIGHT_J: + fmt_is_play = 1; + break; + case SND_SOC_DAIFMT_LEFT_J: + fmt_is_play = 0; + break; + } + + if (dai_is_play != fmt_is_play) { + dev_err(dai->dev, "format error\n"); + return -EINVAL; + } + + return 0; +} + +static int ak4554_dai_hw_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params, + struct snd_soc_dai *dai) +{ + struct ak4554_priv *priv = dev_get_drvdata(dai->dev); + + if ((priv->usrcnt > 0) && + (priv->rate != params_rate(params))) { + dev_err(dai->dev, "asymmetric rate\n"); + return -EIO; + } + + priv->usrcnt++; + priv->rate = params_rate(params); + + return 0; +} + +static void ak4554_dai_shutdown(struct snd_pcm_substream *substream, + struct snd_soc_dai *dai) +{ + struct ak4554_priv *priv = dev_get_drvdata(dai->dev); + + priv->usrcnt--; + + if (priv->usrcnt <= 0) + priv->rate = 0; +} + +static const struct snd_soc_dai_ops ak4554_dai_ops = { + .set_fmt = ak4554_dai_set_fmt, + .hw_params = ak4554_dai_hw_params, + .shutdown = ak4554_dai_shutdown, +}; + +struct snd_soc_dai_driver ak4554_dai[] = { + { + .name = "ak4554-hifi-playback", + .playback = { + .stream_name = "Playback", + .channels_min = 2, + .channels_max = 2, + .rates = SNDRV_PCM_RATE_8000_48000, + .formats = SNDRV_PCM_FMTBIT_S16_LE, + }, + .ops = &ak4554_dai_ops, + }, { + .name = "ak4554-hifi-capture", + .capture = { + .stream_name = "Capture", + .channels_min = 2, + .channels_max = 2, + .rates = SNDRV_PCM_RATE_8000_48000, + .formats = SNDRV_PCM_FMTBIT_S16_LE, + }, + .ops = &ak4554_dai_ops, + } +}; +EXPORT_SYMBOL_GPL(ak4554_dai); + +static int ak4554_probe(struct snd_soc_codec *codec) +{ + dev_info(codec->dev, "probed\n"); + return 0; +} + +static int ak4554_remove(struct snd_soc_codec *codec) +{ + dev_info(codec->dev, "removed\n"); + return 0; +} + +static struct snd_soc_codec_driver soc_codec_dev_ak4554 = { + .probe = ak4554_probe, + .remove = ak4554_remove, +}; + +static int ak4554_soc_probe(struct platform_device *pdev) +{ + struct ak4554_priv *priv; + + priv = devm_kzalloc(&pdev->dev, sizeof(struct ak4554_priv), + GFP_KERNEL); + if (!priv) + return -ENOMEM; + + dev_set_drvdata(&pdev->dev, priv); + + return snd_soc_register_codec(&pdev->dev, + &soc_codec_dev_ak4554, + ak4554_dai, ARRAY_SIZE(ak4554_dai)); +} + +static int ak4554_soc_remove(struct platform_device *pdev) +{ + snd_soc_unregister_codec(&pdev->dev); + return 0; +} + +static struct platform_driver ak4554_driver = { + .driver = { + .name = "ak4554-adc-dac", + .owner = THIS_MODULE, + }, + .probe = ak4554_soc_probe, + .remove = ak4554_soc_remove, +}; +module_platform_driver(ak4554_driver); + +MODULE_LICENSE("GPL"); +MODULE_DESCRIPTION("SoC AK4554 driver"); +MODULE_AUTHOR("Kuninori Morimoto kuninori.morimoto.gx@renesas.com");
On 07/01/2013 08:43 AM, Kuninori Morimoto wrote:
+static int ak4554_dai_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params,
struct snd_soc_dai *dai)
+{
- struct ak4554_priv *priv = dev_get_drvdata(dai->dev);
- if ((priv->usrcnt > 0) &&
(priv->rate != params_rate(params))) {
dev_err(dai->dev, "asymmetric rate\n");
return -EIO;
- }
- priv->usrcnt++;
- priv->rate = params_rate(params);
Set the symmetric_rates field of the dai driver to 1 to enforce symmetric rates. No need to implement this by hand.
- return 0;
+}
[...]
+struct snd_soc_dai_driver ak4554_dai[] = {
- {
.name = "ak4554-hifi-playback",
.playback = {
.stream_name = "Playback",
.channels_min = 2,
.channels_max = 2,
.rates = SNDRV_PCM_RATE_8000_48000,
.formats = SNDRV_PCM_FMTBIT_S16_LE,
},
.ops = &ak4554_dai_ops,
- }, {
.name = "ak4554-hifi-capture",
.capture = {
.stream_name = "Capture",
.channels_min = 2,
.channels_max = 2,
.rates = SNDRV_PCM_RATE_8000_48000,
.formats = SNDRV_PCM_FMTBIT_S16_LE,
},
.ops = &ak4554_dai_ops,
- }
+}; +EXPORT_SYMBOL_GPL(ak4554_dai);
I don't think this needs to be exported and the struct should be static
+static int ak4554_probe(struct snd_soc_codec *codec) +{
- dev_info(codec->dev, "probed\n");
- return 0;
+}
+static int ak4554_remove(struct snd_soc_codec *codec) +{
- dev_info(codec->dev, "removed\n");
- return 0;
+}
Remove the two functions above, there is not much point in printing these messages.
On Mon, Jul 01, 2013 at 05:32:29PM +0200, Lars-Peter Clausen wrote:
On 07/01/2013 08:43 AM, Kuninori Morimoto wrote:
- priv->usrcnt++;
- priv->rate = params_rate(params);
Set the symmetric_rates field of the dai driver to 1 to enforce symmetric rates. No need to implement this by hand.
That won't work because there are separate playback and capture DAIs at the minute. But I think that can be avoided, will comment when I review tomorrow.
Hi Lars
Thank you for checking patch
+static int ak4554_dai_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params,
struct snd_soc_dai *dai)
+{
- struct ak4554_priv *priv = dev_get_drvdata(dai->dev);
- if ((priv->usrcnt > 0) &&
(priv->rate != params_rate(params))) {
dev_err(dai->dev, "asymmetric rate\n");
return -EIO;
- }
- priv->usrcnt++;
- priv->rate = params_rate(params);
Set the symmetric_rates field of the dai driver to 1 to enforce symmetric rates. No need to implement this by hand.
Hmm... I guess this "symmetric_rates" works if dai has both .playback / .capture
ak4554_dai - playback - capture
But, this ak4554 can't use above style, because its format is not symmetric. (playback is SND_SOC_DAIFMT_RIGHT_J, capture is SND_SOC_DAIFMT_LEFT_J) So, it is using
ak4554_dai-playback - playback ak4554_dai-capture - capture
I guess it can't use "symmetric_rates" on this. We talked this on http://comments.gmane.org/gmane.linux.alsa.devel/108346
And, I can update my log comment if it was un-understandable.
+struct snd_soc_dai_driver ak4554_dai[] = {
- {
.name = "ak4554-hifi-playback",
.playback = {
.stream_name = "Playback",
.channels_min = 2,
.channels_max = 2,
.rates = SNDRV_PCM_RATE_8000_48000,
.formats = SNDRV_PCM_FMTBIT_S16_LE,
},
.ops = &ak4554_dai_ops,
- }, {
.name = "ak4554-hifi-capture",
.capture = {
.stream_name = "Capture",
.channels_min = 2,
.channels_max = 2,
.rates = SNDRV_PCM_RATE_8000_48000,
.formats = SNDRV_PCM_FMTBIT_S16_LE,
},
.ops = &ak4554_dai_ops,
- }
+}; +EXPORT_SYMBOL_GPL(ak4554_dai);
I don't think this needs to be exported and the struct should be static
Indeed. Thank you, I will remove it in v2
+static int ak4554_probe(struct snd_soc_codec *codec) +{
- dev_info(codec->dev, "probed\n");
- return 0;
+}
+static int ak4554_remove(struct snd_soc_codec *codec) +{
- dev_info(codec->dev, "removed\n");
- return 0;
+}
Remove the two functions above, there is not much point in printing these messages.
OK, it will be removed in v2
Best regards --- Kuninori Morimoto
On Sun, Jun 30, 2013 at 11:43:51PM -0700, Kuninori Morimoto wrote:
ak4554 is very simple DA/AD converter which has no setting register. But, playback format is SND_SOC_DAIFMT_RIGHT_J, and, capture format is SND_SOC_DAIFMT_LEFT_J on same bit clock, LR clock. Because of that, snd_soc_dai_driver consists from "playback only" and "capture only" here, and it has software base symmetric_rates check.
If the device is hard coded to these formats just use a single DAI without a set_dai_fmt() operation, either the machine can create two links to the DAI or things can be set up without any extra clocks so that both formats are met. Otherwise this looks good apart from the things Lars-Peter already noticed.
Hi Mark
Thank you for your reply
ak4554 is very simple DA/AD converter which has no setting register. But, playback format is SND_SOC_DAIFMT_RIGHT_J, and, capture format is SND_SOC_DAIFMT_LEFT_J on same bit clock, LR clock. Because of that, snd_soc_dai_driver consists from "playback only" and "capture only" here, and it has software base symmetric_rates check.
If the device is hard coded to these formats just use a single DAI without a set_dai_fmt() operation, either the machine can create two links to the DAI or things can be set up without any extra clocks so that both formats are met. Otherwise this looks good apart from the things Lars-Peter already noticed.
OK, then, it can remove self .symmetric_rates check. It works with my current machine.
The image is like this, is it OK ?
CPU-DAI1 (plaback only fmt = RIGHT_J) --+-- ak4554 (it doesn't have set_fmt()) | CPU-DAI2 (capture only fmt = LEFT_J) ---+
Best regards --- Kuninori Morimoto
ak4554 is very simple DA/AD converter which has no setting register. Note that it has hard coded asymmetric data format playback : SND_SOC_DAIFMT_RIGHT_J capture : SND_SOC_DAIFMT_LEFT_J This driver has single DAI and doesn't have set_fmt.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- v1 -> v2
- single DAI - used symmetric_rates = 1 - added CPU/Codec DAI image - git log was updtated - it has no .set_dai()
sound/soc/codecs/Kconfig | 3 ++ sound/soc/codecs/Makefile | 2 ++ sound/soc/codecs/ak4554.c | 79 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+) create mode 100644 sound/soc/codecs/ak4554.c
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index 2f45f00..b4ec4ea 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -196,6 +196,9 @@ config SND_SOC_AK4104 config SND_SOC_AK4535 tristate
+config SND_SOC_AK4554 + tristate + config SND_SOC_AK4641 tristate
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile index b9e41c9..e601a47 100644 --- a/sound/soc/codecs/Makefile +++ b/sound/soc/codecs/Makefile @@ -11,6 +11,7 @@ snd-soc-adav80x-objs := adav80x.o snd-soc-ads117x-objs := ads117x.o snd-soc-ak4104-objs := ak4104.o snd-soc-ak4535-objs := ak4535.o +snd-soc-ak4554-objs := ak4554.o snd-soc-ak4641-objs := ak4641.o snd-soc-ak4642-objs := ak4642.o snd-soc-ak4671-objs := ak4671.o @@ -136,6 +137,7 @@ obj-$(CONFIG_SND_SOC_ADAV80X) += snd-soc-adav80x.o obj-$(CONFIG_SND_SOC_ADS117X) += snd-soc-ads117x.o obj-$(CONFIG_SND_SOC_AK4104) += snd-soc-ak4104.o obj-$(CONFIG_SND_SOC_AK4535) += snd-soc-ak4535.o +obj-$(CONFIG_SND_SOC_AK4554) += snd-soc-ak4554.o obj-$(CONFIG_SND_SOC_AK4641) += snd-soc-ak4641.o obj-$(CONFIG_SND_SOC_AK4642) += snd-soc-ak4642.o obj-$(CONFIG_SND_SOC_AK4671) += snd-soc-ak4671.o diff --git a/sound/soc/codecs/ak4554.c b/sound/soc/codecs/ak4554.c new file mode 100644 index 0000000..c1a1733 --- /dev/null +++ b/sound/soc/codecs/ak4554.c @@ -0,0 +1,79 @@ +/* + * ak4554.c + * + * Copyright (C) 2013 Renesas Solutions Corp. + * Kuninori Morimoto kuninori.morimoto.gx@renesas.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include <linux/module.h> +#include <sound/soc.h> + +/* + * ak4554 is very simple DA/AD converter which has no setting register. + * + * CAUTION + * + * ak4554 playback format is SND_SOC_DAIFMT_RIGHT_J, + * and, capture format is SND_SOC_DAIFMT_LEFT_J + * on same bit clock, LR clock. + * But, this driver doesn't have snd_soc_dai_ops :: set_fmt + * + * CPU/Codec DAI image + * + * CPU-DAI1 (plaback only fmt = RIGHT_J) --+-- ak4554 + * | + * CPU-DAI2 (capture only fmt = LEFT_J) ---+ + */ + +static struct snd_soc_dai_driver ak4554_dai = { + .name = "ak4554-hifi", + .playback = { + .stream_name = "Playback", + .channels_min = 2, + .channels_max = 2, + .rates = SNDRV_PCM_RATE_8000_48000, + .formats = SNDRV_PCM_FMTBIT_S16_LE, + }, + .capture = { + .stream_name = "Capture", + .channels_min = 2, + .channels_max = 2, + .rates = SNDRV_PCM_RATE_8000_48000, + .formats = SNDRV_PCM_FMTBIT_S16_LE, + }, + .symmetric_rates = 1, +}; + +static struct snd_soc_codec_driver soc_codec_dev_ak4554 = { +}; + +static int ak4554_soc_probe(struct platform_device *pdev) +{ + return snd_soc_register_codec(&pdev->dev, + &soc_codec_dev_ak4554, + &ak4554_dai, 1); +} + +static int ak4554_soc_remove(struct platform_device *pdev) +{ + snd_soc_unregister_codec(&pdev->dev); + return 0; +} + +static struct platform_driver ak4554_driver = { + .driver = { + .name = "ak4554-adc-dac", + .owner = THIS_MODULE, + }, + .probe = ak4554_soc_probe, + .remove = ak4554_soc_remove, +}; +module_platform_driver(ak4554_driver); + +MODULE_LICENSE("GPL"); +MODULE_DESCRIPTION("SoC AK4554 driver"); +MODULE_AUTHOR("Kuninori Morimoto kuninori.morimoto.gx@renesas.com");
On Wed, Jul 03, 2013 at 09:15:13PM -0700, Kuninori Morimoto wrote:
ak4554 is very simple DA/AD converter which has no setting register. Note that it has hard coded asymmetric data format playback : SND_SOC_DAIFMT_RIGHT_J capture : SND_SOC_DAIFMT_LEFT_J This driver has single DAI and doesn't have set_fmt.
Applied, thanks.
participants (3)
-
Kuninori Morimoto
-
Lars-Peter Clausen
-
Mark Brown