[alsa-devel] [PATCH] ASoC: dsd1791: Introduce driver for TI DSD1791 stereo codec
This patch introduces a (spi) codec driver for the Texas Instruments DSD1791 24 bit audio stereo DAC.
http://www.ti.com/product/dsd1791
Testing for basic operation using 16 and 24 bit I2S mode has been performed using a MityDSP-L138 SOM and an Industrial I/O board from Critical Link.
Signed-off-by: Michael Williamson michael.williamson@criticallink.com --- This patch incorporates changes from the original RFC as a result of comments received from Mark Brown, Leon Romanovsky, and Lars-Peter Clausen. Thanks.
Summary of Changes: - Use devm_kzalloc() - use regmap cached I/O feature of framework - simplify code applying codec fmt configuration - clean up section attributes on local functions - make local functions/variables static where appropriate - remove set_sysclk method - remove unused tlv header file - remove chatter - strip "-codec" from driver name - fixup driver variable name for consistency - Add Left and Right volume control - Add digital mute
sound/soc/codecs/Kconfig | 4 + sound/soc/codecs/Makefile | 2 + sound/soc/codecs/dsd1791.c | 256 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 262 insertions(+), 0 deletions(-) create mode 100644 sound/soc/codecs/dsd1791.c
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index 4584514..95b7969 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -33,6 +33,7 @@ config SND_SOC_ALL_CODECS select SND_SOC_CX20442 select SND_SOC_DA7210 if I2C select SND_SOC_DFBMCS320 + select SND_SOC_DSD1791 if SPI_MASTER select SND_SOC_JZ4740_CODEC if SOC_JZ4740 select SND_SOC_LM4857 if I2C select SND_SOC_MAX98088 if I2C @@ -205,6 +206,9 @@ config SND_SOC_DA7210 config SND_SOC_DFBMCS320 tristate
+config SND_SOC_DSD1791 + tristate + config SND_SOC_DMIC tristate
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile index a2c7842..d6b5f6a 100644 --- a/sound/soc/codecs/Makefile +++ b/sound/soc/codecs/Makefile @@ -21,6 +21,7 @@ snd-soc-cx20442-objs := cx20442.o snd-soc-da7210-objs := da7210.o snd-soc-dfbmcs320-objs := dfbmcs320.o snd-soc-dmic-objs := dmic.o +snd-soc-dsd1791-objs := dsd1791.o snd-soc-l3-objs := l3.o snd-soc-max98088-objs := max98088.o snd-soc-max98095-objs := max98095.o @@ -120,6 +121,7 @@ obj-$(CONFIG_SND_SOC_CS4271) += snd-soc-cs4271.o obj-$(CONFIG_SND_SOC_CX20442) += snd-soc-cx20442.o obj-$(CONFIG_SND_SOC_DA7210) += snd-soc-da7210.o obj-$(CONFIG_SND_SOC_DFBMCS320) += snd-soc-dfbmcs320.o +obj-$(CONFIG_SND_SOC_DSD1791) += snd-soc-dsd1791.o obj-$(CONFIG_SND_SOC_DMIC) += snd-soc-dmic.o obj-$(CONFIG_SND_SOC_L3) += snd-soc-l3.o obj-$(CONFIG_SND_SOC_JZ4740_CODEC) += snd-soc-jz4740-codec.o diff --git a/sound/soc/codecs/dsd1791.c b/sound/soc/codecs/dsd1791.c new file mode 100644 index 0000000..41bbc61 --- /dev/null +++ b/sound/soc/codecs/dsd1791.c @@ -0,0 +1,256 @@ +/* + * ALSA SoC codec driver for Texas Instruments DSD1791. + * + * Author: (C) Michael Williamson michael.williamson@criticallink.com + * Copyright: (C) 2011 Critical Link, LLC + * + * 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 <linux/moduleparam.h> +#include <linux/init.h> +#include <linux/delay.h> +#include <linux/pm.h> +#include <linux/spi/spi.h> +#include <linux/platform_device.h> +#include <sound/core.h> +#include <sound/pcm.h> +#include <sound/pcm_params.h> +#include <sound/soc.h> +#include <sound/soc-dapm.h> +#include <sound/initval.h> + +#define DSD1791_REG_DIGATT_L 16 +#define DSD1791_REG_DIGATT_R 17 +#define DSD1791_REG_AUDFMT 18 +#define DSD1791_REG_SRST 20 + +#define DSD1791_FMT_16RJ (0<<4) +#define DSD1791_FMT_20RJ (1<<4) +#define DSD1791_FMT_24RJ (2<<4) +#define DSD1791_FMT_24LJ (3<<4) +#define DSD1791_FMT_16I2S (4<<4) +#define DSD1791_FMT_24I2S (5<<4) +#define DSD1791_FMT_MASK 0x70 + +/* DSD1791 register cache (16 through 23 are used) */ +static const u8 dsd1791_reg[] = { + [16] = 0xFF, + [17] = 0xFF, + [18] = 0x50, + [19] = 0x00, + [20] = 0x00, + [21] = 0x01, + [22] = 0x00, + [23] = 0x00, +}; + +struct dsd1791 { + struct spi_device *spi; + struct snd_soc_codec codec; + int dai_fmt; + int pcm_fmt; +}; + +static int dsd1791_set_format_word(struct dsd1791 *dsd1791, + struct snd_soc_codec *codec) +{ + u8 fmt = 0; + u8 reg; + + switch (dsd1791->dai_fmt & SND_SOC_DAIFMT_FORMAT_MASK) { + + case SND_SOC_DAIFMT_I2S: + switch (dsd1791->pcm_fmt) { + case SNDRV_PCM_FORMAT_S16_LE: + fmt = DSD1791_FMT_16I2S; + break; + case SNDRV_PCM_FORMAT_S24_LE: + fmt = DSD1791_FMT_24I2S; + break; + default: + return -EINVAL; + } + break; + + case SND_SOC_DAIFMT_RIGHT_J: + switch (dsd1791->pcm_fmt) { + case SNDRV_PCM_FORMAT_S16_LE: + fmt = DSD1791_FMT_16RJ; + break; + case SNDRV_PCM_FORMAT_S24_LE: + fmt = DSD1791_FMT_24RJ; + break; + default: + return -EINVAL; + } + break; + + case SND_SOC_DAIFMT_LEFT_J: + switch (dsd1791->pcm_fmt) { + case SNDRV_PCM_FORMAT_S24_LE: + fmt = DSD1791_FMT_24LJ; + default: + return -EINVAL; + } + break; + default: + return -EINVAL; + } + reg = snd_soc_read(codec, DSD1791_REG_AUDFMT); + reg &= ~(DSD1791_FMT_MASK); + reg |= fmt; + return snd_soc_write(codec, DSD1791_REG_AUDFMT, reg); +} + +static int dsd1791_mute(struct snd_soc_dai *dai, int mute) +{ + struct snd_soc_codec *codec = dai->codec; + u8 reg; + + reg = snd_soc_read(codec, DSD1791_REG_AUDFMT); + if (mute) + reg |= 1; + else + reg &= ~1; + return snd_soc_write(codec, DSD1791_REG_AUDFMT, reg); +} + +static int dsd1791_hw_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params, + struct snd_soc_dai *dai) +{ + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct snd_soc_codec *codec = rtd->codec; + struct dsd1791 *dsd1791 = snd_soc_codec_get_drvdata(codec); + + dsd1791->pcm_fmt = params_format(params); + + return dsd1791_set_format_word(dsd1791, codec); +} + +static int dsd1791_set_fmt(struct snd_soc_dai *codec_dai, + unsigned int fmt) +{ + struct snd_soc_codec *codec = codec_dai->codec; + struct dsd1791 *dsd1791 = snd_soc_codec_get_drvdata(codec); + + dsd1791->dai_fmt = fmt; + + return dsd1791_set_format_word(dsd1791, codec); +} + +#define DSD1791_RATES SNDRV_PCM_RATE_8000_192000 +#define DSD1791_FORMATS (SNDRV_PCM_FMTBIT_S16_LE |\ + SNDRV_PCM_FMTBIT_S24_LE) + +static const struct snd_soc_dai_ops dsd1791_dai_ops = { + .hw_params = dsd1791_hw_params, + .set_fmt = dsd1791_set_fmt, + .digital_mute = dsd1791_mute, +}; + +static struct snd_soc_dai_driver dsd1791_dai = { + .name = "dsd1791", + .playback = { + .stream_name = "Playback", + .channels_min = 2, + .channels_max = 2, + .rates = DSD1791_RATES, + .formats = DSD1791_FORMATS, + }, + .ops = &dsd1791_dai_ops, +}; + +static const struct snd_kcontrol_new dsd1791_snd_controls[] = { + SOC_SINGLE("Left Playback Volume", DSD1791_REG_DIGATT_L, 0, 255, 0), + SOC_SINGLE("Right Playback Volume", DSD1791_REG_DIGATT_R, 0, 255, 0), +}; + +static int dsd1791_probe(struct snd_soc_codec *codec) +{ + u8 reg; + int ret; + struct dsd1791 *dsd1791 = snd_soc_codec_get_drvdata(codec); + + ret = snd_soc_codec_set_cache_io(codec, 8, 8, SND_SOC_SPI); + if (ret) { + dev_err(codec->dev, "Failed to set Cache I/O: %d\n", ret); + goto err; + } + + ret = snd_soc_write(codec, DSD1791_REG_SRST, 0x40); + if (ret) { + dev_err(codec->dev, "Unable to reset device: %d\n", ret); + goto err; + } + + /* default format after reset */ + dsd1791->dai_fmt = SND_SOC_DAIFMT_I2S; + dsd1791->pcm_fmt = SNDRV_PCM_FORMAT_S24_LE; + + /* enable attenuation control */ + reg = snd_soc_read(codec, DSD1791_REG_AUDFMT); + reg |= 0x80; + snd_soc_write(codec, DSD1791_REG_AUDFMT, reg); + + snd_soc_add_controls(codec, dsd1791_snd_controls, + ARRAY_SIZE(dsd1791_snd_controls)); + return 0; +err: + return ret; +} + +struct snd_soc_codec_driver dsd1791_codec_driver = { + .probe = dsd1791_probe, + .reg_cache_size = ARRAY_SIZE(dsd1791_reg), + .reg_word_size = sizeof(u8), + .reg_cache_default = dsd1791_reg, +}; + +static int __devinit dsd1791_spi_probe(struct spi_device *spi) +{ + struct dsd1791 *dsd1791; + + dsd1791 = devm_kzalloc(&spi->dev, sizeof *dsd1791, GFP_KERNEL); + if (!dsd1791) + return -ENOMEM; + + spi_set_drvdata(spi, dsd1791); + + return snd_soc_register_codec(&spi->dev, + &dsd1791_codec_driver, &dsd1791_dai, 1); +}; + +static int __devexit dsd1791_spi_remove(struct spi_device *spi) +{ + snd_soc_unregister_codec(&spi->dev); + return 0; +} + +static struct spi_driver dsd1791_spi_driver = { + .driver = { + .name = "dsd1791", + .owner = THIS_MODULE, + }, + .probe = dsd1791_spi_probe, + .remove = __devexit_p(dsd1791_spi_remove), +}; + +static int __init dsd1791_init(void) +{ + return spi_register_driver(&dsd1791_spi_driver); +} +module_init(dsd1791_init); + +static void __exit dsd1791_exit(void) +{ + spi_unregister_driver(&dsd1791_spi_driver); +} +module_exit(dsd1791_exit); + +MODULE_DESCRIPTION("ASoC DSD1791 codec driver"); +MODULE_AUTHOR("Michael Williamson"); +MODULE_LICENSE("GPL");
On Mon, 2011-12-19 at 13:53 -0500, Michael Williamson wrote:
This patch introduces a (spi) codec driver for the Texas Instruments DSD1791 24 bit audio stereo DAC.
http://www.ti.com/product/dsd1791
Testing for basic operation using 16 and 24 bit I2S mode has been performed using a MityDSP-L138 SOM and an Industrial I/O board from Critical Link.
Signed-off-by: Michael Williamson michael.williamson@criticallink.com
Looks mostly fine, just a few comments :-
This patch incorporates changes from the original RFC as a result of comments received from Mark Brown, Leon Romanovsky, and Lars-Peter Clausen. Thanks.
Summary of Changes:
- Use devm_kzalloc()
- use regmap cached I/O feature of framework
- simplify code applying codec fmt configuration
- clean up section attributes on local functions
- make local functions/variables static where appropriate
- remove set_sysclk method
- remove unused tlv header file
- remove chatter
- strip "-codec" from driver name
- fixup driver variable name for consistency
- Add Left and Right volume control
- Add digital mute
sound/soc/codecs/Kconfig | 4 + sound/soc/codecs/Makefile | 2 + sound/soc/codecs/dsd1791.c | 256 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 262 insertions(+), 0 deletions(-) create mode 100644 sound/soc/codecs/dsd1791.c
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index 4584514..95b7969 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -33,6 +33,7 @@ config SND_SOC_ALL_CODECS select SND_SOC_CX20442 select SND_SOC_DA7210 if I2C select SND_SOC_DFBMCS320
- select SND_SOC_DSD1791 if SPI_MASTER select SND_SOC_JZ4740_CODEC if SOC_JZ4740 select SND_SOC_LM4857 if I2C select SND_SOC_MAX98088 if I2C
@@ -205,6 +206,9 @@ config SND_SOC_DA7210 config SND_SOC_DFBMCS320 tristate
+config SND_SOC_DSD1791
- tristate
config SND_SOC_DMIC tristate
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile index a2c7842..d6b5f6a 100644 --- a/sound/soc/codecs/Makefile +++ b/sound/soc/codecs/Makefile @@ -21,6 +21,7 @@ snd-soc-cx20442-objs := cx20442.o snd-soc-da7210-objs := da7210.o snd-soc-dfbmcs320-objs := dfbmcs320.o snd-soc-dmic-objs := dmic.o +snd-soc-dsd1791-objs := dsd1791.o snd-soc-l3-objs := l3.o snd-soc-max98088-objs := max98088.o snd-soc-max98095-objs := max98095.o @@ -120,6 +121,7 @@ obj-$(CONFIG_SND_SOC_CS4271) += snd-soc-cs4271.o obj-$(CONFIG_SND_SOC_CX20442) += snd-soc-cx20442.o obj-$(CONFIG_SND_SOC_DA7210) += snd-soc-da7210.o obj-$(CONFIG_SND_SOC_DFBMCS320) += snd-soc-dfbmcs320.o +obj-$(CONFIG_SND_SOC_DSD1791) += snd-soc-dsd1791.o obj-$(CONFIG_SND_SOC_DMIC) += snd-soc-dmic.o obj-$(CONFIG_SND_SOC_L3) += snd-soc-l3.o obj-$(CONFIG_SND_SOC_JZ4740_CODEC) += snd-soc-jz4740-codec.o diff --git a/sound/soc/codecs/dsd1791.c b/sound/soc/codecs/dsd1791.c new file mode 100644 index 0000000..41bbc61 --- /dev/null +++ b/sound/soc/codecs/dsd1791.c @@ -0,0 +1,256 @@ +/*
- ALSA SoC codec driver for Texas Instruments DSD1791.
- Author: (C) Michael Williamson michael.williamson@criticallink.com
- Copyright: (C) 2011 Critical Link, LLC
- 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 <linux/moduleparam.h> +#include <linux/init.h> +#include <linux/delay.h> +#include <linux/pm.h> +#include <linux/spi/spi.h> +#include <linux/platform_device.h> +#include <sound/core.h> +#include <sound/pcm.h> +#include <sound/pcm_params.h> +#include <sound/soc.h> +#include <sound/soc-dapm.h> +#include <sound/initval.h>
+#define DSD1791_REG_DIGATT_L 16 +#define DSD1791_REG_DIGATT_R 17 +#define DSD1791_REG_AUDFMT 18 +#define DSD1791_REG_SRST 20
+#define DSD1791_FMT_16RJ (0<<4) +#define DSD1791_FMT_20RJ (1<<4) +#define DSD1791_FMT_24RJ (2<<4) +#define DSD1791_FMT_24LJ (3<<4) +#define DSD1791_FMT_16I2S (4<<4) +#define DSD1791_FMT_24I2S (5<<4) +#define DSD1791_FMT_MASK 0x70
+/* DSD1791 register cache (16 through 23 are used) */ +static const u8 dsd1791_reg[] = {
- [16] = 0xFF,
- [17] = 0xFF,
- [18] = 0x50,
- [19] = 0x00,
- [20] = 0x00,
- [21] = 0x01,
- [22] = 0x00,
- [23] = 0x00,
+};
+struct dsd1791 {
- struct spi_device *spi;
- struct snd_soc_codec codec;
- int dai_fmt;
- int pcm_fmt;
+};
+static int dsd1791_set_format_word(struct dsd1791 *dsd1791,
struct snd_soc_codec *codec)+{
- u8 fmt = 0;
- u8 reg;
- switch (dsd1791->dai_fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
- case SND_SOC_DAIFMT_I2S:
switch (dsd1791->pcm_fmt) {case SNDRV_PCM_FORMAT_S16_LE:fmt = DSD1791_FMT_16I2S;break;case SNDRV_PCM_FORMAT_S24_LE:fmt = DSD1791_FMT_24I2S;break;default:return -EINVAL;}break;- case SND_SOC_DAIFMT_RIGHT_J:
switch (dsd1791->pcm_fmt) {case SNDRV_PCM_FORMAT_S16_LE:fmt = DSD1791_FMT_16RJ;break;case SNDRV_PCM_FORMAT_S24_LE:fmt = DSD1791_FMT_24RJ;break;default:return -EINVAL;}break;- case SND_SOC_DAIFMT_LEFT_J:
switch (dsd1791->pcm_fmt) {case SNDRV_PCM_FORMAT_S24_LE:fmt = DSD1791_FMT_24LJ;default:return -EINVAL;}break;- default:
return -EINVAL;- }
- reg = snd_soc_read(codec, DSD1791_REG_AUDFMT);
- reg &= ~(DSD1791_FMT_MASK);
- reg |= fmt;
- return snd_soc_write(codec, DSD1791_REG_AUDFMT, reg);
You could make the code flow easier by using snd_soc_update_bits() here and in other places.
+}
+static int dsd1791_mute(struct snd_soc_dai *dai, int mute) +{
- struct snd_soc_codec *codec = dai->codec;
- u8 reg;
- reg = snd_soc_read(codec, DSD1791_REG_AUDFMT);
- if (mute)
reg |= 1;- else
reg &= ~1;- return snd_soc_write(codec, DSD1791_REG_AUDFMT, reg);
+}
+static int dsd1791_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params,struct snd_soc_dai *dai)+{
- struct snd_soc_pcm_runtime *rtd = substream->private_data;
- struct snd_soc_codec *codec = rtd->codec;
- struct dsd1791 *dsd1791 = snd_soc_codec_get_drvdata(codec);
- dsd1791->pcm_fmt = params_format(params);
- return dsd1791_set_format_word(dsd1791, codec);
+}
+static int dsd1791_set_fmt(struct snd_soc_dai *codec_dai,
unsigned int fmt)+{
- struct snd_soc_codec *codec = codec_dai->codec;
- struct dsd1791 *dsd1791 = snd_soc_codec_get_drvdata(codec);
- dsd1791->dai_fmt = fmt;
- return dsd1791_set_format_word(dsd1791, codec);
+}
+#define DSD1791_RATES SNDRV_PCM_RATE_8000_192000 +#define DSD1791_FORMATS (SNDRV_PCM_FMTBIT_S16_LE |\
SNDRV_PCM_FMTBIT_S24_LE)+static const struct snd_soc_dai_ops dsd1791_dai_ops = {
- .hw_params = dsd1791_hw_params,
- .set_fmt = dsd1791_set_fmt,
- .digital_mute = dsd1791_mute,
+};
+static struct snd_soc_dai_driver dsd1791_dai = {
- .name = "dsd1791",
- .playback = {
.stream_name = "Playback",.channels_min = 2,.channels_max = 2,.rates = DSD1791_RATES,.formats = DSD1791_FORMATS,- },
- .ops = &dsd1791_dai_ops,
+};
+static const struct snd_kcontrol_new dsd1791_snd_controls[] = {
- SOC_SINGLE("Left Playback Volume", DSD1791_REG_DIGATT_L, 0, 255, 0),
- SOC_SINGLE("Right Playback Volume", DSD1791_REG_DIGATT_R, 0, 255, 0),
Best to use SOC_DOUBLE_R here and rename to "Master Playback Volume"
+};
+static int dsd1791_probe(struct snd_soc_codec *codec) +{
- u8 reg;
- int ret;
- struct dsd1791 *dsd1791 = snd_soc_codec_get_drvdata(codec);
- ret = snd_soc_codec_set_cache_io(codec, 8, 8, SND_SOC_SPI);
- if (ret) {
dev_err(codec->dev, "Failed to set Cache I/O: %d\n", ret);goto err;
I dont think goto is required in either case here so easier just to return.
- }
- ret = snd_soc_write(codec, DSD1791_REG_SRST, 0x40);
- if (ret) {
dev_err(codec->dev, "Unable to reset device: %d\n", ret);goto err;- }
- /* default format after reset */
- dsd1791->dai_fmt = SND_SOC_DAIFMT_I2S;
- dsd1791->pcm_fmt = SNDRV_PCM_FORMAT_S24_LE;
- /* enable attenuation control */
- reg = snd_soc_read(codec, DSD1791_REG_AUDFMT);
- reg |= 0x80;
- snd_soc_write(codec, DSD1791_REG_AUDFMT, reg);
- snd_soc_add_controls(codec, dsd1791_snd_controls,
ARRAY_SIZE(dsd1791_snd_controls));- return 0;
+err:
- return ret;
+}
+struct snd_soc_codec_driver dsd1791_codec_driver = {
- .probe = dsd1791_probe,
- .reg_cache_size = ARRAY_SIZE(dsd1791_reg),
- .reg_word_size = sizeof(u8),
- .reg_cache_default = dsd1791_reg,
+};
+static int __devinit dsd1791_spi_probe(struct spi_device *spi) +{
- struct dsd1791 *dsd1791;
- dsd1791 = devm_kzalloc(&spi->dev, sizeof *dsd1791, GFP_KERNEL);
- if (!dsd1791)
return -ENOMEM;- spi_set_drvdata(spi, dsd1791);
- return snd_soc_register_codec(&spi->dev,
&dsd1791_codec_driver, &dsd1791_dai, 1);+};
+static int __devexit dsd1791_spi_remove(struct spi_device *spi) +{
- snd_soc_unregister_codec(&spi->dev);
What about your private data ?
- return 0;
+}
+static struct spi_driver dsd1791_spi_driver = {
- .driver = {
.name = "dsd1791",.owner = THIS_MODULE,- },
- .probe = dsd1791_spi_probe,
- .remove = __devexit_p(dsd1791_spi_remove),
+};
+static int __init dsd1791_init(void) +{
- return spi_register_driver(&dsd1791_spi_driver);
+} +module_init(dsd1791_init);
+static void __exit dsd1791_exit(void) +{
- spi_unregister_driver(&dsd1791_spi_driver);
+} +module_exit(dsd1791_exit);
+MODULE_DESCRIPTION("ASoC DSD1791 codec driver"); +MODULE_AUTHOR("Michael Williamson"); +MODULE_LICENSE("GPL");
GPL v2 according to the commnts at the top.
Thanks
Liam
On 12/19/2011 04:44 PM, Liam Girdwood wrote:
On Mon, 2011-12-19 at 13:53 -0500, Michael Williamson wrote:
This patch introduces a (spi) codec driver for the Texas Instruments DSD1791 24 bit audio stereo DAC.
http://www.ti.com/product/dsd1791
Testing for basic operation using 16 and 24 bit I2S mode has been performed using a MityDSP-L138 SOM and an Industrial I/O board from Critical Link.
Signed-off-by: Michael Williamson michael.williamson@criticallink.com
Looks mostly fine, just a few comments :-
Thanks for the review. I will address your comments. One question, below.
-Mike
This patch incorporates changes from the original RFC as a result of comments received from Mark Brown, Leon Romanovsky, and Lars-Peter Clausen. Thanks.
Summary of Changes:
- Use devm_kzalloc()
- use regmap cached I/O feature of framework
- simplify code applying codec fmt configuration
- clean up section attributes on local functions
- make local functions/variables static where appropriate
- remove set_sysclk method
- remove unused tlv header file
- remove chatter
- strip "-codec" from driver name
- fixup driver variable name for consistency
- Add Left and Right volume control
- Add digital mute
sound/soc/codecs/Kconfig | 4 + sound/soc/codecs/Makefile | 2 + sound/soc/codecs/dsd1791.c | 256 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 262 insertions(+), 0 deletions(-) create mode 100644 sound/soc/codecs/dsd1791.c
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index 4584514..95b7969 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -33,6 +33,7 @@ config SND_SOC_ALL_CODECS select SND_SOC_CX20442 select SND_SOC_DA7210 if I2C select SND_SOC_DFBMCS320
- select SND_SOC_DSD1791 if SPI_MASTER select SND_SOC_JZ4740_CODEC if SOC_JZ4740 select SND_SOC_LM4857 if I2C select SND_SOC_MAX98088 if I2C
@@ -205,6 +206,9 @@ config SND_SOC_DA7210 config SND_SOC_DFBMCS320 tristate
+config SND_SOC_DSD1791
- tristate
config SND_SOC_DMIC tristate
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile index a2c7842..d6b5f6a 100644 --- a/sound/soc/codecs/Makefile +++ b/sound/soc/codecs/Makefile @@ -21,6 +21,7 @@ snd-soc-cx20442-objs := cx20442.o snd-soc-da7210-objs := da7210.o snd-soc-dfbmcs320-objs := dfbmcs320.o snd-soc-dmic-objs := dmic.o +snd-soc-dsd1791-objs := dsd1791.o snd-soc-l3-objs := l3.o snd-soc-max98088-objs := max98088.o snd-soc-max98095-objs := max98095.o @@ -120,6 +121,7 @@ obj-$(CONFIG_SND_SOC_CS4271) += snd-soc-cs4271.o obj-$(CONFIG_SND_SOC_CX20442) += snd-soc-cx20442.o obj-$(CONFIG_SND_SOC_DA7210) += snd-soc-da7210.o obj-$(CONFIG_SND_SOC_DFBMCS320) += snd-soc-dfbmcs320.o +obj-$(CONFIG_SND_SOC_DSD1791) += snd-soc-dsd1791.o obj-$(CONFIG_SND_SOC_DMIC) += snd-soc-dmic.o obj-$(CONFIG_SND_SOC_L3) += snd-soc-l3.o obj-$(CONFIG_SND_SOC_JZ4740_CODEC) += snd-soc-jz4740-codec.o diff --git a/sound/soc/codecs/dsd1791.c b/sound/soc/codecs/dsd1791.c new file mode 100644 index 0000000..41bbc61 --- /dev/null +++ b/sound/soc/codecs/dsd1791.c @@ -0,0 +1,256 @@ +/*
- ALSA SoC codec driver for Texas Instruments DSD1791.
- Author: (C) Michael Williamson michael.williamson@criticallink.com
- Copyright: (C) 2011 Critical Link, LLC
- 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 <linux/moduleparam.h> +#include <linux/init.h> +#include <linux/delay.h> +#include <linux/pm.h> +#include <linux/spi/spi.h> +#include <linux/platform_device.h> +#include <sound/core.h> +#include <sound/pcm.h> +#include <sound/pcm_params.h> +#include <sound/soc.h> +#include <sound/soc-dapm.h> +#include <sound/initval.h>
+#define DSD1791_REG_DIGATT_L 16 +#define DSD1791_REG_DIGATT_R 17 +#define DSD1791_REG_AUDFMT 18 +#define DSD1791_REG_SRST 20
+#define DSD1791_FMT_16RJ (0<<4) +#define DSD1791_FMT_20RJ (1<<4) +#define DSD1791_FMT_24RJ (2<<4) +#define DSD1791_FMT_24LJ (3<<4) +#define DSD1791_FMT_16I2S (4<<4) +#define DSD1791_FMT_24I2S (5<<4) +#define DSD1791_FMT_MASK 0x70
+/* DSD1791 register cache (16 through 23 are used) */ +static const u8 dsd1791_reg[] = {
- [16] = 0xFF,
- [17] = 0xFF,
- [18] = 0x50,
- [19] = 0x00,
- [20] = 0x00,
- [21] = 0x01,
- [22] = 0x00,
- [23] = 0x00,
+};
+struct dsd1791 {
- struct spi_device *spi;
- struct snd_soc_codec codec;
- int dai_fmt;
- int pcm_fmt;
+};
+static int dsd1791_set_format_word(struct dsd1791 *dsd1791,
struct snd_soc_codec *codec)+{
- u8 fmt = 0;
- u8 reg;
- switch (dsd1791->dai_fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
- case SND_SOC_DAIFMT_I2S:
switch (dsd1791->pcm_fmt) {case SNDRV_PCM_FORMAT_S16_LE:fmt = DSD1791_FMT_16I2S;break;case SNDRV_PCM_FORMAT_S24_LE:fmt = DSD1791_FMT_24I2S;break;default:return -EINVAL;}break;- case SND_SOC_DAIFMT_RIGHT_J:
switch (dsd1791->pcm_fmt) {case SNDRV_PCM_FORMAT_S16_LE:fmt = DSD1791_FMT_16RJ;break;case SNDRV_PCM_FORMAT_S24_LE:fmt = DSD1791_FMT_24RJ;break;default:return -EINVAL;}break;- case SND_SOC_DAIFMT_LEFT_J:
switch (dsd1791->pcm_fmt) {case SNDRV_PCM_FORMAT_S24_LE:fmt = DSD1791_FMT_24LJ;default:return -EINVAL;}break;- default:
return -EINVAL;- }
- reg = snd_soc_read(codec, DSD1791_REG_AUDFMT);
- reg &= ~(DSD1791_FMT_MASK);
- reg |= fmt;
- return snd_soc_write(codec, DSD1791_REG_AUDFMT, reg);
You could make the code flow easier by using snd_soc_update_bits() here and in other places.
+}
+static int dsd1791_mute(struct snd_soc_dai *dai, int mute) +{
- struct snd_soc_codec *codec = dai->codec;
- u8 reg;
- reg = snd_soc_read(codec, DSD1791_REG_AUDFMT);
- if (mute)
reg |= 1;- else
reg &= ~1;- return snd_soc_write(codec, DSD1791_REG_AUDFMT, reg);
+}
+static int dsd1791_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params,struct snd_soc_dai *dai)+{
- struct snd_soc_pcm_runtime *rtd = substream->private_data;
- struct snd_soc_codec *codec = rtd->codec;
- struct dsd1791 *dsd1791 = snd_soc_codec_get_drvdata(codec);
- dsd1791->pcm_fmt = params_format(params);
- return dsd1791_set_format_word(dsd1791, codec);
+}
+static int dsd1791_set_fmt(struct snd_soc_dai *codec_dai,
unsigned int fmt)+{
- struct snd_soc_codec *codec = codec_dai->codec;
- struct dsd1791 *dsd1791 = snd_soc_codec_get_drvdata(codec);
- dsd1791->dai_fmt = fmt;
- return dsd1791_set_format_word(dsd1791, codec);
+}
+#define DSD1791_RATES SNDRV_PCM_RATE_8000_192000 +#define DSD1791_FORMATS (SNDRV_PCM_FMTBIT_S16_LE |\
SNDRV_PCM_FMTBIT_S24_LE)+static const struct snd_soc_dai_ops dsd1791_dai_ops = {
- .hw_params = dsd1791_hw_params,
- .set_fmt = dsd1791_set_fmt,
- .digital_mute = dsd1791_mute,
+};
+static struct snd_soc_dai_driver dsd1791_dai = {
- .name = "dsd1791",
- .playback = {
.stream_name = "Playback",.channels_min = 2,.channels_max = 2,.rates = DSD1791_RATES,.formats = DSD1791_FORMATS,- },
- .ops = &dsd1791_dai_ops,
+};
+static const struct snd_kcontrol_new dsd1791_snd_controls[] = {
- SOC_SINGLE("Left Playback Volume", DSD1791_REG_DIGATT_L, 0, 255, 0),
- SOC_SINGLE("Right Playback Volume", DSD1791_REG_DIGATT_R, 0, 255, 0),
Best to use SOC_DOUBLE_R here and rename to "Master Playback Volume"
+};
+static int dsd1791_probe(struct snd_soc_codec *codec) +{
- u8 reg;
- int ret;
- struct dsd1791 *dsd1791 = snd_soc_codec_get_drvdata(codec);
- ret = snd_soc_codec_set_cache_io(codec, 8, 8, SND_SOC_SPI);
- if (ret) {
dev_err(codec->dev, "Failed to set Cache I/O: %d\n", ret);goto err;I dont think goto is required in either case here so easier just to return.
- }
- ret = snd_soc_write(codec, DSD1791_REG_SRST, 0x40);
- if (ret) {
dev_err(codec->dev, "Unable to reset device: %d\n", ret);goto err;- }
- /* default format after reset */
- dsd1791->dai_fmt = SND_SOC_DAIFMT_I2S;
- dsd1791->pcm_fmt = SNDRV_PCM_FORMAT_S24_LE;
- /* enable attenuation control */
- reg = snd_soc_read(codec, DSD1791_REG_AUDFMT);
- reg |= 0x80;
- snd_soc_write(codec, DSD1791_REG_AUDFMT, reg);
- snd_soc_add_controls(codec, dsd1791_snd_controls,
ARRAY_SIZE(dsd1791_snd_controls));- return 0;
+err:
- return ret;
+}
+struct snd_soc_codec_driver dsd1791_codec_driver = {
- .probe = dsd1791_probe,
- .reg_cache_size = ARRAY_SIZE(dsd1791_reg),
- .reg_word_size = sizeof(u8),
- .reg_cache_default = dsd1791_reg,
+};
+static int __devinit dsd1791_spi_probe(struct spi_device *spi) +{
- struct dsd1791 *dsd1791;
- dsd1791 = devm_kzalloc(&spi->dev, sizeof *dsd1791, GFP_KERNEL);
- if (!dsd1791)
return -ENOMEM;- spi_set_drvdata(spi, dsd1791);
- return snd_soc_register_codec(&spi->dev,
&dsd1791_codec_driver, &dsd1791_dai, 1);+};
+static int __devexit dsd1791_spi_remove(struct spi_device *spi) +{
- snd_soc_unregister_codec(&spi->dev);
What about your private data ?
My understanding is devm_kzalloc'd memory would be freed up by the device framework. Is this not correct?
- return 0;
+}
+static struct spi_driver dsd1791_spi_driver = {
- .driver = {
.name = "dsd1791",.owner = THIS_MODULE,- },
- .probe = dsd1791_spi_probe,
- .remove = __devexit_p(dsd1791_spi_remove),
+};
+static int __init dsd1791_init(void) +{
- return spi_register_driver(&dsd1791_spi_driver);
+} +module_init(dsd1791_init);
+static void __exit dsd1791_exit(void) +{
- spi_unregister_driver(&dsd1791_spi_driver);
+} +module_exit(dsd1791_exit);
+MODULE_DESCRIPTION("ASoC DSD1791 codec driver"); +MODULE_AUTHOR("Michael Williamson"); +MODULE_LICENSE("GPL");
GPL v2 according to the commnts at the top.
Thanks
Liam
On Mon, Dec 19, 2011 at 05:26:49PM -0500, Michael Williamson wrote:
On 12/19/2011 04:44 PM, Liam Girdwood wrote:
Looks mostly fine, just a few comments :-
Thanks for the review. I will address your comments. One question, below.
Guys, please delete irrelevant context from your mails.
+{
- snd_soc_unregister_codec(&spi->dev);
What about your private data ?
My understanding is devm_kzalloc'd memory would be freed up by the device framework. Is this not correct?
Your understanding is correct, that's the whole point of devm_kzalloc().
On Mon, Dec 19, 2011 at 01:53:30PM -0500, Michael Williamson wrote:
+/* DSD1791 register cache (16 through 23 are used) */ +static const u8 dsd1791_reg[] = {
- [16] = 0xFF,
- [17] = 0xFF,
- [18] = 0x50,
- [19] = 0x00,
- [20] = 0x00,
- [21] = 0x01,
- [22] = 0x00,
- [23] = 0x00,
+};
Use the regmap API.
- reg = snd_soc_read(codec, DSD1791_REG_AUDFMT);
- reg &= ~(DSD1791_FMT_MASK);
- reg |= fmt;
- return snd_soc_write(codec, DSD1791_REG_AUDFMT, reg);
snd_soc_update_bits().
+static const struct snd_kcontrol_new dsd1791_snd_controls[] = {
- SOC_SINGLE("Left Playback Volume", DSD1791_REG_DIGATT_L, 0, 255, 0),
- SOC_SINGLE("Right Playback Volume", DSD1791_REG_DIGATT_R, 0, 255, 0),
This should be a single stereo control and you should supply dB data.
- /* enable attenuation control */
- reg = snd_soc_read(codec, DSD1791_REG_AUDFMT);
- reg |= 0x80;
- snd_soc_write(codec, DSD1791_REG_AUDFMT, reg);
This should probably be runtime controllable, or if it's got a good reason for not being it should be using snd_soc_update_bits().
- snd_soc_add_controls(codec, dsd1791_snd_controls,
ARRAY_SIZE(dsd1791_snd_controls));
Initialize the controls from the driver.
participants (3)
-
Liam Girdwood -
Mark Brown -
Michael Williamson