All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] patch sequence adds analog devices ad1836 asoc support
@ 2009-08-12  4:32 Barry Song
  2009-08-12  4:32 ` [PATCH 1/3] new ad1836 codec driver based on asoc Barry Song
  0 siblings, 1 reply; 17+ messages in thread
From: Barry Song @ 2009-08-12  4:32 UTC (permalink / raw)
  To: broonie; +Cc: uclinux-dist-devel, alsa-devel, Barry Song

Hi Mark,
The patch sequence adds ad1836 support based on asoc

patch 1:
new ad1836 codec driver based on asoc 

patch 2:
add set_tdm_slot entry in tdm-dai to define the relationship between audio channel and tdm slot number

patch 3:
new board driver to connect ad1836 codec and bf5xx

patch 3 depends on patch 2.

Thanks
Barry

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/3] new ad1836 codec driver based on asoc
  2009-08-12  4:32 [PATCH 0/3] patch sequence adds analog devices ad1836 asoc support Barry Song
@ 2009-08-12  4:32 ` Barry Song
  2009-08-12  4:32   ` [PATCH 2/3] add set_tdm_slot in tdm dai to define the relationship between audio channel and tdm slot Barry Song
  2009-08-12 13:46   ` [PATCH 1/3] new ad1836 codec driver based on asoc Mark Brown
  0 siblings, 2 replies; 17+ messages in thread
From: Barry Song @ 2009-08-12  4:32 UTC (permalink / raw)
  To: broonie; +Cc: uclinux-dist-devel, alsa-devel, Barry Song

There has been  an ad1836 driver in sound/blackfin based on traditional alsa.
The new driver is based on asoc. The architecture of ad1836 codec driver
is very much like ad1938.

Signed-off-by: Barry Song <21cnbao@gmail.com>
---
 sound/soc/codecs/Kconfig  |    3 +
 sound/soc/codecs/Makefile |    2 +
 sound/soc/codecs/ad1836.c |  465 +++++++++++++++++++++++++++++++++++++++++++++
 sound/soc/codecs/ad1836.h |   64 ++++++
 4 files changed, 534 insertions(+), 0 deletions(-)
 create mode 100644 sound/soc/codecs/ad1836.c
 create mode 100644 sound/soc/codecs/ad1836.h

diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 31a6d21..05e2279 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -68,6 +68,9 @@ config SND_SOC_AC97_CODEC
 	tristate
 	select SND_AC97_CODEC
 
+config SND_SOC_AD1836
+	tristate
+
 config SND_SOC_AD1938
 	tristate
 
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index 78dce5d..9cbcf8f 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -1,4 +1,5 @@
 snd-soc-ac97-objs := ac97.o
+snd-soc-ad1836-objs := ad1836.o
 snd-soc-ad1938-objs := ad1938.o
 snd-soc-ad1980-objs := ad1980.o
 snd-soc-ad73311-objs := ad73311.o
@@ -45,6 +46,7 @@ snd-soc-wm9713-objs := wm9713.o
 snd-soc-max9877-objs := max9877.o
 
 obj-$(CONFIG_SND_SOC_AC97_CODEC)	+= snd-soc-ac97.o
+obj-$(CONFIG_SND_SOC_AD1836)	+= snd-soc-ad1836.o
 obj-$(CONFIG_SND_SOC_AD1938)	+= snd-soc-ad1938.o
 obj-$(CONFIG_SND_SOC_AD1980)	+= snd-soc-ad1980.o
 obj-$(CONFIG_SND_SOC_AD73311) += snd-soc-ad73311.o
diff --git a/sound/soc/codecs/ad1836.c b/sound/soc/codecs/ad1836.c
new file mode 100644
index 0000000..7d81718
--- /dev/null
+++ b/sound/soc/codecs/ad1836.c
@@ -0,0 +1,465 @@
+/*
+ * File:         sound/soc/codecs/ad1836.c
+ * Author:       Barry Song <Barry.Song@analog.com>
+ *
+ * Created:      Aug 04 2009
+ * Description:  Driver for AD1836 sound chip
+ *
+ * Modified:
+ *               Copyright 2009 Analog Devices Inc.
+ *
+ * Bugs:         Enter bugs at http://blackfin.uclinux.org/
+ *
+ * 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/init.h>
+#include <linux/module.h>
+#include <linux/version.h>
+#include <linux/kernel.h>
+#include <linux/device.h>
+#include <sound/core.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/initval.h>
+#include <sound/soc.h>
+#include <sound/tlv.h>
+#include <sound/soc-dapm.h>
+#include <linux/spi/spi.h>
+#include "ad1836.h"
+
+/* codec private data */
+struct ad1836_priv {
+	struct snd_soc_codec codec;
+	u16 reg_cache[AD1836_NUM_REGS];
+};
+
+static struct snd_soc_codec *ad1836_codec;
+struct snd_soc_codec_device soc_codec_dev_ad1836;
+static int ad1836_register(struct ad1836_priv *ad1836);
+static void ad1836_unregister(struct ad1836_priv *ad1836);
+
+/*
+ * AD1836 volume/mute/de-emphasis etc. controls
+ */
+static const char *ad1836_deemp[] = {"None", "44.1kHz", "32kHz", "48kHz"};
+
+static const struct soc_enum ad1836_deemp_enum =
+	SOC_ENUM_SINGLE(AD1836_DAC_CTRL1, 8, 4, ad1836_deemp);
+
+static const struct snd_kcontrol_new ad1836_snd_controls[] = {
+	/* DAC volume control */
+	SOC_DOUBLE_R("DAC1  Volume", AD1836_DAC_L1_VOL,
+			AD1836_DAC_R1_VOL, 0, 0x3FF, 0),
+	SOC_DOUBLE_R("DAC2  Volume", AD1836_DAC_L2_VOL,
+			AD1836_DAC_R2_VOL, 0, 0x3FF, 0),
+	SOC_DOUBLE_R("DAC3  Volume", AD1836_DAC_L3_VOL,
+			AD1836_DAC_R3_VOL, 0, 0x3FF, 0),
+
+	/* ADC switch control */
+	SOC_DOUBLE("ADC1 Switch", AD1836_ADC_CTRL2, AD1836_ADCL1_MUTE,
+		AD1836_ADCR1_MUTE, 1, 1),
+	SOC_DOUBLE("ADC2 Switch", AD1836_ADC_CTRL2, AD1836_ADCL2_MUTE,
+		AD1836_ADCR2_MUTE, 1, 1),
+
+	/* DAC switch control */
+	SOC_DOUBLE("DAC1 Switch", AD1836_DAC_CTRL2, AD1836_DACL1_MUTE,
+		AD1836_DACR1_MUTE, 1, 1),
+	SOC_DOUBLE("DAC2 Switch", AD1836_DAC_CTRL2, AD1836_DACL2_MUTE,
+		AD1836_DACR2_MUTE, 1, 1),
+	SOC_DOUBLE("DAC3 Switch", AD1836_DAC_CTRL2, AD1836_DACL3_MUTE,
+		AD1836_DACR3_MUTE, 1, 1),
+
+	/* ADC high-pass filter */
+	SOC_SINGLE("ADC High Pass Filter Switch", AD1836_ADC_CTRL1,
+			AD1836_ADC_HIGHPASS_FILTER, 1, 0),
+
+	/* DAC de-emphasis */
+	SOC_ENUM("Playback Deemphasis", ad1836_deemp_enum),
+};
+
+static const struct snd_soc_dapm_widget ad1836_dapm_widgets[] = {
+	SND_SOC_DAPM_DAC("DAC", "Playback", AD1836_DAC_CTRL1,
+				AD1836_DAC_POWERDOWN, 1),
+	SND_SOC_DAPM_ADC("ADC", "Capture", SND_SOC_NOPM, 0, 0),
+	SND_SOC_DAPM_SUPPLY("ADC_PWR", AD1836_ADC_CTRL1,
+				AD1836_ADC_POWERDOWN, 1, NULL, 0),
+	SND_SOC_DAPM_OUTPUT("DAC1OUT"),
+	SND_SOC_DAPM_OUTPUT("DAC2OUT"),
+	SND_SOC_DAPM_OUTPUT("DAC3OUT"),
+	SND_SOC_DAPM_INPUT("ADC1IN"),
+	SND_SOC_DAPM_INPUT("ADC2IN"),
+};
+
+static const struct snd_soc_dapm_route audio_paths[] = {
+	{ "DAC", NULL, "ADC_PWR" },
+	{ "ADC", NULL, "ADC_PWR" },
+	{ "DAC1OUT", "DAC1 Switch", "DAC" },
+	{ "DAC2OUT", "DAC2 Switch", "DAC" },
+	{ "DAC3OUT", "DAC3 Switch", "DAC" },
+	{ "ADC", "ADC1 Switch", "ADC1IN" },
+	{ "ADC", "ADC2 Switch", "ADC2IN" },
+};
+
+/*
+ * DAI ops entries
+ */
+
+static int ad1836_set_tdm_slot(struct snd_soc_dai *dai,
+		unsigned int mask, int slots)
+{
+	switch (slots) {
+	case 8:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int ad1836_set_dai_fmt(struct snd_soc_dai *codec_dai,
+		unsigned int fmt)
+{
+	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+	/* at present, we support adc aux mode to interface with
+	 * blackfin sport tdm mode
+	 */
+	case SND_SOC_DAIFMT_DSP_A:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
+	case SND_SOC_DAIFMT_IB_IF:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
+	/* ALCLK,ABCLK are both output, AD1836 can only be master */
+	case SND_SOC_DAIFMT_CBM_CFM:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int ad1836_hw_params(struct snd_pcm_substream *substream,
+		struct snd_pcm_hw_params *params,
+		struct snd_soc_dai *dai)
+{
+	int word_len = 0, reg = 0;
+
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_device *socdev = rtd->socdev;
+	struct snd_soc_codec *codec = socdev->card->codec;
+
+	/* bit size */
+	switch (params_format(params)) {
+	case SNDRV_PCM_FORMAT_S16_LE:
+		word_len = 3;
+		break;
+	case SNDRV_PCM_FORMAT_S20_3LE:
+		word_len = 1;
+		break;
+	case SNDRV_PCM_FORMAT_S24_LE:
+	case SNDRV_PCM_FORMAT_S32_LE:
+		word_len = 0;
+		break;
+	}
+
+	reg = codec->read(codec, AD1836_DAC_CTRL1);
+	reg = (reg & (~AD1836_DAC_WORD_LEN_MASK)) | word_len;
+	codec->write(codec, AD1836_DAC_CTRL1, reg);
+
+	reg = codec->read(codec, AD1836_ADC_CTRL2);
+	reg = (reg & (~AD1836_ADC_WORD_LEN_MASK)) | word_len;
+	codec->write(codec, AD1836_ADC_CTRL2, reg);
+
+	return 0;
+}
+
+
+/*
+ * interface to read/write ad1836 register
+ */
+#define AD1836_SPI_REG_SHFT 12
+#define AD1836_SPI_READ     (1 << 11)
+#define AD1836_SPI_VAL_MSK  0x3FF
+
+/*
+ * write to the ad1836 register space
+ */
+
+static int ad1836_write_reg(struct snd_soc_codec *codec, unsigned int reg,
+		unsigned int value)
+{
+	u16 *reg_cache = codec->reg_cache;
+	int ret = 0;
+
+	if (value != reg_cache[reg]) {
+		unsigned short buf;
+		struct spi_transfer t = {
+			.tx_buf = &buf,
+			.len = 2,
+		};
+		struct spi_message m;
+
+		buf = (reg << AD1836_SPI_REG_SHFT) |
+			(value & AD1836_SPI_VAL_MSK);
+		spi_message_init(&m);
+		spi_message_add_tail(&t, &m);
+		ret = spi_sync(codec->control_data, &m);
+		if (ret == 0)
+			reg_cache[reg] = value;
+	}
+
+	return ret;
+}
+
+/*
+ * read from the ad1836 register space cache
+ */
+static unsigned int ad1836_read_reg_cache(struct snd_soc_codec *codec,
+					  unsigned int reg)
+{
+	u16 *reg_cache = codec->reg_cache;
+
+	if (reg >= codec->reg_cache_size)
+		return -EINVAL;
+
+	return reg_cache[reg];
+}
+
+static int __devinit ad1836_spi_probe(struct spi_device *spi)
+{
+	struct snd_soc_codec *codec;
+	struct ad1836_priv *ad1836;
+
+	ad1836 = kzalloc(sizeof(struct ad1836_priv), GFP_KERNEL);
+	if (ad1836 == NULL)
+		return -ENOMEM;
+
+	codec = &ad1836->codec;
+	codec->control_data = spi;
+	codec->dev = &spi->dev;
+
+	dev_set_drvdata(&spi->dev, ad1836);
+
+	return ad1836_register(ad1836);
+}
+
+static int __devexit ad1836_spi_remove(struct spi_device *spi)
+{
+	struct ad1836_priv *ad1836 = dev_get_drvdata(&spi->dev);
+
+	ad1836_unregister(ad1836);
+	return 0;
+}
+
+static struct spi_driver ad1836_spi_driver = {
+	.driver = {
+		.name	= "ad1836-spi",
+		.bus	= &spi_bus_type,
+		.owner	= THIS_MODULE,
+	},
+	.probe		= ad1836_spi_probe,
+	.remove		= __devexit_p(ad1836_spi_remove),
+};
+
+static struct snd_soc_dai_ops ad1836_dai_ops = {
+	.hw_params = ad1836_hw_params,
+	.set_tdm_slot = ad1836_set_tdm_slot,
+	.set_fmt = ad1836_set_dai_fmt,
+};
+
+/* codec DAI instance */
+struct snd_soc_dai ad1836_dai = {
+	.name = "AD1836",
+	.playback = {
+		.stream_name = "Playback",
+		.channels_min = 2,
+		.channels_max = 6,
+		.rates = SNDRV_PCM_RATE_48000,
+		.formats = SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_S16_LE |
+			SNDRV_PCM_FMTBIT_S20_3LE | SNDRV_PCM_FMTBIT_S24_LE,
+	},
+	.capture = {
+		.stream_name = "Capture",
+		.channels_min = 2,
+		.channels_max = 4,
+		.rates = SNDRV_PCM_RATE_48000,
+		.formats = SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_S16_LE |
+			SNDRV_PCM_FMTBIT_S20_3LE | SNDRV_PCM_FMTBIT_S24_LE,
+	},
+	.ops = &ad1836_dai_ops,
+};
+EXPORT_SYMBOL_GPL(ad1836_dai);
+
+static int ad1836_register(struct ad1836_priv *ad1836)
+{
+	int ret;
+	struct snd_soc_codec *codec = &ad1836->codec;
+
+	if (ad1836_codec) {
+		dev_err(codec->dev, "Another ad1836 is registered\n");
+		return -EINVAL;
+	}
+
+	mutex_init(&codec->mutex);
+	INIT_LIST_HEAD(&codec->dapm_widgets);
+	INIT_LIST_HEAD(&codec->dapm_paths);
+	codec->private_data = ad1836;
+	codec->reg_cache = ad1836->reg_cache;
+	codec->reg_cache_size = AD1836_NUM_REGS;
+	codec->name = "AD1836";
+	codec->owner = THIS_MODULE;
+	codec->dai = &ad1836_dai;
+	codec->num_dai = 1;
+	codec->write = ad1836_write_reg;
+	codec->read = ad1836_read_reg_cache;
+	INIT_LIST_HEAD(&codec->dapm_widgets);
+	INIT_LIST_HEAD(&codec->dapm_paths);
+
+	ad1836_dai.dev = codec->dev;
+	ad1836_codec = codec;
+
+	/* default setting for ad1836 */
+	/* de-emphasis: 48kHz, power-on dac */
+	codec->write(codec, AD1836_DAC_CTRL1, 0x30);
+	/* unmute dac channels */
+	codec->write(codec, AD1836_DAC_CTRL2, 0x0);
+	/* high-pass filter enable, power-on adc */
+	codec->write(codec, AD1836_ADC_CTRL1, 0x100);
+	/* unmute adc channles, adc aux mode */
+	codec->write(codec, AD1836_ADC_CTRL2, 0x180);
+	/* left/right diff:PGA/MUX */
+	codec->write(codec, AD1836_ADC_CTRL3, 0x3A);
+	/* volume */
+	codec->write(codec, AD1836_DAC_L1_VOL, 0x3FF);
+	codec->write(codec, AD1836_DAC_R1_VOL, 0x3FF);
+	codec->write(codec, AD1836_DAC_L2_VOL, 0x3FF);
+	codec->write(codec, AD1836_DAC_R2_VOL, 0x3FF);
+	codec->write(codec, AD1836_DAC_L3_VOL, 0x3FF);
+	codec->write(codec, AD1836_DAC_R3_VOL, 0x3FF);
+
+	ret = snd_soc_register_codec(codec);
+	if (ret != 0) {
+		dev_err(codec->dev, "Failed to register codec: %d\n", ret);
+		return ret;
+	}
+
+	ret = snd_soc_register_dai(&ad1836_dai);
+	if (ret != 0) {
+		dev_err(codec->dev, "Failed to register DAI: %d\n", ret);
+		snd_soc_unregister_codec(codec);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void ad1836_unregister(struct ad1836_priv *ad1836)
+{
+	snd_soc_unregister_dai(&ad1836_dai);
+	snd_soc_unregister_codec(&ad1836->codec);
+	kfree(ad1836);
+	ad1836_codec = NULL;
+}
+
+static int ad1836_probe(struct platform_device *pdev)
+{
+	struct snd_soc_device *socdev = platform_get_drvdata(pdev);
+	struct snd_soc_codec *codec;
+	int ret = 0;
+
+	if (ad1836_codec == NULL) {
+		dev_err(&pdev->dev, "Codec device not registered\n");
+		return -ENODEV;
+	}
+
+	socdev->card->codec = ad1836_codec;
+	codec = ad1836_codec;
+
+	/* register pcms */
+	ret = snd_soc_new_pcms(socdev, SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1);
+	if (ret < 0) {
+		dev_err(codec->dev, "failed to create pcms: %d\n", ret);
+		goto pcm_err;
+	}
+
+	snd_soc_add_controls(codec, ad1836_snd_controls,
+			     ARRAY_SIZE(ad1836_snd_controls));
+	snd_soc_dapm_new_controls(codec, ad1836_dapm_widgets,
+				  ARRAY_SIZE(ad1836_dapm_widgets));
+	snd_soc_dapm_add_routes(codec, audio_paths, ARRAY_SIZE(audio_paths));
+	snd_soc_dapm_new_widgets(codec);
+
+	ret = snd_soc_init_card(socdev);
+	if (ret < 0) {
+		dev_err(codec->dev, "failed to register card: %d\n", ret);
+		goto card_err;
+	}
+
+	return ret;
+
+card_err:
+	snd_soc_free_pcms(socdev);
+	snd_soc_dapm_free(socdev);
+pcm_err:
+	return ret;
+}
+
+/* power down chip */
+static int ad1836_remove(struct platform_device *pdev)
+{
+	struct snd_soc_device *socdev = platform_get_drvdata(pdev);
+
+	snd_soc_free_pcms(socdev);
+	snd_soc_dapm_free(socdev);
+
+	return 0;
+}
+
+struct snd_soc_codec_device soc_codec_dev_ad1836 = {
+	.probe = 	ad1836_probe,
+	.remove = 	ad1836_remove,
+	/* The power management of ad1836 is very simple. There are
+	 * only adc&dac 2 components to control. Dapm handles them.
+	 */
+	.suspend =      NULL,
+	.resume =       NULL,
+};
+EXPORT_SYMBOL_GPL(soc_codec_dev_ad1836);
+
+static int __init ad1836_init(void)
+{
+	int ret;
+
+	ret = spi_register_driver(&ad1836_spi_driver);
+	if (ret != 0) {
+		printk(KERN_ERR "Failed to register ad1836 SPI driver: %d\n",
+				ret);
+	}
+
+	return ret;
+}
+module_init(ad1836_init);
+
+static void __exit ad1836_exit(void)
+{
+	spi_unregister_driver(&ad1836_spi_driver);
+}
+module_exit(ad1836_exit);
+
+MODULE_DESCRIPTION("ASoC ad1836 driver");
+MODULE_AUTHOR("Barry Song <21cnbao@gmail.com>");
+MODULE_LICENSE("GPL");
diff --git a/sound/soc/codecs/ad1836.h b/sound/soc/codecs/ad1836.h
new file mode 100644
index 0000000..7660ee6
--- /dev/null
+++ b/sound/soc/codecs/ad1836.h
@@ -0,0 +1,64 @@
+/*
+ * File:         sound/soc/codecs/ad1836.h
+ * Based on:
+ * Author:       Barry Song <Barry.Song@analog.com>
+ *
+ * Created:      Aug 04, 2009
+ * Description:  definitions for AD1836 registers
+ *
+ * Modified:
+ *
+ * Bugs:         Enter bugs at http://blackfin.uclinux.org/
+ *
+ * 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.
+ */
+
+#ifndef __AD1836_H__
+#define __AD1836_H__
+
+#define AD1836_DAC_CTRL1               0
+#define AD1836_DAC_POWERDOWN           2
+#define AD1836_DAC_SERFMT_MASK	       0xE0
+#define AD1836_DAC_SERFMT_PCK256       (0x4 << 5)
+#define AD1836_DAC_SERFMT_PCK128       (0x5 << 5)
+#define AD1836_DAC_WORD_LEN_MASK       0x18
+
+#define AD1836_DAC_CTRL2               1
+#define AD1836_DACL1_MUTE              0
+#define AD1836_DACR1_MUTE              1
+#define AD1836_DACL2_MUTE              2
+#define AD1836_DACR2_MUTE              3
+#define AD1836_DACL3_MUTE              4
+#define AD1836_DACR3_MUTE              5
+
+#define AD1836_DAC_L1_VOL              2
+#define AD1836_DAC_R1_VOL              3
+#define AD1836_DAC_L2_VOL              4
+#define AD1836_DAC_R2_VOL              5
+#define AD1836_DAC_L3_VOL              6
+#define AD1836_DAC_R3_VOL              7
+
+#define AD1836_ADC_CTRL1               12
+#define AD1836_ADC_POWERDOWN           7
+#define AD1836_ADC_HIGHPASS_FILTER     8
+
+#define AD1836_ADC_CTRL2               13
+#define AD1836_ADCL1_MUTE 		0
+#define AD1836_ADCR1_MUTE 		1
+#define AD1836_ADCL2_MUTE 		2
+#define AD1836_ADCR2_MUTE 		3
+#define AD1836_ADC_WORD_LEN_MASK       0x30
+#define AD1836_ADC_SERFMT_MASK	       (7 << 6)
+#define AD1836_ADC_SERFMT_PCK256       (0x4 << 6)
+#define AD1836_ADC_SERFMT_PCK128       (0x5 << 6)
+
+#define AD1836_ADC_CTRL3               14
+
+#define AD1836_NUM_REGS                16
+
+extern struct snd_soc_dai ad1836_dai;
+extern struct snd_soc_codec_device soc_codec_dev_ad1836;
+#endif
-- 
1.5.6.3

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/3] add set_tdm_slot in tdm dai to define the relationship between audio channel and tdm slot
  2009-08-12  4:32 ` [PATCH 1/3] new ad1836 codec driver based on asoc Barry Song
@ 2009-08-12  4:32   ` Barry Song
  2009-08-12  4:32     ` [PATCH 3/3] new board driver to connect bfin-5xx with ad1836 codec Barry Song
  2009-08-12 13:56     ` [PATCH 2/3] add set_tdm_slot in tdm dai to define the relationship between audio channel and tdm slot Mark Brown
  2009-08-12 13:46   ` [PATCH 1/3] new ad1836 codec driver based on asoc Mark Brown
  1 sibling, 2 replies; 17+ messages in thread
From: Barry Song @ 2009-08-12  4:32 UTC (permalink / raw)
  To: broonie; +Cc: uclinux-dist-devel, alsa-devel, Barry Song

For ad1938, the audio channel n just uses slot n, but for ad1836, it's
different, channel 0 uses slot 0, channel 1 uses slot 4, channels 2 uses
slot1, ... So add set_tdm_slot entry and use the mask field to define
the relationship between audio channel and slot number.

Signed-off-by: Barry Song <21cnbao@gmail.com>
---
 sound/soc/blackfin/bf5xx-ad1938.c  |    5 +++++
 sound/soc/blackfin/bf5xx-tdm-pcm.c |   12 +++++++++---
 sound/soc/blackfin/bf5xx-tdm.c     |   18 ++++++++++--------
 sound/soc/blackfin/bf5xx-tdm.h     |   10 ++++++++++
 4 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/sound/soc/blackfin/bf5xx-ad1938.c b/sound/soc/blackfin/bf5xx-ad1938.c
index 08269e9..6844098 100644
--- a/sound/soc/blackfin/bf5xx-ad1938.c
+++ b/sound/soc/blackfin/bf5xx-ad1938.c
@@ -73,6 +73,11 @@ static int bf5xx_ad1938_hw_params(struct snd_pcm_substream *substream,
 		SND_SOC_DAIFMT_IB_IF | SND_SOC_DAIFMT_CBM_CFM);
 	if (ret < 0)
 		return ret;
+	
+	/* set cpu DAI slots, 8 channels, mask is defined as slot seq */
+	ret = snd_soc_dai_set_tdm_slot(codec_dai, 0x76543210, 8);
+	if (ret < 0)
+		return ret;
 
 	/* set codec DAI slots, 8 channels, all channels are enabled */
 	ret = snd_soc_dai_set_tdm_slot(codec_dai, 0xFF, 8);
diff --git a/sound/soc/blackfin/bf5xx-tdm-pcm.c b/sound/soc/blackfin/bf5xx-tdm-pcm.c
index ccb5e82..044ef45 100644
--- a/sound/soc/blackfin/bf5xx-tdm-pcm.c
+++ b/sound/soc/blackfin/bf5xx-tdm-pcm.c
@@ -43,7 +43,7 @@
 #include "bf5xx-tdm.h"
 #include "bf5xx-sport.h"
 
-#define PCM_BUFFER_MAX  0x10000
+#define PCM_BUFFER_MAX  0x8000
 #define FRAGMENT_SIZE_MIN  (4*1024)
 #define FRAGMENTS_MIN  2
 #define FRAGMENTS_MAX  32
@@ -177,6 +177,9 @@ out:
 static int bf5xx_pcm_copy(struct snd_pcm_substream *substream, int channel,
 	snd_pcm_uframes_t pos, void *buf, snd_pcm_uframes_t count)
 {
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct sport_device *sport = runtime->private_data;
+	struct bf5xx_tdm_port *tdm_port = sport->private_data;
 	unsigned int *src;
 	unsigned int *dst;
 	int i;
@@ -186,9 +189,11 @@ static int bf5xx_pcm_copy(struct snd_pcm_substream *substream, int channel,
 		dst = (unsigned int *)substream->runtime->dma_area;
 
 		dst += pos * 8;
+
 		while (count--) {
 			for (i = 0; i < substream->runtime->channels; i++)
-				*(dst + i) = *src++;
+				*(dst + (((tdm_port->slot_seq >>
+					(4 * i)) & 0xF))) = *src++;
 			dst += 8;
 		}
 	} else {
@@ -198,7 +203,8 @@ static int bf5xx_pcm_copy(struct snd_pcm_substream *substream, int channel,
 		src += pos * 8;
 		while (count--) {
 			for (i = 0; i < substream->runtime->channels; i++)
-				*dst++ = *(src+i);
+				*dst++ = *(src + (((tdm_port->slot_seq >>
+					(4 * i)) & 0xF)));
 			src += 8;
 		}
 	}
diff --git a/sound/soc/blackfin/bf5xx-tdm.c b/sound/soc/blackfin/bf5xx-tdm.c
index 3096bad..091e178 100644
--- a/sound/soc/blackfin/bf5xx-tdm.c
+++ b/sound/soc/blackfin/bf5xx-tdm.c
@@ -46,14 +46,6 @@
 #include "bf5xx-sport.h"
 #include "bf5xx-tdm.h"
 
-struct bf5xx_tdm_port {
-	u16 tcr1;
-	u16 rcr1;
-	u16 tcr2;
-	u16 rcr2;
-	int configured;
-};
-
 static struct bf5xx_tdm_port bf5xx_tdm;
 static int sport_num = CONFIG_SND_BF5XX_SPORT_NUM;
 
@@ -181,6 +173,13 @@ static void bf5xx_tdm_shutdown(struct snd_pcm_substream *substream,
 		bf5xx_tdm.configured = 0;
 }
 
+static int bf5xx_tdm_set_tdm_slot(struct snd_soc_dai *dai,
+		unsigned int mask, int slots)
+{
+	bf5xx_tdm.slot_seq = mask;
+	return 0;
+}
+
 #ifdef CONFIG_PM
 static int bf5xx_tdm_suspend(struct snd_soc_dai *dai)
 {
@@ -235,6 +234,7 @@ static struct snd_soc_dai_ops bf5xx_tdm_dai_ops = {
 	.hw_params      = bf5xx_tdm_hw_params,
 	.set_fmt        = bf5xx_tdm_set_dai_fmt,
 	.shutdown       = bf5xx_tdm_shutdown,
+	.set_tdm_slot   = bf5xx_tdm_set_tdm_slot,
 };
 
 struct snd_soc_dai bf5xx_tdm_dai = {
@@ -300,6 +300,8 @@ static int __devinit bfin_tdm_probe(struct platform_device *pdev)
 		pr_err("Failed to register DAI: %d\n", ret);
 		goto sport_config_err;
 	}
+
+	sport_handle->private_data = &bf5xx_tdm;
 	return 0;
 
 sport_config_err:
diff --git a/sound/soc/blackfin/bf5xx-tdm.h b/sound/soc/blackfin/bf5xx-tdm.h
index 618ec3d..701c769 100644
--- a/sound/soc/blackfin/bf5xx-tdm.h
+++ b/sound/soc/blackfin/bf5xx-tdm.h
@@ -9,6 +9,16 @@
 #ifndef _BF5XX_TDM_H
 #define _BF5XX_TDM_H
 
+struct bf5xx_tdm_port {
+	u16 tcr1;
+	u16 rcr1;
+	u16 tcr2;
+	u16 rcr2;
+	/* which slot used for the corresponding audio channel? */
+	int slot_seq;
+	int configured;
+};
+
 extern struct snd_soc_dai bf5xx_tdm_dai;
 
 #endif
-- 
1.5.6.3

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 3/3] new board driver to connect bfin-5xx with ad1836 codec
  2009-08-12  4:32   ` [PATCH 2/3] add set_tdm_slot in tdm dai to define the relationship between audio channel and tdm slot Barry Song
@ 2009-08-12  4:32     ` Barry Song
  2009-08-12 16:10       ` Mark Brown
  2009-08-12 13:56     ` [PATCH 2/3] add set_tdm_slot in tdm dai to define the relationship between audio channel and tdm slot Mark Brown
  1 sibling, 1 reply; 17+ messages in thread
From: Barry Song @ 2009-08-12  4:32 UTC (permalink / raw)
  To: broonie; +Cc: uclinux-dist-devel, alsa-devel, Barry Song

Signed-off-by: Barry Song <21cnbao@gmail.com>
---
 sound/soc/blackfin/Kconfig        |    8 ++
 sound/soc/blackfin/Makefile       |    2 +
 sound/soc/blackfin/bf5xx-ad1836.c |  138 +++++++++++++++++++++++++++++++++++++
 3 files changed, 148 insertions(+), 0 deletions(-)
 create mode 100644 sound/soc/blackfin/bf5xx-ad1836.c

diff --git a/sound/soc/blackfin/Kconfig b/sound/soc/blackfin/Kconfig
index 8a4de4d..ac927ff 100644
--- a/sound/soc/blackfin/Kconfig
+++ b/sound/soc/blackfin/Kconfig
@@ -88,6 +88,14 @@ config SND_BF5XX_SOC_AC97
 	select SND_SOC_AC97_BUS
 	select SND_BF5XX_SOC_SPORT
 
+config SND_BF5XX_SOC_AD1836
+	tristate "SoC AD1836 Audio support for BF5xx"
+	depends on SND_BF5XX_TDM
+	select SND_BF5XX_SOC_TDM
+	select SND_SOC_AD1836
+	help
+	  Say Y if you want to add support for SoC audio on BF5xx STAMP/EZKIT.
+
 config SND_BF5XX_SOC_AD1980
 	tristate "SoC AD1980/1 Audio support for BF5xx"
 	depends on SND_BF5XX_AC97
diff --git a/sound/soc/blackfin/Makefile b/sound/soc/blackfin/Makefile
index f4d7607..87e3042 100644
--- a/sound/soc/blackfin/Makefile
+++ b/sound/soc/blackfin/Makefile
@@ -16,11 +16,13 @@ obj-$(CONFIG_SND_BF5XX_SOC_I2S) += snd-soc-bf5xx-i2s.o
 obj-$(CONFIG_SND_BF5XX_SOC_TDM) += snd-soc-bf5xx-tdm.o
 
 # Blackfin Machine Support
+snd-ad1836-objs := bf5xx-ad1836.o
 snd-ad1980-objs := bf5xx-ad1980.o
 snd-ssm2602-objs := bf5xx-ssm2602.o
 snd-ad73311-objs := bf5xx-ad73311.o
 snd-ad1938-objs := bf5xx-ad1938.o
 
+obj-$(CONFIG_SND_BF5XX_SOC_AD1836) += snd-ad1836.o
 obj-$(CONFIG_SND_BF5XX_SOC_AD1980) += snd-ad1980.o
 obj-$(CONFIG_SND_BF5XX_SOC_SSM2602) += snd-ssm2602.o
 obj-$(CONFIG_SND_BF5XX_SOC_AD73311) += snd-ad73311.o
diff --git a/sound/soc/blackfin/bf5xx-ad1836.c b/sound/soc/blackfin/bf5xx-ad1836.c
new file mode 100644
index 0000000..aa367ae
--- /dev/null
+++ b/sound/soc/blackfin/bf5xx-ad1836.c
@@ -0,0 +1,138 @@
+/*
+ * File:         sound/soc/blackfin/bf5xx-ad1836.c
+ * Author:       Barry Song <Barry.Song@analog.com>
+ *
+ * Created:      Aug 4 2009
+ * Description:  Board driver for ad1836 sound chip
+ *
+ * Bugs:         Enter bugs at http://blackfin.uclinux.org/
+ *
+ * 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 <linux/moduleparam.h>
+#include <linux/device.h>
+#include <sound/core.h>
+#include <sound/pcm.h>
+#include <sound/soc.h>
+#include <sound/soc-dapm.h>
+#include <sound/pcm_params.h>
+
+#include <asm/blackfin.h>
+#include <asm/cacheflush.h>
+#include <asm/irq.h>
+#include <asm/dma.h>
+#include <asm/portmux.h>
+
+#include "../codecs/ad1836.h"
+#include "bf5xx-sport.h"
+
+#include "bf5xx-tdm-pcm.h"
+#include "bf5xx-tdm.h"
+
+static struct snd_soc_card bf5xx_ad1836;
+
+static int bf5xx_ad1836_startup(struct snd_pcm_substream *substream)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
+
+	cpu_dai->private_data = sport_handle;
+	return 0;
+}
+
+static int bf5xx_ad1836_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 *cpu_dai = rtd->dai->cpu_dai;
+	struct snd_soc_dai *codec_dai = rtd->dai->codec_dai;
+	int ret = 0;
+	/* set cpu DAI configuration */
+	ret = snd_soc_dai_set_fmt(cpu_dai, SND_SOC_DAIFMT_DSP_A |
+		SND_SOC_DAIFMT_IB_IF | SND_SOC_DAIFMT_CBM_CFM);
+	if (ret < 0)
+		return ret;
+
+	/* set codec DAI configuration */
+	ret = snd_soc_dai_set_fmt(codec_dai, SND_SOC_DAIFMT_DSP_A |
+		SND_SOC_DAIFMT_IB_IF | SND_SOC_DAIFMT_CBM_CFM);
+	if (ret < 0)
+		return ret;
+
+	/* set cpu DAI slots, 8 channels, mask is defined as slot seq */
+	ret = snd_soc_dai_set_tdm_slot(cpu_dai, 0x73625140, 8);
+	if (ret < 0)
+		return ret;
+
+	/* set codec DAI slots, 8 channels, all channels are enabled */
+	ret = snd_soc_dai_set_tdm_slot(codec_dai, 0xFF, 8);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static struct snd_soc_ops bf5xx_ad1836_ops = {
+	.startup = bf5xx_ad1836_startup,
+	.hw_params = bf5xx_ad1836_hw_params,
+};
+
+static struct snd_soc_dai_link bf5xx_ad1836_dai = {
+	.name = "ad1836",
+	.stream_name = "AD1836",
+	.cpu_dai = &bf5xx_tdm_dai,
+	.codec_dai = &ad1836_dai,
+	.ops = &bf5xx_ad1836_ops,
+};
+
+static struct snd_soc_card bf5xx_ad1836 = {
+	.name = "bf5xx_ad1836",
+	.platform = &bf5xx_tdm_soc_platform,
+	.dai_link = &bf5xx_ad1836_dai,
+	.num_links = 1,
+};
+
+static struct snd_soc_device bf5xx_ad1836_snd_devdata = {
+	.card = &bf5xx_ad1836,
+	.codec_dev = &soc_codec_dev_ad1836,
+};
+
+static struct platform_device *bfxx_ad1836_snd_device;
+
+static int __init bf5xx_ad1836_init(void)
+{
+	int ret;
+
+	bfxx_ad1836_snd_device = platform_device_alloc("soc-audio", -1);
+	if (!bfxx_ad1836_snd_device)
+		return -ENOMEM;
+
+	platform_set_drvdata(bfxx_ad1836_snd_device, &bf5xx_ad1836_snd_devdata);
+	bf5xx_ad1836_snd_devdata.dev = &bfxx_ad1836_snd_device->dev;
+	ret = platform_device_add(bfxx_ad1836_snd_device);
+
+	if (ret)
+		platform_device_put(bfxx_ad1836_snd_device);
+
+	return ret;
+}
+
+static void __exit bf5xx_ad1836_exit(void)
+{
+	platform_device_unregister(bfxx_ad1836_snd_device);
+}
+
+module_init(bf5xx_ad1836_init);
+module_exit(bf5xx_ad1836_exit);
+
+/* Module information */
+MODULE_AUTHOR("Barry Song");
+MODULE_DESCRIPTION("ALSA SoC AD1836 board driver");
+MODULE_LICENSE("GPL");
+
-- 
1.5.6.3

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/3] new ad1836 codec driver based on asoc
  2009-08-12  4:32 ` [PATCH 1/3] new ad1836 codec driver based on asoc Barry Song
  2009-08-12  4:32   ` [PATCH 2/3] add set_tdm_slot in tdm dai to define the relationship between audio channel and tdm slot Barry Song
@ 2009-08-12 13:46   ` Mark Brown
  2009-08-13  3:41     ` Barry Song
  1 sibling, 1 reply; 17+ messages in thread
From: Mark Brown @ 2009-08-12 13:46 UTC (permalink / raw)
  To: Barry Song; +Cc: uclinux-dist-devel, alsa-devel

On Wed, Aug 12, 2009 at 12:32:52PM +0800, Barry Song wrote:
> There has been  an ad1836 driver in sound/blackfin based on traditional alsa.
> The new driver is based on asoc. The architecture of ad1836 codec driver
> is very much like ad1938.

This is mostly good, a few relatively minor issues though.

> +	SOC_DOUBLE_R("DAC3  Volume", AD1836_DAC_L3_VOL,
> +			AD1836_DAC_R3_VOL, 0, 0x3FF, 0),

You've got an extra space in the volume control names which I'd expect
to confuse user space.

> +       { "DAC1OUT", "DAC1 Switch", "DAC" },
> +       { "DAC2OUT", "DAC2 Switch", "DAC" },

I'm surprised this works since the switches weren't declared as DAPM
controls but if the core supports it that's OK.

> +static int ad1836_set_tdm_slot(struct snd_soc_dai *dai,
> +		unsigned int mask, int slots)
> +{

This needs updating for the new set_tdm_slot() API but given that
there's only one possible configuration I'd suggest just removing it -
the function does nothing other than check its arguments.

> +	reg = codec->read(codec, AD1836_DAC_CTRL1);
> +	reg = (reg & (~AD1836_DAC_WORD_LEN_MASK)) | word_len;
> +	codec->write(codec, AD1836_DAC_CTRL1, reg);

Not essential but snd_soc_update_bits() does the read/modify/write cycle
for you.

> +static struct spi_driver ad1836_spi_driver = {
> +	.driver = {
> +		.name	= "ad1836-spi",

Just "ad1836" - the fact that it's controlled via SPI is clear from the
fact that this is a struct spi_driver.

> +	ret = snd_soc_register_codec(codec);
> +	if (ret != 0) {
> +		dev_err(codec->dev, "Failed to register codec: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = snd_soc_register_dai(&ad1836_dai);
> +	if (ret != 0) {
> +		dev_err(codec->dev, "Failed to register DAI: %d\n", ret);
> +		snd_soc_unregister_codec(codec);
> +		return ret;
> +	}

Should also be freeing the private data structure on error.

> +struct snd_soc_codec_device soc_codec_dev_ad1836 = {
> +	.probe = 	ad1836_probe,
> +	.remove = 	ad1836_remove,
> +	/* The power management of ad1836 is very simple. There are
> +	 * only adc&dac 2 components to control. Dapm handles them.
> +	 */
> +	.suspend =      NULL,
> +	.resume =       NULL,

The power control will be handled by DAPM but your driver still needs to
restore things like the volume settings - when the driver is powered off
the register settings will be forgotten.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] add set_tdm_slot in tdm dai to define the relationship between audio channel and tdm slot
  2009-08-12  4:32   ` [PATCH 2/3] add set_tdm_slot in tdm dai to define the relationship between audio channel and tdm slot Barry Song
  2009-08-12  4:32     ` [PATCH 3/3] new board driver to connect bfin-5xx with ad1836 codec Barry Song
@ 2009-08-12 13:56     ` Mark Brown
  2009-08-13  4:16       ` Barry Song
  1 sibling, 1 reply; 17+ messages in thread
From: Mark Brown @ 2009-08-12 13:56 UTC (permalink / raw)
  To: Barry Song; +Cc: uclinux-dist-devel, alsa-devel

On Wed, Aug 12, 2009 at 12:32:53PM +0800, Barry Song wrote:
> For ad1938, the audio channel n just uses slot n, but for ad1836, it's
> different, channel 0 uses slot 0, channel 1 uses slot 4, channels 2 uses
> slot1, ... So add set_tdm_slot entry and use the mask field to define
> the relationship between audio channel and slot number.

> Signed-off-by: Barry Song <21cnbao@gmail.com>

OK but this needs updating to the new tdm_slot() API too.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/3] new board driver to connect bfin-5xx with ad1836 codec
  2009-08-12  4:32     ` [PATCH 3/3] new board driver to connect bfin-5xx with ad1836 codec Barry Song
@ 2009-08-12 16:10       ` Mark Brown
  2009-08-13  4:19         ` Barry Song
  0 siblings, 1 reply; 17+ messages in thread
From: Mark Brown @ 2009-08-12 16:10 UTC (permalink / raw)
  To: Barry Song; +Cc: uclinux-dist-devel, alsa-devel

On Wed, Aug 12, 2009 at 12:32:54PM +0800, Barry Song wrote:

This is OK but...

> +	/* set cpu DAI slots, 8 channels, mask is defined as slot seq */
> +	ret = snd_soc_dai_set_tdm_slot(cpu_dai, 0x73625140, 8);
> +	if (ret < 0)
> +		return ret;

This suggests that there's something really odd going on with your
set_tdm_slot() for the CPU which is likely to make it incompatible with
other implementations and with any core support to handle things like
multi-drop DAIs.  The DAI driver side could certainly use more
documenation explaining what's going on if nothing else...

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/3] new ad1836 codec driver based on asoc
  2009-08-12 13:46   ` [PATCH 1/3] new ad1836 codec driver based on asoc Mark Brown
@ 2009-08-13  3:41     ` Barry Song
  2009-08-13  9:08       ` Mark Brown
  0 siblings, 1 reply; 17+ messages in thread
From: Barry Song @ 2009-08-13  3:41 UTC (permalink / raw)
  To: Mark Brown; +Cc: uclinux-dist-devel, alsa-devel

On Wed, Aug 12, 2009 at 9:46 PM, Mark
Brown<broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, Aug 12, 2009 at 12:32:52PM +0800, Barry Song wrote:
>> There has been  an ad1836 driver in sound/blackfin based on traditional alsa.
>> The new driver is based on asoc. The architecture of ad1836 codec driver
>> is very much like ad1938.
>
> This is mostly good, a few relatively minor issues though.
>
>> +     SOC_DOUBLE_R("DAC3  Volume", AD1836_DAC_L3_VOL,
>> +                     AD1836_DAC_R3_VOL, 0, 0x3FF, 0),
>
> You've got an extra space in the volume control names which I'd expect
> to confuse user space.
Fixed.
>
>> +       { "DAC1OUT", "DAC1 Switch", "DAC" },
>> +       { "DAC2OUT", "DAC2 Switch", "DAC" },
>
> I'm surprised this works since the switches weren't declared as DAPM
> controls but if the core supports it that's OK.
>
>> +static int ad1836_set_tdm_slot(struct snd_soc_dai *dai,
>> +             unsigned int mask, int slots)
>> +{
>
> This needs updating for the new set_tdm_slot() API but given that
> there's only one possible configuration I'd suggest just removing it -
> the function does nothing other than check its arguments.
>
Fixed
>> +     reg = codec->read(codec, AD1836_DAC_CTRL1);
>> +     reg = (reg & (~AD1836_DAC_WORD_LEN_MASK)) | word_len;
>> +     codec->write(codec, AD1836_DAC_CTRL1, reg);
>
> Not essential but snd_soc_update_bits() does the read/modify/write cycle
> for you.
>
Fixed
>> +static struct spi_driver ad1836_spi_driver = {
>> +     .driver = {
>> +             .name   = "ad1836-spi",
>
> Just "ad1836" - the fact that it's controlled via SPI is clear from the
> fact that this is a struct spi_driver.
>
I agree the spi structure implies the meaning of spi.  But files in
arch/blackfin/mach... are using names like "xxx-spi" for almost all
spi devices. How about keeping the coherence?

>> +     ret = snd_soc_register_codec(codec);
>> +     if (ret != 0) {
>> +             dev_err(codec->dev, "Failed to register codec: %d\n", ret);
>> +             return ret;
>> +     }
>> +
>> +     ret = snd_soc_register_dai(&ad1836_dai);
>> +     if (ret != 0) {
>> +             dev_err(codec->dev, "Failed to register DAI: %d\n", ret);
>> +             snd_soc_unregister_codec(codec);
>> +             return ret;
>> +     }
>
> Should also be freeing the private data structure on error.
>
Fixed
>> +struct snd_soc_codec_device soc_codec_dev_ad1836 = {
>> +     .probe =        ad1836_probe,
>> +     .remove =       ad1836_remove,
>> +     /* The power management of ad1836 is very simple. There are
>> +      * only adc&dac 2 components to control. Dapm handles them.
>> +      */
>> +     .suspend =      NULL,
>> +     .resume =       NULL,
>
> The power control will be handled by DAPM but your driver still needs to
> restore things like the volume settings - when the driver is powered off
> the register settings will be forgotten.
>
My test shows registers setting doesn't lose after dac/adc shutdown :-)
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] add set_tdm_slot in tdm dai to define the relationship between audio channel and tdm slot
  2009-08-12 13:56     ` [PATCH 2/3] add set_tdm_slot in tdm dai to define the relationship between audio channel and tdm slot Mark Brown
@ 2009-08-13  4:16       ` Barry Song
  0 siblings, 0 replies; 17+ messages in thread
From: Barry Song @ 2009-08-13  4:16 UTC (permalink / raw)
  To: Mark Brown; +Cc: uclinux-dist-devel, alsa-devel

On Wed, Aug 12, 2009 at 9:56 PM, Mark
Brown<broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, Aug 12, 2009 at 12:32:53PM +0800, Barry Song wrote:
>> For ad1938, the audio channel n just uses slot n, but for ad1836, it's
>> different, channel 0 uses slot 0, channel 1 uses slot 4, channels 2 uses
>> slot1, ... So add set_tdm_slot entry and use the mask field to define
>> the relationship between audio channel and slot number.
>
>> Signed-off-by: Barry Song <21cnbao@gmail.com>
>
> OK but this needs updating to the new tdm_slot() API too.
>
Fixed

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/3] new board driver to connect bfin-5xx with ad1836 codec
  2009-08-12 16:10       ` Mark Brown
@ 2009-08-13  4:19         ` Barry Song
  0 siblings, 0 replies; 17+ messages in thread
From: Barry Song @ 2009-08-13  4:19 UTC (permalink / raw)
  To: Mark Brown; +Cc: uclinux-dist-devel, alsa-devel

On Thu, Aug 13, 2009 at 12:10 AM, Mark
Brown<broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, Aug 12, 2009 at 12:32:54PM +0800, Barry Song wrote:
>
> This is OK but...
>
>> +     /* set cpu DAI slots, 8 channels, mask is defined as slot seq */
>> +     ret = snd_soc_dai_set_tdm_slot(cpu_dai, 0x73625140, 8);
>> +     if (ret < 0)
>> +             return ret;
>
> This suggests that there's something really odd going on with your
> set_tdm_slot() for the CPU which is likely to make it incompatible with
> other implementations and with any core support to handle things like
> multi-drop DAIs.  The DAI driver side could certainly use more
> documenation explaining what's going on if nothing else...
>
Fixed
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/3] new ad1836 codec driver based on asoc
  2009-08-13  3:41     ` Barry Song
@ 2009-08-13  9:08       ` Mark Brown
  2009-08-13  9:22         ` [Uclinux-dist-devel] " Mike Frysinger
  2009-08-13  9:37         ` Barry Song
  0 siblings, 2 replies; 17+ messages in thread
From: Mark Brown @ 2009-08-13  9:08 UTC (permalink / raw)
  To: Barry Song; +Cc: uclinux-dist-devel, alsa-devel

On Thu, Aug 13, 2009 at 11:41:13AM +0800, Barry Song wrote:
> On Wed, Aug 12, 2009 at 9:46 PM, Mark
> Brown<broonie@opensource.wolfsonmicro.com> wrote:

> > Just "ad1836" - the fact that it's controlled via SPI is clear from the
> > fact that this is a struct spi_driver.

> I agree the spi structure implies the meaning of spi.  But files in
> arch/blackfin/mach... are using names like "xxx-spi" for almost all
> spi devices. How about keeping the coherence?

Oh dear.  Looks like you've essentially got a choice between being
consistent with the rest of the world and being consistent with what the
blackfin stuff is doing.

> > The power control will be handled by DAPM but your driver still needs to
> > restore things like the volume settings - when the driver is powered off
> > the register settings will be forgotten.

> My test shows registers setting doesn't lose after dac/adc shutdown :-)

Probably your board is maintaining power to the chip even during
suspend.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Uclinux-dist-devel] [PATCH 1/3] new ad1836 codec driver based on asoc
  2009-08-13  9:08       ` Mark Brown
@ 2009-08-13  9:22         ` Mike Frysinger
  2009-08-13  9:34           ` Mike Frysinger
  2009-08-13  9:37         ` Barry Song
  1 sibling, 1 reply; 17+ messages in thread
From: Mike Frysinger @ 2009-08-13  9:22 UTC (permalink / raw)
  To: Mark Brown; +Cc: uclinux-dist-devel, alsa-devel, Barry Song

On Thu, Aug 13, 2009 at 05:08, Mark Brown wrote:
> On Thu, Aug 13, 2009 at 11:41:13AM +0800, Barry Song wrote:
>> On Wed, Aug 12, 2009 at 9:46 PM, Mark Brown wrote:
>> > Just "ad1836" - the fact that it's controlled via SPI is clear from the
>> > fact that this is a struct spi_driver.
>>
>> I agree the spi structure implies the meaning of spi.  But files in
>> arch/blackfin/mach... are using names like "xxx-spi" for almost all
>> spi devices. How about keeping the coherence?
>
> Oh dear.  Looks like you've essentially got a choice between being
> consistent with the rest of the world and being consistent with what the
> blackfin stuff is doing.

it really isnt that bad.  the drivers that use "-spi" suffix:
                .modalias = "ad1836-spi",   # audio
                .modalias = "ad1938-spi",   # audio
                .modalias = "ad7147-spi",   # input
                .modalias = "ad9960-spi",   # dead
                .modalias = "bfin-lq035q1-spi",  # video
                .modalias = "fxo-spi",   # dead
                .modalias = "fxs-spi",   # dead

dropping the dead devices leaves us with 4 drivers to fix, two of
which are audio.  should be easy for us to fix.
-mike
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Uclinux-dist-devel] [PATCH 1/3] new ad1836 codec driver based on asoc
  2009-08-13  9:22         ` [Uclinux-dist-devel] " Mike Frysinger
@ 2009-08-13  9:34           ` Mike Frysinger
  0 siblings, 0 replies; 17+ messages in thread
From: Mike Frysinger @ 2009-08-13  9:34 UTC (permalink / raw)
  To: Mark Brown; +Cc: uclinux-dist-devel, alsa-devel, Barry Song

On Thu, Aug 13, 2009 at 05:22, Mike Frysinger wrote:
> On Thu, Aug 13, 2009 at 05:08, Mark Brown wrote:
>> On Thu, Aug 13, 2009 at 11:41:13AM +0800, Barry Song wrote:
>>> On Wed, Aug 12, 2009 at 9:46 PM, Mark Brown wrote:
>>> > Just "ad1836" - the fact that it's controlled via SPI is clear from the
>>> > fact that this is a struct spi_driver.
>>>
>>> I agree the spi structure implies the meaning of spi.  But files in
>>> arch/blackfin/mach... are using names like "xxx-spi" for almost all
>>> spi devices. How about keeping the coherence?
>>
>> Oh dear.  Looks like you've essentially got a choice between being
>> consistent with the rest of the world and being consistent with what the
>> blackfin stuff is doing.
>
> it really isnt that bad.  the drivers that use "-spi" suffix:
>                .modalias = "ad1836-spi",   # audio
>                .modalias = "ad1938-spi",   # audio
>                .modalias = "ad7147-spi",   # input
>                .modalias = "ad9960-spi",   # dead
>                .modalias = "bfin-lq035q1-spi",  # video
>                .modalias = "fxo-spi",   # dead
>                .modalias = "fxs-spi",   # dead
>
> dropping the dead devices leaves us with 4 drivers to fix, two of
> which are audio.  should be easy for us to fix.

semi-scratch that.  ive punted the dead devices from our tree, but the
bfin-lq035q1-spi is correct as that device driver requires two things
-- a platform driver named "bfin-lq035q1" and a spi driver named
"bfin-lq035q1-spi".

i'll have to send a patch out for the ad7147-spi since it's been
merged into the input tree iirc, but Barry can handle these two
codecs.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/3] new ad1836 codec driver based on asoc
  2009-08-13  9:08       ` Mark Brown
  2009-08-13  9:22         ` [Uclinux-dist-devel] " Mike Frysinger
@ 2009-08-13  9:37         ` Barry Song
  2009-08-13  9:46           ` Mark Brown
  1 sibling, 1 reply; 17+ messages in thread
From: Barry Song @ 2009-08-13  9:37 UTC (permalink / raw)
  To: Mark Brown; +Cc: uclinux-dist-devel, alsa-devel

On Thu, Aug 13, 2009 at 5:08 PM, Mark
Brown<broonie@opensource.wolfsonmicro.com> wrote:
> On Thu, Aug 13, 2009 at 11:41:13AM +0800, Barry Song wrote:
>> On Wed, Aug 12, 2009 at 9:46 PM, Mark
>> Brown<broonie@opensource.wolfsonmicro.com> wrote:
>
>> > Just "ad1836" - the fact that it's controlled via SPI is clear from the
>> > fact that this is a struct spi_driver.
>
>> I agree the spi structure implies the meaning of spi.  But files in
>> arch/blackfin/mach... are using names like "xxx-spi" for almost all
>> spi devices. How about keeping the coherence?
>
> Oh dear.  Looks like you've essentially got a choice between being
> consistent with the rest of the world and being consistent with what the
> blackfin stuff is doing.
Looks like the rest world really doesn't define a spi device with
modalias "xxx-spi" but just "xxx" in arch. So I agree we can change
blackfin stuff as Mike.

>
>> > The power control will be handled by DAPM but your driver still needs to
>> > restore things like the volume settings - when the driver is powered off
>> > the register settings will be forgotten.
>
>> My test shows registers setting doesn't lose after dac/adc shutdown :-)
>
> Probably your board is maintaining power to the chip even during
> suspend.
>
If so, all codec drivers need to save setting then restore them when
resume. The physical shutdown depends on boards great. If we get an
individual programmable PMU/LDO to control the voltage input to this
chip, I can shutdown power-input to codec physically and save the
registers when suspend,  then restore them when resume. If no this
unit, should every codec driver handle this case?
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/3] new ad1836 codec driver based on asoc
  2009-08-13  9:37         ` Barry Song
@ 2009-08-13  9:46           ` Mark Brown
  2009-08-13  9:52             ` Barry Song
  0 siblings, 1 reply; 17+ messages in thread
From: Mark Brown @ 2009-08-13  9:46 UTC (permalink / raw)
  To: Barry Song; +Cc: uclinux-dist-devel, alsa-devel

On Thu, Aug 13, 2009 at 05:37:09PM +0800, Barry Song wrote:

> If so, all codec drivers need to save setting then restore them when
> resume. The physical shutdown depends on boards great. If we get an
> individual programmable PMU/LDO to control the voltage input to this
> chip, I can shutdown power-input to codec physically and save the
> registers when suspend,  then restore them when resume. If no this
> unit, should every codec driver handle this case?

Yes, every CODEC driver that supports suspend and resume needs to handle
this.  Since the CODEC drivers generally use a register cache it's
normally just a case of writing the values in the cache back to the
device at the start of resume.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/3] new ad1836 codec driver based on asoc
  2009-08-13  9:46           ` Mark Brown
@ 2009-08-13  9:52             ` Barry Song
  2009-08-13 10:00               ` Mark Brown
  0 siblings, 1 reply; 17+ messages in thread
From: Barry Song @ 2009-08-13  9:52 UTC (permalink / raw)
  To: Mark Brown; +Cc: uclinux-dist-devel, alsa-devel

On Thu, Aug 13, 2009 at 5:46 PM, Mark
Brown<broonie@opensource.wolfsonmicro.com> wrote:
> On Thu, Aug 13, 2009 at 05:37:09PM +0800, Barry Song wrote:
>
>> If so, all codec drivers need to save setting then restore them when
>> resume. The physical shutdown depends on boards great. If we get an
>> individual programmable PMU/LDO to control the voltage input to this
>> chip, I can shutdown power-input to codec physically and save the
>> registers when suspend,  then restore them when resume. If no this
>> unit, should every codec driver handle this case?
>
> Yes, every CODEC driver that supports suspend and resume needs to handle
> this.  Since the CODEC drivers generally use a register cache it's
> normally just a case of writing the values in the cache back to the
> device at the start of resume.
>
Well. If all chips need this same operation, can it be abstracted to
the soc core layer?
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/3] new ad1836 codec driver based on asoc
  2009-08-13  9:52             ` Barry Song
@ 2009-08-13 10:00               ` Mark Brown
  0 siblings, 0 replies; 17+ messages in thread
From: Mark Brown @ 2009-08-13 10:00 UTC (permalink / raw)
  To: Barry Song; +Cc: uclinux-dist-devel, alsa-devel

On Thu, Aug 13, 2009 at 05:52:42PM +0800, Barry Song wrote:

> Well. If all chips need this same operation, can it be abstracted to
> the soc core layer?

The details of the register cache operation are entirely in the domain
of the chip - it's not mandatory to have a register cache at all and
some devices have complex register layouts which cause problems with
doing this in a fully automated fashion.  There are also issues with
some devices having things like soft reset registers in the middle of
their register map.

That said, there is some work in this direction - see soc-cache.c.

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2009-08-13 10:00 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-08-12  4:32 [PATCH 0/3] patch sequence adds analog devices ad1836 asoc support Barry Song
2009-08-12  4:32 ` [PATCH 1/3] new ad1836 codec driver based on asoc Barry Song
2009-08-12  4:32   ` [PATCH 2/3] add set_tdm_slot in tdm dai to define the relationship between audio channel and tdm slot Barry Song
2009-08-12  4:32     ` [PATCH 3/3] new board driver to connect bfin-5xx with ad1836 codec Barry Song
2009-08-12 16:10       ` Mark Brown
2009-08-13  4:19         ` Barry Song
2009-08-12 13:56     ` [PATCH 2/3] add set_tdm_slot in tdm dai to define the relationship between audio channel and tdm slot Mark Brown
2009-08-13  4:16       ` Barry Song
2009-08-12 13:46   ` [PATCH 1/3] new ad1836 codec driver based on asoc Mark Brown
2009-08-13  3:41     ` Barry Song
2009-08-13  9:08       ` Mark Brown
2009-08-13  9:22         ` [Uclinux-dist-devel] " Mike Frysinger
2009-08-13  9:34           ` Mike Frysinger
2009-08-13  9:37         ` Barry Song
2009-08-13  9:46           ` Mark Brown
2009-08-13  9:52             ` Barry Song
2009-08-13 10:00               ` Mark Brown

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.