alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ASoC: Add MAX9850 codec driver
@ 2011-03-07 12:45 Christian Glindkamp
  2011-03-07 13:48 ` Dimitris Papastamos
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Christian Glindkamp @ 2011-03-07 12:45 UTC (permalink / raw)
  To: alsa-devel

This patch adds ASoC support for the MAX9850 codec with headphone
amplifier.

Supported features:
- Playback
- 16, 20 and 24 bit audio
- 8k - 48k sample rates
- DAPM

Only 16 bit audio was tested while the codec was connected to an
AT91SAM9G20 SSC in master mode.

Signed-off-by: Christian Glindkamp <christian.glindkamp@taskit.de>
---

I've all ready sent this patch some time ago, but it hung in the moderation
queue. This is a slightly modified version. Unfortunately I do not have the
hardware anymore to test suggested changes that alter function.

 sound/soc/codecs/Kconfig   |    4 +
 sound/soc/codecs/Makefile  |    2 +
 sound/soc/codecs/max9850.c |  358 ++++++++++++++++++++++++++++++++++++++++++++
 sound/soc/codecs/max9850.h |   41 +++++
 4 files changed, 405 insertions(+), 0 deletions(-)
 create mode 100644 sound/soc/codecs/max9850.c
 create mode 100644 sound/soc/codecs/max9850.h

diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index e239345..51e9844 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -31,6 +31,7 @@ config SND_SOC_ALL_CODECS
 	select SND_SOC_DA7210 if I2C
 	select SND_SOC_JZ4740_CODEC if SOC_JZ4740
 	select SND_SOC_MAX98088 if I2C
+	select SND_SOC_MAX9850 if I2C
 	select SND_SOC_MAX9877 if I2C
 	select SND_SOC_PCM3008
 	select SND_SOC_SN95031 if INTEL_SCU_IPC
@@ -179,6 +180,9 @@ config SND_SOC_DMIC
 config SND_SOC_MAX98088
        tristate
 
+config SND_SOC_MAX9850
+	tristate
+
 config SND_SOC_PCM3008
        tristate
 
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index ae10507..f2efd1c 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -18,6 +18,7 @@ snd-soc-da7210-objs := da7210.o
 snd-soc-dmic-objs := dmic.o
 snd-soc-l3-objs := l3.o
 snd-soc-max98088-objs := max98088.o
+snd-soc-max9850-objs := max9850.o
 snd-soc-pcm3008-objs := pcm3008.o
 snd-soc-alc5623-objs := alc5623.o
 snd-soc-sn95031-objs := sn95031.o
@@ -102,6 +103,7 @@ obj-$(CONFIG_SND_SOC_DMIC)	+= snd-soc-dmic.o
 obj-$(CONFIG_SND_SOC_L3)	+= snd-soc-l3.o
 obj-$(CONFIG_SND_SOC_JZ4740_CODEC)	+= snd-soc-jz4740-codec.o
 obj-$(CONFIG_SND_SOC_MAX98088)	+= snd-soc-max98088.o
+obj-$(CONFIG_SND_SOC_MAX9850)	+= snd-soc-max9850.o
 obj-$(CONFIG_SND_SOC_PCM3008)	+= snd-soc-pcm3008.o
 obj-$(CONFIG_SND_SOC_SN95031)	+=snd-soc-sn95031.o
 obj-$(CONFIG_SND_SOC_SPDIF)	+= snd-soc-spdif.o
diff --git a/sound/soc/codecs/max9850.c b/sound/soc/codecs/max9850.c
new file mode 100644
index 0000000..a8c1f95
--- /dev/null
+++ b/sound/soc/codecs/max9850.c
@@ -0,0 +1,358 @@
+/*
+ * max9850.c  --  codec driver for max9850
+ *
+ * Copyright (C) 2011 taskit GmbH
+ *
+ * Author: Christian Glindkamp <christian.glindkamp@taskit.de>
+ *
+ * Initial development of this code was funded by
+ * MICRONIC Computer Systeme GmbH, http://www.mcsberlin.de/
+ *
+ * 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/init.h>
+#include <linux/i2c.h>
+#include <linux/slab.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include <sound/tlv.h>
+
+#include "max9850.h"
+
+struct max9850_priv {
+	unsigned int sysclk;
+};
+
+/* max9850 register cache */
+static const u8 max9850_reg[MAX9850_CACHEREGNUM] = {
+	0x00, 0x00, 0x0c, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+};
+
+/* these registers are not used at the moment but provided for the sake of
+ * completeness */
+static int max9850_volatile_register(unsigned int reg)
+{
+	switch (reg) {
+	case MAX9850_STATUSA:
+	case MAX9850_STATUSB:
+		return 1;
+	default:
+		return 0;
+	}
+}
+
+static const unsigned int max9850_tlv[] = {
+	TLV_DB_RANGE_HEAD(4),
+	0x18, 0x1f, TLV_DB_SCALE_ITEM(-7450, 400, 0),
+	0x20, 0x33, TLV_DB_SCALE_ITEM(-4150, 200, 0),
+	0x34, 0x37, TLV_DB_SCALE_ITEM(-150, 100, 0),
+	0x38, 0x3f, TLV_DB_SCALE_ITEM(250, 50, 0),
+};
+
+static const struct snd_kcontrol_new max9850_controls[] = {
+SOC_SINGLE_TLV("Headphone Volume", MAX9850_VOLUME, 0, 0x3f, 1, max9850_tlv),
+SOC_SINGLE("Headphone Switch", MAX9850_VOLUME, 7, 1, 1),
+SOC_SINGLE("Mono", MAX9850_GENERAL_PURPOSE, 2, 1, 0),
+};
+
+static const struct snd_kcontrol_new max9850_mixer_controls[] = {
+	SOC_DAPM_SINGLE("Line In Switch", MAX9850_ENABLE, 1, 1, 0),
+};
+
+static const struct snd_soc_dapm_widget max9850_dapm_widgets[] = {
+SND_SOC_DAPM_DAC("DAC", "HiFi Playback", MAX9850_ENABLE, 0, 0),
+SND_SOC_DAPM_SUPPLY("MCLK", MAX9850_ENABLE, 6, 0, NULL, 0),
+SND_SOC_DAPM_OUTPUT("OUTL"),
+SND_SOC_DAPM_OUTPUT("OUTR"),
+SND_SOC_DAPM_OUTPUT("HPL"),
+SND_SOC_DAPM_OUTPUT("HPR"),
+SND_SOC_DAPM_INPUT("INL"),
+SND_SOC_DAPM_INPUT("INR"),
+SND_SOC_DAPM_PGA("Headphone Output", MAX9850_ENABLE, 3, 0, NULL, 0),
+SND_SOC_DAPM_MIXER("Line Input", SND_SOC_NOPM, 0, 0, NULL, 0),
+SND_SOC_DAPM_MIXER_NAMED_CTL("Output Mixer", MAX9850_ENABLE, 2, 0,
+		&max9850_mixer_controls[0],
+		ARRAY_SIZE(max9850_mixer_controls)),
+};
+
+static const struct snd_soc_dapm_route intercon[] = {
+	/* output mixer */
+	{"Output Mixer", NULL, "DAC"},
+	{"Output Mixer", "Line In Switch", "Line Input"},
+
+	/* outputs */
+	{"Headphone Output", NULL, "Output Mixer"},
+	{"HPL", NULL, "Headphone Output"},
+	{"HPR", NULL, "Headphone Output"},
+	{"OUTL", NULL, "Output Mixer"},
+	{"OUTR", NULL, "Output Mixer"},
+
+	/* inputs */
+	{"Line Input", NULL, "INL"},
+	{"Line Input", NULL, "INR"},
+
+	/* supplies */
+	{"DAC", NULL, "MCLK"},
+};
+
+static int max9850_hw_params(struct snd_pcm_substream *substream,
+			     struct snd_pcm_hw_params *params,
+			     struct snd_soc_dai *dai)
+{
+	struct snd_soc_codec *codec = dai->codec;
+	struct max9850_priv *max9850 = snd_soc_codec_get_drvdata(codec);
+	u64 lrclk_div;
+	u8 sf, da;
+
+	/* lrclk_div = 2^22 * rate / iclk with iclk = mclk / sf */
+	sf = (snd_soc_read(codec, MAX9850_CLOCK) >> 2) + 1;
+	lrclk_div = (1 << 22);
+	lrclk_div *= params_rate(params);
+	lrclk_div *= sf;
+	do_div(lrclk_div, max9850->sysclk);
+
+	snd_soc_write(codec, MAX9850_LRCLK_MSB, (lrclk_div >> 8) & 0x7f);
+	snd_soc_write(codec, MAX9850_LRCLK_LSB, lrclk_div & 0xff);
+
+	da = snd_soc_read(codec, MAX9850_DIGITAL_AUDIO);
+	switch (params_format(params)) {
+	case SNDRV_PCM_FORMAT_S16_LE:
+		break;
+	case SNDRV_PCM_FORMAT_S20_3LE:
+		da |= 0x2;
+		break;
+	case SNDRV_PCM_FORMAT_S24_LE:
+		da |= 0x3;
+		break;
+	default:
+		return -EINVAL;
+	}
+	snd_soc_write(codec, MAX9850_DIGITAL_AUDIO, da);
+
+	return 0;
+}
+
+static int max9850_set_dai_sysclk(struct snd_soc_dai *codec_dai,
+		int clk_id, unsigned int freq, int dir)
+{
+	struct snd_soc_codec *codec = codec_dai->codec;
+	struct max9850_priv *max9850 = snd_soc_codec_get_drvdata(codec);
+
+	/* calculate mclk -> iclk divider */
+	if (freq <= 13000000)
+		snd_soc_write(codec, MAX9850_CLOCK, 0x0);
+	else if (freq <= 26000000)
+		snd_soc_write(codec, MAX9850_CLOCK, 0x4);
+	else if (freq <= 40000000)
+		snd_soc_write(codec, MAX9850_CLOCK, 0x8);
+	else
+		return -EINVAL;
+
+	max9850->sysclk = freq;
+	return 0;
+}
+
+static int max9850_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt)
+{
+	struct snd_soc_codec *codec = codec_dai->codec;
+	u8 da = 0;
+
+	/* set master/slave audio interface */
+	switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
+	case SND_SOC_DAIFMT_CBM_CFM:
+		da |= MAX9850_MASTER;
+		break;
+	case SND_SOC_DAIFMT_CBS_CFS:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/* interface format */
+	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+	case SND_SOC_DAIFMT_I2S:
+		da |= MAX9850_DLY;
+		break;
+	case SND_SOC_DAIFMT_RIGHT_J:
+		da |= MAX9850_RTJ;
+		break;
+	case SND_SOC_DAIFMT_LEFT_J:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/* clock inversion */
+	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
+	case SND_SOC_DAIFMT_NB_NF:
+		break;
+	case SND_SOC_DAIFMT_IB_IF:
+		da |= MAX9850_BCINV | MAX9850_INV;
+		break;
+	case SND_SOC_DAIFMT_IB_NF:
+		da |= MAX9850_BCINV;
+		break;
+	case SND_SOC_DAIFMT_NB_IF:
+		da |= MAX9850_INV;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/* set da */
+	snd_soc_write(codec, MAX9850_DIGITAL_AUDIO, da);
+
+	return 0;
+}
+
+static int max9850_set_bias_level(struct snd_soc_codec *codec,
+				  enum snd_soc_bias_level level)
+{
+	switch (level) {
+	case SND_SOC_BIAS_ON:
+		break;
+	case SND_SOC_BIAS_PREPARE:
+		snd_soc_update_bits(codec, MAX9850_ENABLE, MAX9850_SHDN,
+				MAX9850_SHDN);
+		break;
+	case SND_SOC_BIAS_STANDBY:
+		snd_soc_update_bits(codec, MAX9850_ENABLE, MAX9850_SHDN, 0);
+		break;
+	case SND_SOC_BIAS_OFF:
+		break;
+	}
+	codec->dapm.bias_level = level;
+	return 0;
+}
+
+#define MAX9850_RATES SNDRV_PCM_RATE_8000_48000
+
+#define MAX9850_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE |\
+	SNDRV_PCM_FMTBIT_S24_LE)
+
+static struct snd_soc_dai_ops max9850_dai_ops = {
+	.hw_params	= max9850_hw_params,
+	.set_sysclk	= max9850_set_dai_sysclk,
+	.set_fmt	= max9850_set_dai_fmt,
+};
+
+static struct snd_soc_dai_driver max9850_dai = {
+	.name = "max9850-hifi",
+	.playback = {
+		.stream_name = "Playback",
+		.channels_min = 1,
+		.channels_max = 2,
+		.rates = MAX9850_RATES,
+		.formats = MAX9850_FORMATS
+	},
+	.ops = &max9850_dai_ops,
+};
+
+static int max9850_probe(struct snd_soc_codec *codec)
+{
+	struct snd_soc_dapm_context *dapm = &codec->dapm;
+	int ret;
+
+	ret = snd_soc_codec_set_cache_io(codec, 8, 8, SND_SOC_I2C);
+	if (ret < 0) {
+		dev_err(codec->dev, "Failed to set cache I/O: %d\n", ret);
+		return ret;
+	}
+
+	/* enable zero-detect */
+	snd_soc_update_bits(codec, MAX9850_GENERAL_PURPOSE, 1, 1);
+	/* enable charge pump, disable everything else */
+	snd_soc_write(codec, MAX9850_ENABLE, 0x30);
+	/* enable slew-rate control */
+	snd_soc_update_bits(codec, MAX9850_VOLUME, 0x40, 0x40);
+	/* set slew-rate 125ms */
+	snd_soc_update_bits(codec, MAX9850_CHARGE_PUMP, 0xff, 0xc0);
+
+	snd_soc_dapm_new_controls(dapm, max9850_dapm_widgets,
+				  ARRAY_SIZE(max9850_dapm_widgets));
+	snd_soc_dapm_add_routes(dapm, intercon, ARRAY_SIZE(intercon));
+
+	snd_soc_add_controls(codec, max9850_controls,
+			ARRAY_SIZE(max9850_controls));
+
+	return 0;
+}
+static int max9850_remove(struct snd_soc_codec *codec)
+{
+	return 0;
+}
+
+static struct snd_soc_codec_driver soc_codec_dev_max9850 = {
+	.probe =	max9850_probe,
+	.remove =	max9850_remove,
+	.set_bias_level = max9850_set_bias_level,
+	.reg_cache_size = ARRAY_SIZE(max9850_reg),
+	.reg_word_size = sizeof(u8),
+	.reg_cache_default = max9850_reg,
+	.volatile_register = max9850_volatile_register,
+};
+
+static int __devinit max9850_i2c_probe(struct i2c_client *i2c,
+		const struct i2c_device_id *id)
+{
+	struct max9850_priv *max9850;
+	int ret;
+
+	max9850 = kzalloc(sizeof(struct max9850_priv), GFP_KERNEL);
+	if (max9850 == NULL)
+		return -ENOMEM;
+
+	i2c_set_clientdata(i2c, max9850);
+
+	ret = snd_soc_register_codec(&i2c->dev,
+			&soc_codec_dev_max9850, &max9850_dai, 1);
+	if (ret < 0)
+		kfree(max9850);
+	return ret;
+}
+
+static __devexit int max9850_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 max9850_i2c_id[] = {
+	{ "max9850", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, max9850_i2c_id);
+
+static struct i2c_driver max9850_i2c_driver = {
+	.driver = {
+		.name = "max9850-codec",
+		.owner = THIS_MODULE,
+	},
+	.probe = max9850_i2c_probe,
+	.remove = __devexit_p(max9850_i2c_remove),
+	.id_table = max9850_i2c_id,
+};
+
+static int __init max9850_init(void)
+{
+	return i2c_add_driver(&max9850_i2c_driver);
+}
+module_init(max9850_init);
+
+static void __exit max9850_exit(void)
+{
+	i2c_del_driver(&max9850_i2c_driver);
+}
+module_exit(max9850_exit);
+
+MODULE_AUTHOR("Christian Glindkamp <christian.glindkamp@taskit.de>");
+MODULE_DESCRIPTION("ASoC MAX9850 codec driver");
+MODULE_LICENSE("GPL");
diff --git a/sound/soc/codecs/max9850.h b/sound/soc/codecs/max9850.h
new file mode 100644
index 0000000..5268575
--- /dev/null
+++ b/sound/soc/codecs/max9850.h
@@ -0,0 +1,41 @@
+/*
+ * max9850.h  --  codec driver for max9850
+ *
+ * Copyright (C) 2011 taskit GmbH
+ * Author: Christian Glindkamp <christian.glindkamp@taskit.de>
+ *
+ * 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 _MAX9850_H
+#define _MAX9850_H
+
+#define MAX9850_STATUSA			0x00
+#define MAX9850_STATUSB			0x01
+#define MAX9850_VOLUME			0x02
+#define MAX9850_GENERAL_PURPOSE		0x03
+#define MAX9850_INTERRUPT		0x04
+#define MAX9850_ENABLE			0x05
+#define MAX9850_CLOCK			0x06
+#define MAX9850_CHARGE_PUMP		0x07
+#define MAX9850_LRCLK_MSB		0x08
+#define MAX9850_LRCLK_LSB		0x09
+#define MAX9850_DIGITAL_AUDIO		0x0a
+
+#define MAX9850_CACHEREGNUM 11
+
+/* MAX9850_ENABLE */
+#define MAX9850_SHDN			(1<<7)
+
+/* MAX9850_DIGITAL_AUDIO */
+#define MAX9850_MASTER			(1<<7)
+#define MAX9850_INV			(1<<6)
+#define MAX9850_BCINV			(1<<5)
+#define MAX9850_DLY			(1<<3)
+#define MAX9850_RTJ			(1<<2)
+
+#endif
-- 
1.7.2.3

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

* Re: [PATCH] ASoC: Add MAX9850 codec driver
  2011-03-07 12:45 [PATCH] ASoC: Add MAX9850 codec driver Christian Glindkamp
@ 2011-03-07 13:48 ` Dimitris Papastamos
  2011-03-07 17:04   ` Christian Glindkamp
  2011-03-07 15:15 ` Mark Brown
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 15+ messages in thread
From: Dimitris Papastamos @ 2011-03-07 13:48 UTC (permalink / raw)
  To: Christian Glindkamp; +Cc: alsa-devel

On Mon, Mar 07, 2011 at 01:45:13PM +0100, Christian Glindkamp wrote:
> This patch adds ASoC support for the MAX9850 codec with headphone
> amplifier.
> 
> Supported features:
> - Playback
> - 16, 20 and 24 bit audio
> - 8k - 48k sample rates
> - DAPM
> 
> Only 16 bit audio was tested while the codec was connected to an
> AT91SAM9G20 SSC in master mode.
> 
> Signed-off-by: Christian Glindkamp <christian.glindkamp@taskit.de>
> ---
> 
> I've all ready sent this patch some time ago, but it hung in the moderation
> queue. This is a slightly modified version. Unfortunately I do not have the
> hardware anymore to test suggested changes that alter function.
> 
>  sound/soc/codecs/Kconfig   |    4 +
>  sound/soc/codecs/Makefile  |    2 +
>  sound/soc/codecs/max9850.c |  358 ++++++++++++++++++++++++++++++++++++++++++++
>  sound/soc/codecs/max9850.h |   41 +++++
>  4 files changed, 405 insertions(+), 0 deletions(-)
>  create mode 100644 sound/soc/codecs/max9850.c
>  create mode 100644 sound/soc/codecs/max9850.h
> 
> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> index e239345..51e9844 100644
> --- a/sound/soc/codecs/Kconfig
> +++ b/sound/soc/codecs/Kconfig
> @@ -31,6 +31,7 @@ config SND_SOC_ALL_CODECS
>  	select SND_SOC_DA7210 if I2C
>  	select SND_SOC_JZ4740_CODEC if SOC_JZ4740
>  	select SND_SOC_MAX98088 if I2C
> +	select SND_SOC_MAX9850 if I2C
>  	select SND_SOC_MAX9877 if I2C
>  	select SND_SOC_PCM3008
>  	select SND_SOC_SN95031 if INTEL_SCU_IPC
> @@ -179,6 +180,9 @@ config SND_SOC_DMIC
>  config SND_SOC_MAX98088
>         tristate
>  
> +config SND_SOC_MAX9850
> +	tristate
> +
>  config SND_SOC_PCM3008
>         tristate
>  
> diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
> index ae10507..f2efd1c 100644
> --- a/sound/soc/codecs/Makefile
> +++ b/sound/soc/codecs/Makefile
> @@ -18,6 +18,7 @@ snd-soc-da7210-objs := da7210.o
>  snd-soc-dmic-objs := dmic.o
>  snd-soc-l3-objs := l3.o
>  snd-soc-max98088-objs := max98088.o
> +snd-soc-max9850-objs := max9850.o
>  snd-soc-pcm3008-objs := pcm3008.o
>  snd-soc-alc5623-objs := alc5623.o
>  snd-soc-sn95031-objs := sn95031.o
> @@ -102,6 +103,7 @@ obj-$(CONFIG_SND_SOC_DMIC)	+= snd-soc-dmic.o
>  obj-$(CONFIG_SND_SOC_L3)	+= snd-soc-l3.o
>  obj-$(CONFIG_SND_SOC_JZ4740_CODEC)	+= snd-soc-jz4740-codec.o
>  obj-$(CONFIG_SND_SOC_MAX98088)	+= snd-soc-max98088.o
> +obj-$(CONFIG_SND_SOC_MAX9850)	+= snd-soc-max9850.o
>  obj-$(CONFIG_SND_SOC_PCM3008)	+= snd-soc-pcm3008.o
>  obj-$(CONFIG_SND_SOC_SN95031)	+=snd-soc-sn95031.o
>  obj-$(CONFIG_SND_SOC_SPDIF)	+= snd-soc-spdif.o
> diff --git a/sound/soc/codecs/max9850.c b/sound/soc/codecs/max9850.c
> new file mode 100644
> index 0000000..a8c1f95
> --- /dev/null
> +++ b/sound/soc/codecs/max9850.c
> @@ -0,0 +1,358 @@
> +/*
> + * max9850.c  --  codec driver for max9850
> + *
> + * Copyright (C) 2011 taskit GmbH
> + *
> + * Author: Christian Glindkamp <christian.glindkamp@taskit.de>
> + *
> + * Initial development of this code was funded by
> + * MICRONIC Computer Systeme GmbH, http://www.mcsberlin.de/
> + *
> + * 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/init.h>
> +#include <linux/i2c.h>
> +#include <linux/slab.h>
> +#include <sound/pcm.h>
> +#include <sound/pcm_params.h>
> +#include <sound/soc.h>
> +#include <sound/tlv.h>
> +
> +#include "max9850.h"
> +
> +struct max9850_priv {
> +	unsigned int sysclk;
> +};
> +
> +/* max9850 register cache */
> +static const u8 max9850_reg[MAX9850_CACHEREGNUM] = {
> +	0x00, 0x00, 0x0c, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
> +};
> +
> +/* these registers are not used at the moment but provided for the sake of
> + * completeness */
> +static int max9850_volatile_register(unsigned int reg)
> +{
> +	switch (reg) {
> +	case MAX9850_STATUSA:
> +	case MAX9850_STATUSB:
> +		return 1;
> +	default:
> +		return 0;
> +	}
> +}

This code doesn't seem to have been developed against for-2.6.39.  The
signature of the volatile_register callback has changed to include a
pointer to the snd_soc_codec structure.

> +static const unsigned int max9850_tlv[] = {
> +	TLV_DB_RANGE_HEAD(4),
> +	0x18, 0x1f, TLV_DB_SCALE_ITEM(-7450, 400, 0),
> +	0x20, 0x33, TLV_DB_SCALE_ITEM(-4150, 200, 0),
> +	0x34, 0x37, TLV_DB_SCALE_ITEM(-150, 100, 0),
> +	0x38, 0x3f, TLV_DB_SCALE_ITEM(250, 50, 0),
> +};
> +
> +static const struct snd_kcontrol_new max9850_controls[] = {
> +SOC_SINGLE_TLV("Headphone Volume", MAX9850_VOLUME, 0, 0x3f, 1, max9850_tlv),
> +SOC_SINGLE("Headphone Switch", MAX9850_VOLUME, 7, 1, 1),
> +SOC_SINGLE("Mono", MAX9850_GENERAL_PURPOSE, 2, 1, 0),
> +};

Mono Switch?

> +static const struct snd_kcontrol_new max9850_mixer_controls[] = {
> +	SOC_DAPM_SINGLE("Line In Switch", MAX9850_ENABLE, 1, 1, 0),
> +};
> +
> +static const struct snd_soc_dapm_widget max9850_dapm_widgets[] = {
> +SND_SOC_DAPM_DAC("DAC", "HiFi Playback", MAX9850_ENABLE, 0, 0),
> +SND_SOC_DAPM_SUPPLY("MCLK", MAX9850_ENABLE, 6, 0, NULL, 0),
> +SND_SOC_DAPM_OUTPUT("OUTL"),
> +SND_SOC_DAPM_OUTPUT("OUTR"),
> +SND_SOC_DAPM_OUTPUT("HPL"),
> +SND_SOC_DAPM_OUTPUT("HPR"),
> +SND_SOC_DAPM_INPUT("INL"),
> +SND_SOC_DAPM_INPUT("INR"),
> +SND_SOC_DAPM_PGA("Headphone Output", MAX9850_ENABLE, 3, 0, NULL, 0),
> +SND_SOC_DAPM_MIXER("Line Input", SND_SOC_NOPM, 0, 0, NULL, 0),
> +SND_SOC_DAPM_MIXER_NAMED_CTL("Output Mixer", MAX9850_ENABLE, 2, 0,
> +		&max9850_mixer_controls[0],
> +		ARRAY_SIZE(max9850_mixer_controls)),
> +};

Consider grouping the input and output pins logically separately.

> +static const struct snd_soc_dapm_route intercon[] = {
> +	/* output mixer */
> +	{"Output Mixer", NULL, "DAC"},
> +	{"Output Mixer", "Line In Switch", "Line Input"},
> +
> +	/* outputs */
> +	{"Headphone Output", NULL, "Output Mixer"},
> +	{"HPL", NULL, "Headphone Output"},
> +	{"HPR", NULL, "Headphone Output"},
> +	{"OUTL", NULL, "Output Mixer"},
> +	{"OUTR", NULL, "Output Mixer"},
> +
> +	/* inputs */
> +	{"Line Input", NULL, "INL"},
> +	{"Line Input", NULL, "INR"},
> +
> +	/* supplies */
> +	{"DAC", NULL, "MCLK"},
> +};

Are all these really statically connected? 

> +static int max9850_hw_params(struct snd_pcm_substream *substream,
> +			     struct snd_pcm_hw_params *params,
> +			     struct snd_soc_dai *dai)
> +{
> +	struct snd_soc_codec *codec = dai->codec;
> +	struct max9850_priv *max9850 = snd_soc_codec_get_drvdata(codec);
> +	u64 lrclk_div;
> +	u8 sf, da;
> +
> +	/* lrclk_div = 2^22 * rate / iclk with iclk = mclk / sf */
> +	sf = (snd_soc_read(codec, MAX9850_CLOCK) >> 2) + 1;
> +	lrclk_div = (1 << 22);
> +	lrclk_div *= params_rate(params);
> +	lrclk_div *= sf;
> +	do_div(lrclk_div, max9850->sysclk);
> +
> +	snd_soc_write(codec, MAX9850_LRCLK_MSB, (lrclk_div >> 8) & 0x7f);
> +	snd_soc_write(codec, MAX9850_LRCLK_LSB, lrclk_div & 0xff);
> +
> +	da = snd_soc_read(codec, MAX9850_DIGITAL_AUDIO);
> +	switch (params_format(params)) {
> +	case SNDRV_PCM_FORMAT_S16_LE:
> +		break;
> +	case SNDRV_PCM_FORMAT_S20_3LE:
> +		da |= 0x2;
> +		break;
> +	case SNDRV_PCM_FORMAT_S24_LE:
> +		da |= 0x3;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +	snd_soc_write(codec, MAX9850_DIGITAL_AUDIO, da);
> +
> +	return 0;
> +}
> +
> +static int max9850_set_dai_sysclk(struct snd_soc_dai *codec_dai,
> +		int clk_id, unsigned int freq, int dir)
> +{
> +	struct snd_soc_codec *codec = codec_dai->codec;
> +	struct max9850_priv *max9850 = snd_soc_codec_get_drvdata(codec);
> +
> +	/* calculate mclk -> iclk divider */
> +	if (freq <= 13000000)
> +		snd_soc_write(codec, MAX9850_CLOCK, 0x0);
> +	else if (freq <= 26000000)
> +		snd_soc_write(codec, MAX9850_CLOCK, 0x4);
> +	else if (freq <= 40000000)
> +		snd_soc_write(codec, MAX9850_CLOCK, 0x8);
> +	else
> +		return -EINVAL;
> +
> +	max9850->sysclk = freq;
> +	return 0;
> +}
> +
> +static int max9850_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt)
> +{
> +	struct snd_soc_codec *codec = codec_dai->codec;
> +	u8 da = 0;
> +
> +	/* set master/slave audio interface */
> +	switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
> +	case SND_SOC_DAIFMT_CBM_CFM:
> +		da |= MAX9850_MASTER;
> +		break;
> +	case SND_SOC_DAIFMT_CBS_CFS:
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	/* interface format */
> +	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> +	case SND_SOC_DAIFMT_I2S:
> +		da |= MAX9850_DLY;
> +		break;
> +	case SND_SOC_DAIFMT_RIGHT_J:
> +		da |= MAX9850_RTJ;
> +		break;
> +	case SND_SOC_DAIFMT_LEFT_J:
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	/* clock inversion */
> +	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
> +	case SND_SOC_DAIFMT_NB_NF:
> +		break;
> +	case SND_SOC_DAIFMT_IB_IF:
> +		da |= MAX9850_BCINV | MAX9850_INV;
> +		break;
> +	case SND_SOC_DAIFMT_IB_NF:
> +		da |= MAX9850_BCINV;
> +		break;
> +	case SND_SOC_DAIFMT_NB_IF:
> +		da |= MAX9850_INV;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	/* set da */
> +	snd_soc_write(codec, MAX9850_DIGITAL_AUDIO, da);
> +
> +	return 0;
> +}
> +
> +static int max9850_set_bias_level(struct snd_soc_codec *codec,
> +				  enum snd_soc_bias_level level)
> +{
> +	switch (level) {
> +	case SND_SOC_BIAS_ON:
> +		break;
> +	case SND_SOC_BIAS_PREPARE:
> +		snd_soc_update_bits(codec, MAX9850_ENABLE, MAX9850_SHDN,
> +				MAX9850_SHDN);

Could possibly be handled by DAPM?

> +		break;
> +	case SND_SOC_BIAS_STANDBY:
> +		snd_soc_update_bits(codec, MAX9850_ENABLE, MAX9850_SHDN, 0);

Ditto.

> +		break;
> +	case SND_SOC_BIAS_OFF:
> +		break;
> +	}
> +	codec->dapm.bias_level = level;
> +	return 0;
> +}

I don't see any suspend/resume callbacks.  It'd be good if you could
provide default stubs that'd just set the bias level.  Also syncing the
cache when the bias level changes from BIAS_OFF to STANDBY would be a plus.

> +#define MAX9850_RATES SNDRV_PCM_RATE_8000_48000
> +
> +#define MAX9850_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE |\
> +	SNDRV_PCM_FMTBIT_S24_LE)
> +
> +static struct snd_soc_dai_ops max9850_dai_ops = {
> +	.hw_params	= max9850_hw_params,
> +	.set_sysclk	= max9850_set_dai_sysclk,
> +	.set_fmt	= max9850_set_dai_fmt,
> +};
> +
> +static struct snd_soc_dai_driver max9850_dai = {
> +	.name = "max9850-hifi",
> +	.playback = {
> +		.stream_name = "Playback",
> +		.channels_min = 1,
> +		.channels_max = 2,
> +		.rates = MAX9850_RATES,
> +		.formats = MAX9850_FORMATS
> +	},
> +	.ops = &max9850_dai_ops,
> +};
> +
> +static int max9850_probe(struct snd_soc_codec *codec)
> +{
> +	struct snd_soc_dapm_context *dapm = &codec->dapm;
> +	int ret;
> +
> +	ret = snd_soc_codec_set_cache_io(codec, 8, 8, SND_SOC_I2C);
> +	if (ret < 0) {
> +		dev_err(codec->dev, "Failed to set cache I/O: %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* enable zero-detect */
> +	snd_soc_update_bits(codec, MAX9850_GENERAL_PURPOSE, 1, 1);
> +	/* enable charge pump, disable everything else */
> +	snd_soc_write(codec, MAX9850_ENABLE, 0x30);

DAPM?

> +	/* enable slew-rate control */
> +	snd_soc_update_bits(codec, MAX9850_VOLUME, 0x40, 0x40);
> +	/* set slew-rate 125ms */
> +	snd_soc_update_bits(codec, MAX9850_CHARGE_PUMP, 0xff, 0xc0);
> +
> +	snd_soc_dapm_new_controls(dapm, max9850_dapm_widgets,
> +				  ARRAY_SIZE(max9850_dapm_widgets));
> +	snd_soc_dapm_add_routes(dapm, intercon, ARRAY_SIZE(intercon));
> +
> +	snd_soc_add_controls(codec, max9850_controls,
> +			ARRAY_SIZE(max9850_controls));
> +
> +	return 0;
> +}
> +static int max9850_remove(struct snd_soc_codec *codec)
> +{
> +	return 0;
> +}

Setting the bias level to OFF would be preferable here.

> +static struct snd_soc_codec_driver soc_codec_dev_max9850 = {
> +	.probe =	max9850_probe,
> +	.remove =	max9850_remove,
> +	.set_bias_level = max9850_set_bias_level,
> +	.reg_cache_size = ARRAY_SIZE(max9850_reg),
> +	.reg_word_size = sizeof(u8),
> +	.reg_cache_default = max9850_reg,
> +	.volatile_register = max9850_volatile_register,
> +};
> +
> +static int __devinit max9850_i2c_probe(struct i2c_client *i2c,
> +		const struct i2c_device_id *id)
> +{
> +	struct max9850_priv *max9850;
> +	int ret;
> +
> +	max9850 = kzalloc(sizeof(struct max9850_priv), GFP_KERNEL);
> +	if (max9850 == NULL)
> +		return -ENOMEM;
> +
> +	i2c_set_clientdata(i2c, max9850);
> +
> +	ret = snd_soc_register_codec(&i2c->dev,
> +			&soc_codec_dev_max9850, &max9850_dai, 1);
> +	if (ret < 0)
> +		kfree(max9850);
> +	return ret;
> +}
> +
> +static __devexit int max9850_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 max9850_i2c_id[] = {
> +	{ "max9850", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, max9850_i2c_id);
> +
> +static struct i2c_driver max9850_i2c_driver = {
> +	.driver = {
> +		.name = "max9850-codec",

Remove the `-codec'.

> +		.owner = THIS_MODULE,
> +	},
> +	.probe = max9850_i2c_probe,
> +	.remove = __devexit_p(max9850_i2c_remove),
> +	.id_table = max9850_i2c_id,
> +};
> +
> +static int __init max9850_init(void)
> +{
> +	return i2c_add_driver(&max9850_i2c_driver);
> +}
> +module_init(max9850_init);
> +
> +static void __exit max9850_exit(void)
> +{
> +	i2c_del_driver(&max9850_i2c_driver);
> +}
> +module_exit(max9850_exit);
> +
> +MODULE_AUTHOR("Christian Glindkamp <christian.glindkamp@taskit.de>");
> +MODULE_DESCRIPTION("ASoC MAX9850 codec driver");
> +MODULE_LICENSE("GPL");
> diff --git a/sound/soc/codecs/max9850.h b/sound/soc/codecs/max9850.h
> new file mode 100644
> index 0000000..5268575
> --- /dev/null
> +++ b/sound/soc/codecs/max9850.h
> @@ -0,0 +1,41 @@
> +/*
> + * max9850.h  --  codec driver for max9850
> + *
> + * Copyright (C) 2011 taskit GmbH
> + * Author: Christian Glindkamp <christian.glindkamp@taskit.de>
> + *
> + * 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 _MAX9850_H
> +#define _MAX9850_H
> +
> +#define MAX9850_STATUSA			0x00
> +#define MAX9850_STATUSB			0x01
> +#define MAX9850_VOLUME			0x02
> +#define MAX9850_GENERAL_PURPOSE		0x03
> +#define MAX9850_INTERRUPT		0x04
> +#define MAX9850_ENABLE			0x05
> +#define MAX9850_CLOCK			0x06
> +#define MAX9850_CHARGE_PUMP		0x07
> +#define MAX9850_LRCLK_MSB		0x08
> +#define MAX9850_LRCLK_LSB		0x09
> +#define MAX9850_DIGITAL_AUDIO		0x0a
> +
> +#define MAX9850_CACHEREGNUM 11
> +
> +/* MAX9850_ENABLE */
> +#define MAX9850_SHDN			(1<<7)
> +
> +/* MAX9850_DIGITAL_AUDIO */
> +#define MAX9850_MASTER			(1<<7)
> +#define MAX9850_INV			(1<<6)
> +#define MAX9850_BCINV			(1<<5)
> +#define MAX9850_DLY			(1<<3)
> +#define MAX9850_RTJ			(1<<2)
> +
> +#endif
> -- 
> 1.7.2.3
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

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

* Re: [PATCH] ASoC: Add MAX9850 codec driver
  2011-03-07 12:45 [PATCH] ASoC: Add MAX9850 codec driver Christian Glindkamp
  2011-03-07 13:48 ` Dimitris Papastamos
@ 2011-03-07 15:15 ` Mark Brown
  2011-03-08  9:51   ` Christian Glindkamp
  2011-03-07 16:40 ` Seungwhan Youn
  2011-03-09 10:20 ` [PATCH v2] " Christian Glindkamp
  3 siblings, 1 reply; 15+ messages in thread
From: Mark Brown @ 2011-03-07 15:15 UTC (permalink / raw)
  To: Christian Glindkamp; +Cc: alsa-devel, lrg

On Mon, Mar 07, 2011 at 01:45:13PM +0100, Christian Glindkamp wrote:

> I've all ready sent this patch some time ago, but it hung in the moderation
> queue. This is a slightly modified version. Unfortunately I do not have the
> hardware anymore to test suggested changes that alter function.

As mentioned in SubmittingPatches you should always CC maintainers on
patches.  This will bypass things like list moderation and also helps
ensure that people see your mail on busy mailing lists.

Overall this looks pretty good.

> +static const struct snd_kcontrol_new max9850_controls[] = {
> +SOC_SINGLE_TLV("Headphone Volume", MAX9850_VOLUME, 0, 0x3f, 1, max9850_tlv),
> +SOC_SINGLE("Headphone Switch", MAX9850_VOLUME, 7, 1, 1),
> +SOC_SINGLE("Mono", MAX9850_GENERAL_PURPOSE, 2, 1, 0),

...Switch.

> +	/* lrclk_div = 2^22 * rate / iclk with iclk = mclk / sf */
> +	sf = (snd_soc_read(codec, MAX9850_CLOCK) >> 2) + 1;
> +	lrclk_div = (1 << 22);
> +	lrclk_div *= params_rate(params);
> +	lrclk_div *= sf;
> +	do_div(lrclk_div, max9850->sysclk);

Should check that sysclk() has been set and return an error if not
before doing the division here.

> +	da = snd_soc_read(codec, MAX9850_DIGITAL_AUDIO);
> +	switch (params_format(params)) {
> +	case SNDRV_PCM_FORMAT_S16_LE:
> +		break;
> +	case SNDRV_PCM_FORMAT_S20_3LE:
> +		da |= 0x2;
> +		break;
> +	case SNDRV_PCM_FORMAT_S24_LE:
> +		da |= 0x3;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +	snd_soc_write(codec, MAX9850_DIGITAL_AUDIO, da);

Use snd_soc_update_bits() here.  The above doesn't clear any of the
bits so if 20 or 24 bit is set they'll get latched.

> +static int max9850_set_bias_level(struct snd_soc_codec *codec,
> +				  enum snd_soc_bias_level level)
> +{
> +	switch (level) {
> +	case SND_SOC_BIAS_ON:
> +		break;
> +	case SND_SOC_BIAS_PREPARE:
> +		snd_soc_update_bits(codec, MAX9850_ENABLE, MAX9850_SHDN,
> +				MAX9850_SHDN);
> +		break;
> +	case SND_SOC_BIAS_STANDBY:
> +		snd_soc_update_bits(codec, MAX9850_ENABLE, MAX9850_SHDN, 0);
> +		break;
> +	case SND_SOC_BIAS_OFF:
> +		break;

This pattern looks awfully like you should set idle_bias_off for the
CODEC and then move the code blocks down one power level - _SHDN sounds
like a general power down for the device which fits better with the _OFF
state.

Or use DAPM like Dimitris suggested.

> +	/* enable charge pump, disable everything else */
> +	snd_soc_write(codec, MAX9850_ENABLE, 0x30);

This sounds like it should be in either bias management or DAPM.

> +	/* enable slew-rate control */
> +	snd_soc_update_bits(codec, MAX9850_VOLUME, 0x40, 0x40);
> +	/* set slew-rate 125ms */
> +	snd_soc_update_bits(codec, MAX9850_CHARGE_PUMP, 0xff, 0xc0);

If this is for the charge pump this is fine but the _VOLUME makes it
sound like this should perhaps be user visibile?

> +static int max9850_remove(struct snd_soc_codec *codec)
> +{
> +	return 0;
> +}

Just delete this if not required, all ASoC functions should be optional.

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

* Re: [PATCH] ASoC: Add MAX9850 codec driver
  2011-03-07 12:45 [PATCH] ASoC: Add MAX9850 codec driver Christian Glindkamp
  2011-03-07 13:48 ` Dimitris Papastamos
  2011-03-07 15:15 ` Mark Brown
@ 2011-03-07 16:40 ` Seungwhan Youn
  2011-03-08 10:21   ` Dimitris Papastamos
  2011-03-09 10:20 ` [PATCH v2] " Christian Glindkamp
  3 siblings, 1 reply; 15+ messages in thread
From: Seungwhan Youn @ 2011-03-07 16:40 UTC (permalink / raw)
  To: Christian Glindkamp; +Cc: alsa-devel, Mark Brown, Liam Girdwood

Hi,

> diff --git a/sound/soc/codecs/max9850.h b/sound/soc/codecs/max9850.h
> new file mode 100644
> index 0000000..5268575
> --- /dev/null
> +++ b/sound/soc/codecs/max9850.h
> @@ -0,0 +1,41 @@
> +/*
> + * max9850.h  --  codec driver for max9850
> + *
> + * Copyright (C) 2011 taskit GmbH
> + * Author: Christian Glindkamp <christian.glindkamp@taskit.de>
> + *
> + * 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 _MAX9850_H
> +#define _MAX9850_H
> +
> +#define MAX9850_STATUSA                        0x00
> +#define MAX9850_STATUSB                        0x01
> +#define MAX9850_VOLUME                 0x02
> +#define MAX9850_GENERAL_PURPOSE                0x03
> +#define MAX9850_INTERRUPT              0x04
> +#define MAX9850_ENABLE                 0x05
> +#define MAX9850_CLOCK                  0x06
> +#define MAX9850_CHARGE_PUMP            0x07
> +#define MAX9850_LRCLK_MSB              0x08
> +#define MAX9850_LRCLK_LSB              0x09
> +#define MAX9850_DIGITAL_AUDIO          0x0a
> +
> +#define MAX9850_CACHEREGNUM 11
> +
> +/* MAX9850_ENABLE */
> +#define MAX9850_SHDN                   (1<<7)
> +
> +/* MAX9850_DIGITAL_AUDIO */
> +#define MAX9850_MASTER                 (1<<7)
> +#define MAX9850_INV                    (1<<6)
> +#define MAX9850_BCINV                  (1<<5)
> +#define MAX9850_DLY                    (1<<3)
> +#define MAX9850_RTJ                    (1<<2)
> +
> +#endif

IMHO, because of MAX9850_* macros are only using inside max9850.c, I
think that move MAX9850_* macros into max9850.c and remove max9850.h
header file, is more looks good to me.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

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

* Re: [PATCH] ASoC: Add MAX9850 codec driver
  2011-03-07 13:48 ` Dimitris Papastamos
@ 2011-03-07 17:04   ` Christian Glindkamp
  2011-03-07 17:14     ` Mark Brown
  2011-03-08 10:14     ` Dimitris Papastamos
  0 siblings, 2 replies; 15+ messages in thread
From: Christian Glindkamp @ 2011-03-07 17:04 UTC (permalink / raw)
  To: Dimitris Papastamos; +Cc: alsa-devel

Resend to list, sorry for the noise Dimitris

As all ready said, I can't really test these changes at the moment but
will at least compile test them. Additionally, alsa master currently
does not build here for the arm architecture. Further comments below.

On 2011-03-07 13:48, Dimitris Papastamos wrote:
> On Mon, Mar 07, 2011 at 01:45:13PM +0100, Christian Glindkamp wrote:
> > This patch adds ASoC support for the MAX9850 codec with headphone
> > amplifier.
> > 
> > Supported features:
> > - Playback
> > - 16, 20 and 24 bit audio
> > - 8k - 48k sample rates
> > - DAPM
> > 
> > Only 16 bit audio was tested while the codec was connected to an
> > AT91SAM9G20 SSC in master mode.
> > 
> > Signed-off-by: Christian Glindkamp <christian.glindkamp@taskit.de>
> > ---
> > 
> > I've all ready sent this patch some time ago, but it hung in the moderation
> > queue. This is a slightly modified version. Unfortunately I do not have the
> > hardware anymore to test suggested changes that alter function.
> > 
> >  sound/soc/codecs/Kconfig   |    4 +
> >  sound/soc/codecs/Makefile  |    2 +
> >  sound/soc/codecs/max9850.c |  358 ++++++++++++++++++++++++++++++++++++++++++++
> >  sound/soc/codecs/max9850.h |   41 +++++
> >  4 files changed, 405 insertions(+), 0 deletions(-)
> >  create mode 100644 sound/soc/codecs/max9850.c
> >  create mode 100644 sound/soc/codecs/max9850.h
> > 
> > diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> > index e239345..51e9844 100644
> > --- a/sound/soc/codecs/Kconfig
> > +++ b/sound/soc/codecs/Kconfig
> > @@ -31,6 +31,7 @@ config SND_SOC_ALL_CODECS
> >  	select SND_SOC_DA7210 if I2C
> >  	select SND_SOC_JZ4740_CODEC if SOC_JZ4740
> >  	select SND_SOC_MAX98088 if I2C
> > +	select SND_SOC_MAX9850 if I2C
> >  	select SND_SOC_MAX9877 if I2C
> >  	select SND_SOC_PCM3008
> >  	select SND_SOC_SN95031 if INTEL_SCU_IPC
> > @@ -179,6 +180,9 @@ config SND_SOC_DMIC
> >  config SND_SOC_MAX98088
> >         tristate
> >  
> > +config SND_SOC_MAX9850
> > +	tristate
> > +
> >  config SND_SOC_PCM3008
> >         tristate
> >  
> > diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
> > index ae10507..f2efd1c 100644
> > --- a/sound/soc/codecs/Makefile
> > +++ b/sound/soc/codecs/Makefile
> > @@ -18,6 +18,7 @@ snd-soc-da7210-objs := da7210.o
> >  snd-soc-dmic-objs := dmic.o
> >  snd-soc-l3-objs := l3.o
> >  snd-soc-max98088-objs := max98088.o
> > +snd-soc-max9850-objs := max9850.o
> >  snd-soc-pcm3008-objs := pcm3008.o
> >  snd-soc-alc5623-objs := alc5623.o
> >  snd-soc-sn95031-objs := sn95031.o
> > @@ -102,6 +103,7 @@ obj-$(CONFIG_SND_SOC_DMIC)	+= snd-soc-dmic.o
> >  obj-$(CONFIG_SND_SOC_L3)	+= snd-soc-l3.o
> >  obj-$(CONFIG_SND_SOC_JZ4740_CODEC)	+= snd-soc-jz4740-codec.o
> >  obj-$(CONFIG_SND_SOC_MAX98088)	+= snd-soc-max98088.o
> > +obj-$(CONFIG_SND_SOC_MAX9850)	+= snd-soc-max9850.o
> >  obj-$(CONFIG_SND_SOC_PCM3008)	+= snd-soc-pcm3008.o
> >  obj-$(CONFIG_SND_SOC_SN95031)	+=snd-soc-sn95031.o
> >  obj-$(CONFIG_SND_SOC_SPDIF)	+= snd-soc-spdif.o
> > diff --git a/sound/soc/codecs/max9850.c b/sound/soc/codecs/max9850.c
> > new file mode 100644
> > index 0000000..a8c1f95
> > --- /dev/null
> > +++ b/sound/soc/codecs/max9850.c
> > @@ -0,0 +1,358 @@
> > +/*
> > + * max9850.c  --  codec driver for max9850
> > + *
> > + * Copyright (C) 2011 taskit GmbH
> > + *
> > + * Author: Christian Glindkamp <christian.glindkamp@taskit.de>
> > + *
> > + * Initial development of this code was funded by
> > + * MICRONIC Computer Systeme GmbH, http://www.mcsberlin.de/
> > + *
> > + * 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/init.h>
> > +#include <linux/i2c.h>
> > +#include <linux/slab.h>
> > +#include <sound/pcm.h>
> > +#include <sound/pcm_params.h>
> > +#include <sound/soc.h>
> > +#include <sound/tlv.h>
> > +
> > +#include "max9850.h"
> > +
> > +struct max9850_priv {
> > +	unsigned int sysclk;
> > +};
> > +
> > +/* max9850 register cache */
> > +static const u8 max9850_reg[MAX9850_CACHEREGNUM] = {
> > +	0x00, 0x00, 0x0c, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
> > +};
> > +
> > +/* these registers are not used at the moment but provided for the sake of
> > + * completeness */
> > +static int max9850_volatile_register(unsigned int reg)
> > +{
> > +	switch (reg) {
> > +	case MAX9850_STATUSA:
> > +	case MAX9850_STATUSB:
> > +		return 1;
> > +	default:
> > +		return 0;
> > +	}
> > +}
> 
> This code doesn't seem to have been developed against for-2.6.39.  The
> signature of the volatile_register callback has changed to include a
> pointer to the snd_soc_codec structure.
> 

Have not seen this branch. I will use a newer revision of alsa master if
that is ok.

> > +static const unsigned int max9850_tlv[] = {
> > +	TLV_DB_RANGE_HEAD(4),
> > +	0x18, 0x1f, TLV_DB_SCALE_ITEM(-7450, 400, 0),
> > +	0x20, 0x33, TLV_DB_SCALE_ITEM(-4150, 200, 0),
> > +	0x34, 0x37, TLV_DB_SCALE_ITEM(-150, 100, 0),
> > +	0x38, 0x3f, TLV_DB_SCALE_ITEM(250, 50, 0),
> > +};
> > +
> > +static const struct snd_kcontrol_new max9850_controls[] = {
> > +SOC_SINGLE_TLV("Headphone Volume", MAX9850_VOLUME, 0, 0x3f, 1, max9850_tlv),
> > +SOC_SINGLE("Headphone Switch", MAX9850_VOLUME, 7, 1, 1),
> > +SOC_SINGLE("Mono", MAX9850_GENERAL_PURPOSE, 2, 1, 0),
> > +};
> 
> Mono Switch?
> 
> > +static const struct snd_kcontrol_new max9850_mixer_controls[] = {
> > +	SOC_DAPM_SINGLE("Line In Switch", MAX9850_ENABLE, 1, 1, 0),
> > +};
> > +
> > +static const struct snd_soc_dapm_widget max9850_dapm_widgets[] = {
> > +SND_SOC_DAPM_DAC("DAC", "HiFi Playback", MAX9850_ENABLE, 0, 0),
> > +SND_SOC_DAPM_SUPPLY("MCLK", MAX9850_ENABLE, 6, 0, NULL, 0),
> > +SND_SOC_DAPM_OUTPUT("OUTL"),
> > +SND_SOC_DAPM_OUTPUT("OUTR"),
> > +SND_SOC_DAPM_OUTPUT("HPL"),
> > +SND_SOC_DAPM_OUTPUT("HPR"),
> > +SND_SOC_DAPM_INPUT("INL"),
> > +SND_SOC_DAPM_INPUT("INR"),
> > +SND_SOC_DAPM_PGA("Headphone Output", MAX9850_ENABLE, 3, 0, NULL, 0),
> > +SND_SOC_DAPM_MIXER("Line Input", SND_SOC_NOPM, 0, 0, NULL, 0),
> > +SND_SOC_DAPM_MIXER_NAMED_CTL("Output Mixer", MAX9850_ENABLE, 2, 0,
> > +		&max9850_mixer_controls[0],
> > +		ARRAY_SIZE(max9850_mixer_controls)),
> > +};
> 
> Consider grouping the input and output pins logically separately.
> 

Do you mean something like that?

SND_SOC_DAPM_SUPPLY("MCLK", MAX9850_ENABLE, 6, 0, NULL, 0),
SND_SOC_DAPM_MIXER_NAMED_CTL("Output Mixer", MAX9850_ENABLE, 2, 0,
		&max9850_mixer_controls[0],
		ARRAY_SIZE(max9850_mixer_controls)),
SND_SOC_DAPM_PGA("Headphone Output", MAX9850_ENABLE, 3, 0, NULL, 0),
SND_SOC_DAPM_DAC("DAC", "HiFi Playback", MAX9850_ENABLE, 0, 0),
SND_SOC_DAPM_OUTPUT("OUTL"),
SND_SOC_DAPM_OUTPUT("HPL"),
SND_SOC_DAPM_OUTPUT("OUTR"),
SND_SOC_DAPM_OUTPUT("HPR"),
SND_SOC_DAPM_MIXER("Line Input", SND_SOC_NOPM, 0, 0, NULL, 0),
SND_SOC_DAPM_INPUT("INL"),
SND_SOC_DAPM_INPUT("INR"),

> > +static const struct snd_soc_dapm_route intercon[] = {
> > +	/* output mixer */
> > +	{"Output Mixer", NULL, "DAC"},
> > +	{"Output Mixer", "Line In Switch", "Line Input"},
> > +
> > +	/* outputs */
> > +	{"Headphone Output", NULL, "Output Mixer"},
> > +	{"HPL", NULL, "Headphone Output"},
> > +	{"HPR", NULL, "Headphone Output"},
> > +	{"OUTL", NULL, "Output Mixer"},
> > +	{"OUTR", NULL, "Output Mixer"},
> > +
> > +	/* inputs */
> > +	{"Line Input", NULL, "INL"},
> > +	{"Line Input", NULL, "INR"},
> > +
> > +	/* supplies */
> > +	{"DAC", NULL, "MCLK"},
> > +};
> 
> Are all these really statically connected? 
> 

Yes, it is a relatively primitive codec. You can only switch on/off line
in/out and the headphones. No routing is possible. Actually, what is
called "Output Mixer" here is in reality line out, but it needs to be
enabled for the headphone outputs to work.

> > +static int max9850_hw_params(struct snd_pcm_substream *substream,
> > +			     struct snd_pcm_hw_params *params,
> > +			     struct snd_soc_dai *dai)
> > +{
> > +	struct snd_soc_codec *codec = dai->codec;
> > +	struct max9850_priv *max9850 = snd_soc_codec_get_drvdata(codec);
> > +	u64 lrclk_div;
> > +	u8 sf, da;
> > +
> > +	/* lrclk_div = 2^22 * rate / iclk with iclk = mclk / sf */
> > +	sf = (snd_soc_read(codec, MAX9850_CLOCK) >> 2) + 1;
> > +	lrclk_div = (1 << 22);
> > +	lrclk_div *= params_rate(params);
> > +	lrclk_div *= sf;
> > +	do_div(lrclk_div, max9850->sysclk);
> > +
> > +	snd_soc_write(codec, MAX9850_LRCLK_MSB, (lrclk_div >> 8) & 0x7f);
> > +	snd_soc_write(codec, MAX9850_LRCLK_LSB, lrclk_div & 0xff);
> > +
> > +	da = snd_soc_read(codec, MAX9850_DIGITAL_AUDIO);
> > +	switch (params_format(params)) {
> > +	case SNDRV_PCM_FORMAT_S16_LE:
> > +		break;
> > +	case SNDRV_PCM_FORMAT_S20_3LE:
> > +		da |= 0x2;
> > +		break;
> > +	case SNDRV_PCM_FORMAT_S24_LE:
> > +		da |= 0x3;
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +	snd_soc_write(codec, MAX9850_DIGITAL_AUDIO, da);
> > +
> > +	return 0;
> > +}
> > +
> > +static int max9850_set_dai_sysclk(struct snd_soc_dai *codec_dai,
> > +		int clk_id, unsigned int freq, int dir)
> > +{
> > +	struct snd_soc_codec *codec = codec_dai->codec;
> > +	struct max9850_priv *max9850 = snd_soc_codec_get_drvdata(codec);
> > +
> > +	/* calculate mclk -> iclk divider */
> > +	if (freq <= 13000000)
> > +		snd_soc_write(codec, MAX9850_CLOCK, 0x0);
> > +	else if (freq <= 26000000)
> > +		snd_soc_write(codec, MAX9850_CLOCK, 0x4);
> > +	else if (freq <= 40000000)
> > +		snd_soc_write(codec, MAX9850_CLOCK, 0x8);
> > +	else
> > +		return -EINVAL;
> > +
> > +	max9850->sysclk = freq;
> > +	return 0;
> > +}
> > +
> > +static int max9850_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt)
> > +{
> > +	struct snd_soc_codec *codec = codec_dai->codec;
> > +	u8 da = 0;
> > +
> > +	/* set master/slave audio interface */
> > +	switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
> > +	case SND_SOC_DAIFMT_CBM_CFM:
> > +		da |= MAX9850_MASTER;
> > +		break;
> > +	case SND_SOC_DAIFMT_CBS_CFS:
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
> > +	/* interface format */
> > +	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> > +	case SND_SOC_DAIFMT_I2S:
> > +		da |= MAX9850_DLY;
> > +		break;
> > +	case SND_SOC_DAIFMT_RIGHT_J:
> > +		da |= MAX9850_RTJ;
> > +		break;
> > +	case SND_SOC_DAIFMT_LEFT_J:
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
> > +	/* clock inversion */
> > +	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
> > +	case SND_SOC_DAIFMT_NB_NF:
> > +		break;
> > +	case SND_SOC_DAIFMT_IB_IF:
> > +		da |= MAX9850_BCINV | MAX9850_INV;
> > +		break;
> > +	case SND_SOC_DAIFMT_IB_NF:
> > +		da |= MAX9850_BCINV;
> > +		break;
> > +	case SND_SOC_DAIFMT_NB_IF:
> > +		da |= MAX9850_INV;
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
> > +	/* set da */
> > +	snd_soc_write(codec, MAX9850_DIGITAL_AUDIO, da);
> > +
> > +	return 0;
> > +}
> > +
> > +static int max9850_set_bias_level(struct snd_soc_codec *codec,
> > +				  enum snd_soc_bias_level level)
> > +{
> > +	switch (level) {
> > +	case SND_SOC_BIAS_ON:
> > +		break;
> > +	case SND_SOC_BIAS_PREPARE:
> > +		snd_soc_update_bits(codec, MAX9850_ENABLE, MAX9850_SHDN,
> > +				MAX9850_SHDN);
> 
> Could possibly be handled by DAPM?
> 

I could wire them to be the supply for the "Output Mixer". Without it
being enabled, the codec does nothing anyway.

> > +		break;
> > +	case SND_SOC_BIAS_STANDBY:
> > +		snd_soc_update_bits(codec, MAX9850_ENABLE, MAX9850_SHDN, 0);
> 
> Ditto.
> 
> > +		break;
> > +	case SND_SOC_BIAS_OFF:
> > +		break;
> > +	}
> > +	codec->dapm.bias_level = level;
> > +	return 0;
> > +}
> 
> I don't see any suspend/resume callbacks.  It'd be good if you could
> provide default stubs that'd just set the bias level.  Also syncing the
> cache when the bias level changes from BIAS_OFF to STANDBY would be a plus.
> 

Will look into it.

> > +#define MAX9850_RATES SNDRV_PCM_RATE_8000_48000
> > +
> > +#define MAX9850_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE |\
> > +	SNDRV_PCM_FMTBIT_S24_LE)
> > +
> > +static struct snd_soc_dai_ops max9850_dai_ops = {
> > +	.hw_params	= max9850_hw_params,
> > +	.set_sysclk	= max9850_set_dai_sysclk,
> > +	.set_fmt	= max9850_set_dai_fmt,
> > +};
> > +
> > +static struct snd_soc_dai_driver max9850_dai = {
> > +	.name = "max9850-hifi",
> > +	.playback = {
> > +		.stream_name = "Playback",
> > +		.channels_min = 1,
> > +		.channels_max = 2,
> > +		.rates = MAX9850_RATES,
> > +		.formats = MAX9850_FORMATS
> > +	},
> > +	.ops = &max9850_dai_ops,
> > +};
> > +
> > +static int max9850_probe(struct snd_soc_codec *codec)
> > +{
> > +	struct snd_soc_dapm_context *dapm = &codec->dapm;
> > +	int ret;
> > +
> > +	ret = snd_soc_codec_set_cache_io(codec, 8, 8, SND_SOC_I2C);
> > +	if (ret < 0) {
> > +		dev_err(codec->dev, "Failed to set cache I/O: %d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	/* enable zero-detect */
> > +	snd_soc_update_bits(codec, MAX9850_GENERAL_PURPOSE, 1, 1);
> > +	/* enable charge pump, disable everything else */
> > +	snd_soc_write(codec, MAX9850_ENABLE, 0x30);
> 
> DAPM?
> 

Charge pump could also be a supply for the Output Mixer. Have to find
out how to toggle two bits at once via dapm (preferably without
resorting to callbacks).

> > +	/* enable slew-rate control */
> > +	snd_soc_update_bits(codec, MAX9850_VOLUME, 0x40, 0x40);
> > +	/* set slew-rate 125ms */
> > +	snd_soc_update_bits(codec, MAX9850_CHARGE_PUMP, 0xff, 0xc0);
> > +
> > +	snd_soc_dapm_new_controls(dapm, max9850_dapm_widgets,
> > +				  ARRAY_SIZE(max9850_dapm_widgets));
> > +	snd_soc_dapm_add_routes(dapm, intercon, ARRAY_SIZE(intercon));
> > +
> > +	snd_soc_add_controls(codec, max9850_controls,
> > +			ARRAY_SIZE(max9850_controls));
> > +
> > +	return 0;
> > +}
> > +static int max9850_remove(struct snd_soc_codec *codec)
> > +{
> > +	return 0;
> > +}
> 
> Setting the bias level to OFF would be preferable here.
> 
> > +static struct snd_soc_codec_driver soc_codec_dev_max9850 = {
> > +	.probe =	max9850_probe,
> > +	.remove =	max9850_remove,
> > +	.set_bias_level = max9850_set_bias_level,
> > +	.reg_cache_size = ARRAY_SIZE(max9850_reg),
> > +	.reg_word_size = sizeof(u8),
> > +	.reg_cache_default = max9850_reg,
> > +	.volatile_register = max9850_volatile_register,
> > +};
> > +
> > +static int __devinit max9850_i2c_probe(struct i2c_client *i2c,
> > +		const struct i2c_device_id *id)
> > +{
> > +	struct max9850_priv *max9850;
> > +	int ret;
> > +
> > +	max9850 = kzalloc(sizeof(struct max9850_priv), GFP_KERNEL);
> > +	if (max9850 == NULL)
> > +		return -ENOMEM;
> > +
> > +	i2c_set_clientdata(i2c, max9850);
> > +
> > +	ret = snd_soc_register_codec(&i2c->dev,
> > +			&soc_codec_dev_max9850, &max9850_dai, 1);
> > +	if (ret < 0)
> > +		kfree(max9850);
> > +	return ret;
> > +}
> > +
> > +static __devexit int max9850_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 max9850_i2c_id[] = {
> > +	{ "max9850", 0 },
> > +	{ }
> > +};
> > +MODULE_DEVICE_TABLE(i2c, max9850_i2c_id);
> > +
> > +static struct i2c_driver max9850_i2c_driver = {
> > +	.driver = {
> > +		.name = "max9850-codec",
> 
> Remove the `-codec'.
> 
> > +		.owner = THIS_MODULE,
> > +	},
> > +	.probe = max9850_i2c_probe,
> > +	.remove = __devexit_p(max9850_i2c_remove),
> > +	.id_table = max9850_i2c_id,
> > +};
> > +
> > +static int __init max9850_init(void)
> > +{
> > +	return i2c_add_driver(&max9850_i2c_driver);
> > +}
> > +module_init(max9850_init);
> > +
> > +static void __exit max9850_exit(void)
> > +{
> > +	i2c_del_driver(&max9850_i2c_driver);
> > +}
> > +module_exit(max9850_exit);
> > +
> > +MODULE_AUTHOR("Christian Glindkamp <christian.glindkamp@taskit.de>");
> > +MODULE_DESCRIPTION("ASoC MAX9850 codec driver");
> > +MODULE_LICENSE("GPL");
> > diff --git a/sound/soc/codecs/max9850.h b/sound/soc/codecs/max9850.h
> > new file mode 100644
> > index 0000000..5268575
> > --- /dev/null
> > +++ b/sound/soc/codecs/max9850.h
> > @@ -0,0 +1,41 @@
> > +/*
> > + * max9850.h  --  codec driver for max9850
> > + *
> > + * Copyright (C) 2011 taskit GmbH
> > + * Author: Christian Glindkamp <christian.glindkamp@taskit.de>
> > + *
> > + * 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 _MAX9850_H
> > +#define _MAX9850_H
> > +
> > +#define MAX9850_STATUSA			0x00
> > +#define MAX9850_STATUSB			0x01
> > +#define MAX9850_VOLUME			0x02
> > +#define MAX9850_GENERAL_PURPOSE		0x03
> > +#define MAX9850_INTERRUPT		0x04
> > +#define MAX9850_ENABLE			0x05
> > +#define MAX9850_CLOCK			0x06
> > +#define MAX9850_CHARGE_PUMP		0x07
> > +#define MAX9850_LRCLK_MSB		0x08
> > +#define MAX9850_LRCLK_LSB		0x09
> > +#define MAX9850_DIGITAL_AUDIO		0x0a
> > +
> > +#define MAX9850_CACHEREGNUM 11
> > +
> > +/* MAX9850_ENABLE */
> > +#define MAX9850_SHDN			(1<<7)
> > +
> > +/* MAX9850_DIGITAL_AUDIO */
> > +#define MAX9850_MASTER			(1<<7)
> > +#define MAX9850_INV			(1<<6)
> > +#define MAX9850_BCINV			(1<<5)
> > +#define MAX9850_DLY			(1<<3)
> > +#define MAX9850_RTJ			(1<<2)
> > +
> > +#endif
> > -- 
> > 1.7.2.3
> > 
> > _______________________________________________
> > Alsa-devel mailing list
> > Alsa-devel@alsa-project.org
> > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

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

* Re: [PATCH] ASoC: Add MAX9850 codec driver
  2011-03-07 17:04   ` Christian Glindkamp
@ 2011-03-07 17:14     ` Mark Brown
  2011-03-08 10:14     ` Dimitris Papastamos
  1 sibling, 0 replies; 15+ messages in thread
From: Mark Brown @ 2011-03-07 17:14 UTC (permalink / raw)
  To: Christian Glindkamp; +Cc: Dimitris Papastamos, alsa-devel

On Mon, Mar 07, 2011 at 06:04:20PM +0100, Christian Glindkamp wrote:

> As all ready said, I can't really test these changes at the moment but
> will at least compile test them. Additionally, alsa master currently
> does not build here for the arm architecture. Further comments below.

I'm not sure what tree you're building against but mainline ASoC builds
fine on ARM.  ASoC changes should be against the latest for- branch of:

  git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound-2.6.git

(see MAINTAINERS).

> > > +	/* enable zero-detect */
> > > +	snd_soc_update_bits(codec, MAX9850_GENERAL_PURPOSE, 1, 1);
> > > +	/* enable charge pump, disable everything else */
> > > +	snd_soc_write(codec, MAX9850_ENABLE, 0x30);

> > DAPM?

> Charge pump could also be a supply for the Output Mixer. Have to find
> out how to toggle two bits at once via dapm (preferably without
> resorting to callbacks).

If you implement two widgets of the same type DAPM will try to compress
them down into a single register write when updating so the effect will
be to do a single update to both bits.

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

* Re: [PATCH] ASoC: Add MAX9850 codec driver
  2011-03-07 15:15 ` Mark Brown
@ 2011-03-08  9:51   ` Christian Glindkamp
  0 siblings, 0 replies; 15+ messages in thread
From: Christian Glindkamp @ 2011-03-08  9:51 UTC (permalink / raw)
  To: Mark Brown; +Cc: alsa-devel, lrg

On 2011-03-07 15:15, Mark Brown wrote:
> On Mon, Mar 07, 2011 at 01:45:13PM +0100, Christian Glindkamp wrote:
> > +static int max9850_set_bias_level(struct snd_soc_codec *codec,
> > +				  enum snd_soc_bias_level level)
> > +{
> > +	switch (level) {
> > +	case SND_SOC_BIAS_ON:
> > +		break;
> > +	case SND_SOC_BIAS_PREPARE:
> > +		snd_soc_update_bits(codec, MAX9850_ENABLE, MAX9850_SHDN,
> > +				MAX9850_SHDN);
> > +		break;
> > +	case SND_SOC_BIAS_STANDBY:
> > +		snd_soc_update_bits(codec, MAX9850_ENABLE, MAX9850_SHDN, 0);
> > +		break;
> > +	case SND_SOC_BIAS_OFF:
> > +		break;
> 
> This pattern looks awfully like you should set idle_bias_off for the
> CODEC and then move the code blocks down one power level - _SHDN sounds
> like a general power down for the device which fits better with the _OFF
> state.
> 
> Or use DAPM like Dimitris suggested.

I've done so, because it seemed that after the first use, the codec is
hold in standby (when not entering suspend) for the whole time. So I
thought by moving shutdown to standby might save some milliamps. But I
think I will go the DAPM route.

> > +	/* enable slew-rate control */
> > +	snd_soc_update_bits(codec, MAX9850_VOLUME, 0x40, 0x40);
> > +	/* set slew-rate 125ms */
> > +	snd_soc_update_bits(codec, MAX9850_CHARGE_PUMP, 0xff, 0xc0);
> 
> If this is for the charge pump this is fine but the _VOLUME makes it
> sound like this should perhaps be user visibile?

These bits control, whether the volume is set immediately or smoothly
slewn between levels. I don't see a real reason a user would want to
change it.

> 
> > +static int max9850_remove(struct snd_soc_codec *codec)
> > +{
> > +	return 0;
> > +}
> 
> Just delete this if not required, all ASoC functions should be optional.

Will do this, even though Dimitris suggested setting the bias level to
off, because it would still be a nop.

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

* Re: [PATCH] ASoC: Add MAX9850 codec driver
  2011-03-07 17:04   ` Christian Glindkamp
  2011-03-07 17:14     ` Mark Brown
@ 2011-03-08 10:14     ` Dimitris Papastamos
  2011-03-08 11:52       ` Christian Glindkamp
  1 sibling, 1 reply; 15+ messages in thread
From: Dimitris Papastamos @ 2011-03-08 10:14 UTC (permalink / raw)
  To: Christian Glindkamp; +Cc: alsa-devel, Mark Brown, Liam Girdwood

On Mon, Mar 07, 2011 at 06:04:20PM +0100, Christian Glindkamp wrote:
> > > +static const struct snd_soc_dapm_widget max9850_dapm_widgets[] = {
> > > +SND_SOC_DAPM_DAC("DAC", "HiFi Playback", MAX9850_ENABLE, 0, 0),
> > > +SND_SOC_DAPM_SUPPLY("MCLK", MAX9850_ENABLE, 6, 0, NULL, 0),
> > > +SND_SOC_DAPM_OUTPUT("OUTL"),
> > > +SND_SOC_DAPM_OUTPUT("OUTR"),
> > > +SND_SOC_DAPM_OUTPUT("HPL"),
> > > +SND_SOC_DAPM_OUTPUT("HPR"),
> > > +SND_SOC_DAPM_INPUT("INL"),
> > > +SND_SOC_DAPM_INPUT("INR"),
> > > +SND_SOC_DAPM_PGA("Headphone Output", MAX9850_ENABLE, 3, 0, NULL, 0),
> > > +SND_SOC_DAPM_MIXER("Line Input", SND_SOC_NOPM, 0, 0, NULL, 0),
> > > +SND_SOC_DAPM_MIXER_NAMED_CTL("Output Mixer", MAX9850_ENABLE, 2, 0,
> > > +		&max9850_mixer_controls[0],
> > > +		ARRAY_SIZE(max9850_mixer_controls)),
> > > +};
> > 
> > Consider grouping the input and output pins logically separately.
> > 
> 
> Do you mean something like that?
> 
> SND_SOC_DAPM_SUPPLY("MCLK", MAX9850_ENABLE, 6, 0, NULL, 0),
> SND_SOC_DAPM_MIXER_NAMED_CTL("Output Mixer", MAX9850_ENABLE, 2, 0,
> 		&max9850_mixer_controls[0],
> 		ARRAY_SIZE(max9850_mixer_controls)),
> SND_SOC_DAPM_PGA("Headphone Output", MAX9850_ENABLE, 3, 0, NULL, 0),
> SND_SOC_DAPM_DAC("DAC", "HiFi Playback", MAX9850_ENABLE, 0, 0),
> SND_SOC_DAPM_OUTPUT("OUTL"),
> SND_SOC_DAPM_OUTPUT("HPL"),
> SND_SOC_DAPM_OUTPUT("OUTR"),
> SND_SOC_DAPM_OUTPUT("HPR"),
> SND_SOC_DAPM_MIXER("Line Input", SND_SOC_NOPM, 0, 0, NULL, 0),
> SND_SOC_DAPM_INPUT("INL"),
> SND_SOC_DAPM_INPUT("INR"),

Yea, that's fine.  Although the standard thing to do is to group all
the input pins at the beginning and all the output pins at the end.
However this is a pretty primitive codec so it is not a problem as it
stands now.
 
> Charge pump could also be a supply for the Output Mixer. Have to find
> out how to toggle two bits at once via dapm (preferably without
> resorting to callbacks).

If they are in the same register and same type of widgets then as Mark
mentioned the core guarantees that it'll be done in one register write.

I forgot to mention in the previous e-mail, that if you add the cache
syncing support during resume or whenever we go from BIAS_OFF to
STANDBY use the standard ASoC function for that, snd_soc_cache_sync().
Just in case you are looking at older examples.

Thanks,
Dimitris

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

* Re: [PATCH] ASoC: Add MAX9850 codec driver
  2011-03-07 16:40 ` Seungwhan Youn
@ 2011-03-08 10:21   ` Dimitris Papastamos
  0 siblings, 0 replies; 15+ messages in thread
From: Dimitris Papastamos @ 2011-03-08 10:21 UTC (permalink / raw)
  To: Seungwhan Youn; +Cc: Christian Glindkamp, Mark Brown, alsa-devel, Liam Girdwood

On Tue, Mar 08, 2011 at 01:40:05AM +0900, Seungwhan Youn wrote:
> IMHO, because of MAX9850_* macros are only using inside max9850.c, I
> think that move MAX9850_* macros into max9850.c and remove max9850.h
> header file, is more looks good to me.

It is common practice to have the register definitions / enumerations in
a header file even though they are only used by 1 driver.  However, any
other type declarations or definitions which are not used by multiple
drivers are never put into the header file.

Thanks,
Dimitris

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

* Re: [PATCH] ASoC: Add MAX9850 codec driver
  2011-03-08 10:14     ` Dimitris Papastamos
@ 2011-03-08 11:52       ` Christian Glindkamp
  2011-03-08 12:09         ` Dimitris Papastamos
  0 siblings, 1 reply; 15+ messages in thread
From: Christian Glindkamp @ 2011-03-08 11:52 UTC (permalink / raw)
  To: Dimitris Papastamos; +Cc: alsa-devel, Mark Brown, Liam Girdwood

On 2011-03-08 10:14, Dimitris Papastamos wrote:
> On Mon, Mar 07, 2011 at 06:04:20PM +0100, Christian Glindkamp wrote:
> > > > +static const struct snd_soc_dapm_widget max9850_dapm_widgets[] = {
> > > > +SND_SOC_DAPM_DAC("DAC", "HiFi Playback", MAX9850_ENABLE, 0, 0),
> > > > +SND_SOC_DAPM_SUPPLY("MCLK", MAX9850_ENABLE, 6, 0, NULL, 0),
> > > > +SND_SOC_DAPM_OUTPUT("OUTL"),
> > > > +SND_SOC_DAPM_OUTPUT("OUTR"),
> > > > +SND_SOC_DAPM_OUTPUT("HPL"),
> > > > +SND_SOC_DAPM_OUTPUT("HPR"),
> > > > +SND_SOC_DAPM_INPUT("INL"),
> > > > +SND_SOC_DAPM_INPUT("INR"),
> > > > +SND_SOC_DAPM_PGA("Headphone Output", MAX9850_ENABLE, 3, 0, NULL, 0),
> > > > +SND_SOC_DAPM_MIXER("Line Input", SND_SOC_NOPM, 0, 0, NULL, 0),
> > > > +SND_SOC_DAPM_MIXER_NAMED_CTL("Output Mixer", MAX9850_ENABLE, 2, 0,
> > > > +		&max9850_mixer_controls[0],
> > > > +		ARRAY_SIZE(max9850_mixer_controls)),
> > > > +};
> > > 
> > > Consider grouping the input and output pins logically separately.
> > > 
> > 
> > Do you mean something like that?
> > 
> > SND_SOC_DAPM_SUPPLY("MCLK", MAX9850_ENABLE, 6, 0, NULL, 0),
> > SND_SOC_DAPM_MIXER_NAMED_CTL("Output Mixer", MAX9850_ENABLE, 2, 0,
> > 		&max9850_mixer_controls[0],
> > 		ARRAY_SIZE(max9850_mixer_controls)),
> > SND_SOC_DAPM_PGA("Headphone Output", MAX9850_ENABLE, 3, 0, NULL, 0),
> > SND_SOC_DAPM_DAC("DAC", "HiFi Playback", MAX9850_ENABLE, 0, 0),
> > SND_SOC_DAPM_OUTPUT("OUTL"),
> > SND_SOC_DAPM_OUTPUT("HPL"),
> > SND_SOC_DAPM_OUTPUT("OUTR"),
> > SND_SOC_DAPM_OUTPUT("HPR"),
> > SND_SOC_DAPM_MIXER("Line Input", SND_SOC_NOPM, 0, 0, NULL, 0),
> > SND_SOC_DAPM_INPUT("INL"),
> > SND_SOC_DAPM_INPUT("INR"),
> 
> Yea, that's fine.  Although the standard thing to do is to group all
> the input pins at the beginning and all the output pins at the end.
> However this is a pretty primitive codec so it is not a problem as it
> stands now.
>  
> > Charge pump could also be a supply for the Output Mixer. Have to find
> > out how to toggle two bits at once via dapm (preferably without
> > resorting to callbacks).
> 
> If they are in the same register and same type of widgets then as Mark
> mentioned the core guarantees that it'll be done in one register write.
> 
> I forgot to mention in the previous e-mail, that if you add the cache
> syncing support during resume or whenever we go from BIAS_OFF to
> STANDBY use the standard ASoC function for that, snd_soc_cache_sync().
> Just in case you are looking at older examples.
> 

Do I understand it correctly, that all register writes that I do in the
probe function would be overridden by the syncing so they would have to
be moved to the standby code or put into the register cache array as
default values?

> Thanks,
> Dimitris
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

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

* Re: [PATCH] ASoC: Add MAX9850 codec driver
  2011-03-08 11:52       ` Christian Glindkamp
@ 2011-03-08 12:09         ` Dimitris Papastamos
  0 siblings, 0 replies; 15+ messages in thread
From: Dimitris Papastamos @ 2011-03-08 12:09 UTC (permalink / raw)
  To: Christian Glindkamp; +Cc: alsa-devel, Mark Brown, Liam Girdwood

On Tue, Mar 08, 2011 at 12:52:54PM +0100, Christian Glindkamp wrote:
> Do I understand it correctly, that all register writes that I do in the
> probe function would be overridden by the syncing so they would have to
> be moved to the standby code or put into the register cache array as
> default values?

If you do any registers writes in the probe() function, in effect you
will be changing the hardware state as well as the cache state.  If
after that point you sync the cache to the hardware, the altered cache
will be synced, not the defaults cache.  The core makes a copy of the
defaults cache and uses that for syncing, or if you don't provide a
default's cache it will use a zero-ed cache.

Note that volatile registers don't affect the cache, they bypass it
completely.

Thanks,
Dimitris

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

* [PATCH v2] ASoC: Add MAX9850 codec driver
  2011-03-07 12:45 [PATCH] ASoC: Add MAX9850 codec driver Christian Glindkamp
                   ` (2 preceding siblings ...)
  2011-03-07 16:40 ` Seungwhan Youn
@ 2011-03-09 10:20 ` Christian Glindkamp
  2011-03-09 14:31   ` Dimitris Papastamos
                     ` (2 more replies)
  3 siblings, 3 replies; 15+ messages in thread
From: Christian Glindkamp @ 2011-03-09 10:20 UTC (permalink / raw)
  To: alsa-devel; +Cc: dp, broonie, lrg

This patch adds ASoC support for the MAX9850 codec with headphone
amplifier.

Supported features:
- Playback
- 16, 20 and 24 bit audio
- 8k - 48k sample rates
- DAPM

Signed-off-by: Christian Glindkamp <christian.glindkamp@taskit.de>
---
 sound/soc/codecs/Kconfig   |    4 +
 sound/soc/codecs/Makefile  |    2 +
 sound/soc/codecs/max9850.c |  389 ++++++++++++++++++++++++++++++++++++++++++++
 sound/soc/codecs/max9850.h |   38 +++++
 4 files changed, 433 insertions(+), 0 deletions(-)
 create mode 100644 sound/soc/codecs/max9850.c
 create mode 100644 sound/soc/codecs/max9850.h

diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 10f11dd..d63c175 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -33,6 +33,7 @@ config SND_SOC_ALL_CODECS
 	select SND_SOC_JZ4740_CODEC if SOC_JZ4740
 	select SND_SOC_LM4857 if I2C
 	select SND_SOC_MAX98088 if I2C
+	select SND_SOC_MAX9850 if I2C
 	select SND_SOC_MAX9877 if I2C
 	select SND_SOC_PCM3008
 	select SND_SOC_SGTL5000 if I2C
@@ -186,6 +187,9 @@ config SND_SOC_DMIC
 config SND_SOC_MAX98088
        tristate
 
+config SND_SOC_MAX9850
+	tristate
+
 config SND_SOC_PCM3008
        tristate
 
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index ebb059c..379bc55 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -19,6 +19,7 @@ snd-soc-dfbmcs320-objs := dfbmcs320.o
 snd-soc-dmic-objs := dmic.o
 snd-soc-l3-objs := l3.o
 snd-soc-max98088-objs := max98088.o
+snd-soc-max9850-objs := max9850.o
 snd-soc-pcm3008-objs := pcm3008.o
 snd-soc-sgtl5000-objs := sgtl5000.o
 snd-soc-alc5623-objs := alc5623.o
@@ -107,6 +108,7 @@ obj-$(CONFIG_SND_SOC_DMIC)	+= snd-soc-dmic.o
 obj-$(CONFIG_SND_SOC_L3)	+= snd-soc-l3.o
 obj-$(CONFIG_SND_SOC_JZ4740_CODEC)	+= snd-soc-jz4740-codec.o
 obj-$(CONFIG_SND_SOC_MAX98088)	+= snd-soc-max98088.o
+obj-$(CONFIG_SND_SOC_MAX9850)	+= snd-soc-max9850.o
 obj-$(CONFIG_SND_SOC_PCM3008)	+= snd-soc-pcm3008.o
 obj-$(CONFIG_SND_SOC_SGTL5000)  += snd-soc-sgtl5000.o
 obj-$(CONFIG_SND_SOC_SN95031)	+=snd-soc-sn95031.o
diff --git a/sound/soc/codecs/max9850.c b/sound/soc/codecs/max9850.c
new file mode 100644
index 0000000..6d2c5a3
--- /dev/null
+++ b/sound/soc/codecs/max9850.c
@@ -0,0 +1,389 @@
+/*
+ * max9850.c  --  codec driver for max9850
+ *
+ * Copyright (C) 2011 taskit GmbH
+ *
+ * Author: Christian Glindkamp <christian.glindkamp@taskit.de>
+ *
+ * Initial development of this code was funded by
+ * MICRONIC Computer Systeme GmbH, http://www.mcsberlin.de/
+ *
+ * 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/init.h>
+#include <linux/i2c.h>
+#include <linux/slab.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include <sound/tlv.h>
+
+#include "max9850.h"
+
+struct max9850_priv {
+	unsigned int sysclk;
+};
+
+/* max9850 register cache */
+static const u8 max9850_reg[MAX9850_CACHEREGNUM] = {
+	0x00, 0x00, 0x0c, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+};
+
+/* these registers are not used at the moment but provided for the sake of
+ * completeness */
+static int max9850_volatile_register(struct snd_soc_codec *codec,
+		unsigned int reg)
+{
+	switch (reg) {
+	case MAX9850_STATUSA:
+	case MAX9850_STATUSB:
+		return 1;
+	default:
+		return 0;
+	}
+}
+
+static const unsigned int max9850_tlv[] = {
+	TLV_DB_RANGE_HEAD(4),
+	0x18, 0x1f, TLV_DB_SCALE_ITEM(-7450, 400, 0),
+	0x20, 0x33, TLV_DB_SCALE_ITEM(-4150, 200, 0),
+	0x34, 0x37, TLV_DB_SCALE_ITEM(-150, 100, 0),
+	0x38, 0x3f, TLV_DB_SCALE_ITEM(250, 50, 0),
+};
+
+static const struct snd_kcontrol_new max9850_controls[] = {
+SOC_SINGLE_TLV("Headphone Volume", MAX9850_VOLUME, 0, 0x3f, 1, max9850_tlv),
+SOC_SINGLE("Headphone Switch", MAX9850_VOLUME, 7, 1, 1),
+SOC_SINGLE("Mono Switch", MAX9850_GENERAL_PURPOSE, 2, 1, 0),
+};
+
+static const struct snd_kcontrol_new max9850_mixer_controls[] = {
+	SOC_DAPM_SINGLE("Line In Switch", MAX9850_ENABLE, 1, 1, 0),
+};
+
+static const struct snd_soc_dapm_widget max9850_dapm_widgets[] = {
+SND_SOC_DAPM_SUPPLY("Charge Pump 1", MAX9850_ENABLE, 4, 0, NULL, 0),
+SND_SOC_DAPM_SUPPLY("Charge Pump 2", MAX9850_ENABLE, 5, 0, NULL, 0),
+SND_SOC_DAPM_SUPPLY("MCLK", MAX9850_ENABLE, 6, 0, NULL, 0),
+SND_SOC_DAPM_SUPPLY("SHDN", MAX9850_ENABLE, 7, 0, NULL, 0),
+SND_SOC_DAPM_MIXER_NAMED_CTL("Output Mixer", MAX9850_ENABLE, 2, 0,
+		&max9850_mixer_controls[0],
+		ARRAY_SIZE(max9850_mixer_controls)),
+SND_SOC_DAPM_PGA("Headphone Output", MAX9850_ENABLE, 3, 0, NULL, 0),
+SND_SOC_DAPM_DAC("DAC", "HiFi Playback", MAX9850_ENABLE, 0, 0),
+SND_SOC_DAPM_OUTPUT("OUTL"),
+SND_SOC_DAPM_OUTPUT("HPL"),
+SND_SOC_DAPM_OUTPUT("OUTR"),
+SND_SOC_DAPM_OUTPUT("HPR"),
+SND_SOC_DAPM_MIXER("Line Input", SND_SOC_NOPM, 0, 0, NULL, 0),
+SND_SOC_DAPM_INPUT("INL"),
+SND_SOC_DAPM_INPUT("INR"),
+};
+
+static const struct snd_soc_dapm_route intercon[] = {
+	/* output mixer */
+	{"Output Mixer", NULL, "DAC"},
+	{"Output Mixer", "Line In Switch", "Line Input"},
+
+	/* outputs */
+	{"Headphone Output", NULL, "Output Mixer"},
+	{"HPL", NULL, "Headphone Output"},
+	{"HPR", NULL, "Headphone Output"},
+	{"OUTL", NULL, "Output Mixer"},
+	{"OUTR", NULL, "Output Mixer"},
+
+	/* inputs */
+	{"Line Input", NULL, "INL"},
+	{"Line Input", NULL, "INR"},
+
+	/* supplies */
+	{"Output Mixer", NULL, "Charge Pump 1"},
+	{"Output Mixer", NULL, "Charge Pump 2"},
+	{"Output Mixer", NULL, "SHDN"},
+	{"DAC", NULL, "MCLK"},
+};
+
+static int max9850_hw_params(struct snd_pcm_substream *substream,
+			     struct snd_pcm_hw_params *params,
+			     struct snd_soc_dai *dai)
+{
+	struct snd_soc_codec *codec = dai->codec;
+	struct max9850_priv *max9850 = snd_soc_codec_get_drvdata(codec);
+	u64 lrclk_div;
+	u8 sf, da;
+
+	if(!max9850->sysclk)
+		return -EINVAL;
+
+	/* lrclk_div = 2^22 * rate / iclk with iclk = mclk / sf */
+	sf = (snd_soc_read(codec, MAX9850_CLOCK) >> 2) + 1;
+	lrclk_div = (1 << 22);
+	lrclk_div *= params_rate(params);
+	lrclk_div *= sf;
+	do_div(lrclk_div, max9850->sysclk);
+
+	snd_soc_write(codec, MAX9850_LRCLK_MSB, (lrclk_div >> 8) & 0x7f);
+	snd_soc_write(codec, MAX9850_LRCLK_LSB, lrclk_div & 0xff);
+
+	switch (params_format(params)) {
+	case SNDRV_PCM_FORMAT_S16_LE:
+		da = 0;
+		break;
+	case SNDRV_PCM_FORMAT_S20_3LE:
+		da = 0x2;
+		break;
+	case SNDRV_PCM_FORMAT_S24_LE:
+		da = 0x3;
+		break;
+	default:
+		return -EINVAL;
+	}
+	snd_soc_update_bits(codec, MAX9850_DIGITAL_AUDIO, 0x3, da);
+
+	return 0;
+}
+
+static int max9850_set_dai_sysclk(struct snd_soc_dai *codec_dai,
+		int clk_id, unsigned int freq, int dir)
+{
+	struct snd_soc_codec *codec = codec_dai->codec;
+	struct max9850_priv *max9850 = snd_soc_codec_get_drvdata(codec);
+
+	/* calculate mclk -> iclk divider */
+	if (freq <= 13000000)
+		snd_soc_write(codec, MAX9850_CLOCK, 0x0);
+	else if (freq <= 26000000)
+		snd_soc_write(codec, MAX9850_CLOCK, 0x4);
+	else if (freq <= 40000000)
+		snd_soc_write(codec, MAX9850_CLOCK, 0x8);
+	else
+		return -EINVAL;
+
+	max9850->sysclk = freq;
+	return 0;
+}
+
+static int max9850_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt)
+{
+	struct snd_soc_codec *codec = codec_dai->codec;
+	u8 da = 0;
+
+	/* set master/slave audio interface */
+	switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
+	case SND_SOC_DAIFMT_CBM_CFM:
+		da |= MAX9850_MASTER;
+		break;
+	case SND_SOC_DAIFMT_CBS_CFS:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/* interface format */
+	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+	case SND_SOC_DAIFMT_I2S:
+		da |= MAX9850_DLY;
+		break;
+	case SND_SOC_DAIFMT_RIGHT_J:
+		da |= MAX9850_RTJ;
+		break;
+	case SND_SOC_DAIFMT_LEFT_J:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/* clock inversion */
+	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
+	case SND_SOC_DAIFMT_NB_NF:
+		break;
+	case SND_SOC_DAIFMT_IB_IF:
+		da |= MAX9850_BCINV | MAX9850_INV;
+		break;
+	case SND_SOC_DAIFMT_IB_NF:
+		da |= MAX9850_BCINV;
+		break;
+	case SND_SOC_DAIFMT_NB_IF:
+		da |= MAX9850_INV;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/* set da */
+	snd_soc_write(codec, MAX9850_DIGITAL_AUDIO, da);
+
+	return 0;
+}
+
+static int max9850_set_bias_level(struct snd_soc_codec *codec,
+				  enum snd_soc_bias_level level)
+{
+	int ret;
+
+	switch (level) {
+	case SND_SOC_BIAS_ON:
+		break;
+	case SND_SOC_BIAS_PREPARE:
+		break;
+	case SND_SOC_BIAS_STANDBY:
+		if (codec->dapm.bias_level == SND_SOC_BIAS_OFF) {
+			ret = snd_soc_cache_sync(codec);
+			if (ret) {
+				dev_err(codec->dev,
+					"Failed to sync cache: %d\n", ret);
+				return ret;
+			}
+		}
+		break;
+	case SND_SOC_BIAS_OFF:
+		break;
+	}
+	codec->dapm.bias_level = level;
+	return 0;
+}
+
+#define MAX9850_RATES SNDRV_PCM_RATE_8000_48000
+
+#define MAX9850_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE |\
+	SNDRV_PCM_FMTBIT_S24_LE)
+
+static struct snd_soc_dai_ops max9850_dai_ops = {
+	.hw_params	= max9850_hw_params,
+	.set_sysclk	= max9850_set_dai_sysclk,
+	.set_fmt	= max9850_set_dai_fmt,
+};
+
+static struct snd_soc_dai_driver max9850_dai = {
+	.name = "max9850-hifi",
+	.playback = {
+		.stream_name = "Playback",
+		.channels_min = 1,
+		.channels_max = 2,
+		.rates = MAX9850_RATES,
+		.formats = MAX9850_FORMATS
+	},
+	.ops = &max9850_dai_ops,
+};
+
+#ifdef CONFIG_PM
+static int max9850_suspend(struct snd_soc_codec *codec, pm_message_t state)
+{
+	max9850_set_bias_level(codec, SND_SOC_BIAS_OFF);
+
+	return 0;
+}
+
+static int max9850_resume(struct snd_soc_codec *codec)
+{
+	max9850_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
+
+	return 0;
+}
+#else
+#define max9850_suspend NULL
+#define max9850_resume NULL
+#endif
+
+static int max9850_probe(struct snd_soc_codec *codec)
+{
+	struct snd_soc_dapm_context *dapm = &codec->dapm;
+	int ret;
+
+	ret = snd_soc_codec_set_cache_io(codec, 8, 8, SND_SOC_I2C);
+	if (ret < 0) {
+		dev_err(codec->dev, "Failed to set cache I/O: %d\n", ret);
+		return ret;
+	}
+
+	/* enable zero-detect */
+	snd_soc_update_bits(codec, MAX9850_GENERAL_PURPOSE, 1, 1);
+	/* enable slew-rate control */
+	snd_soc_update_bits(codec, MAX9850_VOLUME, 0x40, 0x40);
+	/* set slew-rate 125ms */
+	snd_soc_update_bits(codec, MAX9850_CHARGE_PUMP, 0xff, 0xc0);
+
+	snd_soc_dapm_new_controls(dapm, max9850_dapm_widgets,
+				  ARRAY_SIZE(max9850_dapm_widgets));
+	snd_soc_dapm_add_routes(dapm, intercon, ARRAY_SIZE(intercon));
+
+	snd_soc_add_controls(codec, max9850_controls,
+			ARRAY_SIZE(max9850_controls));
+
+	return 0;
+}
+
+static struct snd_soc_codec_driver soc_codec_dev_max9850 = {
+	.probe =	max9850_probe,
+	.suspend =	max9850_suspend,
+	.resume =	max9850_resume,
+	.set_bias_level = max9850_set_bias_level,
+	.reg_cache_size = ARRAY_SIZE(max9850_reg),
+	.reg_word_size = sizeof(u8),
+	.reg_cache_default = max9850_reg,
+	.volatile_register = max9850_volatile_register,
+};
+
+static int __devinit max9850_i2c_probe(struct i2c_client *i2c,
+		const struct i2c_device_id *id)
+{
+	struct max9850_priv *max9850;
+	int ret;
+
+	max9850 = kzalloc(sizeof(struct max9850_priv), GFP_KERNEL);
+	if (max9850 == NULL)
+		return -ENOMEM;
+
+	i2c_set_clientdata(i2c, max9850);
+
+	ret = snd_soc_register_codec(&i2c->dev,
+			&soc_codec_dev_max9850, &max9850_dai, 1);
+	if (ret < 0)
+		kfree(max9850);
+	return ret;
+}
+
+static __devexit int max9850_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 max9850_i2c_id[] = {
+	{ "max9850", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, max9850_i2c_id);
+
+static struct i2c_driver max9850_i2c_driver = {
+	.driver = {
+		.name = "max9850",
+		.owner = THIS_MODULE,
+	},
+	.probe = max9850_i2c_probe,
+	.remove = __devexit_p(max9850_i2c_remove),
+	.id_table = max9850_i2c_id,
+};
+
+static int __init max9850_init(void)
+{
+	return i2c_add_driver(&max9850_i2c_driver);
+}
+module_init(max9850_init);
+
+static void __exit max9850_exit(void)
+{
+	i2c_del_driver(&max9850_i2c_driver);
+}
+module_exit(max9850_exit);
+
+MODULE_AUTHOR("Christian Glindkamp <christian.glindkamp@taskit.de>");
+MODULE_DESCRIPTION("ASoC MAX9850 codec driver");
+MODULE_LICENSE("GPL");
diff --git a/sound/soc/codecs/max9850.h b/sound/soc/codecs/max9850.h
new file mode 100644
index 0000000..72b1ddb
--- /dev/null
+++ b/sound/soc/codecs/max9850.h
@@ -0,0 +1,38 @@
+/*
+ * max9850.h  --  codec driver for max9850
+ *
+ * Copyright (C) 2011 taskit GmbH
+ * Author: Christian Glindkamp <christian.glindkamp@taskit.de>
+ *
+ * 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 _MAX9850_H
+#define _MAX9850_H
+
+#define MAX9850_STATUSA			0x00
+#define MAX9850_STATUSB			0x01
+#define MAX9850_VOLUME			0x02
+#define MAX9850_GENERAL_PURPOSE		0x03
+#define MAX9850_INTERRUPT		0x04
+#define MAX9850_ENABLE			0x05
+#define MAX9850_CLOCK			0x06
+#define MAX9850_CHARGE_PUMP		0x07
+#define MAX9850_LRCLK_MSB		0x08
+#define MAX9850_LRCLK_LSB		0x09
+#define MAX9850_DIGITAL_AUDIO		0x0a
+
+#define MAX9850_CACHEREGNUM 11
+
+/* MAX9850_DIGITAL_AUDIO */
+#define MAX9850_MASTER			(1<<7)
+#define MAX9850_INV			(1<<6)
+#define MAX9850_BCINV			(1<<5)
+#define MAX9850_DLY			(1<<3)
+#define MAX9850_RTJ			(1<<2)
+
+#endif
-- 
1.7.2.3

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

* Re: [PATCH v2] ASoC: Add MAX9850 codec driver
  2011-03-09 10:20 ` [PATCH v2] " Christian Glindkamp
@ 2011-03-09 14:31   ` Dimitris Papastamos
  2011-03-11 11:06   ` Liam Girdwood
  2011-03-11 12:09   ` Mark Brown
  2 siblings, 0 replies; 15+ messages in thread
From: Dimitris Papastamos @ 2011-03-09 14:31 UTC (permalink / raw)
  To: Christian Glindkamp; +Cc: alsa-devel, broonie, lrg

On Wed, Mar 09, 2011 at 11:20:04AM +0100, Christian Glindkamp wrote:
> This patch adds ASoC support for the MAX9850 codec with headphone
> amplifier.
> 
> Supported features:
> - Playback
> - 16, 20 and 24 bit audio
> - 8k - 48k sample rates
> - DAPM
> 
> Signed-off-by: Christian Glindkamp <christian.glindkamp@taskit.de>

Looks good.

Thanks,
Dimitris

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

* Re: [PATCH v2] ASoC: Add MAX9850 codec driver
  2011-03-09 10:20 ` [PATCH v2] " Christian Glindkamp
  2011-03-09 14:31   ` Dimitris Papastamos
@ 2011-03-11 11:06   ` Liam Girdwood
  2011-03-11 12:09   ` Mark Brown
  2 siblings, 0 replies; 15+ messages in thread
From: Liam Girdwood @ 2011-03-11 11:06 UTC (permalink / raw)
  To: Christian Glindkamp; +Cc: dp, alsa-devel, broonie

On Wed, 2011-03-09 at 11:20 +0100, Christian Glindkamp wrote:
> This patch adds ASoC support for the MAX9850 codec with headphone
> amplifier.
> 
> Supported features:
> - Playback
> - 16, 20 and 24 bit audio
> - 8k - 48k sample rates
> - DAPM
> 
> Signed-off-by: Christian Glindkamp <christian.glindkamp@taskit.de>

Acked-by: Liam Girdwood <lrg@ti.com>

Two minor comments below, the first can be fixed incrementally. The
second is not urgent but will be useful when this driver is used on
different devices. 

> ---
>  sound/soc/codecs/Kconfig   |    4 +
>  sound/soc/codecs/Makefile  |    2 +
>  sound/soc/codecs/max9850.c |  389 ++++++++++++++++++++++++++++++++++++++++++++
>  sound/soc/codecs/max9850.h |   38 +++++
>  4 files changed, 433 insertions(+), 0 deletions(-)
>  create mode 100644 sound/soc/codecs/max9850.c
>  create mode 100644 sound/soc/codecs/max9850.h
> 
> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> index 10f11dd..d63c175 100644
> --- a/sound/soc/codecs/Kconfig
> +++ b/sound/soc/codecs/Kconfig
> @@ -33,6 +33,7 @@ config SND_SOC_ALL_CODECS
>  	select SND_SOC_JZ4740_CODEC if SOC_JZ4740
>  	select SND_SOC_LM4857 if I2C
>  	select SND_SOC_MAX98088 if I2C
> +	select SND_SOC_MAX9850 if I2C
>  	select SND_SOC_MAX9877 if I2C
>  	select SND_SOC_PCM3008
>  	select SND_SOC_SGTL5000 if I2C
> @@ -186,6 +187,9 @@ config SND_SOC_DMIC
>  config SND_SOC_MAX98088
>         tristate
>  
> +config SND_SOC_MAX9850
> +	tristate
> +
>  config SND_SOC_PCM3008
>         tristate
>  
> diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
> index ebb059c..379bc55 100644
> --- a/sound/soc/codecs/Makefile
> +++ b/sound/soc/codecs/Makefile
> @@ -19,6 +19,7 @@ snd-soc-dfbmcs320-objs := dfbmcs320.o
>  snd-soc-dmic-objs := dmic.o
>  snd-soc-l3-objs := l3.o
>  snd-soc-max98088-objs := max98088.o
> +snd-soc-max9850-objs := max9850.o
>  snd-soc-pcm3008-objs := pcm3008.o
>  snd-soc-sgtl5000-objs := sgtl5000.o
>  snd-soc-alc5623-objs := alc5623.o
> @@ -107,6 +108,7 @@ obj-$(CONFIG_SND_SOC_DMIC)	+= snd-soc-dmic.o
>  obj-$(CONFIG_SND_SOC_L3)	+= snd-soc-l3.o
>  obj-$(CONFIG_SND_SOC_JZ4740_CODEC)	+= snd-soc-jz4740-codec.o
>  obj-$(CONFIG_SND_SOC_MAX98088)	+= snd-soc-max98088.o
> +obj-$(CONFIG_SND_SOC_MAX9850)	+= snd-soc-max9850.o
>  obj-$(CONFIG_SND_SOC_PCM3008)	+= snd-soc-pcm3008.o
>  obj-$(CONFIG_SND_SOC_SGTL5000)  += snd-soc-sgtl5000.o
>  obj-$(CONFIG_SND_SOC_SN95031)	+=snd-soc-sn95031.o
> diff --git a/sound/soc/codecs/max9850.c b/sound/soc/codecs/max9850.c
> new file mode 100644
> index 0000000..6d2c5a3
> --- /dev/null
> +++ b/sound/soc/codecs/max9850.c
> @@ -0,0 +1,389 @@
> +/*
> + * max9850.c  --  codec driver for max9850
> + *
> + * Copyright (C) 2011 taskit GmbH
> + *
> + * Author: Christian Glindkamp <christian.glindkamp@taskit.de>
> + *
> + * Initial development of this code was funded by
> + * MICRONIC Computer Systeme GmbH, http://www.mcsberlin.de/
> + *
> + * 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/init.h>
> +#include <linux/i2c.h>
> +#include <linux/slab.h>
> +#include <sound/pcm.h>
> +#include <sound/pcm_params.h>
> +#include <sound/soc.h>
> +#include <sound/tlv.h>
> +
> +#include "max9850.h"
> +
> +struct max9850_priv {
> +	unsigned int sysclk;
> +};
> +
> +/* max9850 register cache */
> +static const u8 max9850_reg[MAX9850_CACHEREGNUM] = {
> +	0x00, 0x00, 0x0c, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
> +};
> +
> +/* these registers are not used at the moment but provided for the sake of
> + * completeness */
> +static int max9850_volatile_register(struct snd_soc_codec *codec,
> +		unsigned int reg)
> +{
> +	switch (reg) {
> +	case MAX9850_STATUSA:
> +	case MAX9850_STATUSB:
> +		return 1;
> +	default:
> +		return 0;
> +	}
> +}
> +
> +static const unsigned int max9850_tlv[] = {
> +	TLV_DB_RANGE_HEAD(4),
> +	0x18, 0x1f, TLV_DB_SCALE_ITEM(-7450, 400, 0),
> +	0x20, 0x33, TLV_DB_SCALE_ITEM(-4150, 200, 0),
> +	0x34, 0x37, TLV_DB_SCALE_ITEM(-150, 100, 0),
> +	0x38, 0x3f, TLV_DB_SCALE_ITEM(250, 50, 0),
> +};
> +
> +static const struct snd_kcontrol_new max9850_controls[] = {
> +SOC_SINGLE_TLV("Headphone Volume", MAX9850_VOLUME, 0, 0x3f, 1, max9850_tlv),
> +SOC_SINGLE("Headphone Switch", MAX9850_VOLUME, 7, 1, 1),
> +SOC_SINGLE("Mono Switch", MAX9850_GENERAL_PURPOSE, 2, 1, 0),
> +};
> +
> +static const struct snd_kcontrol_new max9850_mixer_controls[] = {
> +	SOC_DAPM_SINGLE("Line In Switch", MAX9850_ENABLE, 1, 1, 0),
> +};
> +
> +static const struct snd_soc_dapm_widget max9850_dapm_widgets[] = {
> +SND_SOC_DAPM_SUPPLY("Charge Pump 1", MAX9850_ENABLE, 4, 0, NULL, 0),
> +SND_SOC_DAPM_SUPPLY("Charge Pump 2", MAX9850_ENABLE, 5, 0, NULL, 0),
> +SND_SOC_DAPM_SUPPLY("MCLK", MAX9850_ENABLE, 6, 0, NULL, 0),
> +SND_SOC_DAPM_SUPPLY("SHDN", MAX9850_ENABLE, 7, 0, NULL, 0),
> +SND_SOC_DAPM_MIXER_NAMED_CTL("Output Mixer", MAX9850_ENABLE, 2, 0,
> +		&max9850_mixer_controls[0],
> +		ARRAY_SIZE(max9850_mixer_controls)),
> +SND_SOC_DAPM_PGA("Headphone Output", MAX9850_ENABLE, 3, 0, NULL, 0),
> +SND_SOC_DAPM_DAC("DAC", "HiFi Playback", MAX9850_ENABLE, 0, 0),
> +SND_SOC_DAPM_OUTPUT("OUTL"),
> +SND_SOC_DAPM_OUTPUT("HPL"),
> +SND_SOC_DAPM_OUTPUT("OUTR"),
> +SND_SOC_DAPM_OUTPUT("HPR"),
> +SND_SOC_DAPM_MIXER("Line Input", SND_SOC_NOPM, 0, 0, NULL, 0),
> +SND_SOC_DAPM_INPUT("INL"),
> +SND_SOC_DAPM_INPUT("INR"),
> +};
> +
> +static const struct snd_soc_dapm_route intercon[] = {
> +	/* output mixer */
> +	{"Output Mixer", NULL, "DAC"},
> +	{"Output Mixer", "Line In Switch", "Line Input"},
> +
> +	/* outputs */
> +	{"Headphone Output", NULL, "Output Mixer"},
> +	{"HPL", NULL, "Headphone Output"},
> +	{"HPR", NULL, "Headphone Output"},
> +	{"OUTL", NULL, "Output Mixer"},
> +	{"OUTR", NULL, "Output Mixer"},
> +
> +	/* inputs */
> +	{"Line Input", NULL, "INL"},
> +	{"Line Input", NULL, "INR"},
> +
> +	/* supplies */
> +	{"Output Mixer", NULL, "Charge Pump 1"},
> +	{"Output Mixer", NULL, "Charge Pump 2"},
> +	{"Output Mixer", NULL, "SHDN"},
> +	{"DAC", NULL, "MCLK"},
> +};
> +
> +static int max9850_hw_params(struct snd_pcm_substream *substream,
> +			     struct snd_pcm_hw_params *params,
> +			     struct snd_soc_dai *dai)
> +{
> +	struct snd_soc_codec *codec = dai->codec;
> +	struct max9850_priv *max9850 = snd_soc_codec_get_drvdata(codec);
> +	u64 lrclk_div;
> +	u8 sf, da;
> +
> +	if(!max9850->sysclk)

space after if

> +		return -EINVAL;
> +
> +	/* lrclk_div = 2^22 * rate / iclk with iclk = mclk / sf */
> +	sf = (snd_soc_read(codec, MAX9850_CLOCK) >> 2) + 1;
> +	lrclk_div = (1 << 22);
> +	lrclk_div *= params_rate(params);
> +	lrclk_div *= sf;
> +	do_div(lrclk_div, max9850->sysclk);
> +
> +	snd_soc_write(codec, MAX9850_LRCLK_MSB, (lrclk_div >> 8) & 0x7f);
> +	snd_soc_write(codec, MAX9850_LRCLK_LSB, lrclk_div & 0xff);
> +
> +	switch (params_format(params)) {
> +	case SNDRV_PCM_FORMAT_S16_LE:
> +		da = 0;
> +		break;
> +	case SNDRV_PCM_FORMAT_S20_3LE:
> +		da = 0x2;
> +		break;
> +	case SNDRV_PCM_FORMAT_S24_LE:
> +		da = 0x3;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +	snd_soc_update_bits(codec, MAX9850_DIGITAL_AUDIO, 0x3, da);
> +
> +	return 0;
> +}
> +
> +static int max9850_set_dai_sysclk(struct snd_soc_dai *codec_dai,
> +		int clk_id, unsigned int freq, int dir)
> +{
> +	struct snd_soc_codec *codec = codec_dai->codec;
> +	struct max9850_priv *max9850 = snd_soc_codec_get_drvdata(codec);
> +
> +	/* calculate mclk -> iclk divider */
> +	if (freq <= 13000000)
> +		snd_soc_write(codec, MAX9850_CLOCK, 0x0);
> +	else if (freq <= 26000000)
> +		snd_soc_write(codec, MAX9850_CLOCK, 0x4);
> +	else if (freq <= 40000000)
> +		snd_soc_write(codec, MAX9850_CLOCK, 0x8);
> +	else
> +		return -EINVAL;
> +
> +	max9850->sysclk = freq;
> +	return 0;
> +}
> +
> +static int max9850_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt)
> +{
> +	struct snd_soc_codec *codec = codec_dai->codec;
> +	u8 da = 0;
> +
> +	/* set master/slave audio interface */
> +	switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
> +	case SND_SOC_DAIFMT_CBM_CFM:
> +		da |= MAX9850_MASTER;
> +		break;
> +	case SND_SOC_DAIFMT_CBS_CFS:
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	/* interface format */
> +	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> +	case SND_SOC_DAIFMT_I2S:
> +		da |= MAX9850_DLY;
> +		break;
> +	case SND_SOC_DAIFMT_RIGHT_J:
> +		da |= MAX9850_RTJ;
> +		break;
> +	case SND_SOC_DAIFMT_LEFT_J:
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	/* clock inversion */
> +	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
> +	case SND_SOC_DAIFMT_NB_NF:
> +		break;
> +	case SND_SOC_DAIFMT_IB_IF:
> +		da |= MAX9850_BCINV | MAX9850_INV;
> +		break;
> +	case SND_SOC_DAIFMT_IB_NF:
> +		da |= MAX9850_BCINV;
> +		break;
> +	case SND_SOC_DAIFMT_NB_IF:
> +		da |= MAX9850_INV;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	/* set da */
> +	snd_soc_write(codec, MAX9850_DIGITAL_AUDIO, da);
> +
> +	return 0;
> +}
> +
> +static int max9850_set_bias_level(struct snd_soc_codec *codec,
> +				  enum snd_soc_bias_level level)
> +{
> +	int ret;
> +
> +	switch (level) {
> +	case SND_SOC_BIAS_ON:
> +		break;
> +	case SND_SOC_BIAS_PREPARE:
> +		break;
> +	case SND_SOC_BIAS_STANDBY:
> +		if (codec->dapm.bias_level == SND_SOC_BIAS_OFF) {
> +			ret = snd_soc_cache_sync(codec);
> +			if (ret) {
> +				dev_err(codec->dev,
> +					"Failed to sync cache: %d\n", ret);
> +				return ret;
> +			}
> +		}
> +		break;
> +	case SND_SOC_BIAS_OFF:
> +		break;
> +	}
> +	codec->dapm.bias_level = level;
> +	return 0;
> +}
> +
> +#define MAX9850_RATES SNDRV_PCM_RATE_8000_48000
> +
> +#define MAX9850_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE |\
> +	SNDRV_PCM_FMTBIT_S24_LE)
> +
> +static struct snd_soc_dai_ops max9850_dai_ops = {
> +	.hw_params	= max9850_hw_params,
> +	.set_sysclk	= max9850_set_dai_sysclk,
> +	.set_fmt	= max9850_set_dai_fmt,
> +};
> +
> +static struct snd_soc_dai_driver max9850_dai = {
> +	.name = "max9850-hifi",
> +	.playback = {
> +		.stream_name = "Playback",
> +		.channels_min = 1,
> +		.channels_max = 2,
> +		.rates = MAX9850_RATES,
> +		.formats = MAX9850_FORMATS
> +	},
> +	.ops = &max9850_dai_ops,
> +};
> +
> +#ifdef CONFIG_PM
> +static int max9850_suspend(struct snd_soc_codec *codec, pm_message_t state)
> +{
> +	max9850_set_bias_level(codec, SND_SOC_BIAS_OFF);
> +
> +	return 0;
> +}
> +
> +static int max9850_resume(struct snd_soc_codec *codec)
> +{
> +	max9850_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
> +
> +	return 0;
> +}
> +#else
> +#define max9850_suspend NULL
> +#define max9850_resume NULL
> +#endif
> +
> +static int max9850_probe(struct snd_soc_codec *codec)
> +{
> +	struct snd_soc_dapm_context *dapm = &codec->dapm;
> +	int ret;
> +
> +	ret = snd_soc_codec_set_cache_io(codec, 8, 8, SND_SOC_I2C);
> +	if (ret < 0) {
> +		dev_err(codec->dev, "Failed to set cache I/O: %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* enable zero-detect */
> +	snd_soc_update_bits(codec, MAX9850_GENERAL_PURPOSE, 1, 1);
> +	/* enable slew-rate control */
> +	snd_soc_update_bits(codec, MAX9850_VOLUME, 0x40, 0x40);
> +	/* set slew-rate 125ms */
> +	snd_soc_update_bits(codec, MAX9850_CHARGE_PUMP, 0xff, 0xc0);
> +

Things like slew rate are often better configured with platform data.

> +	snd_soc_dapm_new_controls(dapm, max9850_dapm_widgets,
> +				  ARRAY_SIZE(max9850_dapm_widgets));
> +	snd_soc_dapm_add_routes(dapm, intercon, ARRAY_SIZE(intercon));
> +
> +	snd_soc_add_controls(codec, max9850_controls,
> +			ARRAY_SIZE(max9850_controls));
> +
> +	return 0;
> +}
> +
> +static struct snd_soc_codec_driver soc_codec_dev_max9850 = {
> +	.probe =	max9850_probe,
> +	.suspend =	max9850_suspend,
> +	.resume =	max9850_resume,
> +	.set_bias_level = max9850_set_bias_level,
> +	.reg_cache_size = ARRAY_SIZE(max9850_reg),
> +	.reg_word_size = sizeof(u8),
> +	.reg_cache_default = max9850_reg,
> +	.volatile_register = max9850_volatile_register,
> +};
> +
> +static int __devinit max9850_i2c_probe(struct i2c_client *i2c,
> +		const struct i2c_device_id *id)
> +{
> +	struct max9850_priv *max9850;
> +	int ret;
> +
> +	max9850 = kzalloc(sizeof(struct max9850_priv), GFP_KERNEL);
> +	if (max9850 == NULL)
> +		return -ENOMEM;
> +
> +	i2c_set_clientdata(i2c, max9850);
> +
> +	ret = snd_soc_register_codec(&i2c->dev,
> +			&soc_codec_dev_max9850, &max9850_dai, 1);
> +	if (ret < 0)
> +		kfree(max9850);
> +	return ret;
> +}
> +
> +static __devexit int max9850_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 max9850_i2c_id[] = {
> +	{ "max9850", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, max9850_i2c_id);
> +
> +static struct i2c_driver max9850_i2c_driver = {
> +	.driver = {
> +		.name = "max9850",
> +		.owner = THIS_MODULE,
> +	},
> +	.probe = max9850_i2c_probe,
> +	.remove = __devexit_p(max9850_i2c_remove),
> +	.id_table = max9850_i2c_id,
> +};
> +
> +static int __init max9850_init(void)
> +{
> +	return i2c_add_driver(&max9850_i2c_driver);
> +}
> +module_init(max9850_init);
> +
> +static void __exit max9850_exit(void)
> +{
> +	i2c_del_driver(&max9850_i2c_driver);
> +}
> +module_exit(max9850_exit);
> +
> +MODULE_AUTHOR("Christian Glindkamp <christian.glindkamp@taskit.de>");
> +MODULE_DESCRIPTION("ASoC MAX9850 codec driver");
> +MODULE_LICENSE("GPL");
> diff --git a/sound/soc/codecs/max9850.h b/sound/soc/codecs/max9850.h
> new file mode 100644
> index 0000000..72b1ddb
> --- /dev/null
> +++ b/sound/soc/codecs/max9850.h
> @@ -0,0 +1,38 @@
> +/*
> + * max9850.h  --  codec driver for max9850
> + *
> + * Copyright (C) 2011 taskit GmbH
> + * Author: Christian Glindkamp <christian.glindkamp@taskit.de>
> + *
> + * 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 _MAX9850_H
> +#define _MAX9850_H
> +
> +#define MAX9850_STATUSA			0x00
> +#define MAX9850_STATUSB			0x01
> +#define MAX9850_VOLUME			0x02
> +#define MAX9850_GENERAL_PURPOSE		0x03
> +#define MAX9850_INTERRUPT		0x04
> +#define MAX9850_ENABLE			0x05
> +#define MAX9850_CLOCK			0x06
> +#define MAX9850_CHARGE_PUMP		0x07
> +#define MAX9850_LRCLK_MSB		0x08
> +#define MAX9850_LRCLK_LSB		0x09
> +#define MAX9850_DIGITAL_AUDIO		0x0a
> +
> +#define MAX9850_CACHEREGNUM 11
> +
> +/* MAX9850_DIGITAL_AUDIO */
> +#define MAX9850_MASTER			(1<<7)
> +#define MAX9850_INV			(1<<6)
> +#define MAX9850_BCINV			(1<<5)
> +#define MAX9850_DLY			(1<<3)
> +#define MAX9850_RTJ			(1<<2)
> +
> +#endif

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

* Re: [PATCH v2] ASoC: Add MAX9850 codec driver
  2011-03-09 10:20 ` [PATCH v2] " Christian Glindkamp
  2011-03-09 14:31   ` Dimitris Papastamos
  2011-03-11 11:06   ` Liam Girdwood
@ 2011-03-11 12:09   ` Mark Brown
  2 siblings, 0 replies; 15+ messages in thread
From: Mark Brown @ 2011-03-11 12:09 UTC (permalink / raw)
  To: Christian Glindkamp; +Cc: dp, alsa-devel, lrg

On Wed, Mar 09, 2011 at 11:20:04AM +0100, Christian Glindkamp wrote:
> This patch adds ASoC support for the MAX9850 codec with headphone
> amplifier.

Applied, thanks.  I also added a commit fixing up the formatting error
Liam mentioned.

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

end of thread, other threads:[~2011-03-11 12:09 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-07 12:45 [PATCH] ASoC: Add MAX9850 codec driver Christian Glindkamp
2011-03-07 13:48 ` Dimitris Papastamos
2011-03-07 17:04   ` Christian Glindkamp
2011-03-07 17:14     ` Mark Brown
2011-03-08 10:14     ` Dimitris Papastamos
2011-03-08 11:52       ` Christian Glindkamp
2011-03-08 12:09         ` Dimitris Papastamos
2011-03-07 15:15 ` Mark Brown
2011-03-08  9:51   ` Christian Glindkamp
2011-03-07 16:40 ` Seungwhan Youn
2011-03-08 10:21   ` Dimitris Papastamos
2011-03-09 10:20 ` [PATCH v2] " Christian Glindkamp
2011-03-09 14:31   ` Dimitris Papastamos
2011-03-11 11:06   ` Liam Girdwood
2011-03-11 12:09   ` Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).