[alsa-devel] [PATCH] ASoC: dsd1791: Introduce driver for TI DSD1791 stereo codec
Michael Williamson
michael.williamson at criticallink.com
Mon Dec 19 23:26:49 CET 2011
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 at 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 at 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
>
>
More information about the Alsa-devel
mailing list