[PATCH 1/3] ASoC: Add ADAU1701 codec driver

[PATCH 1/3] ASoC: Add ADAU1701 codec driver

[Lars-Peter Clausen]

This patch adds support for the Analog Devices ADAU1701 SigmaDSP.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 sound/soc/codecs/Kconfig    |    7 +-
 sound/soc/codecs/Makefile   |    2 +
 sound/soc/codecs/adau1701.c |  547 +++++++++++++++++++++++++++++++++++++++++++
 sound/soc/codecs/adau1701.h |   17 ++
 4 files changed, 572 insertions(+), 1 deletions(-)
 create mode 100644 sound/soc/codecs/adau1701.c
 create mode 100644 sound/soc/codecs/adau1701.h

diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 98175a0..f745e55 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -17,6 +17,7 @@ config SND_SOC_ALL_CODECS
 	select SND_SOC_AD193X if SND_SOC_I2C_AND_SPI
 	select SND_SOC_AD1980 if SND_SOC_AC97_BUS
 	select SND_SOC_AD73311
+	select SND_SOC_ADAU1701 if I2C
 	select SND_SOC_ADS117X
 	select SND_SOC_AK4104 if SPI_MASTER
 	select SND_SOC_AK4535 if I2C
@@ -130,7 +131,11 @@ config SND_SOC_AD1980
 
 config SND_SOC_AD73311
 	tristate
-	
+
+config SND_SOC_ADAU1701
+	select SIGMA
+	tristate
+
 config SND_SOC_ADS117X
 	tristate
 
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index fd85584..30a4c63 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -4,6 +4,7 @@ snd-soc-ad1836-objs := ad1836.o
 snd-soc-ad193x-objs := ad193x.o
 snd-soc-ad1980-objs := ad1980.o
 snd-soc-ad73311-objs := ad73311.o
+snd-soc-adau1701-objs := adau1701.o
 snd-soc-ads117x-objs := ads117x.o
 snd-soc-ak4104-objs := ak4104.o
 snd-soc-ak4535-objs := ak4535.o
@@ -95,6 +96,7 @@ obj-$(CONFIG_SND_SOC_AD1836)	+= snd-soc-ad1836.o
 obj-$(CONFIG_SND_SOC_AD193X)	+= snd-soc-ad193x.o
 obj-$(CONFIG_SND_SOC_AD1980)	+= snd-soc-ad1980.o
 obj-$(CONFIG_SND_SOC_AD73311) += snd-soc-ad73311.o
+obj-$(CONFIG_SND_SOC_ADAU1701)  += snd-soc-adau1701.o
 obj-$(CONFIG_SND_SOC_ADS117X)	+= snd-soc-ads117x.o
 obj-$(CONFIG_SND_SOC_AK4104)	+= snd-soc-ak4104.o
 obj-$(CONFIG_SND_SOC_AK4535)	+= snd-soc-ak4535.o
diff --git a/sound/soc/codecs/adau1701.c b/sound/soc/codecs/adau1701.c
new file mode 100644
index 0000000..df2b1c3
--- /dev/null
+++ b/sound/soc/codecs/adau1701.c
@@ -0,0 +1,547 @@
+/*
+ * Driver for ADAU1701 SigmaDSP processor
+ *
+ * Copyright 2011 Analog Devices Inc.
+ * Author: Lars-Peter Clausen <lars@metafoo.de>
+ *	based on an inital version by Cliff Cai <cliff.cai@analog.com>
+ *
+ * Licensed under the GPL-2 or later.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/i2c.h>
+#include <linux/delay.h>
+#include <linux/sigma.h>
+#include <linux/slab.h>
+#include <sound/core.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+
+#include "adau1701.h"
+
+#define ADAU1701_DSPCTRL	0x1c
+#define ADAU1701_SEROCTL	0x1e
+#define ADAU1701_SERICTL	0x1f
+
+#define ADAU1701_AUXNPOW	0x22
+
+#define ADAU1701_OSCIPOW	0x26
+#define ADAU1701_DACSET		0x27
+
+#define ADAU1701_NUM_REGS	0x28
+
+#define ADAU1701_DSPCTRL_CR		(1 << 2)
+#define ADAU1701_DSPCTRL_DAM		(1 << 3)
+#define ADAU1701_DSPCTRL_ADM		(1 << 4)
+#define ADAU1701_DSPCTRL_SR_48		0x00
+#define ADAU1701_DSPCTRL_SR_96		0x01
+#define ADAU1701_DSPCTRL_SR_192		0x02
+#define ADAU1701_DSPCTRL_SR_MASK	0x03
+
+#define ADAU1701_SEROCTL_INV_LRCLK	0x2000
+#define ADAU1701_SEROCTL_INV_BCLK	0x1000
+#define ADAU1701_SEROCTL_MASTER		0x0800
+
+#define ADAU1701_SEROCTL_OBF16		0x0000
+#define ADAU1701_SEROCTL_OBF8		0x0200
+#define ADAU1701_SEROCTL_OBF4		0x0400
+#define ADAU1701_SEROCTL_OBF2		0x0600
+#define ADAU1701_SEROCTL_OBF_MASK	0x0600
+
+#define ADAU1701_SEROCTL_OLF1024	0x0000
+#define ADAU1701_SEROCTL_OLF512		0x0080
+#define ADAU1701_SEROCTL_OLF256		0x0100
+#define ADAU1701_SEROCTL_OLF_MASK	0x0180
+
+#define ADAU1701_SEROCTL_MSB_DEALY1	0x0000
+#define ADAU1701_SEROCTL_MSB_DEALY0	0x0004
+#define ADAU1701_SEROCTL_MSB_DEALY8	0x0008
+#define ADAU1701_SEROCTL_MSB_DEALY12	0x000c
+#define ADAU1701_SEROCTL_MSB_DEALY16	0x0010
+#define ADAU1701_SEROCTL_MSB_DEALY_MASK	0x001c
+
+#define ADAU1701_SEROCTL_WORD_LEN_24	0x0000
+#define ADAU1701_SEROCTL_WORD_LEN_20	0x0001
+#define ADAU1701_SEROCTL_WORD_LEN_16	0x0010
+#define ADAU1701_SEROCTL_WORD_LEN_MASK	0x0003
+
+#define ADAU1701_AUXNPOW_VBPD		0x40
+#define ADAU1701_AUXNPOW_VRPD		0x20
+
+#define ADAU1701_SERICTL_I2S		0
+#define ADAU1701_SERICTL_LEFTJ		1
+#define ADAU1701_SERICTL_TDM		2
+#define ADAU1701_SERICTL_RIGHTJ_24	3
+#define ADAU1701_SERICTL_RIGHTJ_20	4
+#define ADAU1701_SERICTL_RIGHTJ_18	5
+#define ADAU1701_SERICTL_RIGHTJ_16	6
+#define ADAU1701_SERICTL_MODE_MASK	7
+#define ADAU1701_SERICTL_INV_BCLK	BIT(3)
+#define ADAU1701_SERICTL_INV_LRCLK	BIT(4)
+
+#define ADAU1701_OSCIPOW_OPD		0x04
+#define ADAU1701_DACSET_DACINIT		1
+
+#define ADAU1701_FIRMWARE "adau1701.bin"
+
+struct adau1701 {
+	unsigned int dai_fmt;
+};
+
+static const struct snd_kcontrol_new adau1701_controls[] = {
+	SOC_SINGLE("Master Capture Switch", ADAU1701_DSPCTRL, 4, 1, 0),
+};
+
+static const struct snd_soc_dapm_widget adau1701_dapm_widgets[] = {
+	SND_SOC_DAPM_DAC("DAC0", "Playback", ADAU1701_AUXNPOW, 3, 1),
+	SND_SOC_DAPM_DAC("DAC1", "Playback", ADAU1701_AUXNPOW, 2, 1),
+	SND_SOC_DAPM_DAC("DAC2", "Playback", ADAU1701_AUXNPOW, 1, 1),
+	SND_SOC_DAPM_DAC("DAC3", "Playback", ADAU1701_AUXNPOW, 0, 1),
+	SND_SOC_DAPM_ADC("ADC", "Capture", ADAU1701_AUXNPOW, 7, 1),
+
+	SND_SOC_DAPM_OUTPUT("OUT0"),
+	SND_SOC_DAPM_OUTPUT("OUT1"),
+	SND_SOC_DAPM_OUTPUT("OUT2"),
+	SND_SOC_DAPM_OUTPUT("OUT3"),
+	SND_SOC_DAPM_INPUT("IN0"),
+	SND_SOC_DAPM_INPUT("IN1"),
+};
+
+static const struct snd_soc_dapm_route adau1701_dapm_routes[] = {
+	{ "OUT0", NULL, "DAC0" },
+	{ "OUT1", NULL, "DAC1" },
+	{ "OUT2", NULL, "DAC2" },
+	{ "OUT3", NULL, "DAC3" },
+
+	{ "ADC", NULL, "IN0" },
+	{ "ADC", NULL, "IN1" },
+};
+
+static unsigned int adau1701_register_size(unsigned int reg)
+{
+	switch (reg) {
+	case ADAU1701_DSPCTRL:
+	case ADAU1701_SEROCTL:
+	case ADAU1701_AUXNPOW:
+	case ADAU1701_OSCIPOW:
+	case ADAU1701_DACSET:
+		return 2;
+	case ADAU1701_SERICTL:
+		return 1;
+	}
+
+	return 0;
+}
+
+static int adau1701_write(struct snd_soc_codec *codec, unsigned int reg,
+		unsigned int value)
+{
+	unsigned int i, ret;
+	unsigned int size;
+	uint8_t buf[4];
+
+	size = adau1701_register_size(reg);
+	if (size == 0) {
+		dev_err(codec->dev, "Unsupported register address: %d\n", reg);
+		return -EINVAL;
+	}
+
+	snd_soc_cache_write(codec, reg, value);
+
+	buf[0] = 0x08;
+	buf[1] = reg;
+
+	for (i = size + 1; i >= 2; --i) {
+		buf[i] = value;
+		value >>= 8;
+	}
+
+	ret = i2c_master_send(to_i2c_client(codec->dev), buf, size + 2);
+	if (ret == size + 2)
+		return 0;
+	else if (ret < 0)
+		return ret;
+	else
+		return -EIO;
+}
+
+static unsigned int adau1701_read(struct snd_soc_codec *codec, unsigned int reg)
+{
+	unsigned int value;
+	unsigned int ret;
+
+	ret = snd_soc_cache_read(codec, reg, &value);
+	if (ret)
+		return ret;
+
+	return value;
+}
+
+static int adau1701_load_firmware(struct snd_soc_codec *codec)
+{
+	return process_sigma_firmware(codec->control_data, ADAU1701_FIRMWARE);
+}
+
+static int adau1701_set_capture_pcm_format(struct snd_soc_codec *codec,
+		snd_pcm_format_t format)
+{
+	struct adau1701 *adau1701 = snd_soc_codec_get_drvdata(codec);
+	unsigned int mask = ADAU1701_SEROCTL_WORD_LEN_MASK;
+	unsigned int val;
+
+	switch (format) {
+	case SNDRV_PCM_FORMAT_S16_LE:
+		val = ADAU1701_SEROCTL_WORD_LEN_16;
+		break;
+	case SNDRV_PCM_FORMAT_S20_3LE:
+		val = ADAU1701_SEROCTL_WORD_LEN_20;
+		break;
+	case SNDRV_PCM_FORMAT_S24_LE:
+		val = ADAU1701_SEROCTL_WORD_LEN_24;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (adau1701->dai_fmt == SND_SOC_DAIFMT_RIGHT_J) {
+		switch (format) {
+		case SNDRV_PCM_FORMAT_S16_LE:
+			val |= ADAU1701_SEROCTL_MSB_DEALY16;
+			break;
+		case SNDRV_PCM_FORMAT_S20_3LE:
+			val |= ADAU1701_SEROCTL_MSB_DEALY12;
+			break;
+		case SNDRV_PCM_FORMAT_S24_LE:
+			val |= ADAU1701_SEROCTL_MSB_DEALY8;
+			break;
+		}
+		mask |= ADAU1701_SEROCTL_MSB_DEALY_MASK;
+	}
+
+	snd_soc_update_bits(codec, ADAU1701_SEROCTL, mask, val);
+
+	return 0;
+}
+
+static int adau1701_set_playback_pcm_format(struct snd_soc_codec *codec,
+			snd_pcm_format_t format)
+{
+	struct adau1701 *adau1701 = snd_soc_codec_get_drvdata(codec);
+	unsigned int val;
+
+	if (adau1701->dai_fmt != SND_SOC_DAIFMT_RIGHT_J)
+		return 0;
+
+	switch (format) {
+	case SNDRV_PCM_FORMAT_S16_LE:
+		val = ADAU1701_SERICTL_RIGHTJ_16;
+		break;
+	case SNDRV_PCM_FORMAT_S20_3LE:
+		val = ADAU1701_SERICTL_RIGHTJ_20;
+		break;
+	case SNDRV_PCM_FORMAT_S24_LE:
+		val = ADAU1701_SERICTL_RIGHTJ_24;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	snd_soc_update_bits(codec, ADAU1701_SERICTL,
+		ADAU1701_SERICTL_MODE_MASK, val);
+
+	return 0;
+}
+
+static int adau1701_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;
+	snd_pcm_format_t format;
+	unsigned int val;
+
+	switch (params_rate(params)) {
+	case 192000:
+		val = ADAU1701_DSPCTRL_SR_192;
+		break;
+	case 96000:
+		val = ADAU1701_DSPCTRL_SR_96;
+		break;
+	case 48000:
+		val = ADAU1701_DSPCTRL_SR_48;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	snd_soc_update_bits(codec, ADAU1701_DSPCTRL,
+		ADAU1701_DSPCTRL_SR_MASK, val);
+
+	format = params_format(params);
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+		return adau1701_set_playback_pcm_format(codec, format);
+	else
+		return adau1701_set_capture_pcm_format(codec, format);
+}
+
+static int adau1701_set_dai_fmt(struct snd_soc_dai *codec_dai,
+		unsigned int fmt)
+{
+	struct snd_soc_codec *codec = codec_dai->codec;
+	struct adau1701 *adau1701 = snd_soc_codec_get_drvdata(codec);
+	unsigned int serictl = 0x00, seroctl = 0x00;
+	bool invert_lrclk;
+
+	switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
+	case SND_SOC_DAIFMT_CBM_CFM:
+		/* master, 64-bits per sample, 1 frame per sample */
+		seroctl |= ADAU1701_SEROCTL_MASTER | ADAU1701_SEROCTL_OBF16
+				| ADAU1701_SEROCTL_OLF1024;
+		break;
+	case SND_SOC_DAIFMT_CBS_CFS:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/* clock inversion */
+	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
+	case SND_SOC_DAIFMT_NB_NF:
+		invert_lrclk = false;
+		break;
+	case SND_SOC_DAIFMT_NB_IF:
+		invert_lrclk = true;
+		break;
+	case SND_SOC_DAIFMT_IB_NF:
+		invert_lrclk = false;
+		serictl |= ADAU1701_SERICTL_INV_BCLK;
+		seroctl |= ADAU1701_SEROCTL_INV_BCLK;
+		break;
+	case SND_SOC_DAIFMT_IB_IF:
+		invert_lrclk = true;
+		serictl |= ADAU1701_SERICTL_INV_BCLK;
+		seroctl |= ADAU1701_SEROCTL_INV_BCLK;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+	case SND_SOC_DAIFMT_I2S:
+		break;
+	case SND_SOC_DAIFMT_LEFT_J:
+		serictl |= ADAU1701_SERICTL_LEFTJ;
+		seroctl |= ADAU1701_SEROCTL_MSB_DEALY0;
+		invert_lrclk = !invert_lrclk;
+		break;
+	case SND_SOC_DAIFMT_RIGHT_J:
+		serictl |= ADAU1701_SERICTL_RIGHTJ_24;
+		seroctl |= ADAU1701_SEROCTL_MSB_DEALY8;
+		invert_lrclk = !invert_lrclk;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (invert_lrclk) {
+		seroctl |= ADAU1701_SEROCTL_INV_LRCLK;
+		serictl |= ADAU1701_SERICTL_INV_LRCLK;
+	}
+
+	adau1701->dai_fmt = fmt & SND_SOC_DAIFMT_FORMAT_MASK;
+
+	snd_soc_write(codec, ADAU1701_SERICTL, serictl);
+	snd_soc_update_bits(codec, ADAU1701_SEROCTL,
+		~ADAU1701_SEROCTL_WORD_LEN_MASK, seroctl);
+
+	return 0;
+}
+
+static int adau1701_set_bias_level(struct snd_soc_codec *codec,
+		enum snd_soc_bias_level level)
+{
+	unsigned int mask = ADAU1701_AUXNPOW_VBPD | ADAU1701_AUXNPOW_VRPD;
+
+	switch (level) {
+	case SND_SOC_BIAS_ON:
+		break;
+	case SND_SOC_BIAS_PREPARE:
+		break;
+	case SND_SOC_BIAS_STANDBY:
+		/* Enable VREF and VREF buffer */
+		snd_soc_update_bits(codec, ADAU1701_AUXNPOW, mask, 0x00);
+		break;
+	case SND_SOC_BIAS_OFF:
+		/* Disable VREF and VREF buffer */
+		snd_soc_update_bits(codec, ADAU1701_AUXNPOW, mask, mask);
+		break;
+	}
+
+	codec->dapm.bias_level = level;
+	return 0;
+}
+
+static int adau1701_digital_mute(struct snd_soc_dai *dai, int mute)
+{
+	struct snd_soc_codec *codec = dai->codec;
+	unsigned int mask = ADAU1701_DSPCTRL_DAM;
+	unsigned int val;
+
+	if (mute)
+		val = 0;
+	else
+		val = mask;
+
+	snd_soc_update_bits(codec, ADAU1701_DSPCTRL, mask, val);
+
+	return 0;
+}
+
+static int adau1701_set_sysclk(struct snd_soc_codec *codec, int clk_id,
+	unsigned int freq, int dir)
+{
+	unsigned int val;
+
+	switch (clk_id) {
+	case ADAU1701_CLK_SRC_OSC:
+		val = 0x0;
+		break;
+	case ADAU1701_CLK_SRC_MCLK:
+		val = ADAU1701_OSCIPOW_OPD;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	snd_soc_update_bits(codec, ADAU1701_OSCIPOW, ADAU1701_OSCIPOW_OPD, val);
+
+	return 0;
+}
+
+#define ADAU1701_RATES (SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_96000 | \
+	SNDRV_PCM_RATE_192000)
+
+#define ADAU1701_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE |\
+	SNDRV_PCM_FMTBIT_S24_LE)
+
+static const struct snd_soc_dai_ops adau1701_dai_ops = {
+	.set_fmt	= adau1701_set_dai_fmt,
+	.hw_params	= adau1701_hw_params,
+	.digital_mute	= adau1701_digital_mute,
+};
+
+static struct snd_soc_dai_driver adau1701_dai = {
+	.name = "adau1701",
+	.playback = {
+		.stream_name = "Playback",
+		.channels_min = 2,
+		.channels_max = 8,
+		.rates = ADAU1701_RATES,
+		.formats = ADAU1701_FORMATS,
+	},
+	.capture = {
+		.stream_name = "Capture",
+		.channels_min = 2,
+		.channels_max = 8,
+		.rates = ADAU1701_RATES,
+		.formats = ADAU1701_FORMATS,
+	},
+	.ops = &adau1701_dai_ops,
+	.symmetric_rates = 1,
+};
+
+static int adau1701_probe(struct snd_soc_codec *codec)
+{
+	int ret;
+
+	codec->dapm.idle_bias_off = 1;
+
+	ret = adau1701_load_firmware(codec);
+	if (ret)
+		dev_warn(codec->dev, "Failed to load firmware\n");
+
+	snd_soc_write(codec, ADAU1701_DACSET, ADAU1701_DACSET_DACINIT);
+	snd_soc_write(codec, ADAU1701_DSPCTRL, ADAU1701_DSPCTRL_CR);
+
+	return 0;
+}
+
+static struct snd_soc_codec_driver adau1701_codec_drv = {
+	.probe			= adau1701_probe,
+	.set_bias_level		= adau1701_set_bias_level,
+
+	.reg_cache_size		= ADAU1701_NUM_REGS,
+	.reg_word_size		= sizeof(u16),
+
+	.controls		= adau1701_controls,
+	.num_controls		= ARRAY_SIZE(adau1701_controls),
+	.dapm_widgets		= adau1701_dapm_widgets,
+	.num_dapm_widgets	= ARRAY_SIZE(adau1701_dapm_widgets),
+	.dapm_routes		= adau1701_dapm_routes,
+	.num_dapm_routes	= ARRAY_SIZE(adau1701_dapm_routes),
+
+	.write			= adau1701_write,
+	.read			= adau1701_read,
+
+	.set_sysclk		= adau1701_set_sysclk,
+};
+
+static __devinit int adau1701_i2c_probe(struct i2c_client *client,
+		const struct i2c_device_id *id)
+{
+	struct adau1701 *adau1701;
+	int ret;
+
+	adau1701 = kzalloc(sizeof(*adau1701), GFP_KERNEL);
+	if (!adau1701)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, adau1701);
+	ret = snd_soc_register_codec(&client->dev, &adau1701_codec_drv,
+			&adau1701_dai, 1);
+	if (ret < 0)
+		kfree(adau1701);
+
+	return ret;
+}
+
+static __devexit int adau1701_i2c_remove(struct i2c_client *client)
+{
+	snd_soc_unregister_codec(&client->dev);
+	kfree(i2c_get_clientdata(client));
+	return 0;
+}
+
+static const struct i2c_device_id adau1701_i2c_id[] = {
+	{ "adau1701", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, adau1701_i2c_id);
+
+static struct i2c_driver adau1701_i2c_driver = {
+	.driver = {
+		.name	= "adau1701",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= adau1701_i2c_probe,
+	.remove		= __devexit_p(adau1701_i2c_remove),
+	.id_tabl	= adau1701_i2c_id,
+};
+
+static int __init adau1701_init(void)
+{
+	return i2c_add_driver(&adau1701_i2c_driver);
+}
+module_init(adau1701_init);
+
+static void __exit adau1701_exit(void)
+{
+	i2c_del_driver(&adau1701_i2c_driver);
+}
+module_exit(adau1701_exit);
+
+MODULE_DESCRIPTION("ASoC ADAU1701 SigmaDSP driver");
+MODULE_AUTHOR("Lars-Peter Clausen <lars@metafoo.de>");
+MODULE_LICENSE("GPL");
diff --git a/sound/soc/codecs/adau1701.h b/sound/soc/codecs/adau1701.h
new file mode 100644
index 0000000..8d0949a
--- /dev/null
+++ b/sound/soc/codecs/adau1701.h
@@ -0,0 +1,17 @@
+/*
+ * header file for ADAU1701 SigmaDSP processor
+ *
+ * Copyright 2011 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2 or later.
+ */
+
+#ifndef _ADAU1701_H
+#define _ADAU1701_H
+
+enum adau1701_clk_src {
+	ADAU1701_CLK_SRC_OSC,
+	ADAU1701_CLK_SRC_MCLK,
+};
+
+#endif
-- 
1.7.2.5

[PATCH 2/3] ASoC: Blackfin: Add bf5xx-adau1701 machine driver

[Lars-Peter Clausen]

Add a machine driver to support the ADAU1701 SigmaDSP processors on
Analog Devices BF5XX evaluation boards.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Cc: Mike Frysinger <vapier@gentoo.org>
---
 sound/soc/blackfin/Kconfig          |   10 +++
 sound/soc/blackfin/Makefile         |    2 +
 sound/soc/blackfin/bf5xx-adau1701.c |  118 +++++++++++++++++++++++++++++++++++
 3 files changed, 130 insertions(+), 0 deletions(-)
 create mode 100644 sound/soc/blackfin/bf5xx-adau1701.c

diff --git a/sound/soc/blackfin/Kconfig b/sound/soc/blackfin/Kconfig
index ae40359..0777210 100644
--- a/sound/soc/blackfin/Kconfig
+++ b/sound/soc/blackfin/Kconfig
@@ -17,6 +17,16 @@ config SND_BF5XX_SOC_SSM2602
 	help
 	  Say Y if you want to add support for SoC audio on BF527-EZKIT.
 
+config SND_BF5XX_SOC_ADAU1701
+	tristate "SoC ADAU1701 Audio support for BF5xx eval boards"
+	depends on SND_BF5XX_I2S
+	select SND_BF5XX_SOC_I2S
+	select SND_SOC_ADAU1701
+	select I2C
+	help
+	  Say Y if you want to add support for the ADAU1701 SigmaDSP processor on
+	  Analog Devices BF5xx evaluation boards.
+
 config SND_BF5XX_SOC_AD73311
 	tristate "SoC AD73311 Audio support for Blackfin"
 	depends on SND_BF5XX_I2S
diff --git a/sound/soc/blackfin/Makefile b/sound/soc/blackfin/Makefile
index 49af3f3..1d75173 100644
--- a/sound/soc/blackfin/Makefile
+++ b/sound/soc/blackfin/Makefile
@@ -21,9 +21,11 @@ snd-ad1980-objs := bf5xx-ad1980.o
 snd-ssm2602-objs := bf5xx-ssm2602.o
 snd-ad73311-objs := bf5xx-ad73311.o
 snd-ad193x-objs := bf5xx-ad193x.o
+snd-adau1701-objs := bf5xx-adau1701.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
 obj-$(CONFIG_SND_BF5XX_SOC_AD193X) += snd-ad193x.o
+obj-$(CONFIG_SND_BF5XX_SOC_ADAU1701) += snd-adau1701.o
diff --git a/sound/soc/blackfin/bf5xx-adau1701.c b/sound/soc/blackfin/bf5xx-adau1701.c
new file mode 100644
index 0000000..dcdd6cd
--- /dev/null
+++ b/sound/soc/blackfin/bf5xx-adau1701.c
@@ -0,0 +1,118 @@
+/*
+ * Machine driver for ADAU1701 SigmaDSP processor on Analog Devices BF5XX
+ * evaluation boards.
+ *
+ * Copyright 2011 Analog Devices Inc.
+ * Author: Lars-Peter Clausen <lars@metafoo.de>
+ *
+ * Licensed under the GPL-2 or later.
+ */
+
+#include <linux/module.h>
+#include <linux/device.h>
+#include <sound/core.h>
+#include <sound/pcm.h>
+#include <sound/soc.h>
+#include <sound/pcm_params.h>
+
+#include "../codecs/adau1701.h"
+
+static int bf5xx_adau1701_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->cpu_dai;
+	struct snd_soc_dai *codec_dai = rtd->codec_dai;
+	int ret;
+
+	ret = snd_soc_dai_set_fmt(cpu_dai, SND_SOC_DAIFMT_I2S |
+			SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBM_CFM);
+	if (ret)
+		return ret;
+
+	ret = snd_soc_dai_set_fmt(codec_dai, SND_SOC_DAIFMT_I2S |
+			SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBM_CFM);
+	if (ret)
+		return ret;
+
+	ret = snd_soc_dai_set_sysclk(codec_dai, ADAU1701_CLK_SRC_OSC, 12288000,
+			SND_SOC_CLOCK_IN);
+
+	return ret;
+}
+
+static struct snd_soc_ops bf5xx_adau1701_ops = {
+	.hw_params = bf5xx_adau1701_hw_params,
+};
+
+static struct snd_soc_dai_link bf5xx_adau1701_dai[] = {
+	{
+		.name = "adau1701",
+		.stream_name = "adau1701",
+		.cpu_dai_name = "bfin-i2s.0",
+		.codec_dai_name = "adau1701",
+		.platform_name = "bfin-i2s-pcm-audio",
+		.codec_name = "adau1701.0-0034",
+		.ops = &bf5xx_adau1701_ops,
+	},
+	{
+		.name = "adau1701",
+		.stream_name = "adau1701",
+		.cpu_dai_name = "bfin-i2s.1",
+		.codec_dai_name = "adau1701",
+		.platform_name = "bfin-i2s-pcm-audio",
+		.codec_name = "adau1701.0-0034",
+		.ops = &bf5xx_adau1701_ops,
+	},
+};
+
+static struct snd_soc_card bf5xx_adau1701 = {
+	.name = "bfin-adau1701",
+	.dai_link = &bf5xx_adau1701_dai[CONFIG_SND_BF5XX_SPORT_NUM],
+	.num_links = 1,
+};
+
+static int bf5xx_adau1701_probe(struct platform_device *pdev)
+{
+	struct snd_soc_card *card = &bf5xx_adau1701;
+
+	card->dev = &pdev->dev;
+
+	return snd_soc_register_card(&bf5xx_adau1701);
+}
+
+static int __devexit bf5xx_adau1701_remove(struct platform_device *pdev)
+{
+	struct snd_soc_card *card = platform_get_drvdata(pdev);
+
+	snd_soc_unregister_card(card);
+
+	return 0;
+}
+
+static struct platform_driver bf5xx_adau1701_driver = {
+	.driver = {
+		.name = "bf5xx-adau1701",
+		.owner = THIS_MODULE,
+		.pm = &snd_soc_pm_ops,
+	},
+	.probe = bf5xx_adau1701_probe,
+	.remove = __devexit_p(bf5xx_adau1701_remove),
+};
+
+static int __init bf5xx_adau1701_init(void)
+{
+	return platform_driver_register(&bf5xx_adau1701_driver);
+}
+module_init(bf5xx_adau1701_init);
+
+static void __exit bf5xx_adau1701_exit(void)
+{
+	platform_driver_unregister(&bf5xx_adau1701_driver);
+}
+module_exit(bf5xx_adau1701_exit);
+
+MODULE_AUTHOR("Lars-Peter Clausen <lars@metafoo.de>");
+MODULE_DESCRIPTION("ALSA SoC BF5XX ADAU1701 driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:bfin-adau1701");
-- 
1.7.2.5

[PATCH 3/3] Blackfin: bf537: Stamp: Register adau1701 codec and ASoC machine driver

[Lars-Peter Clausen]

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Cc: Mike Frysinger <vapier@gentoo.org>
---
 arch/blackfin/mach-bf537/boards/stamp.c |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/arch/blackfin/mach-bf537/boards/stamp.c b/arch/blackfin/mach-bf537/boards/stamp.c
index 76db1d4..1365cbd 100644
--- a/arch/blackfin/mach-bf537/boards/stamp.c
+++ b/arch/blackfin/mach-bf537/boards/stamp.c
@@ -2390,6 +2390,11 @@ static struct i2c_board_info __initdata bfin_i2c_board_info[] = {
 		I2C_BOARD_INFO("adau1361", 0x38),
 	},
 #endif
+#if defined(CONFIG_SND_SOC_ADAU1701) || defined(CONFIG_SND_SOC_ADAU1701_MODULE)
+	{
+		I2C_BOARD_INFO("adau1701", 0x34),
+	},
+#endif
 #if defined(CONFIG_AD525X_DPOT) || defined(CONFIG_AD525X_DPOT_MODULE)
 	{
 		I2C_BOARD_INFO("ad5258", 0x18),
@@ -2760,6 +2765,13 @@ static struct platform_device iio_gpio_trigger = {
 };
 #endif
 
+#if defined(CONFIG_SND_BF5XX_SOC_ADAU1701) || \
+	defined(CONFIG_SND_BF5XX_SOC_ADAU1701_MODULE)
+static struct platform_device bf5xx_adau1701_device = {
+	.name = "bf5xx-adau1701",
+};
+#endif
+
 static struct platform_device *stamp_devices[] __initdata = {
 
 	&bfin_dpmc,
@@ -2914,6 +2926,11 @@ static struct platform_device *stamp_devices[] __initdata = {
 	defined(CONFIG_IIO_GPIO_TRIGGER_MODULE)
 	&iio_gpio_trigger,
 #endif
+
+#if defined(CONFIG_SND_BF5XX_SOC_ADAU1701) || \
+	defined(CONFIG_SND_BF5XX_SOC_ADAU1701_MODULE)
+	&bf5xx_adau1701_device,
+#endif
 };
 
 static int __init stamp_init(void)
-- 
1.7.2.5

Re: [alsa-devel] [PATCH 1/3] ASoC: Add ADAU1701 codec driver

[Mark Brown]

On Fri, Jun 10, 2011 at 07:18:49PM +0200, Lars-Peter Clausen wrote:
> This patch adds support for the Analog Devices ADAU1701 SigmaDSP.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>

Looks OK but one thing it'd be nice to do:

> +static unsigned int adau1701_register_size(unsigned int reg)
> +{
> +	switch (reg) {
> +	case ADAU1701_DSPCTRL:
> +	case ADAU1701_SEROCTL:
> +	case ADAU1701_AUXNPOW:
> +	case ADAU1701_OSCIPOW:
> +	case ADAU1701_DACSET:
> +		return 2;
> +	case ADAU1701_SERICTL:
> +		return 1;
> +	}
> +
> +	return 0;
> +}

I'd be happier if this complained loudly when it fell through the
switch in case something goes wrong.  You've got checks in the callers
but I'd feel safer if the log were here in case we missed a caller.

Also someone should remonstrate with the hardware designers.

Re: [alsa-devel] [PATCH 2/3] ASoC: Blackfin: Add bf5xx-adau1701 machine driver

[Mark Brown]

On Fri, Jun 10, 2011 at 07:18:50PM +0200, Lars-Peter Clausen wrote:
> Add a machine driver to support the ADAU1701 SigmaDSP processors on
> Analog Devices BF5XX evaluation boards.

So, I keep on complaining about the way these drivers are just generic
to any random Blackfin plus CODEC combination rather than being board
specific.  It'd be good if we could improve this, even adding something
into the driver name to make it clear these are for the EVB would help. 

But it's about the same as all the other drivers so...

> +static struct snd_soc_dai_link bf5xx_adau1701_dai[] = {
> +	{
> +		.name = "adau1701",
> +		.stream_name = "adau1701",
> +		.cpu_dai_name = "bfin-i2s.0",
> +		.codec_dai_name = "adau1701",
> +		.platform_name = "bfin-i2s-pcm-audio",
> +		.codec_name = "adau1701.0-0034",
> +		.ops = &bf5xx_adau1701_ops,
> +	},
> +	{
> +		.name = "adau1701",
> +		.stream_name = "adau1701",
> +		.cpu_dai_name = "bfin-i2s.1",
> +		.codec_dai_name = "adau1701",
> +		.platform_name = "bfin-i2s-pcm-audio",
> +		.codec_name = "adau1701.0-0034",
> +		.ops = &bf5xx_adau1701_ops,
> +	},

For example this mapping could vary between systems, and I guess there
may be some possiblity that the CODEC I2C address could change.

Re: [Device-drivers-devel] [PATCH 1/3] ASoC: Add ADAU1701 codec driver

[Mike Frysinger]

On Fri, Jun 10, 2011 at 13:18, Lars-Peter Clausen wrote:
> +MODULE_AUTHOR("Lars-Peter Clausen <lars@metafoo.de>");

did you actually rewrite this thing from scratch ?  seems like you
should keep Cliff as the author in git/MODULE_AUTHOR, and then add
your name to the s-o-b list and this macro.

friendly git commits can easily be cherry picked from my tree for asoc stuff:
http://git.kernel.org/?p=linux/kernel/git/vapier/blackfin.git;a=summary
(see the "trunk" branch)
-mike

Re: [alsa-devel] [PATCH 1/3] ASoC: Add ADAU1701 codec driver

[Mike Frysinger]

On Fri, Jun 10, 2011 at 13:27, Mark Brown wrote:
> Also someone should remonstrate with the hardware designers.

if there's things we can take back, i can forward them along.  the
last conversation i had with them sounded like they have no problem
accepting feedback.
-mike

Re: [Device-drivers-devel] [alsa-devel] [PATCH 2/3] ASoC: Blackfin: Add bf5xx-adau1701 machine driver

[Mike Frysinger]

On Fri, Jun 10, 2011 at 13:30, Mark Brown wrote:
> On Fri, Jun 10, 2011 at 07:18:50PM +0200, Lars-Peter Clausen wrote:
>> Add a machine driver to support the ADAU1701 SigmaDSP processors on
>> Analog Devices BF5XX evaluation boards.
>
> So, I keep on complaining about the way these drivers are just generic
> to any random Blackfin plus CODEC combination rather than being board
> specific.  It'd be good if we could improve this, even adding something
> into the driver name to make it clear these are for the EVB would help.

i know you keep complaining, but i honestly dont understand why this
is undesirable.  connecting a codec to a Blackfin is pretty much
always the same.  you pick a SPORT # and that's about it.

the spi cs and i2c address could differ (so maybe make that a field
for the platform resources), but otherwise i dont see why people
should have to copy & paste the same code to change all of 4 bytes.
-mike

Re: [Device-drivers-devel] [PATCH 3/3] Blackfin: bf537: Stamp: Register adau1701 codec and ASoC machine driver

[Mike Frysinger]

looks straightforward.  once the codec/machine driver get merged, i'll
run this through my Blackfin tree of course.
-mike

Re: [alsa-devel] [PATCH 2/3] ASoC: Blackfin: Add bf5xx-adau1701 machine driver

[Lars-Peter Clausen]

On 06/10/2011 07:30 PM, Mark Brown wrote:
> On Fri, Jun 10, 2011 at 07:18:50PM +0200, Lars-Peter Clausen wrote:
>> Add a machine driver to support the ADAU1701 SigmaDSP processors on
>> Analog Devices BF5XX evaluation boards.
> 
> So, I keep on complaining about the way these drivers are just generic
> to any random Blackfin plus CODEC combination rather than being board
> specific.  It'd be good if we could improve this, even adding something
> into the driver name to make it clear these are for the EVB would help.

I think this is due, that the codecs them-self sit on an add-on board to the
bf5xx eval boards.
So you have two boards, the bf5xx eval board with has the codec eval board
connected to it. That's why it it is kept so generic. You could connect the
codec eval board to any of the bf5xx boards.

> 
> But it's about the same as all the other drivers so...
> 
>> +static struct snd_soc_dai_link bf5xx_adau1701_dai[] = {
>> +	{
>> +		.name = "adau1701",
>> +		.stream_name = "adau1701",
>> +		.cpu_dai_name = "bfin-i2s.0",
>> +		.codec_dai_name = "adau1701",
>> +		.platform_name = "bfin-i2s-pcm-audio",
>> +		.codec_name = "adau1701.0-0034",
>> +		.ops = &bf5xx_adau1701_ops,
>> +	},
>> +	{
>> +		.name = "adau1701",
>> +		.stream_name = "adau1701",
>> +		.cpu_dai_name = "bfin-i2s.1",
>> +		.codec_dai_name = "adau1701",
>> +		.platform_name = "bfin-i2s-pcm-audio",
>> +		.codec_name = "adau1701.0-0034",
>> +		.ops = &bf5xx_adau1701_ops,
>> +	},
> 
> For example this mapping could vary between systems, and I guess there
> may be some possiblity that the CODEC I2C address could change.

The I2C address is specific to the add-on board and since there is only one
adau1701 eval board it should be static.

- Lars

Re: [Device-drivers-devel] [PATCH 1/3] ASoC: Add ADAU1701 codec driver

[Lars-Peter Clausen]

On 06/10/2011 07:43 PM, Mike Frysinger wrote:
> On Fri, Jun 10, 2011 at 13:18, Lars-Peter Clausen wrote:
>> +MODULE_AUTHOR("Lars-Peter Clausen <lars@metafoo.de>");
> 
> did you actually rewrite this thing from scratch ?  seems like you
> should keep Cliff as the author in git/MODULE_AUTHOR, and then add
> your name to the s-o-b list and this macro.

Not from scratch, but I guess except for the register definitions and the
copyright header not much of the original driver is left:
sound/soc/codecs/adau1701.c |  662 ++++++++++++++++++++++++-------------------
 1 files changed, 371 insertions(+), 291 deletions(-)



> 
> friendly git commits can easily be cherry picked from my tree for asoc stuff:
> http://git.kernel.org/?p=linux/kernel/git/vapier/blackfin.git;a=summary
> (see the "trunk" branch)
> -mike

Re: [Device-drivers-devel] [alsa-devel] [PATCH 2/3] ASoC: Blackfin: Add bf5xx-adau1701 machine driver

[Mark Brown]

On Fri, Jun 10, 2011 at 01:47:34PM -0400, Mike Frysinger wrote:
> On Fri, Jun 10, 2011 at 13:30, Mark Brown wrote:

> > So, I keep on complaining about the way these drivers are just generic
> > to any random Blackfin plus CODEC combination rather than being board
> > specific.  It'd be good if we could improve this, even adding something
> > into the driver name to make it clear these are for the EVB would help.

> i know you keep complaining, but i honestly dont understand why this
> is undesirable.  connecting a codec to a Blackfin is pretty much
> always the same.  you pick a SPORT # and that's about it.

Only if you're looking at very basic boards - as soon as you start
adding any external components on the boards like speaker amps, start
handling jacks (with detection or without) or start dealing with more
flexible CODEC devices with a wider range of connection possibilities
the number of possibilities expands dramatically.

> the spi cs and i2c address could differ (so maybe make that a field
> for the platform resources), but otherwise i dont see why people
> should have to copy & paste the same code to change all of 4 bytes.

Reusing code where there's similiarities is fine - Tegra is doing that
for WM8903 based systems in a fairly tasteful fashion - but that's not
really what this stuff feels like it is doing.  For example, the SPORT
selection and reset GPIO selection should be platform data not Kconfig
options and drivers that explicitly say they're for a particular board
in Kconfig should probably say so in code also.

Re: [alsa-devel] [PATCH 2/3] ASoC: Blackfin: Add bf5xx-adau1701 machine driver

[Mark Brown]

On Fri, Jun 10, 2011 at 07:50:17PM +0200, Lars-Peter Clausen wrote:
> On 06/10/2011 07:30 PM, Mark Brown wrote:

> > So, I keep on complaining about the way these drivers are just generic
> > to any random Blackfin plus CODEC combination rather than being board
> > specific.  It'd be good if we could improve this, even adding something
> > into the driver name to make it clear these are for the EVB would help.

> I think this is due, that the codecs them-self sit on an add-on board to the
> bf5xx eval boards.
> So you have two boards, the bf5xx eval board with has the codec eval board
> connected to it. That's why it it is kept so generic. You could connect the
> codec eval board to any of the bf5xx boards.

I'd be more happy with that if there were some indication in the code
that this was for the standard Blackfin EVBs rather than all systems
based on Blackfin.

Re: [alsa-devel] [PATCH 1/3] ASoC: Add ADAU1701 codec driver

[Mark Brown]

On Fri, Jun 10, 2011 at 01:44:00PM -0400, Mike Frysinger wrote:
> On Fri, Jun 10, 2011 at 13:27, Mark Brown wrote:

> > Also someone should remonstrate with the hardware designers.

> if there's things we can take back, i can forward them along.  the
> last conversation i had with them sounded like they have no problem
> accepting feedback.

I was referring to the fact that different registers have different
sizes which does rather complicate the code.

Re: [Device-drivers-devel] [PATCH 1/3] ASoC: Add ADAU1701 codec driver

[Mike Frysinger]

On Fri, Jun 10, 2011 at 13:57, Lars-Peter Clausen wrote:
> On 06/10/2011 07:43 PM, Mike Frysinger wrote:
>> On Fri, Jun 10, 2011 at 13:18, Lars-Peter Clausen wrote:
>>> +MODULE_AUTHOR("Lars-Peter Clausen <lars@metafoo.de>");
>>
>> did you actually rewrite this thing from scratch ?  seems like you
>> should keep Cliff as the author in git/MODULE_AUTHOR, and then add
>> your name to the s-o-b list and this macro.
>
> Not from scratch, but I guess except for the register definitions and the
> copyright header not much of the original driver is left:
> sound/soc/codecs/adau1701.c |  662 ++++++++++++++++++++++++-------------------
>  1 files changed, 371 insertions(+), 291 deletions(-)

based purely on LoC, Cliff wrote 467, so you took that and punted 291
(62%) and then added 371.  that leaves the final file with 32% Cliff
and 68% you.

however, some of that was register renaming and shuffling between the
adau1701.c and adau1701.h, and it's easier to start with a base and
clean up than from scratch.

so i'm definitely not comfortable dropping Cliff's completely from the
file and the log (which means he needs to be readded and made sure to
be retained in all the other ADAU drivers going forward), and i'm not
entirely sold about changing of the Author field in the git commit.

don't get me wrong ... i'm not a fanboi of Cliff or something, i just
think credit is due to him for his work.
-mike

Re: [alsa-devel] [PATCH 1/3] ASoC: Add ADAU1701 codec driver

[Mike Frysinger]

On Fri, Jun 10, 2011 at 14:07, Mark Brown wrote:
> On Fri, Jun 10, 2011 at 01:44:00PM -0400, Mike Frysinger wrote:
>> On Fri, Jun 10, 2011 at 13:27, Mark Brown wrote:
>> > Also someone should remonstrate with the hardware designers.
>
>> if there's things we can take back, i can forward them along.  the
>> last conversation i had with them sounded like they have no problem
>> accepting feedback.
>
> I was referring to the fact that different registers have different
> sizes which does rather complicate the code.

np.  i'll forward that along.  "uniform codec register sizes simplifies code".
-mike

Re: [alsa-devel] [PATCH 2/3] ASoC: Blackfin: Add bf5xx-adau1701 machine driver

[Lars-Peter Clausen]

On 06/10/2011 08:02 PM, Mark Brown wrote:
> On Fri, Jun 10, 2011 at 07:50:17PM +0200, Lars-Peter Clausen wrote:
>> On 06/10/2011 07:30 PM, Mark Brown wrote:
> 
>>> So, I keep on complaining about the way these drivers are just generic
>>> to any random Blackfin plus CODEC combination rather than being board
>>> specific.  It'd be good if we could improve this, even adding something
>>> into the driver name to make it clear these are for the EVB would help.
> 
>> I think this is due, that the codecs them-self sit on an add-on board to the
>> bf5xx eval boards.
>> So you have two boards, the bf5xx eval board with has the codec eval board
>> connected to it. That's why it it is kept so generic. You could connect the
>> codec eval board to any of the bf5xx boards.
> 
> I'd be more happy with that if there were some indication in the code
> that this was for the standard Blackfin EVBs rather than all systems
> based on Blackfin.

I could rename the machine driver to bf5xx-adau1701-evb for v2.

- Lars

Re: [Device-drivers-devel] [alsa-devel] [PATCH 2/3] ASoC: Blackfin: Add bf5xx-adau1701 machine driver

[Mike Frysinger]

On Fri, Jun 10, 2011 at 14:01, Mark Brown wrote:
> On Fri, Jun 10, 2011 at 01:47:34PM -0400, Mike Frysinger wrote:
>> On Fri, Jun 10, 2011 at 13:30, Mark Brown wrote:
>> > So, I keep on complaining about the way these drivers are just generic
>> > to any random Blackfin plus CODEC combination rather than being board
>> > specific.  It'd be good if we could improve this, even adding something
>> > into the driver name to make it clear these are for the EVB would help.
>>
>> i know you keep complaining, but i honestly dont understand why this
>> is undesirable.  connecting a codec to a Blackfin is pretty much
>> always the same.  you pick a SPORT # and that's about it.
>
> Only if you're looking at very basic boards - as soon as you start
> adding any external components on the boards like speaker amps, start
> handling jacks (with detection or without) or start dealing with more
> flexible CODEC devices with a wider range of connection possibilities
> the number of possibilities expands dramatically.

often times, people do have just basic needs, but i see what you mean.
 i want the base driver to be flexible enough to handle all the base
changes (diff SPORT num and perhaps addresses), but anything beyond
that i'd expect someone to write a custom driver.  perhaps renaming
the Kconfig description to be like "Basic Blackfin connection to XXX
Codec" would be sufficient ?  and then add a few more details to the
help text ?

>> the spi cs and i2c address could differ (so maybe make that a field
>> for the platform resources), but otherwise i dont see why people
>> should have to copy & paste the same code to change all of 4 bytes.
>
> Reusing code where there's similiarities is fine - Tegra is doing that
> for WM8903 based systems in a fairly tasteful fashion - but that's not
> really what this stuff feels like it is doing.  For example, the SPORT
> selection and reset GPIO selection should be platform data not Kconfig
> options and drivers that explicitly say they're for a particular board
> in Kconfig should probably say so in code also.

i absolutely agree with this 100%.  i keep banging on our guys to get
all Kconfig knobs thrown out and moved to proper resources, and some
of it has, but not all of it just yet.
-mike

Re: [Device-drivers-devel] [PATCH 1/3] ASoC: Add ADAU1701 codec driver

[Mike Frysinger]

On Fri, Jun 10, 2011 at 14:08, Mike Frysinger wrote:
> On Fri, Jun 10, 2011 at 13:57, Lars-Peter Clausen wrote:
>> On 06/10/2011 07:43 PM, Mike Frysinger wrote:
>>> On Fri, Jun 10, 2011 at 13:18, Lars-Peter Clausen wrote:
>>>> +MODULE_AUTHOR("Lars-Peter Clausen <lars@metafoo.de>");
>>>
>>> did you actually rewrite this thing from scratch ?  seems like you
>>> should keep Cliff as the author in git/MODULE_AUTHOR, and then add
>>> your name to the s-o-b list and this macro.
>>
>> Not from scratch, but I guess except for the register definitions and the
>> copyright header not much of the original driver is left:
>> sound/soc/codecs/adau1701.c |  662 ++++++++++++++++++++++++-------------------
>>  1 files changed, 371 insertions(+), 291 deletions(-)
>
> based purely on LoC, Cliff wrote 467, so you took that and punted 291
> (62%) and then added 371.  that leaves the final file with 32% Cliff
> and 68% you.
>
> however, some of that was register renaming and shuffling between the
> adau1701.c and adau1701.h, and it's easier to start with a base and
> clean up than from scratch.
>
> so i'm definitely not comfortable dropping Cliff's completely from the
> file and the log (which means he needs to be readded and made sure to
> be retained in all the other ADAU drivers going forward), and i'm not
> entirely sold about changing of the Author field in the git commit.
>
> don't get me wrong ... i'm not a fanboi of Cliff or something, i just
> think credit is due to him for his work.

to be clear, i'm like 50/50 on the git author field.  maybe someone
else here has an opinion, or you feel strongly enough that your
rewrite supersedes his seeding work.  but please send a v2 with
updated source/changelog with his name in it.
-mike

Re: [Device-drivers-devel] [alsa-devel] [PATCH 2/3] ASoC: Blackfin: Add bf5xx-adau1701 machine driver

[Mark Brown]

On Fri, Jun 10, 2011 at 02:13:05PM -0400, Mike Frysinger wrote:

> often times, people do have just basic needs, but i see what you mean.
>  i want the base driver to be flexible enough to handle all the base
> changes (diff SPORT num and perhaps addresses), but anything beyond
> that i'd expect someone to write a custom driver.  perhaps renaming
> the Kconfig description to be like "Basic Blackfin connection to XXX
> Codec" would be sufficient ?  and then add a few more details to the
> help text ?

Yes, that's exactly the sort of thing I'd like to see.  It shouldn't be
that big a change to the code, it's more about making it clear that it's
a 90% case driver rather than the only way to connect things up.

Re: [Device-drivers-devel] [alsa-devel] [PATCH 2/3] ASoC: Blackfin: Add bf5xx-adau1701 machine driver

[Mike Frysinger]

On Fri, Jun 10, 2011 at 14:13, Lars-Peter Clausen wrote:
> On 06/10/2011 08:02 PM, Mark Brown wrote:
>> On Fri, Jun 10, 2011 at 07:50:17PM +0200, Lars-Peter Clausen wrote:
>>> On 06/10/2011 07:30 PM, Mark Brown wrote:
>>>> So, I keep on complaining about the way these drivers are just generic
>>>> to any random Blackfin plus CODEC combination rather than being board
>>>> specific.  It'd be good if we could improve this, even adding something
>>>> into the driver name to make it clear these are for the EVB would help.
>>
>>> I think this is due, that the codecs them-self sit on an add-on board to the
>>> bf5xx eval boards.
>>> So you have two boards, the bf5xx eval board with has the codec eval board
>>> connected to it. That's why it it is kept so generic. You could connect the
>>> codec eval board to any of the bf5xx boards.
>>
>> I'd be more happy with that if there were some indication in the code
>> that this was for the standard Blackfin EVBs rather than all systems
>> based on Blackfin.
>
> I could rename the machine driver to bf5xx-adau1701-evb for v2.

i'd like to get away from "bf5xx" actually and start using "bfin".
ive started doing this in some of the files, so let's not add more.

also, if we do decide to add more to the name, it should be closer to
what ADI calls the actual product, so probably "bfin-eval-adau1701".
although, once we push the SPORT # and I2C address to platform
resources, this should work for more than just the eval.  it'd fall
into our "basic blackfin" category, so we'd only tweak the Kconfig
desc/help text.
-mike

Re: [Device-drivers-devel] [PATCH 1/3] ASoC: Add ADAU1701 codec driver

[Lars-Peter Clausen]

On 06/10/2011 08:15 PM, Mike Frysinger wrote:
> On Fri, Jun 10, 2011 at 14:08, Mike Frysinger wrote:
>> On Fri, Jun 10, 2011 at 13:57, Lars-Peter Clausen wrote:
>>> On 06/10/2011 07:43 PM, Mike Frysinger wrote:
>>>> On Fri, Jun 10, 2011 at 13:18, Lars-Peter Clausen wrote:
>>>>> +MODULE_AUTHOR("Lars-Peter Clausen <lars@metafoo.de>");
>>>>
>>>> did you actually rewrite this thing from scratch ?  seems like you
>>>> should keep Cliff as the author in git/MODULE_AUTHOR, and then add
>>>> your name to the s-o-b list and this macro.
>>>
>>> Not from scratch, but I guess except for the register definitions and the
>>> copyright header not much of the original driver is left:
>>> sound/soc/codecs/adau1701.c |  662 ++++++++++++++++++++++++-------------------
>>>  1 files changed, 371 insertions(+), 291 deletions(-)
>>
>> based purely on LoC, Cliff wrote 467, so you took that and punted 291
>> (62%) and then added 371.  that leaves the final file with 32% Cliff
>> and 68% you.
>>
>> however, some of that was register renaming and shuffling between the
>> adau1701.c and adau1701.h, and it's easier to start with a base and
>> clean up than from scratch.
>>
>> so i'm definitely not comfortable dropping Cliff's completely from the
>> file and the log (which means he needs to be readded and made sure to
>> be retained in all the other ADAU drivers going forward), and i'm not
>> entirely sold about changing of the Author field in the git commit.
>>
>> don't get me wrong ... i'm not a fanboi of Cliff or something, i just
>> think credit is due to him for his work.
>
You have both the orignal driver and the new driver, so you can do the diff for
yourself. You see that the lines unchanged are basically boilerplate code and
stuff that you'll find in any ASoC driver. The diff to any random ASoC driver
called adau1701.

But of course if you want to see his name as the author, you get his name as
the author.

> to be clear, i'm like 50/50 on the git author field.  maybe someone
> else here has an opinion, or you feel strongly enough that your
> rewrite supersedes his seeding work.  but please send a v2 with
> updated source/changelog with his name in it.
His name is already in the copyright header.

- Lars

Re: [Device-drivers-devel] [PATCH 1/3] ASoC: Add ADAU1701 codec driver

[Mike Frysinger]

On Fri, Jun 10, 2011 at 14:22, Lars-Peter Clausen wrote:
> On 06/10/2011 08:15 PM, Mike Frysinger wrote:
>> On Fri, Jun 10, 2011 at 14:08, Mike Frysinger wrote:
>>> On Fri, Jun 10, 2011 at 13:57, Lars-Peter Clausen wrote:
>>>> On 06/10/2011 07:43 PM, Mike Frysinger wrote:
>>>>> On Fri, Jun 10, 2011 at 13:18, Lars-Peter Clausen wrote:
>>>>>> +MODULE_AUTHOR("Lars-Peter Clausen <lars@metafoo.de>");
>>>>>
>>>>> did you actually rewrite this thing from scratch ?  seems like you
>>>>> should keep Cliff as the author in git/MODULE_AUTHOR, and then add
>>>>> your name to the s-o-b list and this macro.
>>>>
>>>> Not from scratch, but I guess except for the register definitions and the
>>>> copyright header not much of the original driver is left:
>>>> sound/soc/codecs/adau1701.c |  662 ++++++++++++++++++++++++-------------------
>>>>  1 files changed, 371 insertions(+), 291 deletions(-)
>>>
>>> based purely on LoC, Cliff wrote 467, so you took that and punted 291
>>> (62%) and then added 371.  that leaves the final file with 32% Cliff
>>> and 68% you.
>>>
>>> however, some of that was register renaming and shuffling between the
>>> adau1701.c and adau1701.h, and it's easier to start with a base and
>>> clean up than from scratch.
>>>
>>> so i'm definitely not comfortable dropping Cliff's completely from the
>>> file and the log (which means he needs to be readded and made sure to
>>> be retained in all the other ADAU drivers going forward), and i'm not
>>> entirely sold about changing of the Author field in the git commit.
>>>
>>> don't get me wrong ... i'm not a fanboi of Cliff or something, i just
>>> think credit is due to him for his work.
>
> You have both the orignal driver and the new driver, so you can do the diff for
> yourself.

i did do that already which is why i commented on the register renaming

> But of course if you want to see his name as the author, you get his name as
> the author.

if you feel strongly about it, then you may keep your name there

>> to be clear, i'm like 50/50 on the git author field.  maybe someone
>> else here has an opinion, or you feel strongly enough that your
>> rewrite supersedes his seeding work.  but please send a v2 with
>> updated source/changelog with his name in it.
>
> His name is already in the copyright header.

sorry, i missed that the first time.  but also list him in
MODULE_AUTHOR (i'll leave order preference to you).
-mike