Re: [alsa-devel] [PATCH v2 3/3] ALSA SoC: Add Texas Instruments TLV320AIC26 codec driver
On Mon, Jul 14, 2008 at 12:45:55PM +0100, Mark Brown wrote:
On Sat, Jul 12, 2008 at 02:39:39AM -0600, Grant Likely wrote:
ASoC Codec driver for the TLV320AIC26 device. This driver uses the ASoC
v1 API, so I don't expect it to get merged as-is, but I want to get it
out there for review.
This looks basically good - most of the issues below are nitpicky.
+ /* Configure PLL */
+ pval = 1;
+ jval = (fsref == 44100) ? 7 : 8;
+ dval = (fsref == 44100) ? 5264 : 1920;
+ qval = 0;
+ reg = 0x8000 | qval 11 | pval 8 | jval 2;
+ aic26_reg_write(codec, AIC26_REG_PLL_PROG1, reg);
+ reg = dval 2;
+ aic26_reg_write(codec, AIC26_REG_PLL_PROG2, reg);
Does the PLL configuration not depend on the input clock frequency? You
have a set_sysclk() method to configure the MCLK supplied but the driver
never appears to reference the value anywhere.
Yes, this should be done better. I'm working on making this better, but
I'm hoping it is okay to have that as follow-on patches.
+ /* Power up CODEC */
+ aic26_reg_write(codec, AIC26_REG_POWER_CTRL, 0);
As discussed this should probably just be removed from hw_params().
done
+static int aic26_set_fmt(struct snd_soc_codec_dai *codec_dai, unsigned int
fmt)
+{
+ /* interface format */
+ switch (fmt SND_SOC_DAIFMT_FORMAT_MASK) {
+ case SND_SOC_DAIFMT_I2S: aic26-datfm = AIC26_DATFM_I2S; break;
+ case SND_SOC_DAIFMT_DSP_A: aic26-datfm = AIC26_DATFM_DSP; break;
+ case SND_SOC_DAIFMT_RIGHT_J: aic26-datfm = AIC26_DATFM_RIGHTJ; break;
+ case SND_SOC_DAIFMT_LEFT_J: aic26-datfm = AIC26_DATFM_LEFTJ; break;
+ default: dev_dbg(aic26-spi-dev, bad format\n); return -EINVAL;
+ }
I'm having a hard time liking this indentation style. It's not an
obstacle to merging but it doesn't really help legibility - there's not
enough white space and once you have a non-standard line like the
default: one.
Cleaned up.
+static const char *aic26_capture_src_text[] = {Mic, Aux};
+static const struct soc_enum aic26_capture_src_enum =
+ SOC_ENUM_SINGLE(AIC26_REG_AUDIO_CTRL1, 12,2, aic26_capture_src_text);
checkpatch complains about the 12,2 here and a bunch of other stuff -
ALSA is generally very strict about that.
Originally, that had been to keep the line under 80 chars, but some of
the names have changed since then to make it no longer necessary. I'll
clean up the checkpatch stuff.
+ SOC_SINGLE(Keyclick activate, AIC26_REG_AUDIO_CTRL2, 15, 0x1, 0),
+ SOC_SINGLE(Keyclick amplitude, AIC26_REG_AUDIO_CTRL2, 12, 0x7, 0),
+ SOC_SINGLE(Keyclick frequency, AIC26_REG_AUDIO_CTRL2, 8, 0x7, 0),
+ SOC_SINGLE(Keyclick period, AIC26_REG_AUDIO_CTRL2, 4, 0xf, 0),
This configuration is also exposed via a sysfs file, including some of
the configurability. Exposing the information via sysfs isn't a
particular problem but allowing it to be written to without going
through ALSA seems wrong.
All the written bit does is trigger the keyclick event. It doesn't
adjust the parameters in any way.
+static ssize_t aic26_regs_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
As discussed this is replicating the existing codec register display
sysfs file.
removed
+#if defined(CONFIG_SND_SOC_OF)
+ /* Tell the of_soc helper about this codec */
+ of_snd_soc_register_codec(aic26_soc_codec_dev, aic26, aic26_dai,
+ spi-dev.archdata.of_node);
+#endif
CONFIG_SND_SOC_OF could be a module - this needs to also check for it
being a module.
Doesn't #if defined() match on both static and module selection?
+#define AIC26_RATES(SNDRV_PCM_RATE_8000 | SNDRV_PCM_RATE_11025 |\
+SNDRV_PCM_RATE_16000 | SNDRV_PCM_RATE_22050 |\
+SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_44100 |\
+SNDRV_PCM_RATE_48000)
+#define AIC26_FORMATS (SNDRV_PCM_FMTBIT_S8 |
SNDRV_PCM_FMTBIT_S16_BE |\
+SNDRV_PCM_FMTBIT_S24_BE | SNDRV_PCM_FMTBIT_S32_BE)
These are usually kept in the driver code.
fixed.
Thanks,
g.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [alsa-devel] [PATCH v2 3/3] ALSA SoC: Add Texas Instruments TLV320AIC26 codec driver
On Fri, Jul 18, 2008 at 12:29:37AM -0600, Grant Likely wrote: On Mon, Jul 14, 2008 at 12:45:55PM +0100, Mark Brown wrote: On Sat, Jul 12, 2008 at 02:39:39AM -0600, Grant Likely wrote: This configuration is also exposed via a sysfs file, including some of the configurability. Exposing the information via sysfs isn't a particular problem but allowing it to be written to without going through ALSA seems wrong. All the written bit does is trigger the keyclick event. It doesn't adjust the parameters in any way. That should be OK for now but we ought to work out a way of doing this via an ALSA control so it can be standardised over the devices that support it. I can't see anything suitable currently, though. +#if defined(CONFIG_SND_SOC_OF) + /* Tell the of_soc helper about this codec */ + of_snd_soc_register_codec(aic26_soc_codec_dev, aic26, aic26_dai, + spi-dev.archdata.of_node); +#endif CONFIG_SND_SOC_OF could be a module - this needs to also check for it being a module. Doesn't #if defined() match on both static and module selection? Sadly not. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [alsa-devel] [PATCH v2 3/3] ALSA SoC: Add Texas Instruments TLV320AIC26 codec driver
Hi
I have one query about this soc driver.
I want to know when u will merge it with kernel then
where and by which name this device will be available
in /dev directory of your file system.
As i am following the same structure for my driver so please help me. I
am ot able to recognise the device in the file system.
-Original Message-
From: Grant Likely [EMAIL PROTECTED]
To: linuxppc-dev@ozlabs.org, [EMAIL PROTECTED],
[EMAIL PROTECTED], [EMAIL PROTECTED]
Subject: [alsa-devel] [PATCH v2 3/3] ALSA SoC: Add Texas Instruments
TLV320AIC26 codec driver
Date: Sat, 12 Jul 2008 02:39:39 -0600
From: Grant Likely [EMAIL PROTECTED]
ASoC Codec driver for the TLV320AIC26 device. This driver uses the ASoC
v1 API, so I don't expect it to get merged as-is, but I want to get it
out there for review.
---
sound/soc/codecs/Kconfig |4
sound/soc/codecs/Makefile |2
sound/soc/codecs/tlv320aic26.c | 546
sound/soc/codecs/tlv320aic26.h | 103
4 files changed, 655 insertions(+), 0 deletions(-)
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 3903ab7..96c7bfe 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -41,6 +41,10 @@ config SND_SOC_CS4270_VD33_ERRATA
bool
depends on SND_SOC_CS4270
+config SND_SOC_TLV320AIC26
+ tristate TI TLB320AIC26 Codec support
+ depends on SND_SOC SPI
+
config SND_SOC_TLV320AIC3X
tristate
depends on SND_SOC I2C
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index 4e1314c..ec0cd93 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -5,6 +5,7 @@ snd-soc-wm8753-objs := wm8753.o
snd-soc-wm9712-objs := wm9712.o
snd-soc-wm9713-objs := wm9713.o
snd-soc-cs4270-objs := cs4270.o
+snd-soc-tlv320aic26-objs := tlv320aic26.o
snd-soc-tlv320aic3x-objs := tlv320aic3x.o
obj-$(CONFIG_SND_SOC_AC97_CODEC) += snd-soc-ac97.o
@@ -14,4 +15,5 @@ obj-$(CONFIG_SND_SOC_WM8753) += snd-soc-wm8753.o
obj-$(CONFIG_SND_SOC_WM9712) += snd-soc-wm9712.o
obj-$(CONFIG_SND_SOC_WM9713) += snd-soc-wm9713.o
obj-$(CONFIG_SND_SOC_CS4270) += snd-soc-cs4270.o
+obj-$(CONFIG_SND_SOC_TLV320AIC26) += snd-soc-tlv320aic26.o
obj-$(CONFIG_SND_SOC_TLV320AIC3X) += snd-soc-tlv320aic3x.o
diff --git a/sound/soc/codecs/tlv320aic26.c b/sound/soc/codecs/tlv320aic26.c
new file mode 100644
index 000..3ebfa23
--- /dev/null
+++ b/sound/soc/codecs/tlv320aic26.c
@@ -0,0 +1,546 @@
+/*
+ * Texas Instruments TLV320AIC26 low power audio CODEC
+ * ALSA SoC CODEC driver
+ *
+ * Copyright (C) 2008 Secret Lab Technologies Ltd.
+ */
+
+#include linux/module.h
+#include linux/moduleparam.h
+#include linux/init.h
+#include linux/delay.h
+#include linux/pm.h
+#include linux/device.h
+#include linux/sysfs.h
+#include linux/spi/spi.h
+#include sound/core.h
+#include sound/pcm.h
+#include sound/pcm_params.h
+#include sound/soc.h
+#include sound/soc-dapm.h
+#include sound/soc-of.h
+#include sound/initval.h
+
+#include tlv320aic26.h
+
+MODULE_DESCRIPTION(ASoC TLV320AIC26 codec driver);
+MODULE_AUTHOR(Grant Likely [EMAIL PROTECTED]);
+MODULE_LICENSE(GPL);
+
+/* AIC26 driver private data */
+struct aic26 {
+ struct spi_device *spi;
+ struct snd_soc_codec codec;
+ u16 reg_cache[AIC26_REG_CACHE_SIZE];/* shadow registers */
+ int master;
+ int datfm;
+ int mclk;
+
+ /* Keyclick parameters */
+ int keyclick_amplitude;
+ int keyclick_freq;
+ int keyclick_len;
+};
+
+/* -
+ * Register access routines
+ */
+static unsigned int aic26_reg_read(struct snd_soc_codec *codec,
+ unsigned int reg)
+{
+ struct aic26 *aic26 = codec-private_data;
+ u16 *cache = codec-reg_cache;
+ u16 cmd, value;
+ u8 buffer[2];
+ int rc;
+
+ if (reg = AIC26_NUM_REGS) {
+ WARN_ON_ONCE(1);
+ return 0;
+ }
+
+ /* Do SPI transfer; first 16bits are command; remaining is
+* register contents */
+ cmd = AIC26_READ_COMMAND_WORD(reg);
+ buffer[0] = (cmd 8) 0xff;
+ buffer[1] = cmd 0xff;
+ rc = spi_write_then_read(aic26-spi, buffer, 2, buffer, 2);
+ if (rc) {
+ dev_err(aic26-spi-dev, AIC26 reg read error\n);
+ return -EIO;
+ }
+ value = (buffer[0] 8) | buffer[1];
+
+ /* Update the cache before returning with the value */
+ if (AIC26_REG_IS_CACHED(reg))
+ cache[AIC26_REG_CACHE_ADDR(reg)] = value;
+ return value;
+}
+
+static unsigned int aic26_reg_read_cache(struct snd_soc_codec *codec,
+unsigned int reg)
+{
+ u16 *cache = codec-reg_cache;
+
+ if ((reg 0) || (reg = AIC26_NUM_REGS)) {
+ WARN_ON_ONCE(1);
+ return 0
Re: [alsa-devel] [PATCH v2 3/3] ALSA SoC: Add Texas Instruments TLV320AIC26 codec driver
thanks for your response but there is no /dev/snd directory in my file structure is there any special method to create it please tell in detail. -Original Message- From: Mark Brown [EMAIL PROTECTED] To: dinesh [EMAIL PROTECTED] Cc: Grant Likely [EMAIL PROTECTED], linuxppc-dev@ozlabs.org, [EMAIL PROTECTED], [EMAIL PROTECTED] Subject: Re: [alsa-devel] [PATCH v2 3/3] ALSA SoC: Add Texas Instruments TLV320AIC26 codec driver Date: Tue, 15 Jul 2008 11:33:34 +0100 On Tue, Jul 15, 2008 at 01:27:52PM +0530, dinesh wrote: I have one query about this soc driver. I want to know when u will merge it with kernel then where and by which name this device will be available in /dev directory of your file system. As i am following the same structure for my driver so please help me. I am ot able to recognise the device in the file system. It will appear via the standard ALSA interfaces (and OSS interfaces if you enable OSS compatibility). The standard location for ALSA device files is under /dev/snd where you'll see several files per device. ___ Alsa-devel mailing list [EMAIL PROTECTED] http://mailman.alsa-project.org/mailman/listinfo/alsa-devel ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [alsa-devel] [PATCH v2 3/3] ALSA SoC: Add Texas Instruments TLV320AIC26 codec driver
On Tue, Jul 15, 2008 at 06:08:19PM +0530, dinesh wrote: thanks for your response but there is no /dev/snd directory in my file structure is there any special method to create it please tell in detail. The devices appear via the standard kernel interfaces so if you are using udev or mdev they should be created automatically. Otherwise you'll need to create them statically using whatever method you usually use (eg, a MAKEDEV script supplied by your distribution or manually by reference to the device numbers exposed in /sys/class/sound/*/dev). The ALSA devices use the standard Linux mechanisms to create their devices so whatever you normally use to create devices should work for ALSA too. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [alsa-devel] [PATCH v2 3/3] ALSA SoC: Add Texas Instruments TLV320AIC26 codec driver
On Sat, Jul 12, 2008 at 02:39:39AM -0600, Grant Likely wrote:
ASoC Codec driver for the TLV320AIC26 device. This driver uses the ASoC
v1 API, so I don't expect it to get merged as-is, but I want to get it
out there for review.
This looks basically good - most of the issues below are nitpicky.
+ /* Configure PLL */
+ pval = 1;
+ jval = (fsref == 44100) ? 7 : 8;
+ dval = (fsref == 44100) ? 5264 : 1920;
+ qval = 0;
+ reg = 0x8000 | qval 11 | pval 8 | jval 2;
+ aic26_reg_write(codec, AIC26_REG_PLL_PROG1, reg);
+ reg = dval 2;
+ aic26_reg_write(codec, AIC26_REG_PLL_PROG2, reg);
Does the PLL configuration not depend on the input clock frequency? You
have a set_sysclk() method to configure the MCLK supplied but the driver
never appears to reference the value anywhere.
+ /* Power up CODEC */
+ aic26_reg_write(codec, AIC26_REG_POWER_CTRL, 0);
As discussed this should probably just be removed from hw_params().
+static int aic26_set_fmt(struct snd_soc_codec_dai *codec_dai, unsigned int
fmt)
+{
+ /* interface format */
+ switch (fmt SND_SOC_DAIFMT_FORMAT_MASK) {
+ case SND_SOC_DAIFMT_I2S: aic26-datfm = AIC26_DATFM_I2S; break;
+ case SND_SOC_DAIFMT_DSP_A: aic26-datfm = AIC26_DATFM_DSP; break;
+ case SND_SOC_DAIFMT_RIGHT_J: aic26-datfm = AIC26_DATFM_RIGHTJ; break;
+ case SND_SOC_DAIFMT_LEFT_J: aic26-datfm = AIC26_DATFM_LEFTJ; break;
+ default: dev_dbg(aic26-spi-dev, bad format\n); return -EINVAL;
+ }
I'm having a hard time liking this indentation style. It's not an
obstacle to merging but it doesn't really help legibility - there's not
enough white space and once you have a non-standard line like the
default: one.
+static const char *aic26_capture_src_text[] = {Mic, Aux};
+static const struct soc_enum aic26_capture_src_enum =
+ SOC_ENUM_SINGLE(AIC26_REG_AUDIO_CTRL1, 12,2, aic26_capture_src_text);
checkpatch complains about the 12,2 here and a bunch of other stuff -
ALSA is generally very strict about that.
+ SOC_SINGLE(Keyclick activate, AIC26_REG_AUDIO_CTRL2, 15, 0x1, 0),
+ SOC_SINGLE(Keyclick amplitude, AIC26_REG_AUDIO_CTRL2, 12, 0x7, 0),
+ SOC_SINGLE(Keyclick frequency, AIC26_REG_AUDIO_CTRL2, 8, 0x7, 0),
+ SOC_SINGLE(Keyclick period, AIC26_REG_AUDIO_CTRL2, 4, 0xf, 0),
This configuration is also exposed via a sysfs file, including some of
the configurability. Exposing the information via sysfs isn't a
particular problem but allowing it to be written to without going
through ALSA seems wrong.
+static ssize_t aic26_regs_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
As discussed this is replicating the existing codec register display
sysfs file.
+#if defined(CONFIG_SND_SOC_OF)
+ /* Tell the of_soc helper about this codec */
+ of_snd_soc_register_codec(aic26_soc_codec_dev, aic26, aic26_dai,
+ spi-dev.archdata.of_node);
+#endif
CONFIG_SND_SOC_OF could be a module - this needs to also check for it
being a module.
+#define AIC26_RATES (SNDRV_PCM_RATE_8000 | SNDRV_PCM_RATE_11025 |\
+ SNDRV_PCM_RATE_16000 | SNDRV_PCM_RATE_22050 |\
+ SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_44100 |\
+ SNDRV_PCM_RATE_48000)
+#define AIC26_FORMATS(SNDRV_PCM_FMTBIT_S8 |
SNDRV_PCM_FMTBIT_S16_BE |\
+ SNDRV_PCM_FMTBIT_S24_BE | SNDRV_PCM_FMTBIT_S32_BE)
These are usually kept in the driver code.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [alsa-devel] [PATCH v2 3/3] ALSA SoC: Add Texas Instruments TLV320AIC26 codec driver
On Sat, Jul 12, 2008 at 12:10 PM, Mark Brown [EMAIL PROTECTED] wrote: On Sat, Jul 12, 2008 at 02:39:39AM -0600, Grant Likely wrote: ASoC Codec driver for the TLV320AIC26 device. This driver uses the ASoC v1 API, so I don't expect it to get merged as-is, but I want to get it out there for review. I've not reviewed this revision of these drivers yet but I just wanted to point out that there's absoluely no problem with merging v1 drivers - so long as a driver uses the existing merged APIs there's no issue from that point of view. Oops, I forgot to update my commit messages. I'll probably repost v3 of the series this evening and I'll fix this. Cheers, g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
