All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3 v5] sound/soc/codecs: add LAPIS Semiconductor ML26124
@ 2011-12-20  2:45 Tomoya MORINAGA
  2011-12-20  2:45 ` [PATCH 2/3 v4] sound/soc/lapis: add machine driver for ML7213 Carrier Board Tomoya MORINAGA
                   ` (3 more replies)
  0 siblings, 4 replies; 29+ messages in thread
From: Tomoya MORINAGA @ 2011-12-20  2:45 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
	Lars-Peter Clausen, Dimitris Papastamos, Mike Frysinger,
	Daniel Mack, alsa-devel, linux-kernel
  Cc: qi.wang, yong.y.wang, joel.clark, kok.howg.ewe, Tomoya MORINAGA

ML26124-01HB/ML26124-02GD is 16bit monaural audio CODEC which has high
resistance to voltage noise. On chip regulator realizes power supply rejection
ratio be over 90dB so more than 50dB is improved than ever. ML26124-01HB/
ML26124-02GD can deliver stable audio performance without being affected by noise
from the power supply circuit and peripheral components. The chip also includes
a composite video signal output, which can be applied to various portable device
 requirements. The ML26124 is realized these functions into very small package
the size is only 2.56mm x 2.46mm therefore can be construct high quality sound
system easily.
ML26124-01HB is 25pin WCSP package; ML26124-02GD is 32pin WQFN package.

Signed-off-by: Tomoya MORINAGA <tomoya.rohm@gmail.com>
---
V5: Add clock setting processing
---
 sound/soc/codecs/Kconfig   |    4 +
 sound/soc/codecs/Makefile  |    1 +
 sound/soc/codecs/ml26124.c |  594 ++++++++++++++++++++++++++++++++++++++++++++
 sound/soc/codecs/ml26124.h |  129 ++++++++++
 4 files changed, 728 insertions(+), 0 deletions(-)
 create mode 100644 sound/soc/codecs/ml26124.c
 create mode 100644 sound/soc/codecs/ml26124.h

diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 7c205e7..2b4435f 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -41,6 +41,7 @@ config SND_SOC_ALL_CODECS
 	select SND_SOC_MAX98095 if I2C
 	select SND_SOC_MAX9850 if I2C
 	select SND_SOC_MAX9877 if I2C
+	select SND_SOC_ML26124 if I2C
 	select SND_SOC_PCM3008
 	select SND_SOC_RT5631 if I2C
 	select SND_SOC_SGTL5000 if I2C
@@ -224,6 +225,9 @@ config SND_SOC_MAX98095
 config SND_SOC_MAX9850
 	tristate
 
+config SND_SOC_ML26124
+	tristate
+
 config SND_SOC_PCM3008
        tristate
 
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index 9aa6e66..22adcbd 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -28,6 +28,7 @@ snd-soc-lm4857-objs := lm4857.o
 snd-soc-max98088-objs := max98088.o
 snd-soc-max98095-objs := max98095.o
 snd-soc-max9850-objs := max9850.o
+snd-soc-ml26124-objs := ml26124.o
 snd-soc-pcm3008-objs := pcm3008.o
 snd-soc-rt5631-objs := rt5631.o
 snd-soc-sgtl5000-objs := sgtl5000.o
diff --git a/sound/soc/codecs/ml26124.c b/sound/soc/codecs/ml26124.c
new file mode 100644
index 0000000..fe2dc26
--- /dev/null
+++ b/sound/soc/codecs/ml26124.c
@@ -0,0 +1,594 @@
+/*
+ * Copyright (C) 2011 LAPIS Semiconductor Co., Ltd.
+ *
+ * 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; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307, USA.
+ */
+
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/init.h>
+#include <linux/delay.h>
+#include <linux/pm.h>
+#include <linux/i2c.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <sound/core.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include <sound/tlv.h>
+#include "ml26124.h"
+
+struct ml26124_priv {
+	u32 mclk;
+	u32 channels;
+};
+
+/* ML26124 configuration */
+static const DECLARE_TLV_DB_SCALE(digital_tlv, -7150, 50, 0);
+static const DECLARE_TLV_DB_SCALE(alclvl, -2250, 150, 0);
+static const DECLARE_TLV_DB_SCALE(mingain, -1200, 600, 0);
+static const DECLARE_TLV_DB_SCALE(maxgain, -675, 600, 0);
+static const DECLARE_TLV_DB_SCALE(boost_vol, -1200, 75, 0);
+static const DECLARE_TLV_DB_SCALE(ngth, -7650, 150, 0);
+
+static const char * const ml26124_companding[] = {"16bit PCM", "u-law",
+						  "A-law"};
+static const struct soc_enum ml26124_adc_companding_enum
+	= SOC_ENUM_SINGLE(ML26124_SAI_TRANS_CTL, 6, 3, ml26124_companding);
+static const struct soc_enum ml26124_dac_companding_enum
+	= SOC_ENUM_SINGLE(ML26124_SAI_RCV_CTL, 6, 3, ml26124_companding);
+
+static const struct snd_kcontrol_new ml26124_snd_controls[] = {
+	SOC_SINGLE_TLV("Capture Digital Volume", ML26124_RECORD_DIG_VOL, 0,
+			0xff, 1, digital_tlv),
+	SOC_SINGLE_TLV("Playback Digital Volume", ML26124_PLBAK_DIG_VOL, 0,
+			0xff, 1, digital_tlv),
+	SOC_SINGLE_TLV("Digital Boost Volume", ML26124_DIGI_BOOST_VOL, 0,
+			0x3f, 0, boost_vol),
+	SOC_SINGLE_TLV("EQ Band0 Volume", ML26124_EQ_GAIN_BRAND0, 0,
+			0xff, 1, digital_tlv),
+	SOC_SINGLE_TLV("EQ Band1 Volume", ML26124_EQ_GAIN_BRAND1, 0,
+			0xff, 1, digital_tlv),
+	SOC_SINGLE_TLV("EQ Band2 Volume", ML26124_EQ_GAIN_BRAND2, 0,
+			0xff, 1, digital_tlv),
+	SOC_SINGLE_TLV("EQ Band3 Volume", ML26124_EQ_GAIN_BRAND3, 0,
+			0xff, 1, digital_tlv),
+	SOC_SINGLE_TLV("EQ Band4 Volume", ML26124_EQ_GAIN_BRAND4, 0,
+			0xff, 1, digital_tlv),
+	SOC_SINGLE_TLV("ALC Target Level", ML26124_ALC_TARGET_LEV, 0,
+			0xf, 1, alclvl),
+	SOC_SINGLE_TLV("ALC Min Input Volume", ML26124_ALC_MAXMIN_GAIN, 0,
+			7, 0, mingain),
+	SOC_SINGLE_TLV("ALC Max Input Volume", ML26124_ALC_MAXMIN_GAIN, 4,
+			7, 1, maxgain),
+	SOC_SINGLE_TLV("Playback Limitter Min Input Volume",
+			ML26124_PL_MAXMIN_GAIN, 0, 7, 0, mingain),
+	SOC_SINGLE_TLV("Playback Limitter Max Input Volume",
+			ML26124_PL_MAXMIN_GAIN, 4, 7, 1, maxgain),
+	SOC_SINGLE_TLV("Playback Boost Volume", ML26124_PLYBAK_BOST_VOL, 0,
+			0x3f, 0, boost_vol),
+	SOC_SINGLE("DC High Pass Filter Switch", ML26124_FILTER_EN, 0, 1, 0),
+	SOC_SINGLE("Noise High Pass Filter Switch", ML26124_FILTER_EN, 1, 1, 0),
+	SOC_SINGLE("Zero Cross Comparator Switch", ML26124_PW_ZCCMP_PW_MNG, 1,
+		    1, 0),
+	SOC_SINGLE("EQ Band0 Switch", ML26124_FILTER_EN, 2, 1, 0),
+	SOC_SINGLE("EQ Band1 Switch", ML26124_FILTER_EN, 3, 1, 0),
+	SOC_SINGLE("EQ Band2 Switch", ML26124_FILTER_EN, 4, 1, 0),
+	SOC_SINGLE("EQ Band3 Switch", ML26124_FILTER_EN, 5, 1, 0),
+	SOC_SINGLE("EQ Band4 Switch", ML26124_FILTER_EN, 6, 1, 0),
+	SOC_SINGLE("Play Limitter", ML26124_DVOL_CTL, 0, 1, 0),
+	SOC_SINGLE("Capture Limitter", ML26124_DVOL_CTL, 1, 1, 0),
+	SOC_SINGLE("Digital Volume Fade Switch", ML26124_DVOL_CTL, 3, 1, 0),
+	SOC_SINGLE("Digital Switch", ML26124_DVOL_CTL, 4, 1, 0),
+	SOC_ENUM("DAC Companding", ml26124_dac_companding_enum),
+	SOC_ENUM("ADC Companding", ml26124_adc_companding_enum),
+};
+
+static const struct snd_kcontrol_new ml26124_output_mixer_controls[] = {
+	SOC_DAPM_SINGLE("DAC Switch", ML26124_SPK_AMP_OUT, 1, 1, 0),
+	SOC_DAPM_SINGLE("Line in Switch", ML26124_SPK_AMP_OUT, 3, 1, 0),
+	SOC_DAPM_SINGLE("PGA Switch", ML26124_SPK_AMP_OUT, 5, 1, 0),
+};
+
+/* Input mux */
+static const char * const ml26124_input_select[] = {"Analog MIC SingleEnded in",
+						"Analog MIC Differential in",
+						"Digital MIC in"};
+
+static const struct soc_enum ml26124_insel_enum =
+	SOC_ENUM_SINGLE(ML26124_MIC_IF_CTL, 0, 2, ml26124_input_select);
+
+static const struct snd_kcontrol_new ml26124_input_mux_controls =
+	SOC_DAPM_ENUM("Input Select", ml26124_insel_enum);
+
+static const struct snd_kcontrol_new ml26124_line_control =
+	SOC_DAPM_SINGLE("Switch", ML26124_PW_LOUT_PW_MNG, 1, 1, 0);
+
+static const struct snd_soc_dapm_widget ml26124_dapm_widgets[] = {
+	SND_SOC_DAPM_SUPPLY("MCLK", ML26124_CLK_EN, 0, 0, NULL, 0),
+	SND_SOC_DAPM_SUPPLY("PLL", ML26124_CLK_EN, 1, 0, NULL, 0),
+	SND_SOC_DAPM_MICBIAS("MICBIAS", ML26124_PW_REF_PW_MNG, 0, 0),
+	SND_SOC_DAPM_MIXER("Output Mixer", ML26124_PW_SPAMP_PW_MNG, 0, 0,
+			   &ml26124_output_mixer_controls[0],
+			   ARRAY_SIZE(ml26124_output_mixer_controls)),
+	SND_SOC_DAPM_DAC("DAC", "Playback", ML26124_PW_DAC_PW_MNG, 1, 0),
+	SND_SOC_DAPM_ADC("ADC", "Capture", ML26124_PW_IN_PW_MNG, 1, 0),
+	SND_SOC_DAPM_PGA("PGA", ML26124_PW_IN_PW_MNG, 3, 0, NULL, 0),
+	SND_SOC_DAPM_MUX("Input Mux", SND_SOC_NOPM, 0, 0,
+			  &ml26124_input_mux_controls),
+	SND_SOC_DAPM_SWITCH("Line Out Enable", SND_SOC_NOPM, 0, 0,
+			     &ml26124_line_control),
+	SND_SOC_DAPM_INPUT("MDIN"),
+	SND_SOC_DAPM_INPUT("MIN"),
+	SND_SOC_DAPM_INPUT("LIN"),
+	SND_SOC_DAPM_OUTPUT("SPOUT"),
+	SND_SOC_DAPM_OUTPUT("LOUT"),
+};
+
+static const struct snd_soc_dapm_route ml26124_intercon[] = {
+	/* Supply */
+	{"DAC", NULL, "MCLK"},
+	{"ADC", NULL, "MCLK"},
+	{"DAC", NULL, "PLL"},
+	{"ADC", NULL, "PLL"},
+
+	/* output mixer */
+	{"Output Mixer", "PGA Switch", "PGA"},
+	{"Output Mixer", "DAC Switch", "DAC"},
+	{"Output Mixer", "Line in Switch", "LIN"},
+
+	/* outputs */
+	{"LOUT", NULL, "Output Mixer"},
+	{"SPOUT", NULL, "Output Mixer"},
+
+	/* input */
+	{"Input Mux", "Analog MIC SingleEnded in", "MIN"},
+	{"Input Mux", "Analog MIC Differential in", "MIN"},
+	{"ADC", NULL, "Input Mux"},
+};
+
+struct clk_coeff {
+	u32 mclk;
+	u32 rate;
+	u8 pllnl;
+	u8 pllnh;
+	u8 pllml;
+	u8 pllmh;
+	u8 plldiv;
+};
+
+/* PLLOutputFreq(Hz) = InputMclkFreq(Hz) * PLLM / (PLLN * PLLDIV) */
+static const struct clk_coeff coeff_div[] = {
+	{12288800, 16000, 0xc, 0x0, 0x20, 0x0, 0x4},
+	{12288800, 32000, 0xc, 0x0, 0x20, 0x0, 0x4},
+	{12288800, 48000, 0xc, 0x0, 0x30, 0x0, 0x4},
+};
+
+/* Get sampling rate value of sampling rate setting register (0x0) */
+static inline int get_srate(int rate)
+{
+	int srate;
+
+	switch (rate) {
+	case 16000:
+		srate = 3;
+		break;
+	case 32000:
+		srate = 6;
+		break;
+	case 48000:
+		srate = 8;
+		break;
+	default:
+		return -EINVAL;
+	}
+	return srate;
+}
+
+static inline int get_coeff(int mclk, int rate)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(coeff_div); i++) {
+		if (coeff_div[i].rate == rate && coeff_div[i].mclk == mclk)
+			return i;
+	}
+	return -EINVAL;
+}
+
+static int ml26124_hw_params(struct snd_pcm_substream *substream,
+			    struct snd_pcm_hw_params *hw_params,
+			    struct snd_soc_dai *dai)
+{
+	struct snd_soc_codec *codec = dai->codec;
+	struct ml26124_priv *priv = snd_soc_codec_get_drvdata(codec);
+	int i = get_coeff(priv->mclk, params_rate(hw_params));
+
+	priv->channels = params_channels(hw_params);
+
+	switch (params_rate(hw_params)) {
+	case 16000:
+		snd_soc_update_bits(codec, ML26124_SMPLING_RATE, 0xf,
+				    get_srate(coeff_div[i].rate));
+		snd_soc_update_bits(codec, ML26124_PLLNL, 0xff,
+				    coeff_div[i].pllnl);
+		snd_soc_update_bits(codec, ML26124_PLLNH, 0x1,
+				    coeff_div[i].pllnh);
+		snd_soc_update_bits(codec, ML26124_PLLML, 0xff,
+				    coeff_div[i].pllml);
+		snd_soc_update_bits(codec, ML26124_PLLMH, 0x3f,
+				    coeff_div[i].pllmh);
+		snd_soc_update_bits(codec, ML26124_PLLDIV, 0x1f,
+				    coeff_div[i].plldiv);
+		break;
+	case 32000:
+		snd_soc_update_bits(codec, ML26124_SMPLING_RATE, 0xf,
+				    get_srate(coeff_div[i].rate));
+		snd_soc_update_bits(codec, ML26124_PLLNL, 0xff,
+				    coeff_div[i].pllnl);
+		snd_soc_update_bits(codec, ML26124_PLLNH, 0x1,
+				    coeff_div[i].pllnh);
+		snd_soc_update_bits(codec, ML26124_PLLML, 0xff,
+				    coeff_div[i].pllml);
+		snd_soc_update_bits(codec, ML26124_PLLMH, 0x3f,
+				    coeff_div[i].pllmh);
+		snd_soc_update_bits(codec, ML26124_PLLDIV, 0x1f,
+				    coeff_div[i].plldiv);
+		break;
+	case 48000:
+		snd_soc_update_bits(codec, ML26124_SMPLING_RATE, 0xf,
+				    get_srate(coeff_div[i].rate));
+		snd_soc_update_bits(codec, ML26124_PLLNL, 0xff,
+				    coeff_div[i].pllnl);
+		snd_soc_update_bits(codec, ML26124_PLLNH, 0x1,
+				    coeff_div[i].pllnh);
+		snd_soc_update_bits(codec, ML26124_PLLML, 0xff,
+				    coeff_div[i].pllml);
+		snd_soc_update_bits(codec, ML26124_PLLMH, 0x3f,
+				    coeff_div[i].pllmh);
+		snd_soc_update_bits(codec, ML26124_PLLDIV, 0x1f,
+				    coeff_div[i].plldiv);
+		break;
+	default:
+		pr_err("%s:this rate is no support for ml26124\n", __func__);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+#define DVOL_CTL_DVMUTE_ON		BIT(4)	/* Digital volume MUTE On */
+#define DVOL_CTL_DVMUTE_OFF		0	/* Digital volume MUTE Off */
+
+static int ml26124_mute(struct snd_soc_dai *dai, int mute)
+{
+	struct snd_soc_codec *codec = dai->codec;
+
+	if (mute)
+		snd_soc_update_bits(codec, ML26124_DVOL_CTL, BIT(4),
+				    DVOL_CTL_DVMUTE_ON);
+	else
+		snd_soc_update_bits(codec, ML26124_DVOL_CTL, BIT(4),
+				    DVOL_CTL_DVMUTE_OFF);
+	return 0;
+}
+
+#define ML26124_SAI_NO_DELAY	BIT(1)
+#define ML26124_SAI_FRAME_SYNC	(BIT(5) | BIT(0)) /* For mono (Telecodec) */
+
+static int ml26124_set_dai_fmt(struct snd_soc_dai *codec_dai,
+		unsigned int fmt)
+{
+	unsigned char mode;
+	struct snd_soc_codec *codec = codec_dai->codec;
+	struct ml26124_priv *priv = snd_soc_codec_get_drvdata(codec);
+	unsigned char mask = ML26124_SAI_NO_DELAY | ML26124_SAI_FRAME_SYNC;
+
+	/* set master/slave audio interface */
+	switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
+	case SND_SOC_DAIFMT_CBM_CFM:
+		mode = 1;
+		break;
+	case SND_SOC_DAIFMT_CBS_CFS:
+		mode = 0;
+		break;
+	default:
+		return -EINVAL;
+	}
+	snd_soc_update_bits(codec, ML26124_SAI_MODE_SEL, BIT(0), mode);
+
+	/* interface format */
+	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+	case SND_SOC_DAIFMT_I2S:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/* clock inversion */
+	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
+	case SND_SOC_DAIFMT_NB_NF:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/* SAI configuration */
+	if (priv->channels == 1) {
+		snd_soc_update_bits(codec, ML26124_SAI_TRANS_CTL, mask,
+				ML26124_SAI_NO_DELAY | ML26124_SAI_FRAME_SYNC);
+		snd_soc_update_bits(codec, ML26124_SAI_RCV_CTL, mask,
+				ML26124_SAI_NO_DELAY | ML26124_SAI_FRAME_SYNC);
+	} else {
+		snd_soc_update_bits(codec, ML26124_SAI_TRANS_CTL, mask,
+				    ML26124_SAI_NO_DELAY);
+		snd_soc_update_bits(codec, ML26124_SAI_RCV_CTL, mask,
+				    ML26124_SAI_NO_DELAY);
+	}
+
+	return 0;
+}
+
+static int ml26124_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 ml26124_priv *priv = snd_soc_codec_get_drvdata(codec);
+
+	switch (clk_id) {
+	case ML26124_USE_PLL:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	switch (freq) {
+	case 12288000:
+		priv->mclk = freq;
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+#define ML26134_CACHESIZE 79
+static const u16 ml26124_reg[ML26134_CACHESIZE] = {
+	0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
+	0x0, 0x0, 0x0,				/* System Control */
+	0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,	/* Power Management */
+	0x4,					/* Analog Reference Control */
+	0x10, 0x0, 0x33, 0x0, 0x0,	/* Input/Output Amplifier Control */
+	0x0, 0x0, 0x1,				/* Analog Path Contorl */
+	0x0, 0x0, 0x0,				/* Audio Interface Control */
+	0x1, 0x0, 0x0, 0xff, 0xff, 0x10,	/* DSP Control */
+	0x7, 0x7, 0x7, 0x7, 0x7,		/* DSP Control */
+	0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,	/* DSP Control */
+	0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,	/* DSP Control */
+	0x0, 0x0, 0x0,					/* DSP Control */
+	0x0, 0x2, 0x3, 0x0, 0xb, 0x70, 0x0, 0x0,	/* ALC Control */
+	0x4, 0x5, 0xd, 0x70, 0x10, 0x0,		/* Playback Limiter Control */
+	0x1, 0x1, 0x1				/* Video Amplifer Control */
+};
+
+#define ML26124_VMID	BIT(0)
+
+static void ml26124_sync_cache(struct snd_soc_codec *codec)
+{
+	u8 *reg_cache = codec->reg_cache;
+	int i;
+
+	if (!codec->cache_sync)
+		return;
+
+	codec->cache_only = 0;
+
+	for (i = 0; i < codec->driver->reg_cache_size; i++) {
+		if (reg_cache[i] == ml26124_reg[i])
+			continue;
+
+		snd_soc_write(codec, i, reg_cache[i]);
+	}
+
+	codec->cache_sync = 0;
+}
+
+static int ml26124_set_bias_level(struct snd_soc_codec *codec,
+		enum snd_soc_bias_level level)
+{
+	switch (level) {
+	case SND_SOC_BIAS_ON:
+		/* VMID ON */
+		snd_soc_update_bits(codec, ML26124_PW_REF_PW_MNG,
+				    ML26124_VMID, ML26124_VMID);
+		msleep(500);
+	case SND_SOC_BIAS_PREPARE:
+	case SND_SOC_BIAS_STANDBY:
+		if (codec->dapm.bias_level == SND_SOC_BIAS_OFF)
+			ml26124_sync_cache(codec);
+		break;
+	case SND_SOC_BIAS_OFF:
+		/* VMID OFF */
+		snd_soc_update_bits(codec, ML26124_PW_REF_PW_MNG,
+				    ML26124_VMID, 0);
+		break;
+	}
+	codec->dapm.bias_level = level;
+	return 0;
+}
+
+static int ml26124_pcm_trigger(struct snd_pcm_substream *substream,
+			      int cmd, struct snd_soc_dai *codec_dai)
+{
+	struct snd_soc_codec *codec = codec_dai->codec;
+
+	if (cmd == SNDRV_PCM_TRIGGER_STOP) {
+		snd_soc_update_bits(codec, ML26124_REC_PLYBAK_RUN, 0x3, 0);
+		return 0;
+	} else if (cmd == SNDRV_PCM_TRIGGER_START) {
+		if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
+			snd_soc_update_bits(codec, ML26124_REC_PLYBAK_RUN, 0x3,
+					    1);
+		else
+			snd_soc_update_bits(codec, ML26124_REC_PLYBAK_RUN, 0x3,
+					    2);
+	}
+
+	return 0;
+}
+
+#define ML26124_RATES (SNDRV_PCM_RATE_16000 | SNDRV_PCM_RATE_32000 |\
+		       SNDRV_PCM_RATE_48000)
+#define ML26124_FORMATS SNDRV_PCM_FMTBIT_S16_LE
+
+static const struct snd_soc_dai_ops ml26124_dai_ops = {
+	.hw_params	= ml26124_hw_params,
+	.digital_mute	= ml26124_mute,
+	.set_fmt	= ml26124_set_dai_fmt,
+	.set_sysclk	= ml26124_set_dai_sysclk,
+	.trigger	= ml26124_pcm_trigger,
+};
+
+static struct snd_soc_dai_driver ml26124_dai = {
+	.name = "ml26124-hifi",
+	.playback = {
+		.stream_name = "Playback",
+		.channels_min = 1,
+		.channels_max = 2,
+		.rates = ML26124_RATES,
+		.formats = ML26124_FORMATS,},
+	.capture = {
+		.stream_name = "Capture",
+		.channels_min = 1,
+		.channels_max = 2,
+		.rates = ML26124_RATES,
+		.formats = ML26124_FORMATS,},
+	.ops = &ml26124_dai_ops,
+	.symmetric_rates = 1,
+};
+
+#ifdef CONFIG_PM
+static int ml26124_suspend(struct snd_soc_codec *codec, pm_message_t state)
+{
+	ml26124_set_bias_level(codec, SND_SOC_BIAS_OFF);
+
+	return 0;
+}
+
+static int ml26124_resume(struct snd_soc_codec *codec)
+{
+	ml26124_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
+
+	return 0;
+}
+#else
+#define ml26124_suspend NULL
+#define ml26124_resume NULL
+#endif
+
+static int ml26124_probe(struct snd_soc_codec *codec)
+{
+	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;
+	}
+
+	/* Software Reset */
+	snd_soc_update_bits(codec, ML26124_SW_RST, 0x01, 1);
+	snd_soc_update_bits(codec, ML26124_SW_RST, 0x01, 0);
+
+	ml26124_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
+
+	return 0;
+}
+
+/* power down chip */
+static struct snd_soc_codec_driver soc_codec_dev_ml26124 = {
+	.probe =	ml26124_probe,
+	.suspend =	ml26124_suspend,
+	.resume =	ml26124_resume,
+	.set_bias_level = ml26124_set_bias_level,
+	.reg_cache_size = ML26134_CACHESIZE,
+	.reg_word_size = sizeof(u8),
+	.reg_cache_default = ml26124_reg,
+	.dapm_widgets = ml26124_dapm_widgets,
+	.num_dapm_widgets = ARRAY_SIZE(ml26124_dapm_widgets),
+	.dapm_routes = ml26124_intercon,
+	.num_dapm_routes = ARRAY_SIZE(ml26124_intercon),
+	.controls = ml26124_snd_controls,
+	.num_controls = ARRAY_SIZE(ml26124_snd_controls),
+};
+
+static __devinit int ml26124_i2c_probe(struct i2c_client *i2c,
+				      const struct i2c_device_id *id)
+{
+	struct ml26124_priv *priv;
+
+	priv = devm_kzalloc(&i2c->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	i2c_set_clientdata(i2c, priv);
+
+	return snd_soc_register_codec(&i2c->dev,
+			&soc_codec_dev_ml26124, &ml26124_dai, 1);
+}
+
+static __devexit int ml26124_i2c_remove(struct i2c_client *client)
+{
+	snd_soc_unregister_codec(&client->dev);
+	return 0;
+}
+
+static const struct i2c_device_id ml26124_i2c_id[] = {
+	{ "ml26124", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, ml26124_i2c_id);
+
+static struct i2c_driver ml26124_i2c_driver = {
+	.driver = {
+		.name = "ml26124",
+		.owner = THIS_MODULE,
+	},
+	.probe = ml26124_i2c_probe,
+	.remove = __devexit_p(ml26124_i2c_remove),
+	.id_table = ml26124_i2c_id,
+};
+
+static int __init ml26124_modinit(void)
+{
+	int ret = 0;
+
+	ret = i2c_add_driver(&ml26124_i2c_driver);
+	if (ret != 0)
+		pr_err("Failed to register ML26124 I2C driver: %d\n", ret);
+
+	return ret;
+}
+module_init(ml26124_modinit);
+
+static void __exit ml26124_exit(void)
+{
+	i2c_del_driver(&ml26124_i2c_driver);
+}
+module_exit(ml26124_exit);
+
+MODULE_AUTHOR("Tomoya MORINAGA <tomoya.rohm@gmail.com>");
+MODULE_DESCRIPTION("LAPIS Semiconductor ML26124 ALSA SoC codec driver");
+MODULE_LICENSE("GPL");
diff --git a/sound/soc/codecs/ml26124.h b/sound/soc/codecs/ml26124.h
new file mode 100644
index 0000000..4fe7337
--- /dev/null
+++ b/sound/soc/codecs/ml26124.h
@@ -0,0 +1,129 @@
+/*
+ * Copyright (C) 2011 LAPIS Semiconductor Co., Ltd.
+ *
+ * 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; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307, USA.
+ */
+
+#ifndef ML26124_H
+#define ML26124_H
+
+/* Clock Control Register */
+#define ML26124_SMPLING_RATE		0x00
+#define ML26124_PLLNL			0x02
+#define ML26124_PLLNH			0x04
+#define ML26124_PLLML			0x06
+#define ML26124_PLLMH			0x08
+#define ML26124_PLLDIV			0x0a
+#define ML26124_CLK_EN			0x0c
+#define ML26124_CLK_CTL			0x0e
+
+/* System Control Register */
+#define ML26124_SW_RST			0x10
+#define ML26124_REC_PLYBAK_RUN		0x12
+#define ML26124_MIC_TIM			0x14
+
+/* Power Mnagement Register */
+#define ML26124_PW_REF_PW_MNG		0x20
+#define ML26124_PW_IN_PW_MNG		0x22
+#define ML26124_PW_DAC_PW_MNG		0x24
+#define ML26124_PW_SPAMP_PW_MNG		0x26
+#define ML26124_PW_LOUT_PW_MNG		0x28
+#define ML26124_PW_VOUT_PW_MNG		0x2a
+#define ML26124_PW_ZCCMP_PW_MNG		0x2e
+
+/* Analog Reference Control Register */
+#define ML26124_PW_MICBIAS_VOL		0x30
+
+/* Input/Output Amplifier Control Register */
+#define ML26124_PW_MIC_IN_VOL		0x32
+#define ML26124_PW_MIC_BOST_VOL		0x38
+#define ML26124_PW_SPK_AMP_VOL		0x3a
+#define ML26124_PW_AMP_VOL_FUNC		0x48
+#define ML26124_PW_AMP_VOL_FADE		0x4a
+
+/* Analog Path Control Register */
+#define ML26124_SPK_AMP_OUT		0x54
+#define ML26124_MIC_IF_CTL		0x5a
+#define ML26124_MIC_SELECT		0xe8
+
+/* Audio Interface Control Register */
+#define ML26124_SAI_TRANS_CTL		0x60
+#define ML26124_SAI_RCV_CTL		0x62
+#define ML26124_SAI_MODE_SEL		0x64
+
+/* DSP Control Register */
+#define ML26124_FILTER_EN		0x66
+#define ML26124_DVOL_CTL		0x68
+#define ML26124_MIXER_VOL_CTL		0x6a
+#define ML26124_RECORD_DIG_VOL		0x6c
+#define ML26124_PLBAK_DIG_VOL		0x70
+#define ML26124_DIGI_BOOST_VOL		0x72
+#define ML26124_EQ_GAIN_BRAND0		0x74
+#define ML26124_EQ_GAIN_BRAND1		0x76
+#define ML26124_EQ_GAIN_BRAND2		0x78
+#define ML26124_EQ_GAIN_BRAND3		0x7a
+#define ML26124_EQ_GAIN_BRAND4		0x7c
+#define ML26124_HPF2_CUTOFF		0x7e
+#define ML26124_EQBRAND0_F0L		0x80
+#define ML26124_EQBRAND0_F0H		0x82
+#define ML26124_EQBRAND0_F1L		0x84
+#define ML26124_EQBRAND0_F1H		0x86
+#define ML26124_EQBRAND1_F0L		0x88
+#define ML26124_EQBRAND1_F0H		0x8a
+#define ML26124_EQBRAND1_F1L		0x8c
+#define ML26124_EQBRAND1_F1H		0x8e
+#define ML26124_EQBRAND2_F0L		0x90
+#define ML26124_EQBRAND2_F0H		0x92
+#define ML26124_EQBRAND2_F1L		0x94
+#define ML26124_EQBRAND2_F1H		0x96
+#define ML26124_EQBRAND3_F0L		0x98
+#define ML26124_EQBRAND3_F0H		0x9a
+#define ML26124_EQBRAND3_F1L		0x9c
+#define ML26124_EQBRAND3_F1H		0x9e
+#define ML26124_EQBRAND4_F0L		0xa0
+#define ML26124_EQBRAND4_F0H		0xa2
+#define ML26124_EQBRAND4_F1L		0xa4
+#define ML26124_EQBRAND4_F1H		0xa6
+
+/* ALC Control Register */
+#define ML26124_ALC_MODE		0xb0
+#define ML26124_ALC_ATTACK_TIM		0xb2
+#define ML26124_ALC_DECAY_TIM		0xb4
+#define ML26124_ALC_HOLD_TIM		0xb6
+#define ML26124_ALC_TARGET_LEV		0xb8
+#define ML26124_ALC_MAXMIN_GAIN		0xba
+#define ML26124_NOIS_GATE_THRSH		0xbc
+#define ML26124_ALC_ZERO_TIMOUT		0xbe
+
+/* Playback Limiter Control Register */
+#define ML26124_PL_ATTACKTIME		0xc0
+#define ML26124_PL_DECAYTIME		0xc2
+#define ML26124_PL_TARGETTIME		0xc4
+#define ML26124_PL_MAXMIN_GAIN		0xc6
+#define ML26124_PLYBAK_BOST_VOL		0xc8
+#define ML26124_PL_0CROSS_TIMOUT	0xca
+
+/* Video Amplifer Control Register */
+#define ML26124_VIDEO_AMP_GAIN_CTL	0xd0
+#define ML26124_VIDEO_AMP_SETUP1	0xd2
+#define ML26124_VIDEO_AMP_CTL2		0xd4
+
+/* Clock select for machine driver */
+#define ML26124_USE_PLL			0
+#define ML26124_USE_MCLKI_256FS		1
+#define ML26124_USE_MCLKI_512FS		2
+#define ML26124_USE_MCLKI_1024FS	3
+
+
+#endif
-- 
1.7.4.4


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

* [PATCH 2/3 v4] sound/soc/lapis: add machine driver for ML7213 Carrier Board
  2011-12-20  2:45 [PATCH 1/3 v5] sound/soc/codecs: add LAPIS Semiconductor ML26124 Tomoya MORINAGA
@ 2011-12-20  2:45 ` Tomoya MORINAGA
  2011-12-20 10:45     ` Mark Brown
  2011-12-20  2:45 ` [PATCH 3/3 v2] sound/soc/lapis: add platform driver for ML7213 IOH I2S Tomoya MORINAGA
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 29+ messages in thread
From: Tomoya MORINAGA @ 2011-12-20  2:45 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
	Lars-Peter Clausen, Dimitris Papastamos, Mike Frysinger,
	Daniel Mack, alsa-devel, linux-kernel
  Cc: qi.wang, yong.y.wang, joel.clark, kok.howg.ewe, Tomoya MORINAGA

This driver is for LAPIS Semiconductor ML7213 Carrier Board.
This driver controls ML26124 CODEC and ML7213 IOH I2S.

Signed-off-by: Tomoya MORINAGA <tomoya.rohm@gmail.com>
---
V4
 - Add MCLKFS / BCLKFS processing
 - Delete ml7213_ioh_init
---
 sound/soc/Kconfig                   |    1 +
 sound/soc/Makefile                  |    1 +
 sound/soc/lapis/Kconfig             |    5 +
 sound/soc/lapis/Makefile            |    4 +
 sound/soc/lapis/ml7213ioh-machine.c |  214 +++++++++++++++++++++++++++++++++++
 5 files changed, 225 insertions(+), 0 deletions(-)
 create mode 100644 sound/soc/lapis/Kconfig
 create mode 100644 sound/soc/lapis/Makefile
 create mode 100644 sound/soc/lapis/ml7213ioh-machine.c

diff --git a/sound/soc/Kconfig b/sound/soc/Kconfig
index 35e662d..63f8582 100644
--- a/sound/soc/Kconfig
+++ b/sound/soc/Kconfig
@@ -34,6 +34,7 @@ source "sound/soc/ep93xx/Kconfig"
 source "sound/soc/fsl/Kconfig"
 source "sound/soc/imx/Kconfig"
 source "sound/soc/jz4740/Kconfig"
+source "sound/soc/lapis/Kconfig"
 source "sound/soc/nuc900/Kconfig"
 source "sound/soc/omap/Kconfig"
 source "sound/soc/kirkwood/Kconfig"
diff --git a/sound/soc/Makefile b/sound/soc/Makefile
index 9ea8ac8..002951f 100644
--- a/sound/soc/Makefile
+++ b/sound/soc/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_SND_SOC)	+= ep93xx/
 obj-$(CONFIG_SND_SOC)	+= fsl/
 obj-$(CONFIG_SND_SOC)   += imx/
 obj-$(CONFIG_SND_SOC)	+= jz4740/
+obj-$(CONFIG_SND_SOC)   += lapis/
 obj-$(CONFIG_SND_SOC)	+= mid-x86/
 obj-$(CONFIG_SND_SOC)	+= mxs/
 obj-$(CONFIG_SND_SOC)	+= nuc900/
diff --git a/sound/soc/lapis/Kconfig b/sound/soc/lapis/Kconfig
new file mode 100644
index 0000000..f6c2fe5
--- /dev/null
+++ b/sound/soc/lapis/Kconfig
@@ -0,0 +1,5 @@
+config SND_SOC_ML7213_MACHINE
+	tristate "ML7213 IOH ASoC machine driver"
+	select SND_SOC_ML7213_PLATFORM
+	help
+	  This is ASoC machine driver for ML7213 IOH
diff --git a/sound/soc/lapis/Makefile b/sound/soc/lapis/Makefile
new file mode 100644
index 0000000..cdb196a
--- /dev/null
+++ b/sound/soc/lapis/Makefile
@@ -0,0 +1,4 @@
+# Platform
+snd-soc-ml7213-machine-objs := ml7213ioh-machine.o
+
+obj-$(CONFIG_SND_SOC_ML7213_MACHINE) += snd-soc-ml7213-machine.o
diff --git a/sound/soc/lapis/ml7213ioh-machine.c b/sound/soc/lapis/ml7213ioh-machine.c
new file mode 100644
index 0000000..4787793
--- /dev/null
+++ b/sound/soc/lapis/ml7213ioh-machine.c
@@ -0,0 +1,214 @@
+/*
+ * ml7213ioh-machine.c -- SoC Audio for LAPIS Semiconductor ML7213 IOH CRB
+ *
+ * Copyright (C) 2011 LAPIS Semiconductor Co., Ltd.
+ *
+ * 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; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307, USA.
+ */
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <sound/core.h>
+#include <sound/pcm.h>
+#include <sound/soc.h>
+#include <sound/pcm_params.h>
+#include <linux/i2c.h>
+#include "../codecs/ml26124.h"
+#include "ioh_i2s.h"
+
+static const struct snd_soc_dapm_widget ml7213_dapm_widgets[] = {
+	SND_SOC_DAPM_SPK("Speaker", NULL),
+	SND_SOC_DAPM_LINE("LINEOUT", NULL),
+	SND_SOC_DAPM_LINE("LINEIN", NULL),
+	SND_SOC_DAPM_MIC("AnalogMIC", NULL),
+	SND_SOC_DAPM_MIC("DigitalMIC", NULL),
+};
+
+static const struct snd_soc_dapm_route ml7213_routes[] = {
+	{"Speaker", NULL, "SPOUT"},
+	{"LINEOUT", NULL, "LOUT"},
+	{"MIN", NULL, "AnalogMIC"},
+	{"MDIN", NULL, "DigitalMIC"},
+	{"MIN", NULL, "LINEIN"},
+};
+
+static int ml7213_hw_params(struct snd_pcm_substream *substream,
+			    struct snd_pcm_hw_params *hw_params)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_dai *codec_dai = rtd->codec_dai;
+	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+
+	unsigned int clk = 0;
+	int ret = 0;
+	int bclkfs;
+	int mclkfs;
+	int rate = params_rate(hw_params);
+
+	switch (rate) {
+	case 16000:
+	case 32000:
+	case 48000:
+		clk = 12288000;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	mclkfs = clk / rate;
+
+	switch (params_format(hw_params)) {
+	case SNDRV_PCM_FORMAT_U8:
+	case SNDRV_PCM_FORMAT_S16_LE:
+		bclkfs = 32;
+		break;
+	case SNDRV_PCM_FORMAT_S32_LE:
+		bclkfs = 64;
+		break;
+	default:
+		pr_err("%s: Failed not support format\n", __func__);
+		return -1;
+		break;
+	}
+
+	/* set the codec system clock for DAC and ADC */
+	ret = snd_soc_dai_set_sysclk(codec_dai, ML26124_USE_PLL, clk,
+				     SND_SOC_CLOCK_IN);
+	if (ret < 0)
+		return ret;
+
+	ret = snd_soc_dai_set_clkdiv(cpu_dai, ML7213IOH_BCLKFS0, bclkfs);
+	if (ret < 0)
+		return ret;
+
+	ret = snd_soc_dai_set_clkdiv(cpu_dai, ML7213IOH_BCLKFS1, bclkfs);
+	if (ret < 0)
+		return ret;
+
+	ret = snd_soc_dai_set_clkdiv(cpu_dai, ML7213IOH_BCLKFS2, bclkfs);
+	if (ret < 0)
+		return ret;
+
+	ret = snd_soc_dai_set_clkdiv(cpu_dai, ML7213IOH_BCLKFS3, bclkfs);
+	if (ret < 0)
+		return ret;
+
+	ret = snd_soc_dai_set_clkdiv(cpu_dai, ML7213IOH_BCLKFS4, bclkfs);
+	if (ret < 0)
+		return ret;
+
+	ret = snd_soc_dai_set_clkdiv(cpu_dai, ML7213IOH_BCLKFS5, bclkfs);
+	if (ret < 0)
+		return ret;
+
+	ret = snd_soc_dai_set_clkdiv(cpu_dai, ML7213IOH_MCLKFS0, mclkfs);
+	if (ret < 0)
+		return ret;
+
+	ret = snd_soc_dai_set_clkdiv(cpu_dai, ML7213IOH_MCLKFS1, mclkfs);
+	if (ret < 0)
+		return ret;
+
+	ret = snd_soc_dai_set_clkdiv(cpu_dai, ML7213IOH_MCLKFS2, mclkfs);
+	if (ret < 0)
+		return ret;
+
+	ret = snd_soc_dai_set_clkdiv(cpu_dai, ML7213IOH_MCLKFS3, mclkfs);
+	if (ret < 0)
+		return ret;
+
+	ret = snd_soc_dai_set_clkdiv(cpu_dai, ML7213IOH_MCLKFS4, mclkfs);
+	if (ret < 0)
+		return ret;
+
+	ret = snd_soc_dai_set_clkdiv(cpu_dai, ML7213IOH_MCLKFS5, mclkfs);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static struct snd_soc_ops ml7213_ops = {
+	.hw_params = ml7213_hw_params,
+};
+
+static struct snd_soc_dai_link ioh_i2s_dai = {
+	.name = "ML7213",
+	.stream_name = "I2S HiFi",
+	.cpu_dai_name = "ml7213-i2s",
+	.codec_dai_name = "ml26124-hifi",
+	.platform_name	= "ml7213-i2s-pcm-audio",
+	.codec_name	= "ml26124-00GD",
+	.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
+					SND_SOC_DAIFMT_CBS_CFS,
+	.ops = &ml7213_ops,
+};
+
+static struct snd_soc_card ioh_i2s_card = {
+	.name		= "ml7213",
+	.dai_link	= &ioh_i2s_dai,
+	.num_links	= 1,
+	.dapm_widgets	= ml7213_dapm_widgets,
+	.num_dapm_widgets	= ARRAY_SIZE(ml7213_dapm_widgets),
+	.dapm_routes	= ml7213_routes,
+	.num_dapm_routes	= ARRAY_SIZE(ml7213_routes),
+};
+
+static int __init ioh_i2s_probe(struct platform_device *pdev)
+{
+	ioh_i2s_card.dev = &pdev->dev;
+	snd_soc_register_card(&ioh_i2s_card);
+
+	return 0;
+}
+
+static int __exit ioh_i2s_remove(struct platform_device *pdev)
+{
+	int ret = snd_soc_unregister_card(&ioh_i2s_card);
+
+	if (ret) {
+		dev_err(&pdev->dev, "snd_soc_register_card failed (%d)\n", ret);
+		ioh_i2s_card.dev = NULL;
+		return ret;
+	}
+
+	return 0;
+}
+
+static struct platform_driver ioh_i2s_driver = {
+	.remove = ioh_i2s_remove,
+	.driver = {
+		.name = "ml7213ioh-i2s",
+		.owner = THIS_MODULE,
+	},
+	.probe	= ioh_i2s_probe,
+	.remove	= __devexit_p(ioh_i2s_remove),
+};
+
+static int __init ioh_i2s_init(void)
+{
+	return platform_driver_register(&ioh_i2s_driver);
+}
+
+static void __exit ioh_i2s_exit(void)
+{
+	return platform_driver_unregister(&ioh_i2s_driver);
+}
+
+module_init(ioh_i2s_init);
+module_exit(ioh_i2s_exit);
+
+MODULE_AUTHOR("Tomoya MORINAGA <tomoya.rohm@gmail.com>");
+MODULE_DESCRIPTION("LAPIS Semiconductor ML7213 IOH ALSA SoC machine driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:ml7213ioh-i2s");
-- 
1.7.4.4


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

* [PATCH 3/3 v2] sound/soc/lapis: add platform driver for ML7213 IOH I2S
  2011-12-20  2:45 [PATCH 1/3 v5] sound/soc/codecs: add LAPIS Semiconductor ML26124 Tomoya MORINAGA
  2011-12-20  2:45 ` [PATCH 2/3 v4] sound/soc/lapis: add machine driver for ML7213 Carrier Board Tomoya MORINAGA
@ 2011-12-20  2:45 ` Tomoya MORINAGA
  2011-12-20 13:23     ` Mark Brown
  2011-12-20 10:40   ` Mark Brown
  2011-12-20 10:42   ` Mark Brown
  3 siblings, 1 reply; 29+ messages in thread
From: Tomoya MORINAGA @ 2011-12-20  2:45 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
	Lars-Peter Clausen, Dimitris Papastamos, Mike Frysinger,
	Daniel Mack, alsa-devel, linux-kernel
  Cc: qi.wang, yong.y.wang, joel.clark, kok.howg.ewe, Tomoya MORINAGA

Signed-off-by: Tomoya MORINAGA <tomoya.rohm@gmail.com>
---
V2
 - Delete unused module_param "index"
 - Re-desing data structure. So, some internal functions interface are changed
 - Add dai interface functions (Includes format, sysclk, clkdiv)
 - Delete snd_device_new, snd_card_create snd_card_register
---
 sound/soc/lapis/Kconfig          |    5 +
 sound/soc/lapis/Makefile         |    2 +
 sound/soc/lapis/ioh_i2s.h        |   39 +
 sound/soc/lapis/ml7213ioh-plat.c | 2033 ++++++++++++++++++++++++++++++++++++++
 sound/soc/lapis/ml7213ioh-plat.h |  583 +++++++++++
 5 files changed, 2662 insertions(+), 0 deletions(-)
 create mode 100644 sound/soc/lapis/ioh_i2s.h
 create mode 100644 sound/soc/lapis/ml7213ioh-plat.c
 create mode 100644 sound/soc/lapis/ml7213ioh-plat.h

diff --git a/sound/soc/lapis/Kconfig b/sound/soc/lapis/Kconfig
index f6c2fe5..e10e729 100644
--- a/sound/soc/lapis/Kconfig
+++ b/sound/soc/lapis/Kconfig
@@ -1,3 +1,8 @@
+config SND_SOC_ML7213_PLATFORM
+	tristate "ML7213 IOH ASoC platform driver"
+	help
+	  This option enables support for the AC Link Controllers in ML7213 IOH SoC.
+
 config SND_SOC_ML7213_MACHINE
 	tristate "ML7213 IOH ASoC machine driver"
 	select SND_SOC_ML7213_PLATFORM
diff --git a/sound/soc/lapis/Makefile b/sound/soc/lapis/Makefile
index cdb196a..812290b 100644
--- a/sound/soc/lapis/Makefile
+++ b/sound/soc/lapis/Makefile
@@ -1,4 +1,6 @@
 # Platform
 snd-soc-ml7213-machine-objs := ml7213ioh-machine.o
+snd-soc-ml7213-plat-objs := ml7213ioh-plat.o
 
 obj-$(CONFIG_SND_SOC_ML7213_MACHINE) += snd-soc-ml7213-machine.o
+obj-$(CONFIG_SND_SOC_ML7213_PLATFORM) += snd-soc-ml7213-plat.o
diff --git a/sound/soc/lapis/ioh_i2s.h b/sound/soc/lapis/ioh_i2s.h
new file mode 100644
index 0000000..9f19f70
--- /dev/null
+++ b/sound/soc/lapis/ioh_i2s.h
@@ -0,0 +1,39 @@
+/*
+ * Copyright (C) 2011 LAPIS Semiconductor Co., Ltd.
+ *
+ * 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; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307, USA.
+ */
+
+#ifndef ML7213_IOH_I2S
+#define ML7213_IOH_I2S
+
+enum ioh_bclkfs {
+	ML7213IOH_BCLKFS0 = 0,
+	ML7213IOH_BCLKFS1,
+	ML7213IOH_BCLKFS2,
+	ML7213IOH_BCLKFS3,
+	ML7213IOH_BCLKFS4,
+	ML7213IOH_BCLKFS5,
+};
+
+enum ioh_mclkfs {
+	ML7213IOH_MCLKFS0 = 6,
+	ML7213IOH_MCLKFS1,
+	ML7213IOH_MCLKFS2,
+	ML7213IOH_MCLKFS3,
+	ML7213IOH_MCLKFS4,
+	ML7213IOH_MCLKFS5,
+};
+
+#endif
diff --git a/sound/soc/lapis/ml7213ioh-plat.c b/sound/soc/lapis/ml7213ioh-plat.c
new file mode 100644
index 0000000..4b9ed3c
--- /dev/null
+++ b/sound/soc/lapis/ml7213ioh-plat.c
@@ -0,0 +1,2033 @@
+/*
+ * Copyright (C) 2011 LAPIS Semiconductor Co., Ltd.
+ *
+ * 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; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307, USA.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/scatterlist.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+#include <linux/pci.h>
+#include <linux/clk.h>
+#include <linux/dma-mapping.h>
+#include <sound/core.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include <sound/initval.h>
+
+#include "ioh_i2s.h"
+#include "ml7213ioh-plat.h"
+
+static struct ioh_i2s_data *i2s_data;
+static struct ioh_i2s_dma dmadata[MAX_I2S_CH];
+static int ignore_overrun = 1;
+module_param(ignore_overrun, int, 0444);
+MODULE_PARM_DESC(ignore_overrun, "ignore RX overruns (default=0)");
+
+/*****************************************************************************
+ *	I2S HAL (Hardware Abstruction Layer)
+ *****************************************************************************/
+static void ioh_i2s_reset(int ch)
+{
+	iowrite32(1 << ch, i2s_data->iobase + I2SSRST_OFFSET);
+	iowrite32(0, i2s_data->iobase + I2SSRST_OFFSET);
+}
+
+static void ioh_i2s_enable_interrupts(int ch, enum dma_data_direction dir)
+{
+	unsigned int intr_lines;
+
+	if (dir)
+		intr_lines = 1 << (I2S_IMASK_RX_BIT_START + ch);
+
+	else
+		intr_lines = 1 << (I2S_IMASK_TX_BIT_START + ch);
+
+	/*enable interrupts for specified channel */
+	iowrite32(intr_lines, i2s_data->iobase + I2SIMASKCLR_OFFSET);
+}
+
+static void ioh_i2s_disable_interrupts(int ch, enum dma_data_direction dir)
+{
+	unsigned int intr_lines;
+
+	/*intr_lines&=I2S_ALL_INTERRUPT_BITS; */
+	intr_lines = ioread32(i2s_data->iobase + I2SIMASK_OFFSET);
+
+	/*disable interrupts for specified channel */
+	if (dir)
+		intr_lines |= 1 << (I2S_IMASK_RX_BIT_START + ch);
+	else
+		intr_lines |= 1 << (I2S_IMASK_TX_BIT_START + ch);
+
+	/*Mask the specific interrupt bits */
+	iowrite32(intr_lines, i2s_data->iobase + I2SIMASK_OFFSET);
+}
+
+/* Run FIFO */
+static void ioh_i2s_run_tx_fifo(int ch)
+{
+	int offset = ch * 0x800;
+	u32 val;
+
+	val = ioread32(i2s_data->iobase + I2SFIFOCTX_OFFSET + offset);
+	val |= I2S_FIFO_TX_RUN;
+
+	iowrite32(val, i2s_data->iobase + I2SFIFOCTX_OFFSET + offset);
+}
+
+/* Clear TX FIFO */
+static void ioh_i2s_clear_tx_fifo(int ch)
+{
+	int offset = ch * 0x800;
+	u32 val;
+
+	val = ioread32(i2s_data->iobase + I2SFIFOCTX_OFFSET + offset);
+	val |= I2S_FIFO_TX_FCLR;
+
+	iowrite32(val, i2s_data->iobase + I2SFIFOCTX_OFFSET + offset);
+}
+
+/* Clear interrupt status */
+static void ioh_i2s_clear_tx_sts_ir(int ch)
+{
+	int offset = ch * 0x800;
+
+	iowrite32(I2S_TX_FINT | I2S_TX_AFINT | I2S_TX_EINT | I2S_TX_AEINT,
+		i2s_data->iobase + I2SISTTX_OFFSET + offset);
+}
+
+/* Run FIFO */
+static void ioh_i2s_run_rx_fifo(int ch)
+{
+	int offset = ch * 0x800;
+	u32 val;
+
+	val = ioread32(i2s_data->iobase + I2SFIFOCRX_OFFSET + offset);
+	val |= I2S_FIFO_RX_RUN;
+	iowrite32(val, i2s_data->iobase + I2SFIFOCRX_OFFSET + offset);
+}
+
+/* Clear RX FIFO */
+static void ioh_i2s_clear_rx_fifo(int ch)
+{
+	int offset = ch * 0x800;
+	u32 val;
+
+	val = ioread32(i2s_data->iobase + I2SFIFOCRX_OFFSET + offset);
+	val |= I2S_FIFO_RX_FCLR;
+	iowrite32(val, i2s_data->iobase + I2SFIFOCRX_OFFSET + offset);
+}
+
+/* Clear interrupt status */
+static void ioh_i2s_clear_rx_sts_ir(int ch)
+{
+	int offset = ch * 0x800;
+
+	iowrite32(I2S_RX_FINT | I2S_RX_AFINT | I2S_RX_EINT | I2S_RX_AEINT,
+		i2s_data->iobase + I2SISTRX_OFFSET + offset);
+}
+
+/* Clear DMA mask setting */
+static void ioh_i2s_tx_clear_dma_mask(int ch)
+{
+	int offset = ch * 0x800;
+	u32 val;
+
+	val = ioread32(i2s_data->iobase + I2SMSKTX_OFFSET + offset);
+	val &= ~TX_BIT_DMAMSK; /* Enable Tx DMA Request */
+
+	iowrite32(val, i2s_data->iobase + I2SMSKTX_OFFSET + offset);
+}
+
+/* Clear DMA mask setting */
+static void ioh_i2s_rx_clear_dma_mask(int ch)
+{
+	int offset = ch * 0x800;
+	u32 val;
+
+	val = ioread32(i2s_data->iobase + I2SMSKRX_OFFSET + offset);
+	val &= ~RX_BIT_DMAMSK; /* Enable Rx DMA Request */
+	iowrite32(val, i2s_data->iobase + I2SMSKRX_OFFSET + offset);
+}
+
+/* Clear the mask setting of the corresponding interrupt source bit */
+static void ioh_i2s_enable_tx_empty_ir(int ch)
+{
+	int offset = ch * 0x800;
+	u32 val;
+
+	val = ioread32(i2s_data->iobase + I2SMSKTX_OFFSET + offset);
+	val &= ~TX_BIT_AEIMSK; /* Enable Almost empty interrupt */
+	val &= ~TX_BIT_EIMSK; /* Enable Empty interrupt */
+
+	iowrite32(val, i2s_data->iobase + I2SMSKTX_OFFSET + offset);
+}
+
+/* Clear the mask setting of the corresponding interrupt source bit */
+static void ioh_i2s_enable_rx_full_ir(int ch)
+{
+	int offset = ch * 0x800;
+	u32 val;
+	val = ioread32(i2s_data->iobase + I2SMSKRX_OFFSET + offset);
+
+	val &= ~RX_BIT_AFIMSK; /* Enable Almost empty interrupt */
+	val &= ~RX_BIT_FIMSK; /* Enable Empty interrupt */
+
+	iowrite32(val, i2s_data->iobase + I2SMSKRX_OFFSET + offset);
+}
+
+static void ioh_i2s_disable_tx_empty_ir(int ch)
+{
+	int offset = ch * 0x800;
+	u32 val;
+
+	val = ioread32(i2s_data->iobase + I2SMSKTX_OFFSET + offset);
+	val |= TX_BIT_AEIMSK; /* Disble Almost empty interrupt */
+	val |= TX_BIT_EIMSK; /* Disble Empty interrupt */
+
+	iowrite32(val, i2s_data->iobase + I2SMSKTX_OFFSET + offset);
+}
+
+static void ioh_i2s_disable_rx_full_ir(int ch)
+{
+	int offset = ch * 0x800;
+	u32 val;
+
+	val = ioread32(i2s_data->iobase + I2SMSKRX_OFFSET + offset);
+	val |= RX_BIT_AFIMSK; /* Disble Almost full interrupt */
+	val |= RX_BIT_FIMSK; /* Disble full interrupt */
+
+	iowrite32(val, i2s_data->iobase + I2SMSKRX_OFFSET + offset);
+}
+
+/*****************************************************************************
+ *	I2S Middle ware
+ *****************************************************************************/
+static bool filter(struct dma_chan *chan, void *slave)
+{
+	struct pch_dma_slave *param = slave;
+
+	if ((chan->chan_id == param->chan_id) && (param->dma_dev ==
+						  chan->device->dev)) {
+		chan->private = param;
+		return true;
+	} else {
+		return false;
+	}
+}
+
+static struct dma_chan *ioh_request_dma_channel(
+		   int ch, struct ioh_i2s_dma *dma, enum dma_data_direction dir)
+{
+	dma_cap_mask_t mask;
+	struct dma_chan *chan;
+	struct pci_dev *dma_dev;
+	dma_cap_zero(mask);
+	dma_cap_set(DMA_SLAVE, mask);
+
+	dma_dev = pci_get_bus_and_slot(2, PCI_DEVFN(0, 1)); /* Get DMA's dev
+								information */
+
+	if (dir == DMA_FROM_DEVICE) { /* Rx */
+		dma->param_rx.width = dma->dma_rx_width;
+		dma->param_rx.dma_dev = &dma_dev->dev;
+		dma->param_rx.chan_id = ch * 2 + 1; /* ch Rx=1,3,...11 */
+		dma->param_rx.rx_reg = (dma_addr_t)(i2s_data->mapbase +\
+					ch * 0x800 + I2SDRRXMIRROR_OFFSET);
+		chan = dma_request_channel(mask, filter, &dma->param_rx);
+		if (chan == NULL) {
+			dev_err(i2s_data->dev, "Failed dma_request_channel for"
+				" I2S %d\n", ch);
+			return NULL;
+		}
+		dma->chan_rx = chan;
+
+	} else if (dir == DMA_TO_DEVICE) { /* Tx */
+		dma->param_tx.width = dma->dma_tx_width;
+		dma->param_tx.dma_dev = &dma_dev->dev;
+		dma->param_tx.chan_id = ch * 2; /* DMA ch Tx=0,2,...10 */
+
+		dma->param_tx.tx_reg = (dma_addr_t)(i2s_data->mapbase +\
+					ch * 0x800 + I2SDRTXMIRROR_OFFSET);
+
+		chan = dma_request_channel(mask, filter, &dma->param_tx);
+		if (chan == NULL) {
+			dev_err(i2s_data->dev, "Failed dma_request_channel for"
+				" I2S %d\n", ch);
+			return NULL;
+		}
+		dma->chan_tx = chan;
+	} else {
+		dev_err(i2s_data->dev, "Invalid direction (%d)\n", dir);
+		return NULL;
+	}
+
+	return chan;
+}
+
+static void ioh_i2s_ignore_rx_overrun(void)
+{
+	i2s_data->ignore_rx_overrun = 1;
+}
+
+static void
+ioh_i2s_write(struct snd_pcm_substream *substream, const void *data, int len)
+{
+	int rem1;
+	int rem2;
+	int tx_index;
+	int ch = substream->number;
+	struct ioh_i2s_dma *dma = &dmadata[ch];
+	struct scatterlist *sg = dma->sg_tx_p;
+	int t_num = 0;
+	int l;
+	int *ptr_fmt;
+	int *ptr32;
+	short *ptr16;
+	char *ptr8;
+	int tx_unit = dmadata[ch].dma_tx_unit;
+	struct device *dev = substream->pcm->card->dev;
+
+	if (dma->tx_avail >= INTER_BUFF_SIZE) {
+		dev_err(dev, "%s[%d]: internal buffer full\n",
+			__func__, ch);
+		return;
+	}
+
+	dev_dbg(dev, "%s: [ch%d] len=%d data_head=%p data_complete=%p",
+		__func__, ch, len, dma->tx_data_head, dma->tx_complete);
+
+	if ((dma->tx_data_head + ((len/tx_unit) * 4)) <= dma->tx_tail) {
+		tx_index = (int)(dma->tx_data_head - dma->tx_head) /
+				(I2S_AEMPTY_THRESH * 4);
+		sg = sg + tx_index;
+		t_num = len/(I2S_AEMPTY_THRESH * tx_unit);
+		dma_sync_sg_for_cpu(dev, sg, t_num, DMA_TO_DEVICE);
+
+		ptr_fmt = (int *)dma->tx_data_head;
+		switch (tx_unit) {
+		case 1:
+			ptr8 = (char *)data;
+			for (l = 0; l < (len/tx_unit); l++)
+				*ptr_fmt++ = (int)*ptr8++;
+			break;
+		case 2:
+			ptr16 = (short *)data;
+			for (l = 0; l < (len/tx_unit); l++)
+				*ptr_fmt++ = (int)*ptr16++;
+			break;
+		case 4:
+			ptr32 = (int *)data;
+			for (l = 0; l < (len/tx_unit); l++)
+				*ptr_fmt++ = *ptr32++;
+			break;
+		}
+		dma_sync_sg_for_device(dev, sg, t_num, DMA_TO_DEVICE);
+		dma->tx_data_head += (len/tx_unit) * 4;
+	} else {
+		rem1 = (dma->tx_tail - dma->tx_data_head) / 4;
+		rem2 = (len/tx_unit) - rem1;
+		tx_index = (int)(dma->tx_data_head-dma->tx_head) /
+				(I2S_AEMPTY_THRESH * 4);
+		sg = sg + tx_index;
+		t_num = rem1/I2S_AEMPTY_THRESH;
+		dma_sync_sg_for_cpu(dev, sg, t_num, DMA_TO_DEVICE);
+		ptr_fmt = (int *)dma->tx_data_head;
+		switch (tx_unit) {
+		case 1:
+			ptr8 = (char *)data;
+			for (l = 0; l < rem1; l++)
+				*ptr_fmt++ = (int)*ptr8++;
+			break;
+		case 2:
+			ptr16 = (short *)data;
+			for (l = 0; l < rem1; l++)
+				*ptr_fmt++ = (int)*ptr16++;
+			break;
+		case 4:
+			ptr32 = (int *)data;
+			for (l = 0; l < rem1; l++)
+				*ptr_fmt++ = *ptr32++;
+			break;
+		}
+
+		dma_sync_sg_for_device(dev, sg, t_num, DMA_TO_DEVICE);
+		dma->tx_data_head = dma->tx_head;
+		sg = dma->sg_tx_p;
+		t_num = rem2/I2S_AEMPTY_THRESH;
+		dma_sync_sg_for_cpu(dev, sg, t_num, DMA_TO_DEVICE);
+
+		ptr_fmt = (int *)dma->tx_data_head;
+
+		switch (tx_unit) {
+		case 1:
+			ptr8 = (char *)(data+rem1*tx_unit);
+			for (l = 0; l < rem2; l++)
+				*ptr_fmt++ = (int)*ptr8++;
+			break;
+		case 2:
+			ptr16 = (short *)(data+rem1*tx_unit);
+			for (l = 0; l < rem2; l++)
+				*ptr_fmt++ = (int)*ptr16++;
+			break;
+		case 4:
+			ptr32 = (int *)(data+rem1*tx_unit);
+			for (l = 0; l < rem2; l++)
+				*ptr_fmt++ = *ptr32++;
+			break;
+		}
+
+		dma_sync_sg_for_device(dev, sg, t_num, DMA_TO_DEVICE);
+		dma->tx_data_head += rem2 * 4;
+	}
+
+	if (dma->tx_data_head >= dma->tx_tail)
+		dma->tx_data_head = dma->tx_head;
+
+	dev_dbg(dev, "-->data_head=%p\n", dma->tx_data_head);
+
+	dma->tx_avail += (len/tx_unit) * 4;
+}
+
+static void ioh_i2s_stop_i2s_regs(int ch, enum ioh_direction dir)
+{
+	if (dir) {
+		/* Interrupt stop */
+		ioh_i2s_disable_rx_full_ir(ch);
+
+		/* FIFO setting */
+		ioh_i2s_clear_rx_fifo(ch);
+		ioh_i2s_clear_rx_sts_ir(ch);
+	} else {
+		/* Interrupt stop */
+		ioh_i2s_disable_tx_empty_ir(ch);
+
+		/* FIFO setting */
+		ioh_i2s_clear_tx_fifo(ch);
+		ioh_i2s_clear_tx_sts_ir(ch);
+	}
+}
+
+static void ioh_i2s_configure_i2s_regs(int ch, enum ioh_direction dir)
+{
+	int offset = ch * 0x800;
+
+	if (dir) {
+		/* Rx register */
+		iowrite32(I2S_AFULL_THRESH / 2,
+			  i2s_data->iobase + I2SAFRX_OFFSET + offset);
+		iowrite32(0x1F, i2s_data->iobase + I2SAERX_OFFSET + offset);
+		iowrite32(0x1F, i2s_data->iobase + I2SMSKRX_OFFSET + offset);
+		iowrite32(0xC, i2s_data->iobase + I2SISTRX_OFFSET + offset);
+
+		/* FIFO setting */
+		ioh_i2s_clear_rx_fifo(ch);
+		ioh_i2s_run_rx_fifo(ch);
+
+		/* Interrupt setting */
+		ioh_i2s_clear_rx_sts_ir(ch);
+		ioh_i2s_enable_rx_full_ir(ch);
+
+	} else {
+		iowrite32(0x0, i2s_data->iobase + I2SAFTX_OFFSET + offset);
+		iowrite32(I2S_AEMPTY_THRESH / 2,
+			  i2s_data->iobase + I2SAETX_OFFSET + offset);
+		iowrite32(0x1F, i2s_data->iobase + I2SMSKTX_OFFSET + offset);
+		iowrite32(0xC, i2s_data->iobase + I2SISTTX_OFFSET + offset);
+
+		/* FIFO setting */
+		ioh_i2s_clear_tx_fifo(ch);
+		ioh_i2s_run_tx_fifo(ch);
+
+		/* Interrupt setting */
+		ioh_i2s_clear_tx_sts_ir(ch);
+		ioh_i2s_enable_tx_empty_ir(ch);
+	}
+}
+
+static void i2s_rx_tasklet(unsigned long data)
+{
+	struct ioh_i2s_data *priv = (struct ioh_i2s_data *)data;
+	struct ioh_i2s_dma *dma = &dmadata[priv->ch];
+	int num = 0;
+
+	if (dma->rxexe_flag) {
+		if (dma->rx_done) {
+			switch (dma->dma_rx_unit) {
+			case 1:
+				num = dma->rx_avail / 4;
+				break;
+			case 2:
+				num = dma->rx_avail / 2;
+				break;
+			case 4:
+				num = dma->rx_avail;
+				break;
+			}
+			dma->rx_done(dma->rx_callback_data, IOH_EOK, num, num);
+		}
+	}
+}
+
+static void i2s_tx_tasklet(unsigned long data)
+{
+	struct ioh_i2s_data *priv = (struct ioh_i2s_data *)data;
+	struct ioh_i2s_dma *dma = &dmadata[priv->ch];
+	int num = 0;
+	int avail = 0;
+
+	if (dma->txexe_flag) {
+		if (dma->tx_done) {
+			switch (dmadata[priv->ch].dma_tx_unit) {
+			case 1:
+				num = (INTER_BUFF_SIZE - dma->tx_avail) / 4;
+				avail = dma->tx_avail / 4;
+				break;
+			case 2:
+				num = (INTER_BUFF_SIZE - dma->tx_avail) / 2;
+				avail = dma->tx_avail / 2;
+				break;
+			case 4:
+				num = (INTER_BUFF_SIZE - dma->tx_avail);
+				avail = dma->tx_avail;
+				break;
+			}
+			dma->tx_done(dma->tx_callback_data,
+				      IOH_EOK, num, avail);
+		}
+	}
+}
+
+static void ioh_i2s_release(int ch, enum ioh_direction dir)
+{
+	struct ioh_i2s_dma *dma;
+
+	dma = &dmadata[ch];
+	if (dir) {
+		dma_sync_sg_for_cpu(i2s_data->dev, dma->sg_rx_p, dma->rx_nent,
+				    DMA_FROM_DEVICE);
+
+		ioh_i2s_disable_interrupts(ch, IOH_CAPTURE);
+		ioh_i2s_disable_rx_full_ir(ch);
+		if (dma->chan_rx) {
+			dma->chan_rx->device->device_control(dma->chan_rx,
+							     DMA_TERMINATE_ALL,
+							     0);
+			dma_release_channel(dma->chan_rx);
+			dma->chan_rx = NULL;
+		}
+
+		kfree(dma->sg_rx_p);
+		if (dma->rxbuf_virt)
+			dma_free_coherent(i2s_data->dev, INTER_BUFF_SIZE,
+					  dma->rxbuf_virt, dma->rx_buf_dma);
+
+		dma->rxbuf_virt = NULL;
+		dma->rx_buf_dma = 0;
+		atomic_dec(&dma->rx_busy);
+
+		tasklet_disable(&dma->rx_tasklet);
+		tasklet_kill(&dma->rx_tasklet);
+
+	} else {
+		dma_sync_sg_for_cpu(i2s_data->dev, dma->sg_tx_p, dma->tx_nent,
+				    DMA_TO_DEVICE);
+
+		ioh_i2s_disable_interrupts(ch, IOH_PLAYBACK);
+		ioh_i2s_disable_tx_empty_ir(ch);
+		if (dma->chan_tx) {
+			dma->chan_tx->device->device_control(dma->chan_tx,
+							     DMA_TERMINATE_ALL,
+							     0);
+			dma_release_channel(dma->chan_tx);
+			dma->chan_tx = NULL;
+		}
+
+		kfree(dma->sg_tx_p);
+		if (dma->txbuf_virt)
+			dma_free_coherent(i2s_data->dev, INTER_BUFF_SIZE,
+					  dma->txbuf_virt, dma->tx_buf_dma);
+
+		dma->txbuf_virt = NULL;
+		dma->tx_buf_dma = 0;
+		atomic_dec(&dma->tx_busy);
+
+		tasklet_disable(&dma->tx_tasklet);
+		tasklet_kill(&dma->tx_tasklet);
+	}
+}
+
+static struct ioh_i2s_data *ioh_i2s_open(int ch, enum ioh_direction dir,
+					 const char *name)
+{
+	struct ioh_i2s_data *obj = NULL;
+	struct scatterlist *sg;
+	int rx_size;
+	int rx_num;
+	int tx_size;
+	int tx_num;
+	int i;
+	struct ioh_i2s_dma *dma;
+
+	if (ch >= MAX_I2S_CH) {
+		dev_err(obj->dev,
+			"Tried to open i2s with number %d which is more then"
+			" the available number\n", ch);
+		return 0;
+	}
+
+	dma = &dmadata[ch];
+	i2s_data = kzalloc(sizeof(*i2s_data), GFP_KERNEL);
+	i2s_data->ignore_rx_overrun = 0;
+
+	atomic_set(&dma->rx_busy, 0);
+	atomic_set(&dma->tx_busy, 0);
+
+	dma->dma_tx_width = PCH_DMA_WIDTH_4_BYTES;
+	dma->dma_rx_width = PCH_DMA_WIDTH_4_BYTES;
+
+	if (dir) {
+		/* Rx configuration */
+		if (atomic_read(&dma->rx_busy)) {
+			dev_err(i2s_data->dev, "rx i2s%dalready opened\n", ch);
+			atomic_dec(&dma->rx_busy);
+			return NULL;
+		}
+		atomic_inc(&dma->rx_busy);
+
+		ioh_request_dma_channel(ch, dma, DMA_FROM_DEVICE);
+		if (!dma->chan_rx) {
+			dev_err(obj->dev, "%s:ioh_setup_rx_dma failed\n",
+				__func__);
+			return NULL;
+		}
+
+		dma->rxbuf_virt = dma_alloc_coherent(obj->dev, INTER_BUFF_SIZE,
+						&dma->rx_buf_dma, GFP_KERNEL);
+		if (!dma->rxbuf_virt) {
+			dev_err(obj->dev, "dma_alloc_coherent Failed\n");
+			return NULL;
+		}
+
+		rx_size = I2S_AFULL_THRESH * 4;
+		/* The number of scatter list (Franction area is not used) */
+		rx_num = INTER_BUFF_SIZE / rx_size;
+
+		dev_dbg(obj->dev, "%s: rx: scatter_num=%d scatter_size=%d\n",
+			__func__, rx_num, rx_size);
+
+		dma->sg_rx_p =\
+		       kzalloc(sizeof(struct scatterlist) * rx_num, GFP_ATOMIC);
+
+		sg = dma->sg_rx_p;
+		sg_init_table(sg, rx_num); /* Initialize SG table */
+
+		for (i = 0; i < rx_num; i++, sg++) {
+			sg_set_page(sg, virt_to_page(dma->rxbuf_virt), rx_size,
+				    rx_size * i);
+			sg_dma_len(sg) = rx_size / 4;
+			sg_dma_address(sg) = dma->rx_buf_dma + sg->offset;
+		}
+
+		dma->rx_head = (unsigned char *)dma->rxbuf_virt;
+		dma->rx_tail = (unsigned char *)dma->rxbuf_virt +
+			       rx_num * rx_size;
+		dma->rx_data_head = (unsigned char *)dma->rxbuf_virt;
+		dma->rx_complete = (unsigned char *)dma->rxbuf_virt;
+		dma->rx_avail = 0;
+
+		dma->rx_nent = rx_num;
+		dma_sync_sg_for_device(obj->dev, dma->sg_rx_p, dma->rx_nent,
+				       DMA_FROM_DEVICE);
+
+		tasklet_init(&dma->rx_tasklet, i2s_rx_tasklet,
+			     (unsigned long)obj);
+	} else {
+		/* Tx configuration */
+		if (atomic_read(&dma->tx_busy)) {
+			dev_err(obj->dev, "tx i2s%d have already opened\n", ch);
+			atomic_dec(&dma->tx_busy);
+			return NULL;
+		}
+		atomic_inc(&dma->tx_busy);
+
+		ioh_request_dma_channel(ch, dma, DMA_TO_DEVICE);
+
+		if (!dma->chan_tx) {
+			dev_err(obj->dev, "%s:ioh_setup_tx_dma failed\n",
+				__func__);
+			return NULL;
+		}
+
+		tx_size = I2S_AEMPTY_THRESH * 4;
+		if (INTER_BUFF_SIZE % tx_size)
+			/* tx_num = The number of scatter list */
+			tx_num = INTER_BUFF_SIZE / tx_size + 1;
+		else
+			tx_num = INTER_BUFF_SIZE / tx_size;
+
+		dma->txbuf_virt = dma_alloc_coherent(obj->dev, INTER_BUFF_SIZE,
+						&dma->tx_buf_dma, GFP_KERNEL);
+
+		if (!dma->txbuf_virt) {
+			dev_err(obj->dev, "dma_alloc_coherent Failed\n");
+			return NULL;
+		}
+
+		dma->tx_head = (unsigned char *)dma->txbuf_virt;
+		dma->tx_tail = (unsigned char *)dma->txbuf_virt +
+			       INTER_BUFF_SIZE;
+		dma->tx_data_head = (unsigned char *)dma->txbuf_virt;
+		dma->tx_complete = (unsigned char *)dma->txbuf_virt;
+		dma->tx_avail = 0;
+
+		dev_dbg(obj->dev, "%s: tx: scatter_num=%d scatter_size=%d\n",
+			__func__, tx_num, tx_size);
+
+		dma->sg_tx_p =\
+		       kzalloc(sizeof(struct scatterlist) * tx_num, GFP_ATOMIC);
+
+		sg_init_table(dma->sg_tx_p, tx_num); /* Initialize SG table */
+		sg = dma->sg_tx_p;
+
+		for (i = 0; i < tx_num; i++, sg++) {
+			if (i == (tx_num - 1)) {
+				if (INTER_BUFF_SIZE % tx_size) {
+					sg_set_page(sg,
+						  virt_to_page(dma->txbuf_virt),
+						  INTER_BUFF_SIZE % tx_size,
+						  tx_size * i);
+					sg_dma_len(sg) =
+						  (INTER_BUFF_SIZE % tx_size)
+						  / 4;
+				} else {
+					sg_set_page(sg,
+						  virt_to_page(dma->txbuf_virt),
+						  tx_size, tx_size * i);
+					sg_dma_len(sg) = tx_size / 4;
+				}
+			} else {
+				sg_set_page(sg, virt_to_page(dma->txbuf_virt),
+					    tx_size, tx_size * i);
+				sg_dma_len(sg) = tx_size / 4;
+			}
+			sg_dma_address(sg) = dma->tx_buf_dma + sg->offset;
+		}
+		dma->tx_nent = tx_num;
+		dma_sync_sg_for_device(obj->dev, dma->sg_tx_p, dma->tx_nent,
+				       DMA_TO_DEVICE);
+
+		tasklet_init(&dma->tx_tasklet, i2s_tx_tasklet,
+			     (unsigned long)obj);
+	}
+
+	return obj;
+}
+
+static void
+ioh_i2s_read(struct snd_pcm_substream *substream, void *data, int len)
+{
+	unsigned int rem1 = 0, rem2 = 0;
+	int ch = substream->number;
+	struct ioh_i2s_dma *dma = &dmadata[ch];
+	struct scatterlist *sg = dma->sg_rx_p;
+	int rx_index;
+	int t_num = 0;
+	int *ptr_fmt;
+	int *ptr32;
+	short *ptr16;
+	char *ptr8;
+	int l;
+	int rx_unit = dma->dma_rx_unit;
+	struct device *dev = substream->pcm->card->dev;
+
+	switch (rx_unit) {
+	case 1:
+		t_num = dma->rx_avail / 4;
+		break;
+	case 2:
+		t_num = dma->rx_avail / 2;
+		break;
+	case 4:
+		t_num = dma->rx_avail;
+		break;
+	}
+
+	if (t_num < len) {
+		dev_err(dev, "%s[%d]: internal buffer empty\n",
+			__func__, ch);
+		return;
+	}
+
+	if ((dma->rx_complete + ((len/rx_unit) * 4)) <= dma->rx_tail) {
+		rx_index = (int)(dma->rx_complete - dma->rx_head) /
+				(I2S_AFULL_THRESH * 4);
+		sg = sg + rx_index;
+		t_num = len/(I2S_AFULL_THRESH * rx_unit);
+		dma_sync_sg_for_cpu(dev, sg, t_num, DMA_FROM_DEVICE);
+
+		ptr_fmt = (int *)dma->rx_complete;
+		switch (rx_unit) {
+		case 1:
+			ptr8 = (char *)data;
+			for (l = 0; l < (len/rx_unit); l++)
+				*ptr8++ = (char)*ptr_fmt++;
+			break;
+		case 2:
+			ptr16 = (short *)data;
+			for (l = 0; l < (len/rx_unit); l++)
+				*ptr16++ = (short)*ptr_fmt++;
+			break;
+		case 4:
+			ptr32 = (int *)data;
+			for (l = 0; l < (len/rx_unit); l++)
+				*ptr32++ = *ptr_fmt++;
+			break;
+		}
+		dma_sync_sg_for_device(dev, sg, t_num, DMA_FROM_DEVICE);
+		dma->rx_complete += (len/rx_unit) * 4;
+	} else {
+		rem1 = (dma->rx_tail - dma->rx_complete) / 4;
+		rem2 = (len/rx_unit) - rem1;
+		rx_index = (int)(dma->rx_complete-dma->rx_head) /
+				(I2S_AFULL_THRESH * 4);
+		sg = sg + rx_index;
+		t_num = rem1/I2S_AFULL_THRESH;
+		dma_sync_sg_for_cpu(dev, sg, t_num, DMA_FROM_DEVICE);
+		ptr_fmt = (int *)dma->rx_complete;
+		switch (rx_unit) {
+		case 1:
+			ptr8 = (char *)data;
+			for (l = 0; l < rem1; l++)
+				*ptr8++ = (char)*ptr_fmt++;
+			break;
+		case 2:
+			ptr16 = (short *)data;
+			for (l = 0; l < rem1; l++)
+				*ptr16++ = (short)*ptr_fmt++;
+			break;
+		case 4:
+			ptr32 = (int *)data;
+			for (l = 0; l < rem1; l++)
+				*ptr32++ = *ptr_fmt++;
+			break;
+		}
+		dma_sync_sg_for_device(dev, sg, t_num, DMA_FROM_DEVICE);
+		dma->rx_complete = dma->rx_head;
+		sg = dma->sg_rx_p;
+		t_num = rem2/I2S_AFULL_THRESH;
+		dma_sync_sg_for_cpu(dev, sg, t_num, DMA_FROM_DEVICE);
+		ptr_fmt = (int *)dma->rx_complete;
+		switch (rx_unit) {
+		case 1:
+			ptr8 = (char *)(data+rem1*rx_unit);
+			for (l = 0; l < rem2; l++)
+				*ptr8++ = (char)*ptr_fmt++;
+			break;
+		case 2:
+			ptr16 = (short *)(data+rem1*rx_unit);
+			for (l = 0; l < rem2; l++)
+				*ptr16++ = (short)*ptr_fmt++;
+			break;
+		case 4:
+			ptr32 = (int *)(data+rem1*rx_unit);
+			for (l = 0; l < rem2; l++)
+				*ptr32++ = *ptr_fmt++;
+			break;
+		}
+		dma_sync_sg_for_device(dev, sg, t_num, DMA_FROM_DEVICE);
+		dma->rx_complete += rem2 * 4;
+	}
+
+	if (dma->rx_complete >= dma->rx_tail)
+		dma->rx_complete = dma->rx_head;
+
+	dma->rx_avail -= (len/rx_unit) * 4;
+}
+
+static void i2s_dma_rx_complete(void *arg)
+{
+	int ch = (int)arg;
+	struct ioh_i2s_dma *dma = &dmadata[ch];
+	struct scatterlist *sg = dma->sg_rx_cur;
+	int num = dma->rx_num;
+	int i;
+
+	async_tx_ack(dma->desc_rx);
+
+	for (i = 0; i < num; i++, sg++) {
+		dma->rx_data_head += sg_dma_len(sg) * 4;
+		dma->rx_avail += sg_dma_len(sg) * 4;
+	}
+
+	if (dma->rx_data_head >= dma->rx_tail)
+		dma->rx_data_head = dma->rx_head;
+
+	ioh_i2s_clear_rx_sts_ir(ch);
+	ioh_i2s_enable_rx_full_ir(ch);
+	kfree(arg);
+}
+
+static void i2s_dma_tx_complete(void *arg)
+{
+	int ch = (int)arg;
+	struct ioh_i2s_dma *dma = &dmadata[ch];
+	struct scatterlist *sg = dma->sg_tx_cur;
+	int num = dma->tx_num;
+	int i;
+
+	async_tx_ack(dma->desc_tx);
+
+	for (i = 0; i < num; i++, sg++) {
+		dma->tx_complete += sg_dma_len(sg) * 4;
+		dma->tx_avail -= sg_dma_len(sg) * 4;
+	}
+
+	if (dma->tx_complete >= dma->tx_tail)
+		dma->tx_complete = dma->tx_head;
+	ioh_i2s_clear_tx_sts_ir(ch);
+	ioh_i2s_enable_tx_empty_ir(ch);
+	kfree(arg);
+}
+
+/*****************************************************************************
+ *	Interrupt control
+ *****************************************************************************/
+static void i2s_tx_almost_empty_ir(int ch)
+{
+	struct dma_async_tx_descriptor *desc;
+	int num;
+	int tx_comp_index;
+	struct ioh_i2s_dma *dma = &dmadata[ch];
+	struct scatterlist *sg = dma->sg_tx_p;
+	void *cb_ch;
+
+	dev_dbg(i2s_data->dev, "%s: data_head=%p data_complete=%p\n", __func__,
+		dma->tx_data_head, dma->tx_complete);
+
+	num = ((int)dma->tx_avail) / (I2S_AEMPTY_THRESH * 4);
+
+	tx_comp_index = (((int)(dma->tx_complete - dma->tx_head))) /\
+			(I2S_AEMPTY_THRESH * 4);
+
+	if ((tx_comp_index + num) >= dma->tx_nent)
+		num = dma->tx_nent - tx_comp_index;
+
+	if (num > I2S_DMA_SG_NUM)
+		num = I2S_DMA_SG_NUM;
+
+	if (!num) {
+		dev_err(i2s_data->dev, "%s:Internal buffer empty\n",
+			__func__);
+		tasklet_schedule(&dma->tx_tasklet);
+		return; /* No data to transmit */
+	}
+
+	sg = sg + tx_comp_index; /* Point head of sg must be sent */
+	dma->sg_tx_cur = sg; /* Save tx condition */
+	dma->tx_num = num; /* Save tx condition */
+
+	desc = dma->chan_tx->device->device_prep_slave_sg(dma->chan_tx,
+					sg, num, DMA_TO_DEVICE,
+					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+
+	if (!desc) {
+		dev_err(i2s_data->dev, "%s:device_prep_slave_sg Failed\n",
+			__func__);
+		return;
+	}
+
+	/* To prevent this function from calling again before DMA completion */
+	ioh_i2s_disable_tx_empty_ir(ch);
+
+	dma->desc_tx = desc;
+
+	cb_ch = kzalloc(sizeof(int), GFP_KERNEL);
+	cb_ch = (void *)ch;
+
+	desc->callback = i2s_dma_tx_complete;
+	desc->callback_param = cb_ch;
+
+	atomic_inc(&dma->pending_tx);
+	desc->tx_submit(desc);
+
+	tasklet_schedule(&dma->tx_tasklet);
+}
+
+void i2s_tx_empty_ir(int ch)
+{
+	dev_warn(i2s_data->dev, "%s:I2S%d under flow occurs\n", __func__, ch);
+}
+
+void i2s_rx_full_ir(int ch)
+{
+	dev_warn(i2s_data->dev, "%s:I2S%d overrun occurs\n", __func__, ch);
+}
+
+static inline void ioh_i2s_interrupt_sub_tx(int ch)
+{
+	unsigned int status;
+	int offset = ch * 0x800;
+
+	status = ioread32(i2s_data->iobase + I2SISTTX_OFFSET + offset);
+	if (status & I2S_TX_EINT)
+		i2s_tx_empty_ir(ch);
+	if (status & I2S_TX_AEINT)
+		i2s_tx_almost_empty_ir(ch);
+
+	/*Clear the interrupt status */
+	iowrite32(status, i2s_data->iobase + I2SISTTX_OFFSET + offset);
+}
+
+static void i2s_rx_almost_full_ir(int ch)
+{
+	struct dma_async_tx_descriptor *desc;
+	struct scatterlist *sg;
+	int rx_data_index;
+	int num;
+	void *cb_ch;
+	struct ioh_i2s_dma *dma = &dmadata[ch];
+
+	num = (int)(INTER_BUFF_SIZE - dma->rx_avail) / (I2S_AFULL_THRESH * 4);
+	if (num < 1) {
+		dev_err(i2s_data->dev, "%s:Internal buffer full\n",
+			__func__);
+		tasklet_schedule(&dma->rx_tasklet);
+		return;
+	}
+
+	sg = dma->sg_rx_p;
+	rx_data_index = ((int)(dma->rx_data_head - dma->rx_head)) /\
+			(I2S_AFULL_THRESH * 4);
+
+	if ((rx_data_index + num) >= dma->rx_nent)
+		num = dma->rx_nent - rx_data_index;
+
+	if (num > I2S_DMA_SG_NUM)
+		num = I2S_DMA_SG_NUM;
+
+	sg += rx_data_index;
+
+	desc = dma->chan_rx->device->device_prep_slave_sg(dma->chan_rx,
+			sg, num, DMA_FROM_DEVICE,
+			DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+	if (!desc) {
+		dev_err(i2s_data->dev, "%s:device_prep_slave_sg Failed\n",
+			__func__);
+		return;
+	}
+
+	dma->sg_rx_cur = sg; /* Save rx condition */
+	ioh_i2s_disable_rx_full_ir(ch);
+	dma->rx_num = num;
+
+	cb_ch = kzalloc(sizeof(int), GFP_KERNEL);
+	cb_ch = (void *)ch;
+
+	dma->desc_rx = desc;
+	desc->callback = i2s_dma_rx_complete;
+	desc->callback_param = cb_ch;
+	desc->tx_submit(desc);
+
+	tasklet_schedule(&dma->rx_tasklet);
+}
+
+static inline void ioh_i2s_interrupt_sub_rx(int ch)
+{
+	unsigned int status;
+	int offset = ch * 0x800;
+
+	status = ioread32(i2s_data->iobase + I2SISTRX_OFFSET + offset);
+	if (status & I2S_RX_FINT)
+		i2s_rx_full_ir(ch);
+	if (status & I2S_RX_AFINT)
+		i2s_rx_almost_full_ir(ch);
+
+	/*Clear the interrupt status */
+	iowrite32(status, i2s_data->iobase + I2SISTRX_OFFSET + offset);
+}
+
+void ioh_i2s_event(u32 idisp, int ch)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&i2s_data->tx_lock, flags);
+
+	if (idisp & BIT(ch + 16)) {
+		dev_dbg(i2s_data->dev, "Rx%d interrupt occures\n", ch);
+		ioh_i2s_interrupt_sub_rx(ch);
+	}
+
+	if (idisp & BIT(ch)) {
+		dev_dbg(i2s_data->dev, "Tx%d interrupt occures\n", ch);
+		ioh_i2s_interrupt_sub_tx(ch);
+	}
+
+	spin_unlock_irqrestore(&i2s_data->tx_lock, flags);
+	return;
+}
+
+static irqreturn_t ioh_i2s_irq(int irq, void *data)
+{
+	int i;
+	u32 idisp;
+
+	idisp = ioread32(i2s_data->iobase + I2SIDISP_OFFSET);
+	for (i = 0; i < MAX_I2S_CH; i++)
+		ioh_i2s_event(idisp, i);
+
+	return IRQ_HANDLED;
+}
+
+/*****************************************************************************
+ *	Sound Card
+ *****************************************************************************/
+static void i2s_read_period(struct snd_pcm_substream *substream)
+{
+	struct ml7213i2s_runtime_data *rtd;
+	int period;
+	void *read_ptr;
+	int read_size;
+
+	rtd = substream->runtime->private_data;
+	period = substream->runtime->period_size;
+
+	read_ptr = substream->runtime->dma_area
+			  +(snd_pcm_lib_period_bytes(substream) * rtd->irq_pos);
+	read_size = period * (substream->runtime->sample_bits / 8) *
+						(substream->runtime->channels);
+
+	ioh_i2s_read(substream, read_ptr, read_size);
+	rtd->irq_pos = (rtd->irq_pos + 1) % substream->runtime->periods;
+}
+
+static void read_done(void *callback_data, int status, int num, int avail)
+{
+	struct snd_pcm_substream *substream =
+				      (struct snd_pcm_substream *)callback_data;
+	struct ml7213i2s_runtime_data *rtd;
+	int ch = substream->number;
+	rtd = substream->runtime->private_data;
+
+	if (num < snd_card_ml7213i2s_capture[ch].period_bytes_max)
+		return;
+
+	switch (status) {
+	case IOH_EOK:
+		if (!rtd->stop) {
+			i2s_read_period(substream);
+			rtd->buf_pos = (rtd->buf_pos + 1) %
+				substream->runtime->periods;
+			snd_pcm_period_elapsed(substream);
+		}
+
+		rtd->cnt++;
+		break;
+	case IOH_EDONE:
+		pr_debug("Done stopping channel %d\n", rtd->stop);
+		rtd->stop = 2;
+		break;
+
+	case IOH_EOVERRUN:
+		if (ignore_overrun)
+			pr_debug("overrun ignore\n");
+		else {
+			pr_err("RX overrun\n");
+			rtd->stop = 2;
+		}
+		break;
+
+	case IOH_EFRAMESYNC:
+		pr_err("Frame sync error\n");
+		rtd->stop = 2;
+		break;
+	}
+	if (rtd->stop)
+		pr_debug("stopping... %d\n", rtd->stop);
+}
+
+static int setup_i2s_read(struct snd_pcm_substream *substream)
+{
+	struct ml7213i2s_runtime_data *rtd;
+	int ret = 0;
+	int ch = substream->number;
+
+	rtd = substream->runtime->private_data;
+
+	if (!ioh_i2s_open(ch, IOH_CAPTURE, "radio-i2s-in")) {
+		pr_err("%s: Cannot open the device\n", __func__);
+		return -1;
+	}
+
+	if (ignore_overrun)
+		ioh_i2s_ignore_rx_overrun();
+
+	ioh_i2s_configure_i2s_regs(ch, IOH_CAPTURE);
+	return ret;
+}
+
+static void i2s_write_period(struct snd_pcm_substream *substream)
+{
+	struct ml7213i2s_runtime_data *rtd;
+	int period;
+	void *write_ptr;
+	int write_size;
+
+	rtd = substream->runtime->private_data;
+	period = substream->runtime->period_size;
+	write_ptr = substream->runtime->dma_area
+			 + (snd_pcm_lib_period_bytes(substream) * rtd->irq_pos);
+	write_size = period * (substream->runtime->sample_bits / 8) *
+						(substream->runtime->channels);
+
+	ioh_i2s_write(substream, write_ptr, write_size);
+	rtd->irq_pos = (rtd->irq_pos + 1) % substream->runtime->periods;
+}
+
+static void write_done(void *callback_data, int status, int num, int avail)
+{
+	struct snd_pcm_substream *substream =
+				      (struct snd_pcm_substream *)callback_data;
+	struct ml7213i2s_runtime_data *rtd;
+	int ch = substream->number;
+
+	rtd = substream->runtime->private_data;
+
+	if (num < snd_card_ml7213i2s_playback[ch].period_bytes_max)
+		return;
+	if (avail >= snd_card_ml7213i2s_playback[ch].period_bytes_max * 2)
+		return;
+
+	if (!substream) {
+		pr_debug("%s:!substream NULL\n", __func__);
+		return;
+	}
+	if (!substream->runtime) {
+		pr_debug("%s:!substream->runtime NULL\n", __func__);
+		return;
+	}
+	if (!substream->runtime->private_data) {
+		pr_debug("%s:!substream->runtime->private_data NULL\n",
+			__func__);
+		return;
+	}
+	switch (status) {
+	case IOH_EOK:
+		if (!rtd->stop) {
+			i2s_write_period(substream);
+			rtd->buf_pos = (rtd->buf_pos + 1) %
+					substream->runtime->periods;
+			snd_pcm_period_elapsed(rtd->substream);
+		}
+		rtd->cnt++;
+		break;
+	case IOH_EDONE:
+		pr_debug("Done stopping channel %d\n", rtd->stop);
+		rtd->stop = 2;
+		break;
+	default:
+		pr_debug("%s:default(%d)\n", __func__, status);
+	break;
+	}
+}
+
+static int setup_i2s_write(struct snd_pcm_substream *substream)
+{
+	int ret = 0;
+	struct ml7213i2s_runtime_data *rtd;
+	int ch = substream->number;
+
+	rtd = substream->runtime->private_data;
+
+	if (!ioh_i2s_open(ch, IOH_PLAYBACK, "radio-i2s-out")) {
+		pr_err("%s: Cannot open the device\n", __func__);
+		return -1;
+	}
+
+	if (ignore_overrun)
+		ioh_i2s_ignore_rx_overrun();
+
+	ioh_i2s_configure_i2s_regs(ch, IOH_PLAYBACK);
+	return ret;
+}
+
+static void __snd_card_ml7213i2s_runtime_free(struct snd_pcm_runtime *runtime)
+{
+	struct ml7213i2s_runtime_data *rtd;
+	static int cnt;
+
+	rtd = (struct ml7213i2s_runtime_data *)runtime->private_data;
+	if (!rtd->stop)
+		rtd->stop = 1;
+	else {
+		while (rtd->stop != 2) {
+			if (cnt++ > 100) {
+				pr_debug("oops, failed to close ml7213i2s..\n");
+				pr_debug("it's ok if i2s isn't running\n");
+				break;
+			}
+			msleep(20);
+		}
+	}
+}
+
+static void snd_card_ml7213i2s_runtime_capture_free
+					(struct snd_pcm_runtime *runtime)
+{
+	__snd_card_ml7213i2s_runtime_free(runtime);
+	kfree(runtime->private_data);
+}
+
+static struct ml7213i2s_runtime_data *
+ml7213i2s_new_pcm_stream(struct snd_pcm_substream *substream)
+{
+	struct ml7213i2s_runtime_data *rtd;
+
+	rtd = kzalloc(sizeof(*rtd), GFP_KERNEL);
+	if (!rtd)
+		return NULL;
+	spin_lock_init(&rtd->lock);
+	rtd->substream = substream;
+	return rtd;
+}
+
+static int snd_card_ml7213i2s_open(struct snd_pcm_substream *substream)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct ml7213i2s_runtime_data *rtd;
+	int err;
+
+	rtd = ml7213i2s_new_pcm_stream(substream);
+	if (!rtd)
+		return -ENOMEM;
+
+	runtime->private_data = rtd;
+	/* makes the infrastructure responsible for freeing dma */
+	runtime->private_free = snd_card_ml7213i2s_runtime_capture_free;
+
+	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
+		rtd->rw = SND_CAPTURE_SUBSTREAM;
+	else
+		rtd->rw = SND_PLAYBACK_SUBSTREAM;
+
+	snd_soc_set_runtime_hwparams(substream,
+				&snd_card_ml7213i2s_capture[substream->number]);
+
+	err = add_capture_constraints(runtime);
+	if (err < 0)
+		return err;
+
+	return 0;
+}
+
+static int snd_card_ml7213i2s_close(struct snd_pcm_substream *substream)
+{
+	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
+		ioh_i2s_release(substream->number, IOH_CAPTURE);
+	else
+		ioh_i2s_release(substream->number, IOH_PLAYBACK);
+	return 0;
+}
+
+static int snd_card_ml7213i2s_hw_params(struct snd_pcm_substream *substream,
+				    struct snd_pcm_hw_params *hw_params)
+{
+	return snd_pcm_lib_malloc_pages(substream,
+					params_buffer_bytes(hw_params));
+}
+
+static int snd_card_ml7213i2s_hw_free(struct snd_pcm_substream *substream)
+{
+	return snd_pcm_lib_free_pages(substream);
+}
+
+void ioh_i2s_irq_stop(int ch, enum ioh_direction dir)
+{
+	struct ioh_i2s_dma *dma = &dmadata[ch];
+
+	if (dir) {
+		ioh_i2s_disable_interrupts(ch, IOH_CAPTURE);
+		ioh_i2s_disable_rx_full_ir(ch);
+		ioh_i2s_clear_rx_sts_ir(ch);
+		dma->rxexe_flag = 0;
+
+	} else {
+		ioh_i2s_disable_interrupts(ch, IOH_PLAYBACK);
+		ioh_i2s_disable_tx_empty_ir(ch);
+		ioh_i2s_clear_tx_sts_ir(ch);
+		dma->txexe_flag = 0;
+	}
+}
+
+void ioh_i2s_write_start(void *callback_data,
+			 void (*done) (void *callback_data, int status,
+				       int num, int avail))
+{
+	struct snd_pcm_substream *substream = callback_data;
+	int ch = substream->number;
+	struct ioh_i2s_dma *dma = &dmadata[ch];
+
+	dma->txexe_flag = 1;
+	dma->tx_data_head = dma->tx_head;
+	dma->tx_complete = dma->tx_head;
+	dma->tx_avail = 0;
+	dma->tx_callback_data = callback_data;
+	dma->tx_done = done;
+
+	ioh_i2s_clear_tx_sts_ir(ch);
+	ioh_i2s_tx_clear_dma_mask(ch);
+	ioh_i2s_enable_interrupts(ch, IOH_PLAYBACK);
+}
+
+void ioh_i2s_read_start(void *callback_data,
+			void (*done) (void *callback_data, int status,
+				      int num, int avail))
+{
+	struct snd_pcm_substream *substream = callback_data;
+	int ch = substream->number;
+	struct ioh_i2s_dma *dma = &dmadata[ch];
+
+	dma->rxexe_flag = 1;
+	dma->rx_data_head = dma->rx_head;
+	dma->rx_complete = dma->rx_head;
+	dma->rx_avail = 0;
+	dma->rx_callback_data = callback_data;
+	dma->rx_done = done;
+
+	ioh_i2s_clear_rx_sts_ir(ch);
+	ioh_i2s_rx_clear_dma_mask(ch);
+	ioh_i2s_enable_interrupts(ch, IOH_CAPTURE);
+}
+
+static inline void
+snd_card_ml7213i2s_pcm_i2s_start(struct snd_pcm_substream *substream)
+{
+	struct ml7213i2s_runtime_data *rtd;
+	rtd = substream->runtime->private_data;
+
+	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
+		ioh_i2s_read_start(substream, read_done);
+	} else {
+		ioh_i2s_write_start(substream, write_done);
+		i2s_write_period(substream);
+	}
+}
+
+static int snd_card_ml7213i2s_pcm_trigger
+				(struct snd_pcm_substream *substream, int cmd)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct ml7213i2s_runtime_data *rtd = runtime->private_data;
+	int err = 0;
+
+	spin_lock(&rtd->lock);
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+	case SNDRV_PCM_TRIGGER_RESUME:
+		rtd->stop = 0;
+		snd_card_ml7213i2s_pcm_i2s_start(substream);
+		break;
+	case SNDRV_PCM_TRIGGER_STOP:
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+		pr_debug("stop..\n");
+		if (!rtd->stop)
+			rtd->stop = 1;
+		else
+			pr_debug("already stopped %d\n", rtd->stop);
+		if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
+			ioh_i2s_irq_stop(substream->number, IOH_CAPTURE);
+		else
+			ioh_i2s_irq_stop(substream->number, IOH_PLAYBACK);
+		break;
+	default:
+		err = -EINVAL;
+		break;
+	}
+
+	spin_unlock(&rtd->lock);
+	return 0;
+}
+
+static int snd_card_ml7213i2s_pcm_prepare(struct snd_pcm_substream *substream)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct ml7213i2s_runtime_data *rtd = runtime->private_data;
+
+	rtd->irq_pos = 0;
+	rtd->buf_pos = 0;
+
+	snd_pcm_format_set_silence(runtime->format, runtime->dma_area,
+				bytes_to_samples(runtime, runtime->dma_bytes));
+
+	return 0;
+}
+
+static snd_pcm_uframes_t
+snd_card_ml7213i2s_pcm_pointer(struct snd_pcm_substream *substream)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct ml7213i2s_runtime_data *rtd = runtime->private_data;
+
+	return substream->runtime->period_size*rtd->buf_pos;
+}
+
+static struct snd_pcm_ops snd_card_ml7213i2s_capture_ops = {
+	.open =			snd_card_ml7213i2s_open,
+	.close =		snd_card_ml7213i2s_close,
+	.ioctl =		snd_pcm_lib_ioctl,
+	.hw_params =		snd_card_ml7213i2s_hw_params,
+	.hw_free =		snd_card_ml7213i2s_hw_free,
+	.prepare =		snd_card_ml7213i2s_pcm_prepare,
+	.trigger =		snd_card_ml7213i2s_pcm_trigger,
+	.pointer =		snd_card_ml7213i2s_pcm_pointer,
+};
+
+static int ml7213ioh_pcm_new(struct snd_soc_pcm_runtime *rtd)
+{
+	struct snd_pcm *pcm = rtd->pcm;
+
+	snd_pcm_lib_preallocate_pages_for_all(
+		pcm, SNDRV_DMA_TYPE_CONTINUOUS,
+		snd_dma_continuous_data(GFP_KERNEL),
+		0, 64 * 1024);
+
+	return 0;
+}
+
+static struct snd_soc_platform_driver ml7213ioh_soc_platform = {
+	.pcm_new	= ml7213ioh_pcm_new,
+	.ops		= &snd_card_ml7213i2s_capture_ops,
+};
+
+/*****************************************************************************
+ *	DAI functions
+ *****************************************************************************/
+static int ml7213i2s_dai_hw_params(struct snd_pcm_substream *substream,
+			    struct snd_pcm_hw_params *hw_params,
+			    struct snd_soc_dai *dai)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct ml7213i2s_runtime_data *rtd = runtime->private_data;
+	int ch = substream->number;
+	int byte;
+
+	switch (params_format(hw_params)) {
+	case SNDRV_PCM_FORMAT_U8:
+		byte = 8;
+	case SNDRV_PCM_FORMAT_S16_LE:
+		byte = 16;
+		break;
+	case SNDRV_PCM_FORMAT_S32_LE:
+		byte = 24;
+		break;
+	default:
+		pr_err("%s: Failed not support format\n", __func__);
+		return -1;
+		break;
+	}
+
+	switch (rtd->rw) {
+	case SND_CAPTURE_SUBSTREAM:
+		dmadata[ch].dma_rx_unit = byte;
+		if (setup_i2s_read(substream))
+			return -1;
+		break;
+	case SND_PLAYBACK_SUBSTREAM:
+		dmadata[ch].dma_rx_unit = byte;
+		if (setup_i2s_write(substream))
+			return -1;
+		break;
+	default:
+		return -1;
+	}
+
+	return 0;
+}
+
+static int ml7213i2s_dai_hw_free(struct snd_pcm_substream *substream,
+				 struct snd_soc_dai *dai)
+{
+	struct ml7213i2s_runtime_data *rtd = substream->runtime->private_data;
+	unsigned int ch = substream->number;
+
+	switch (rtd->rw) {
+	case SND_CAPTURE_SUBSTREAM:
+		ioh_i2s_stop_i2s_regs(ch, IOH_CAPTURE);
+		break;
+
+	case SND_PLAYBACK_SUBSTREAM:
+		ioh_i2s_stop_i2s_regs(ch, IOH_PLAYBACK);
+		break;
+
+	default:
+		return -1;
+	}
+
+	return 0;
+}
+
+static int ml7213i2s_dai_set_dai_fmt(struct snd_soc_dai *dai,
+				     unsigned int fmt)
+{
+	struct ml7213i2s_dai *i2s = snd_soc_dai_get_drvdata(dai);
+	u32 cmn_reg[MAX_I2S_CH];
+	u32 tx_reg[MAX_I2S_CH];
+	u32 rx_reg[MAX_I2S_CH];
+	int i;
+	int offset = 0;
+	void *iobase = i2s->iobase;
+
+	/* set master/slave audio interface */
+	for (i = 0; i < MAX_I2S_CH; i++, offset = i * 0x800) {
+		cmn_reg[i] = ioread32(iobase + I2SCLKCNT0_OFFSET + 0x10 * i);
+		tx_reg[i] = ioread32(iobase + offset + I2SCNTTX_OFFSET);
+		rx_reg[i] = ioread32(iobase + offset + I2SCNTRX_OFFSET);
+
+		switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
+		case SND_SOC_DAIFMT_CBM_CFM:
+			cmn_reg[i] |= I2SCLKCNT_MSSEL;
+			break;
+		case SND_SOC_DAIFMT_CBS_CFS:
+			cmn_reg[i] &= ~I2SCLKCNT_MSSEL;
+			break;
+		default:
+			return -EINVAL;
+		}
+
+		/* interface format */
+		switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+		case SND_SOC_DAIFMT_I2S:
+			cmn_reg[i] &= ~ML7213I2S_LRCLK_FMT;
+			tx_reg[i] &= ~ML7213I2S_TX_I2S | ~ML7213I2S_TX_DLY |\
+				  ~ML7213I2S_TX_MSB_LSB |\
+				  ~ML7213I2S_TX_LR_POL | ~ML7213I2S_TX_AFT;
+			rx_reg[i] &= ~ML7213I2S_RX_I2S | ~ML7213I2S_RX_DLY |\
+				  ~ML7213I2S_RX_MSB_LSB |\
+				  ~ML7213I2S_RX_LR_POL | ~ML7213I2S_RX_AFT;
+			break;
+		default:
+			return -EINVAL;
+		}
+
+		/* clock inversion */
+		switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
+		case SND_SOC_DAIFMT_NB_NF:
+			cmn_reg[i] &= ~ML7213I2S_BCLKPOL;
+			break;
+		default:
+			return -EINVAL;
+		}
+
+		iowrite32(cmn_reg[i], iobase + I2SCLKCNT0_OFFSET + 0x10 * i);
+		iowrite32(tx_reg[i], iobase + offset + I2SCNTTX_OFFSET);
+		iowrite32(rx_reg[i], iobase + offset + I2SCNTRX_OFFSET);
+	}
+
+	return 0;
+}
+
+static int ml7213i2s_dai_set_dai_sysclk(struct snd_soc_dai *dai,
+					int clk_id, unsigned int freq, int dir)
+{
+	u32 reg[MAX_I2S_CH];
+	struct ml7213i2s_dai *i2s = snd_soc_dai_get_drvdata(dai);
+	void *iobase = i2s->iobase;
+	int i;
+
+	if (i2s->freq != freq)
+		i2s->freq = freq;
+
+	for (i = 0; i < MAX_I2S_CH; i++) {
+		reg[i] = ioread32(iobase + I2SCLKCNT0_OFFSET + 0x10 * i);
+		if (clk_id == IOH_MASTERCLKSEL_MCLK)
+			reg[i] &= ~ML7213I2S_MASTER_CLK_SEL;
+		else if (clk_id == IOH_MASTERCLKSEL_MLBCLK)
+			reg[i] |= ML7213I2S_MASTER_CLK_SEL;
+		iowrite32(reg[i], iobase + I2SCLKCNT0_OFFSET + 0x10 * i);
+	}
+
+	return 0;
+}
+
+static int ml7213i2s_dai_set_clkdiv(struct snd_soc_dai *dai,
+				  int div_id, int div)
+{
+	struct ml7213i2s_dai *i2s = snd_soc_dai_get_drvdata(dai);
+	u32 bclkfs = 0;
+	u32 mclkfs = 0;
+	int ch;
+	void *iobase = i2s->iobase;
+	u32 i2sclkcnt;
+
+	switch (div_id) {
+	case ML7213IOH_BCLKFS0:
+	case ML7213IOH_MCLKFS0:
+		ch = 0;
+		break;
+	case ML7213IOH_BCLKFS1:
+	case ML7213IOH_MCLKFS1:
+		ch = 1;
+		break;
+	case ML7213IOH_BCLKFS2:
+	case ML7213IOH_MCLKFS2:
+		ch = 2;
+		break;
+	case ML7213IOH_BCLKFS3:
+	case ML7213IOH_MCLKFS3:
+		ch = 3;
+		break;
+	case ML7213IOH_BCLKFS4:
+	case ML7213IOH_MCLKFS4:
+		ch = 4;
+		break;
+	case ML7213IOH_BCLKFS5:
+	case ML7213IOH_MCLKFS5:
+		ch = 5;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	switch (div_id) {
+	case ML7213IOH_BCLKFS0:
+	case ML7213IOH_BCLKFS1:
+	case ML7213IOH_BCLKFS2:
+	case ML7213IOH_BCLKFS3:
+	case ML7213IOH_BCLKFS4:
+	case ML7213IOH_BCLKFS5:
+		if (div == 8)
+			bclkfs = IOH_BCLKFS_8FS;
+		else if (div == 16)
+			bclkfs = IOH_BCLKFS_16FS;
+		else if (div == 32)
+			bclkfs = IOH_BCLKFS_32FS;
+		else if (div == 64)
+			bclkfs = IOH_BCLKFS_64FS;
+		break;
+	case ML7213IOH_MCLKFS0:
+	case ML7213IOH_MCLKFS1:
+	case ML7213IOH_MCLKFS2:
+	case ML7213IOH_MCLKFS3:
+	case ML7213IOH_MCLKFS4:
+	case ML7213IOH_MCLKFS5:
+		if (div == 64)
+			mclkfs = IOH_MCLKFS_64FS;
+		else if (div == 128)
+			mclkfs = IOH_MCLKFS_128FS;
+		else if (div == 192)
+			mclkfs = IOH_MCLKFS_192FS;
+		else if (div == 256)
+			mclkfs = IOH_MCLKFS_256FS;
+		else if (div == 384)
+			mclkfs = IOH_MCLKFS_384FS;
+		else if (div == 512)
+			mclkfs = IOH_MCLKFS_512FS;
+		else if (div == 768)
+			mclkfs = IOH_MCLKFS_768FS;
+		else if (div == 1024)
+			mclkfs = IOH_MCLKFS_1024FS;
+		break;
+	default:
+		return -EINVAL;
+	}
+	i2sclkcnt = ioread32(i2s_data->iobase + I2SCLKCNT0_OFFSET + 0x10 * ch);
+	i2sclkcnt |= bclkfs << I2SCLKCNT_BCLKFS_OFFSET;
+	i2sclkcnt |= mclkfs << I2SCLKCNT_MCLKFS_OFFSET;
+	iowrite32(i2sclkcnt, iobase + I2SCLKCNT0_OFFSET + 0x10 * ch);
+
+	return 0;
+}
+
+static const struct snd_soc_dai_ops ml7213i2s_dai_ops = {
+	.hw_params	= ml7213i2s_dai_hw_params,
+	.hw_free	= ml7213i2s_dai_hw_free,
+	.set_fmt	= ml7213i2s_dai_set_dai_fmt,
+	.set_sysclk	= ml7213i2s_dai_set_dai_sysclk,
+	.set_clkdiv	= ml7213i2s_dai_set_clkdiv,
+};
+
+static struct snd_soc_dai_driver ml7213i2s_dai_data = {
+	.playback = {
+		.channels_min = USE_CHANNELS_MIN,
+		.channels_max = USE_CHANNELS_MAX,
+		.rates = ML7213_I2S_RATES,
+		.formats = SNDRV_PCM_FMTBIT_U8 | SNDRV_PCM_FMTBIT_S16_LE |\
+			   SNDRV_PCM_FMTBIT_S32_LE,
+	},
+	.capture = {
+		.channels_min = USE_CHANNELS_MIN,
+		.channels_max = USE_CHANNELS_MAX,
+		.rates = ML7213_I2S_RATES,
+		.formats = SNDRV_PCM_FMTBIT_U8 | SNDRV_PCM_FMTBIT_S16_LE |\
+			   SNDRV_PCM_FMTBIT_S32_LE,
+	},
+	.ops = &ml7213i2s_dai_ops,
+};
+
+/*****************************************************************************
+ *	PCI functions
+ *****************************************************************************/
+DEFINE_PCI_DEVICE_TABLE(ioh_pci_tbl) = {
+	{
+		.vendor = PCI_VENDOR_ID_ROHM,
+		.device = PCI_DEVICE_ID_ML7213_I2S,
+		.subvendor = PCI_ANY_ID,
+		.subdevice = PCI_ANY_ID,
+	},
+	{0,}
+};
+
+static int ioh_i2s_pci_probe(struct pci_dev *pdev,
+			     const struct pci_device_id *id)
+{
+	int rv = 0;
+	void __iomem *tbl;
+	unsigned int mapbase;
+	struct ml7213i2s_dai *i2s_dai;
+
+	rv = pci_enable_device(pdev);
+	if (rv)
+		goto enable_device;
+
+	tbl = pci_iomap(pdev, 1, 0);
+	if (!tbl) {
+		rv = -ENOMEM;
+		printk(KERN_ERR "pci_iomap failed\n");
+		goto out_ipmap;
+	}
+
+	mapbase = pci_resource_start(pdev, 1);
+	if (!mapbase) {
+		rv = -ENOMEM;
+		printk(KERN_ERR "pci_resource_start failed\n");
+		goto out_pci_resource;
+	}
+
+	i2s_data = devm_kzalloc(&pdev->dev, sizeof(*i2s_data), GFP_KERNEL);
+	if (!i2s_data) {
+		dev_err(&pdev->dev, "Can't allocate i2s_data\n");
+		rv = -ENOMEM;
+		goto out_kzalloc_data;
+	}
+
+	i2s_data->dev = &pdev->dev;
+	i2s_data->iobase = tbl;
+	i2s_data->mapbase = mapbase;
+	spin_lock_init(&i2s_data->tx_lock);
+
+	rv = request_irq(pdev->irq, ioh_i2s_irq, IRQF_SHARED, "ml7213_ioh",
+			 pdev);
+	if (rv != 0) {
+		printk(KERN_ERR "Failed to allocate irq\n");
+		goto out_irq;
+	}
+
+	i2s_dai = devm_kzalloc(&pdev->dev, sizeof(*i2s_dai), GFP_KERNEL);
+	if (!i2s_dai) {
+		dev_err(&pdev->dev, "Can't allocate i2s_dai\n");
+		rv = -ENOMEM;
+		goto out_kzalloc_i2s_dai;
+	}
+	dev_set_drvdata(&pdev->dev, i2s_dai);
+
+	rv = snd_soc_register_platform(&pdev->dev, &ml7213ioh_soc_platform);
+	if (rv < 0) {
+		printk(KERN_ERR "Failed to snd_soc_register_platform\n");
+		goto out_register_plat;
+	}
+
+	i2s_dai->iobase = tbl;
+	i2s_dai->dev = &pdev->dev;
+	rv = snd_soc_register_dai(&pdev->dev, &ml7213i2s_dai_data);
+	if (rv < 0) {
+		printk(KERN_ERR "Failed to snd_soc_register_dai\n");
+		goto out_register_dai;
+	}
+
+	return 0;
+
+out_register_dai:
+	snd_soc_unregister_platform(&pdev->dev);
+out_register_plat:
+out_kzalloc_i2s_dai:
+	free_irq(pdev->irq, pdev);
+out_irq:
+out_kzalloc_data:
+out_pci_resource:
+	pci_iounmap(pdev, i2s_data->iobase);
+out_ipmap:
+	pci_disable_device(pdev);
+enable_device:
+
+	return rv;
+}
+
+static void ioh_i2s_pci_remove(struct pci_dev *pdev)
+{
+	int i;
+
+	for (i = 0; i < MAX_I2S_CH; i++)
+		ioh_i2s_reset(i);
+
+	snd_soc_unregister_platform(&pdev->dev);
+	free_irq(pdev->irq, pdev);
+	pci_iounmap(pdev, i2s_data->iobase);
+	pci_disable_device(pdev);
+}
+
+static void ioh_i2s_save_reg_conf(struct pci_dev *pdev)
+{
+	int i;
+	void *iobase;
+	struct ioh_i2s_pm_ch_reg *save;
+	struct ioh_i2s_pm_ch_reg_cmn *save_cmn;
+	int offset;
+
+	iobase = i2s_data->iobase;
+	for (i = 0, offset = 0; i < MAX_I2S_CH; i++, offset = i * 0x800) {
+		save = &i2s_data->ch_reg_save[i];
+
+		save->i2sdrtx = ioread32(iobase + offset + I2SDRTX_OFFSET);
+		save->i2scnttx = ioread32(iobase + offset + I2SCNTTX_OFFSET);
+		save->i2sfifoctx =
+				ioread32(iobase + offset + I2SFIFOCTX_OFFSET);
+		save->i2saftx = ioread32(iobase + offset + I2SAFTX_OFFSET);
+		save->i2saetx = ioread32(iobase + offset + I2SAETX_OFFSET);
+		save->i2smsktx = ioread32(iobase + offset + I2SMSKTX_OFFSET);
+		save->i2sisttx = ioread32(iobase + offset + I2SISTTX_OFFSET);
+
+		save->i2scntrx = ioread32(iobase + offset + I2SCNTRX_OFFSET);
+		save->i2sfifocrx =
+				ioread32(iobase + offset + I2SFIFOCRX_OFFSET);
+		save->i2safrx = ioread32(iobase + offset + I2SAFRX_OFFSET);
+		save->i2saerx = ioread32(iobase + offset + I2SAERX_OFFSET);
+		save->i2smskrx = ioread32(iobase + offset + I2SMSKRX_OFFSET);
+		save->i2sistrx = ioread32(iobase + offset + I2SISTRX_OFFSET);
+	}
+
+	save_cmn = &i2s_data->cmn_reg_save;
+	for (i = 0; i < MAX_I2S_CH; i++) {
+		save_cmn->i2sclkcnt[i] =
+		ioread32(i2s_data->iobase + I2SCLKCNT0_OFFSET + 0x10 * i);
+	}
+	save_cmn->i2simask = ioread32(i2s_data->iobase + I2SIMASK_OFFSET);
+}
+
+static int ioh_i2s_pci_suspend(struct pci_dev *pdev, pm_message_t state)
+{
+	int ret;
+
+	ioh_i2s_save_reg_conf(pdev);
+	ret = pci_save_state(pdev);
+	if (ret) {
+		dev_err(&pdev->dev,
+			" %s -pci_save_state returns %d\n", __func__, ret);
+		return ret;
+	}
+	pci_enable_wake(pdev, PCI_D3hot, 0);
+	pci_disable_device(pdev);
+	pci_set_power_state(pdev, pci_choose_state(pdev, state));
+
+	return 0;
+}
+
+static void ioh_i2s_restore_reg_conf(struct pci_dev *pdev)
+{
+	int i;
+	void *iobase;
+	struct ioh_i2s_pm_ch_reg *save;
+	int offset;
+
+	iobase = i2s_data->iobase;
+	save = &i2s_data->ch_reg_save[0];
+	for (i = 0, offset = 0; i < MAX_I2S_CH; i++, offset = i * 0x800) {
+		iowrite32(save->i2sdrtx, iobase + offset + I2SDRTX_OFFSET);
+		iowrite32(save->i2scnttx, iobase + offset + I2SCNTTX_OFFSET);
+		iowrite32(save->i2sfifoctx,
+			  iobase + offset + I2SFIFOCTX_OFFSET);
+		iowrite32(save->i2saftx, iobase + offset + I2SAFTX_OFFSET);
+		iowrite32(save->i2saetx, iobase + offset + I2SAETX_OFFSET);
+		iowrite32(save->i2smsktx, iobase + offset + I2SMSKTX_OFFSET);
+		iowrite32(save->i2sisttx, iobase + offset + I2SISTTX_OFFSET);
+
+		iowrite32(save->i2scntrx, iobase + offset + I2SCNTRX_OFFSET);
+		iowrite32(save->i2sfifocrx,
+			  iobase + offset + I2SFIFOCRX_OFFSET);
+		iowrite32(save->i2safrx, iobase + offset + I2SAFRX_OFFSET);
+		iowrite32(save->i2saerx, iobase + offset + I2SAERX_OFFSET);
+		iowrite32(save->i2smskrx, iobase + offset + I2SMSKRX_OFFSET);
+		iowrite32(save->i2sistrx, iobase + offset + I2SISTRX_OFFSET);
+	}
+
+	for (i = 0; i < MAX_I2S_CH; i++) {
+		iowrite32(i2s_data->cmn_reg_save.i2sclkcnt[i],
+			 i2s_data->iobase + I2SCLKCNT0_OFFSET + 0x10 * i);
+	}
+
+	iowrite32(i2s_data->cmn_reg_save.i2simask,
+		  i2s_data->iobase + I2SIMASK_OFFSET);
+}
+
+static int ioh_i2s_pci_resume(struct pci_dev *pdev)
+{
+	int ret;
+
+	pci_set_power_state(pdev, PCI_D0);
+	pci_restore_state(pdev);
+	ret = pci_enable_device(pdev);
+	if (ret) {
+		dev_err(&pdev->dev,
+		"%s-pci_enable_device failed(ret=%d) ", __func__, ret);
+		return ret;
+	}
+
+	pci_enable_wake(pdev, PCI_D3hot, 0);
+	ioh_i2s_restore_reg_conf(pdev);
+
+	return 0;
+}
+
+static struct pci_driver ioh_i2s_driver = {
+	.name = DRV_NAME,
+	.probe = ioh_i2s_pci_probe,
+	.remove = __devexit_p(ioh_i2s_pci_remove),
+	.id_table = ioh_pci_tbl,
+#ifdef CONFIG_PM
+	.suspend = ioh_i2s_pci_suspend,
+	.resume = ioh_i2s_pci_resume,
+#endif
+};
+
+static int __init ioh_i2s_init(void)
+{
+	return pci_register_driver(&ioh_i2s_driver);
+}
+
+static void __exit ioh_i2s_cleanup(void)
+{
+	pci_unregister_driver(&ioh_i2s_driver);
+}
+
+module_init(ioh_i2s_init);
+module_exit(ioh_i2s_cleanup);
+
+MODULE_AUTHOR("Tomoya MORINAGA <tomoya.rohm@gmail.com>");
+MODULE_DESCRIPTION("LAPIS Semiconductor ML7213 IOH ALSA SoC platform driver");
+MODULE_LICENSE("GPL");
diff --git a/sound/soc/lapis/ml7213ioh-plat.h b/sound/soc/lapis/ml7213ioh-plat.h
new file mode 100644
index 0000000..614f762
--- /dev/null
+++ b/sound/soc/lapis/ml7213ioh-plat.h
@@ -0,0 +1,583 @@
+/*
+ * Copyright (C) 2011 LAPIS Semiconductor Co., Ltd.
+ *
+ * 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; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307, USA.
+ */
+
+#ifndef ML7213IOH_PLAT_H
+#define ML7213IOH_PLAT_H
+
+#include <linux/interrupt.h>
+#include <linux/pch_dma.h>
+
+#ifndef add_capture_constraints
+#define add_capture_constraints(x) 0
+#endif
+
+#define	SND_CAPTURE_SUBSTREAM	0
+#define	SND_PLAYBACK_SUBSTREAM	1
+
+#define I2SCLKCNT_MSSEL		BIT(0)
+#define I2SCLKCNT_BCLKPOL	BIT(1)
+
+#define DRV_NAME "ml7213_ioh_i2s"
+#define PCI_VENDOR_ID_ROHM	0X10DB
+#define PCI_DEVICE_ID_ML7213_I2S	0X8033
+
+#define ML7213I2S_BCLKPOL	BIT(1)
+#define ML7213I2S_LRCLK_FMT	(BIT(4) | BIT(5))
+#define ML7213I2S_TX_I2S	BIT(0)
+#define ML7213I2S_TX_DLY	BIT(12)
+#define ML7213I2S_TX_MSB_LSB	BIT(13)
+#define ML7213I2S_TX_LR_POL	BIT(14)
+#define ML7213I2S_TX_AFT	BIT(15)
+#define ML7213I2S_RX_I2S	BIT(0)
+#define ML7213I2S_RX_DLY	BIT(12)
+#define ML7213I2S_RX_MSB_LSB	BIT(13)
+#define ML7213I2S_RX_LR_POL	BIT(14)
+#define ML7213I2S_RX_AFT	BIT(15)
+#define ML7213I2S_MASTER_CLK_SEL	BIT(2)
+
+#define ML7213_I2S_RATES \
+	(SNDRV_PCM_RATE_16000 | SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_48000)
+
+/* ioh_bclkfs_t */
+#define	IOH_BCLKFS_8FS		0
+#define	IOH_BCLKFS_16FS		1
+#define	IOH_BCLKFS_32FS		2
+#define	IOH_BCLKFS_64FS		3
+
+#define	I2SCLKCNT_MCLKFS_OFFSET		(8)
+#define	I2SCLKCNT_BCLKFS_OFFSET		(12)
+
+#define I2SCLKCNT0_OFFSET	0x3000
+#define I2SCLKCNT1_OFFSET	0x3010
+#define I2SCLKCNT2_OFFSET	0x3020
+#define I2SCLKCNT3_OFFSET	0x3030
+#define I2SCLKCNT4_OFFSET	0x3040
+#define I2SCLKCNT5_OFFSET	0x3050
+#define I2SISTATUS_OFFSET	0x3080
+#define I2SIDISP_OFFSET		0x3084
+#define I2SIMASK_OFFSET		0x3088
+#define I2SIMASKCLR_OFFSET	0x308C
+#define I2SSRST_OFFSET		0x3FFC
+#define I2SDRTX_OFFSET		0x0
+#define I2SCNTTX_OFFSET		0x4
+#define I2SFIFOCTX_OFFSET	0x8
+#define I2SAFTX_OFFSET		0xC
+#define I2SAETX_OFFSET		0x10
+#define I2SMSKTX_OFFSET		0x14
+#define I2SISTTX_OFFSET		0x18
+#define I2SMONTX_OFFSET		0x1C
+#define I2SDRRX_OFFSET		0x20
+#define I2SCNTRX_OFFSET		0x24
+#define I2SFIFOCRX_OFFSET	0x28
+#define I2SAFRX_OFFSET		0x2C
+#define I2SAERX_OFFSET		0x30
+#define I2SMSKRX_OFFSET		0x34
+#define I2SISTRX_OFFSET		0x38
+#define I2SMONRX_OFFSET		0x3C
+#define FIRST_TX_OFFSET		0x0
+#define FIRST_RX_OFFSET		0x0
+
+#define I2SDRTXMIRROR_OFFSET	0x100
+#define I2SDRRXMIRROR_OFFSET	0x400
+
+#define I2S_ALL_INTERRUPT_BITS	0x3F003F
+#define I2S_IDISP_BITS		0x3F003F
+#define I2S_IDISP_TX_BITS	0x00003F
+#define I2S_IDISP_RX_BITS	0x3F0000
+#define TX_BIT_FIMSK	0x1	/*Fifo full interrupt mask bit*/
+#define TX_BIT_AFIMSK	0x2	/*Fifo Almost full interrupt mask bit*/
+#define TX_BIT_EIMSK	0x4	/*Fifo empty interrupt mask bit*/
+#define TX_BIT_AEIMSK	0x8	/*Fifo Almost empty interrupt mask bit*/
+#define TX_BIT_DMAMSK	0x10	/*Masks DMA*/
+#define TX_BIT_DMATC	0x100
+#define I2S_TX_ALL_INTR_MASK_BITS (TX_BIT_FIMSK | TX_BIT_AFIMSK | TX_BIT_EIMSK \
+							| TX_BIT_AEIMSK)
+#define I2S_TX_NORMAL_INTR_MASK_BITS (TX_BIT_FIMSK | TX_BIT_AFIMSK)
+#define RX_BIT_FIMSK	0x1	/*Fifo full interrupt mask bit*/
+#define RX_BIT_AFIMSK	0x2	/*Fifo Almost full interrupt mask bit*/
+#define RX_BIT_EIMSK	0x4	/*Fifo empty interrupt mask bit*/
+#define RX_BIT_AEIMSK	0x8	/*Fifo Almost empty interrupt mask bit*/
+#define RX_BIT_DMAMSK	0x10	/*Masks DMA*/
+#define RX_BIT_DMATC	0x100
+#define I2S_RX_ALL_INTR_MASK_BITS (RX_BIT_FIMSK | RX_BIT_AFIMSK | RX_BIT_EIMSK \
+							| RX_BIT_AEIMSK)
+#define I2S_RX_NORMAL_INTR_MASK_BITS (RX_BIT_EIMSK | RX_BIT_AEIMSK)
+#define I2S_TX_FINT	0x1	/*Full Interrupt*/
+#define I2S_TX_AFINT	0x2	/*Almost full interrupt*/
+#define I2S_TX_EINT	0x4	/*Empty interrupt*/
+#define I2S_TX_AEINT	0x8	/*Almost empty interrupt*/
+#define I2S_RX_FINT	0x1	/*Full Interrupt*/
+#define I2S_RX_AFINT	0x2	/*Almost full interrupt*/
+#define I2S_RX_EINT	0x4	/*Empty interrupt*/
+#define I2S_RX_AEINT	0x8	/*Almost empty interrupt*/
+
+#define I2S_FIFO_TX_FCLR	BIT(0)
+#define I2S_FIFO_TX_RUN		BIT(4)
+#define I2S_FIFO_RX_FCLR	BIT(0)
+#define I2S_FIFO_RX_RUN		BIT(4)
+
+#define FIFO_CTRL_BIT_TX_RUN	0x10
+#define FIFO_CTRL_BIT_RX_RUN	0x10
+#define I2S_CNT_BIT_TEL		0x1
+#define I2S_IMASK_TX_BIT_START	0
+#define I2S_IMASK_RX_BIT_START	16
+
+
+/* DMA processing */
+#define	PERIOD_POS_MAX		I2S_DMA_SG_NUM
+#define	PERIOD_LEN_TX		(I2S_AEMPTY_THRESH * PERIOD_POS_MAX)
+#define	PERIOD_LEN_RX		(I2S_AFULL_THRESH * PERIOD_POS_MAX)
+
+#define	SUPPORT_FORMAT		(SNDRV_PCM_FMTBIT_U8 | \
+				 SNDRV_PCM_FMTBIT_S16_LE | \
+				 SNDRV_PCM_FMTBIT_S32_LE)
+#define	MAX_PERIOD_SIZE_TX	(PERIOD_LEN_TX * 4)
+#define	MAX_PERIOD_SIZE_RX	(PERIOD_LEN_RX * 4)
+
+#define USE_CHANNELS_MIN	1
+#define USE_CHANNELS_MAX	2
+#define	MAX_I2S_CH		6		/*I2S0 ~ I2S5*/
+#define USE_PERIODS_MIN		(I2S_DMA_SG_MAX)
+#define USE_PERIODS_MAX		(I2S_DMA_SG_MAX)
+
+#define I2S_AEMPTY_THRESH	64	/* Almost  Empty Threshold */
+#define I2S_AFULL_THRESH	64	/* Almost  Full Threshold */
+
+#define	INTER_BUFF_SIZE		(I2S_AEMPTY_THRESH * 4 \
+				 * I2S_DMA_SG_NUM \
+				 * I2S_DMA_SG_MAX)
+#define	I2S_DMA_SG_NUM		(128)
+#define	I2S_DMA_SG_MAX		(64)
+
+#define	IOH_MSSEL_MASTER	1
+
+enum ioh_direction {
+	IOH_PLAYBACK = 0,
+	IOH_CAPTURE,
+};
+
+enum ioh_i2s_fifo_type {
+	IOH_FIFO_32 = 4,
+	IOH_FIFO_16 = 2,
+	IOH_FIFO_8 = 1,
+};
+
+enum ioh_i2s_status {
+	IOH_EOK = 0,
+	IOH_EDONE = 1,
+	IOH_EUNDERRUN = 2,
+	IOH_EOVERRUN = 3,
+	IOH_EFRAMESYNC = 4,
+};
+
+struct ml7213i2s_runtime_data {
+	spinlock_t lock;
+	unsigned int irq_pos;
+	unsigned int buf_pos;
+	struct snd_pcm_substream *substream;
+	int stop;
+	int cnt;
+	unsigned int rw;
+};
+
+enum ioh_bclkpol_t {
+	ioh_BCLKPOL_FALLING = 0,
+	ioh_BCLKPOL_RISING,
+};
+
+enum ioh_masterclksel_t {
+	IOH_MASTERCLKSEL_MCLK = 0,
+	IOH_MASTERCLKSEL_MLBCLK,
+};
+
+enum ioh_lrckfmt_t {
+	IOH_LRCLKFMT_I2S = 1,
+	IOH_LRCLKFMT_LONGFRAME,
+	IOH_LRCLKFMT_SHORTFRAME,
+};
+
+enum ioh_mclkfs_t {
+	IOH_MCLKFS_64FS = 0,
+	IOH_MCLKFS_128FS,
+	IOH_MCLKFS_192FS,
+	IOH_MCLKFS_256FS,
+	IOH_MCLKFS_384FS,
+	IOH_MCLKFS_512FS,
+	IOH_MCLKFS_768FS,
+	IOH_MCLKFS_1024FS,
+};
+
+enum ioh_dlyoff_t {
+	IOH_DLYOFF_DLY_ON = 0,		/* date delat on */
+	IOH_DLYOFF_DLY_OFF,		/* date delat off */
+};
+
+
+
+enum ioh_lrpol_t {
+	IOH_LRPOL_NO_INVERT = 0,	/* Low of LRCLK is L data.
+					   High of LRCLK is R data. */
+	IOH_LRPOL_INVERT,		/* Low of LRCLK is R data.
+					   High of LRCLK is L data. */
+};
+
+enum ioh_aft_t {
+	IOH_AFR_FRONT = 0,
+	IOH_AFR_BACK,
+};
+
+struct ioh_i2s_pm_ch_reg {
+	u32 i2sdrtx;	/* Tx: data register */
+	u32 i2scnttx; /* Tx: control register */
+	u32 i2sfifoctx;	/* Tx: FIFO control register */
+	u32 i2saftx;	/* Tx: almost full threshold setting */
+	u32 i2saetx;	/* Tx: almost empty threshold setting */
+	u32 i2smsktx;	/* Tx: interrupt mask settings */
+	u32 i2sisttx;	/* Tx: for acknowledging interrupts */
+	u32 i2scntrx;	/* Rx: control register */
+	u32 i2sfifocrx; /* Rx: FIFO control register */
+	u32 i2safrx;	/* Rx: almost full threshold setting */
+	u32 i2saerx;	/* Rx: almost empty threshold setting */
+	u32 i2smskrx;	/* Rx: interrupt mask settings */
+	u32 i2sistrx;	/* Rx: for acknowledging interrupts */
+};
+
+struct ioh_i2s_pm_ch_reg_cmn {
+	u32 i2sclkcnt[MAX_I2S_CH];	/*clock control register(ch0~5) */
+	u32 i2simask;		/*interrupt mask */
+};
+
+struct ioh_i2s_data {
+	struct device *dev;
+	void *iobase;
+	unsigned int mapbase;
+	int ch;
+	int ignore_rx_overrun;
+	spinlock_t tx_lock;
+	struct ioh_i2s_pm_ch_reg_cmn cmn_reg_save;
+	struct ioh_i2s_pm_ch_reg ch_reg_save[MAX_I2S_CH];
+};
+
+struct ioh_i2s_dma {
+	atomic_t rx_busy;
+	atomic_t tx_busy;
+
+	/* Transmit side DMA */
+	atomic_t pending_tx;
+
+	struct ioh_dma_config *dma_config;
+
+	struct scatterlist	*sg_tx_p;
+	struct scatterlist	*sg_rx_p;
+
+	struct scatterlist	*sg_tx_cur; /* current head of tx sg */
+	struct scatterlist	*sg_rx_cur; /* current head of tx sg */
+
+	int tx_num;	/* The number of sent sg */
+	int rx_num;	/* The number of sent sg */
+
+	void *rxbuf_virt;
+	void *txbuf_virt;
+	unsigned char *tx_tail;
+	unsigned char *tx_head;
+	unsigned char *tx_data_head;
+	unsigned char *tx_complete;
+	unsigned int  tx_avail;
+	unsigned char *rx_tail;
+	unsigned char *rx_head;
+	unsigned char *rx_data_head;
+	unsigned char *rx_complete;
+	unsigned int rx_avail;
+
+	struct dma_chan			*chan_tx;
+	struct dma_chan			*chan_rx;
+
+	int rx_nent;	/* The number of rx scatter list */
+	int tx_nent;	/* The number of tx scatter list */
+
+	struct dma_async_tx_descriptor	*desc_tx;
+	struct dma_async_tx_descriptor	*desc_rx;
+
+	dma_addr_t			tx_buf_dma;
+	dma_addr_t			rx_buf_dma;
+
+	struct pch_dma_slave		param_tx;
+	struct pch_dma_slave		param_rx;
+
+	void *rx_callback_data;
+	void (*rx_done) (void *callback_data, int status, int num, int avail);
+	void *tx_callback_data;
+	void (*tx_done) (void *callback_data, int status, int num, int avail);
+
+	int dma_tx_unit; /* 1Byte of 2Byte or 4Byte */
+	int dma_rx_unit; /* 1Byte of 2Byte or 4Byte */
+	int dma_tx_width;
+	int dma_rx_width;
+
+	int txexe_flag;
+	int rxexe_flag;
+
+	struct tasklet_struct	tx_tasklet;
+	struct tasklet_struct	rx_tasklet;
+};
+
+struct ml7213i2s_dai {
+	struct snd_soc_dai_driver dai;
+	struct device	*dev;
+	void *iobase;
+	u32 freq;
+};
+
+struct ioh_i2s_config_common_reg {
+	u32 i2sclkcnt;	/*clock control register(ch0~5) */
+	u32 i2sistatus;	/*interrupt status */
+	u32 i2sidisp;		/*active interrupts */
+	u32 i2simask;		/*interrupt mask */
+	u32 i2simaskclr;	/*interrupt mask clear */
+};
+
+struct ioh_i2s_config_tx_reg {
+	u32 i2sdrtx;	/*data register */
+	u32 i2scnttx; /*control register */
+	u32 i2sfifoctx;	/*FIFO control register */
+	u32 i2saftx;	/*almost full threshold setting */
+	u32 i2saetx;	/*almost empty threshold setting */
+	u32 i2smsktx;	/*interrupt mask settings */
+	u32 i2sisttx;	/*for acknowledging interrupts */
+	u32 i2smontx;	/*monitor register */
+};
+
+struct ioh_i2s_config_rx_reg {
+	u32 i2sdrrx;	/* data register */
+	u32 i2scntrx;	/* control register */
+	u32 i2sfifocrx;/* FIFO control register */
+	u32 i2safrx;	/* almost full threshold setting */
+	u32 i2saerx;	/* almost empty threshold setting */
+	u32 i2smskrx;	/* interrupt mask settings */
+	u32 i2sistrx;	/* for acknowledging interrupts */
+	u32 i2smonrx;	/* monitor register */
+};
+
+struct ioh_i2s_config_reg {
+	/* The common register settings */
+	struct ioh_i2s_config_common_reg cmn;
+
+	/* TX channel settings */
+	struct ioh_i2s_config_tx_reg tx;
+
+	/* RX channel settings */
+	struct ioh_i2s_config_rx_reg rx;
+};
+
+static struct snd_pcm_hardware snd_card_ml7213i2s_capture[MAX_I2S_CH] = {
+	{
+		.info =			(SNDRV_PCM_INFO_MMAP |
+					 SNDRV_PCM_INFO_INTERLEAVED |
+					 SNDRV_PCM_INFO_RESUME |
+					 SNDRV_PCM_INFO_MMAP_VALID),
+		.formats =		SUPPORT_FORMAT,
+		.channels_min =		USE_CHANNELS_MIN,
+		.channels_max =		USE_CHANNELS_MAX,
+		.buffer_bytes_max =	(MAX_PERIOD_SIZE_RX *
+					 USE_PERIODS_MAX),
+		.period_bytes_min =	MAX_PERIOD_SIZE_RX,
+		.period_bytes_max =	MAX_PERIOD_SIZE_RX,
+		.periods_min =		USE_PERIODS_MIN,
+		.periods_max =		USE_PERIODS_MAX,
+		.fifo_size =		0,
+	},
+	{
+		.info =			(SNDRV_PCM_INFO_MMAP |
+					 SNDRV_PCM_INFO_INTERLEAVED |
+					 SNDRV_PCM_INFO_RESUME |
+					 SNDRV_PCM_INFO_MMAP_VALID),
+		.formats =		SUPPORT_FORMAT,
+		.channels_min =		USE_CHANNELS_MIN,
+		.channels_max =		USE_CHANNELS_MAX,
+		.buffer_bytes_max =	(MAX_PERIOD_SIZE_RX *
+					 USE_PERIODS_MAX),
+		.period_bytes_min =	MAX_PERIOD_SIZE_RX,
+		.period_bytes_max =	MAX_PERIOD_SIZE_RX,
+		.periods_min =		USE_PERIODS_MIN,
+		.periods_max =		USE_PERIODS_MAX,
+		.fifo_size =		0,
+	},
+	{
+		.info =			(SNDRV_PCM_INFO_MMAP |
+					 SNDRV_PCM_INFO_INTERLEAVED |
+					 SNDRV_PCM_INFO_RESUME |
+					 SNDRV_PCM_INFO_MMAP_VALID),
+		.formats =		SUPPORT_FORMAT,
+		.channels_min =		USE_CHANNELS_MIN,
+		.channels_max =		USE_CHANNELS_MAX,
+		.buffer_bytes_max =	(MAX_PERIOD_SIZE_RX *
+					 USE_PERIODS_MAX),
+		.period_bytes_min =	MAX_PERIOD_SIZE_RX,
+		.period_bytes_max =	MAX_PERIOD_SIZE_RX,
+		.periods_min =		USE_PERIODS_MIN,
+		.periods_max =		USE_PERIODS_MAX,
+		.fifo_size =		0,
+	},
+	{
+		.info =			(SNDRV_PCM_INFO_MMAP |
+					 SNDRV_PCM_INFO_INTERLEAVED |
+					 SNDRV_PCM_INFO_RESUME |
+					 SNDRV_PCM_INFO_MMAP_VALID),
+		.formats =		SUPPORT_FORMAT,
+		.channels_min =		USE_CHANNELS_MIN,
+		.channels_max =		USE_CHANNELS_MAX,
+		.buffer_bytes_max =	(MAX_PERIOD_SIZE_RX *
+					 USE_PERIODS_MAX),
+		.period_bytes_min =	MAX_PERIOD_SIZE_RX,
+		.period_bytes_max =	MAX_PERIOD_SIZE_RX,
+		.periods_min =		USE_PERIODS_MIN,
+		.periods_max =		USE_PERIODS_MAX,
+		.fifo_size =		0,
+	},
+	{
+		.info =			(SNDRV_PCM_INFO_MMAP |
+					 SNDRV_PCM_INFO_INTERLEAVED |
+					 SNDRV_PCM_INFO_RESUME |
+					 SNDRV_PCM_INFO_MMAP_VALID),
+		.formats =		SUPPORT_FORMAT,
+		.channels_min =		USE_CHANNELS_MIN,
+		.channels_max =		USE_CHANNELS_MAX,
+		.buffer_bytes_max =	(MAX_PERIOD_SIZE_RX *
+					 USE_PERIODS_MAX),
+		.period_bytes_min =	MAX_PERIOD_SIZE_RX,
+		.period_bytes_max =	MAX_PERIOD_SIZE_RX,
+		.periods_min =		USE_PERIODS_MIN,
+		.periods_max =		USE_PERIODS_MAX,
+		.fifo_size =		0,
+	},
+	{
+		.info =			(SNDRV_PCM_INFO_MMAP |
+					 SNDRV_PCM_INFO_INTERLEAVED |
+					 SNDRV_PCM_INFO_RESUME |
+					 SNDRV_PCM_INFO_MMAP_VALID),
+		.formats =		SUPPORT_FORMAT,
+		.channels_min =		USE_CHANNELS_MIN,
+		.channels_max =		USE_CHANNELS_MAX,
+		.buffer_bytes_max =	(MAX_PERIOD_SIZE_RX *
+					 USE_PERIODS_MAX),
+		.period_bytes_min =	MAX_PERIOD_SIZE_RX,
+		.period_bytes_max =	MAX_PERIOD_SIZE_RX,
+		.periods_min =		USE_PERIODS_MIN,
+		.periods_max =		USE_PERIODS_MAX,
+		.fifo_size =		0,
+	},
+};
+
+static struct snd_pcm_hardware snd_card_ml7213i2s_playback[MAX_I2S_CH] = {
+	{
+		.info =			(SNDRV_PCM_INFO_MMAP |
+					 SNDRV_PCM_INFO_INTERLEAVED |
+					 SNDRV_PCM_INFO_RESUME |
+					 SNDRV_PCM_INFO_MMAP_VALID),
+		.formats =		SUPPORT_FORMAT,
+		.channels_min =		USE_CHANNELS_MIN,
+		.channels_max =		USE_CHANNELS_MAX,
+		.buffer_bytes_max =	(MAX_PERIOD_SIZE_TX *
+					 USE_PERIODS_MAX),
+		.period_bytes_min =	MAX_PERIOD_SIZE_TX,
+		.period_bytes_max =	MAX_PERIOD_SIZE_TX,
+		.periods_min =		USE_PERIODS_MIN,
+		.periods_max =		USE_PERIODS_MAX,
+		.fifo_size =		0,
+	},
+	{
+		.info =			(SNDRV_PCM_INFO_MMAP |
+					 SNDRV_PCM_INFO_INTERLEAVED |
+					 SNDRV_PCM_INFO_RESUME |
+					 SNDRV_PCM_INFO_MMAP_VALID),
+		.formats =		SUPPORT_FORMAT,
+		.channels_min =		USE_CHANNELS_MIN,
+		.channels_max =		USE_CHANNELS_MAX,
+		.buffer_bytes_max =	(MAX_PERIOD_SIZE_TX *
+					 USE_PERIODS_MAX),
+		.period_bytes_min =	MAX_PERIOD_SIZE_TX,
+		.period_bytes_max =	MAX_PERIOD_SIZE_TX,
+		.periods_min =		USE_PERIODS_MIN,
+		.periods_max =		USE_PERIODS_MAX,
+		.fifo_size =		0,
+	},
+	{
+		.info =			(SNDRV_PCM_INFO_MMAP |
+					 SNDRV_PCM_INFO_INTERLEAVED |
+					 SNDRV_PCM_INFO_RESUME |
+					 SNDRV_PCM_INFO_MMAP_VALID),
+		.formats =		SUPPORT_FORMAT,
+		.channels_min =		USE_CHANNELS_MIN,
+		.channels_max =		USE_CHANNELS_MAX,
+		.buffer_bytes_max =	(MAX_PERIOD_SIZE_TX *
+					 USE_PERIODS_MAX),
+		.period_bytes_min =	MAX_PERIOD_SIZE_TX,
+		.period_bytes_max =	MAX_PERIOD_SIZE_TX,
+		.periods_min =		USE_PERIODS_MIN,
+		.periods_max =		USE_PERIODS_MAX,
+		.fifo_size =		0,
+	},
+	{
+		.info =			(SNDRV_PCM_INFO_MMAP |
+					 SNDRV_PCM_INFO_INTERLEAVED |
+					 SNDRV_PCM_INFO_RESUME |
+					 SNDRV_PCM_INFO_MMAP_VALID),
+		.formats =		SUPPORT_FORMAT,
+		.channels_min =		USE_CHANNELS_MIN,
+		.channels_max =		USE_CHANNELS_MAX,
+		.buffer_bytes_max =	(MAX_PERIOD_SIZE_TX *
+					 USE_PERIODS_MAX),
+		.period_bytes_min =	MAX_PERIOD_SIZE_TX,
+		.period_bytes_max =	MAX_PERIOD_SIZE_TX,
+		.periods_min =		USE_PERIODS_MIN,
+		.periods_max =		USE_PERIODS_MAX,
+		.fifo_size =		0,
+	},
+	{
+		.info =			(SNDRV_PCM_INFO_MMAP |
+					 SNDRV_PCM_INFO_INTERLEAVED |
+					 SNDRV_PCM_INFO_RESUME |
+					 SNDRV_PCM_INFO_MMAP_VALID),
+		.formats =		SUPPORT_FORMAT,
+		.channels_min =		USE_CHANNELS_MIN,
+		.channels_max =		USE_CHANNELS_MAX,
+		.buffer_bytes_max =	(MAX_PERIOD_SIZE_TX *
+					 USE_PERIODS_MAX),
+		.period_bytes_min =	MAX_PERIOD_SIZE_TX,
+		.period_bytes_max =	MAX_PERIOD_SIZE_TX,
+		.periods_min =		USE_PERIODS_MIN,
+		.periods_max =		USE_PERIODS_MAX,
+		.fifo_size =		0,
+	},
+	{
+		.info =			(SNDRV_PCM_INFO_MMAP |
+					 SNDRV_PCM_INFO_INTERLEAVED |
+					 SNDRV_PCM_INFO_RESUME |
+					 SNDRV_PCM_INFO_MMAP_VALID),
+		.formats =		SUPPORT_FORMAT,
+		.channels_min =		USE_CHANNELS_MIN,
+		.channels_max =		USE_CHANNELS_MAX,
+		.buffer_bytes_max =	(MAX_PERIOD_SIZE_TX *
+					 USE_PERIODS_MAX),
+		.period_bytes_min =	MAX_PERIOD_SIZE_TX,
+		.period_bytes_max =	MAX_PERIOD_SIZE_TX,
+		.periods_min =		USE_PERIODS_MIN,
+		.periods_max =		USE_PERIODS_MAX,
+		.fifo_size =		0,
+	},
+};
+#endif
-- 
1.7.4.4


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

* Re: [PATCH 1/3 v5] sound/soc/codecs: add LAPIS Semiconductor ML26124
  2011-12-20  2:45 [PATCH 1/3 v5] sound/soc/codecs: add LAPIS Semiconductor ML26124 Tomoya MORINAGA
@ 2011-12-20 10:40   ` Mark Brown
  2011-12-20  2:45 ` [PATCH 3/3 v2] sound/soc/lapis: add platform driver for ML7213 IOH I2S Tomoya MORINAGA
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 29+ messages in thread
From: Mark Brown @ 2011-12-20 10:40 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai, Lars-Peter Clausen,
	Dimitris Papastamos, Mike Frysinger, Daniel Mack, alsa-devel,
	linux-kernel, qi.wang, yong.y.wang, joel.clark, kok.howg.ewe

On Tue, Dec 20, 2011 at 11:45:42AM +0900, Tomoya MORINAGA wrote:

> +	switch (level) {
> +	case SND_SOC_BIAS_ON:
> +		/* VMID ON */
> +		snd_soc_update_bits(codec, ML26124_PW_REF_PW_MNG,
> +				    ML26124_VMID, ML26124_VMID);
> +		msleep(500);
> +	case SND_SOC_BIAS_PREPARE:

You're missing a break here.

> +static int ml26124_pcm_trigger(struct snd_pcm_substream *substream,
> +			      int cmd, struct snd_soc_dai *codec_dai)
> +{
> +	struct snd_soc_codec *codec = codec_dai->codec;
> +
> +	if (cmd == SNDRV_PCM_TRIGGER_STOP) {
> +		snd_soc_update_bits(codec, ML26124_REC_PLYBAK_RUN, 0x3, 0);
> +		return 0;
> +	} else if (cmd == SNDRV_PCM_TRIGGER_START) {
> +		if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)

Use switch statements, though frankly I'd be astonished if this code
actually runs as the trigger() callback is called in atomic context and
the register I/O functionality needs interrupts.  How have you tested
this code?

> +	.reg_cache_size = ML26134_CACHESIZE,
> +	.reg_word_size = sizeof(u8),
> +	.reg_cache_default = ml26124_reg,

New drivers really should use regmap rather than the ASoC cache code.

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

* Re: [PATCH 1/3 v5] sound/soc/codecs: add LAPIS Semiconductor ML26124
@ 2011-12-20 10:40   ` Mark Brown
  0 siblings, 0 replies; 29+ messages in thread
From: Mark Brown @ 2011-12-20 10:40 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: Dimitris Papastamos, alsa-devel, Lars-Peter Clausen,
	Mike Frysinger, qi.wang, Takashi Iwai, linux-kernel, yong.y.wang,
	kok.howg.ewe, Daniel Mack, Liam Girdwood, joel.clark

On Tue, Dec 20, 2011 at 11:45:42AM +0900, Tomoya MORINAGA wrote:

> +	switch (level) {
> +	case SND_SOC_BIAS_ON:
> +		/* VMID ON */
> +		snd_soc_update_bits(codec, ML26124_PW_REF_PW_MNG,
> +				    ML26124_VMID, ML26124_VMID);
> +		msleep(500);
> +	case SND_SOC_BIAS_PREPARE:

You're missing a break here.

> +static int ml26124_pcm_trigger(struct snd_pcm_substream *substream,
> +			      int cmd, struct snd_soc_dai *codec_dai)
> +{
> +	struct snd_soc_codec *codec = codec_dai->codec;
> +
> +	if (cmd == SNDRV_PCM_TRIGGER_STOP) {
> +		snd_soc_update_bits(codec, ML26124_REC_PLYBAK_RUN, 0x3, 0);
> +		return 0;
> +	} else if (cmd == SNDRV_PCM_TRIGGER_START) {
> +		if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)

Use switch statements, though frankly I'd be astonished if this code
actually runs as the trigger() callback is called in atomic context and
the register I/O functionality needs interrupts.  How have you tested
this code?

> +	.reg_cache_size = ML26134_CACHESIZE,
> +	.reg_word_size = sizeof(u8),
> +	.reg_cache_default = ml26124_reg,

New drivers really should use regmap rather than the ASoC cache code.

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

* Re: [PATCH 1/3 v5] sound/soc/codecs: add LAPIS Semiconductor ML26124
  2011-12-20  2:45 [PATCH 1/3 v5] sound/soc/codecs: add LAPIS Semiconductor ML26124 Tomoya MORINAGA
@ 2011-12-20 10:42   ` Mark Brown
  2011-12-20  2:45 ` [PATCH 3/3 v2] sound/soc/lapis: add platform driver for ML7213 IOH I2S Tomoya MORINAGA
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 29+ messages in thread
From: Mark Brown @ 2011-12-20 10:42 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai, Lars-Peter Clausen,
	Dimitris Papastamos, Mike Frysinger, Daniel Mack, alsa-devel,
	linux-kernel, qi.wang, yong.y.wang, joel.clark, kok.howg.ewe

On Tue, Dec 20, 2011 at 11:45:42AM +0900, Tomoya MORINAGA wrote:

> +	SND_SOC_DAPM_MICBIAS("MICBIAS", ML26124_PW_REF_PW_MNG, 0, 0),

You should use a supply widget for MICBIAS, the specific widget type is
legacy.

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

* Re: [PATCH 1/3 v5] sound/soc/codecs: add LAPIS Semiconductor ML26124
@ 2011-12-20 10:42   ` Mark Brown
  0 siblings, 0 replies; 29+ messages in thread
From: Mark Brown @ 2011-12-20 10:42 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: Dimitris Papastamos, alsa-devel, Lars-Peter Clausen,
	Mike Frysinger, qi.wang, Takashi Iwai, linux-kernel, yong.y.wang,
	kok.howg.ewe, Daniel Mack, Liam Girdwood, joel.clark

On Tue, Dec 20, 2011 at 11:45:42AM +0900, Tomoya MORINAGA wrote:

> +	SND_SOC_DAPM_MICBIAS("MICBIAS", ML26124_PW_REF_PW_MNG, 0, 0),

You should use a supply widget for MICBIAS, the specific widget type is
legacy.

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

* Re: [PATCH 2/3 v4] sound/soc/lapis: add machine driver for ML7213 Carrier Board
  2011-12-20  2:45 ` [PATCH 2/3 v4] sound/soc/lapis: add machine driver for ML7213 Carrier Board Tomoya MORINAGA
@ 2011-12-20 10:45     ` Mark Brown
  0 siblings, 0 replies; 29+ messages in thread
From: Mark Brown @ 2011-12-20 10:45 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai, Lars-Peter Clausen,
	Dimitris Papastamos, Mike Frysinger, Daniel Mack, alsa-devel,
	linux-kernel, qi.wang, yong.y.wang, joel.clark, kok.howg.ewe

On Tue, Dec 20, 2011 at 11:45:43AM +0900, Tomoya MORINAGA wrote:

> +{
> +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> +	struct snd_soc_dai *codec_dai = rtd->codec_dai;
> +	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
> +
> +	unsigned int clk = 0;
> +	int ret = 0;
> +	int bclkfs;
> +	int mclkfs;
> +	int rate = params_rate(hw_params);

The coding style here is really odd, looks like you're mixing
declarations and code.

> +	default:
> +		pr_err("%s: Failed not support format\n", __func__);
> +		return -1;
> +		break;

Return sensible error codes and there's no reason for the break after
the return.

> +	ret = snd_soc_dai_set_clkdiv(cpu_dai, ML7213IOH_BCLKFS0, bclkfs);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = snd_soc_dai_set_clkdiv(cpu_dai, ML7213IOH_BCLKFS1, bclkfs);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = snd_soc_dai_set_clkdiv(cpu_dai, ML7213IOH_BCLKFS2, bclkfs);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = snd_soc_dai_set_clkdiv(cpu_dai, ML7213IOH_BCLKFS3, bclkfs);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = snd_soc_dai_set_clkdiv(cpu_dai, ML7213IOH_BCLKFS4, bclkfs);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = snd_soc_dai_set_clkdiv(cpu_dai, ML7213IOH_BCLKFS5, bclkfs);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = snd_soc_dai_set_clkdiv(cpu_dai, ML7213IOH_MCLKFS0, mclkfs);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = snd_soc_dai_set_clkdiv(cpu_dai, ML7213IOH_MCLKFS1, mclkfs);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = snd_soc_dai_set_clkdiv(cpu_dai, ML7213IOH_MCLKFS2, mclkfs);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = snd_soc_dai_set_clkdiv(cpu_dai, ML7213IOH_MCLKFS3, mclkfs);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = snd_soc_dai_set_clkdiv(cpu_dai, ML7213IOH_MCLKFS4, mclkfs);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = snd_soc_dai_set_clkdiv(cpu_dai, ML7213IOH_MCLKFS5, mclkfs);
> +	if (ret < 0)
> +		return ret;

This looks like the CODEC driver ought to be working these things out
for itself rather than having every machine using the device know about
these dividers.

You're also still missing the drivers for the CPU...

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

* Re: [PATCH 2/3 v4] sound/soc/lapis: add machine driver for ML7213 Carrier Board
@ 2011-12-20 10:45     ` Mark Brown
  0 siblings, 0 replies; 29+ messages in thread
From: Mark Brown @ 2011-12-20 10:45 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: Dimitris Papastamos, alsa-devel, Lars-Peter Clausen,
	Mike Frysinger, qi.wang, Takashi Iwai, linux-kernel, yong.y.wang,
	kok.howg.ewe, Daniel Mack, Liam Girdwood, joel.clark

On Tue, Dec 20, 2011 at 11:45:43AM +0900, Tomoya MORINAGA wrote:

> +{
> +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> +	struct snd_soc_dai *codec_dai = rtd->codec_dai;
> +	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
> +
> +	unsigned int clk = 0;
> +	int ret = 0;
> +	int bclkfs;
> +	int mclkfs;
> +	int rate = params_rate(hw_params);

The coding style here is really odd, looks like you're mixing
declarations and code.

> +	default:
> +		pr_err("%s: Failed not support format\n", __func__);
> +		return -1;
> +		break;

Return sensible error codes and there's no reason for the break after
the return.

> +	ret = snd_soc_dai_set_clkdiv(cpu_dai, ML7213IOH_BCLKFS0, bclkfs);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = snd_soc_dai_set_clkdiv(cpu_dai, ML7213IOH_BCLKFS1, bclkfs);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = snd_soc_dai_set_clkdiv(cpu_dai, ML7213IOH_BCLKFS2, bclkfs);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = snd_soc_dai_set_clkdiv(cpu_dai, ML7213IOH_BCLKFS3, bclkfs);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = snd_soc_dai_set_clkdiv(cpu_dai, ML7213IOH_BCLKFS4, bclkfs);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = snd_soc_dai_set_clkdiv(cpu_dai, ML7213IOH_BCLKFS5, bclkfs);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = snd_soc_dai_set_clkdiv(cpu_dai, ML7213IOH_MCLKFS0, mclkfs);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = snd_soc_dai_set_clkdiv(cpu_dai, ML7213IOH_MCLKFS1, mclkfs);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = snd_soc_dai_set_clkdiv(cpu_dai, ML7213IOH_MCLKFS2, mclkfs);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = snd_soc_dai_set_clkdiv(cpu_dai, ML7213IOH_MCLKFS3, mclkfs);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = snd_soc_dai_set_clkdiv(cpu_dai, ML7213IOH_MCLKFS4, mclkfs);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = snd_soc_dai_set_clkdiv(cpu_dai, ML7213IOH_MCLKFS5, mclkfs);
> +	if (ret < 0)
> +		return ret;

This looks like the CODEC driver ought to be working these things out
for itself rather than having every machine using the device know about
these dividers.

You're also still missing the drivers for the CPU...

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

* Re: [PATCH 3/3 v2] sound/soc/lapis: add platform driver for ML7213 IOH I2S
  2011-12-20  2:45 ` [PATCH 3/3 v2] sound/soc/lapis: add platform driver for ML7213 IOH I2S Tomoya MORINAGA
@ 2011-12-20 13:23     ` Mark Brown
  0 siblings, 0 replies; 29+ messages in thread
From: Mark Brown @ 2011-12-20 13:23 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai, Lars-Peter Clausen,
	Dimitris Papastamos, Mike Frysinger, Daniel Mack, alsa-devel,
	linux-kernel, qi.wang, yong.y.wang, joel.clark, kok.howg.ewe

On Tue, Dec 20, 2011 at 11:45:44AM +0900, Tomoya MORINAGA wrote:

> +static int ignore_overrun = 1;
> +module_param(ignore_overrun, int, 0444);
> +MODULE_PARM_DESC(ignore_overrun, "ignore RX overruns (default=0)");

This shouldn't be a driver specific thing, and if we were to have such a
feature a module parameter doesn't seem like a great way of doing it.

> +/*****************************************************************************
> + *	I2S HAL (Hardware Abstruction Layer)
> + *****************************************************************************/

Please follow the kernel coding style in terms of comments.

> +static void ioh_i2s_enable_interrupts(int ch, enum dma_data_direction dir)
> +{
> +	unsigned int intr_lines;
> +
> +	if (dir)
> +		intr_lines = 1 << (I2S_IMASK_RX_BIT_START + ch);
> +
> +	else
> +		intr_lines = 1 << (I2S_IMASK_TX_BIT_START + ch);

I'd expect a switch statement corresponding to the enum, not an if
statement.

> +static void ioh_i2s_disable_interrupts(int ch, enum dma_data_direction dir)
> +{
> +	unsigned int intr_lines;
> +
> +	/*intr_lines&=I2S_ALL_INTERRUPT_BITS; */
> +	intr_lines = ioread32(i2s_data->iobase + I2SIMASK_OFFSET);

What's this commented out code for?  Also as a coding style thing
you should have a space between /* and the text in the comment (this
applies throughout the driver).

> +	/*disable interrupts for specified channel */
> +	if (dir)
> +		intr_lines |= 1 << (I2S_IMASK_RX_BIT_START + ch);
> +	else
> +		intr_lines |= 1 << (I2S_IMASK_TX_BIT_START + ch);
> +
> +	/*Mask the specific interrupt bits */
> +	iowrite32(intr_lines, i2s_data->iobase + I2SIMASK_OFFSET);

What ensures that this read/modify/write cycle can't race with another
caller?

> +/* Clear interrupt status */
> +static void ioh_i2s_clear_tx_sts_ir(int ch)
> +{
> +	int offset = ch * 0x800;
> +
> +	iowrite32(I2S_TX_FINT | I2S_TX_AFINT | I2S_TX_EINT | I2S_TX_AEINT,
> +		i2s_data->iobase + I2SISTTX_OFFSET + offset);
> +}

This appears to unconditionally acknowledge all interrupts, generally
you should only acknowledge interrupts that have been handled.
Otherwise it's possible that interrupts may be dropped if they're
flagged between the status read and acknowledgement write.

> +/*****************************************************************************
> + *	I2S Middle ware
> + *****************************************************************************/

I'm really not convinced of the value of these layers, reading the code
it really feels incredibly verbose in comparison with what I'd expect
from such a driver and I can't help that think that a lot of this
verbosity is down to muddling through these abstraction layers.

> +static bool filter(struct dma_chan *chan, void *slave)

This needs a better name.  It's not namespaced and it's not clear what
it's filtering.

> +static struct dma_chan *ioh_request_dma_channel(
> +		   int ch, struct ioh_i2s_dma *dma, enum dma_data_direction dir)
> +{
> +	dma_cap_mask_t mask;
> +	struct dma_chan *chan;
> +	struct pci_dev *dma_dev;
> +	dma_cap_zero(mask);
> +	dma_cap_set(DMA_SLAVE, mask);
> +
> +	dma_dev = pci_get_bus_and_slot(2, PCI_DEVFN(0, 1)); /* Get DMA's dev
> +								information */
> +
> +	if (dir == DMA_FROM_DEVICE) { /* Rx */

switch statement to select between multiple options in the enum.  This
applies throughout the code.

> +static void
> +ioh_i2s_write(struct snd_pcm_substream *substream, const void *data, int len)
> +{
> +	int rem1;
> +	int rem2;

What is this supposed to do?  There's an awful lot of it, and it looks
like it's supposed to be rewriting the data format for some reason which
isn't something that a driver ought to be doing (apart from anything
else it does rather defeat the point of DMA).

> +static void ioh_i2s_configure_i2s_regs(int ch, enum ioh_direction dir)
> +{
> +	int offset = ch * 0x800;

You should have a function to map the channel into a number.

> +
> +	if (dir) {
> +		/* Rx register */
> +		iowrite32(I2S_AFULL_THRESH / 2,
> +			  i2s_data->iobase + I2SAFRX_OFFSET + offset);
> +		iowrite32(0x1F, i2s_data->iobase + I2SAERX_OFFSET + offset);
> +		iowrite32(0x1F, i2s_data->iobase + I2SMSKRX_OFFSET + offset);
> +		iowrite32(0xC, i2s_data->iobase + I2SISTRX_OFFSET + offset);

Lots of magic numbers here and given that the function is called just
"configure" it's not clear what the intended purpose is.  This needs to
be clarified.

> +		/* Rx configuration */
> +		if (atomic_read(&dma->rx_busy)) {
> +			dev_err(i2s_data->dev, "rx i2s%dalready opened\n", ch);

...

> +	} else {
> +		/* Tx configuration */
> +		if (atomic_read(&dma->tx_busy)) {

There's a *lot* of duplicate code in the driver for TX and RX, it should
be possible to abstract out much more common code.

> +static void i2s_tx_almost_empty_ir(int ch)
> +{
> +	struct dma_async_tx_descriptor *desc;
> +	int num;
> +	int tx_comp_index;
> +	struct ioh_i2s_dma *dma = &dmadata[ch];
> +	struct scatterlist *sg = dma->sg_tx_p;
> +	void *cb_ch;
> +
> +	dev_dbg(i2s_data->dev, "%s: data_head=%p data_complete=%p\n", __func__,
> +		dma->tx_data_head, dma->tx_complete);
> +
> +	num = ((int)dma->tx_avail) / (I2S_AEMPTY_THRESH * 4);

That case looks *very* suspect.

> +	tx_comp_index = (((int)(dma->tx_complete - dma->tx_head))) /\
> +			(I2S_AEMPTY_THRESH * 4);

There is very rarely any need for line continuations outside of macros.

> +static inline void ioh_i2s_interrupt_sub_tx(int ch)
> +{
> +	unsigned int status;
> +	int offset = ch * 0x800;
> +
> +	status = ioread32(i2s_data->iobase + I2SISTTX_OFFSET + offset);
> +	if (status & I2S_TX_EINT)
> +		i2s_tx_empty_ir(ch);
> +	if (status & I2S_TX_AEINT)
> +		i2s_tx_almost_empty_ir(ch);

Given that all you're doing is logging just open code the log message.

> +	default:
> +		return -1;

Return error codes, you EPERM almost certainly isn't the error you meant
to report.

> +static int ioh_i2s_pci_suspend(struct pci_dev *pdev, pm_message_t state)
> +{
> +	int ret;
> +
> +	ioh_i2s_save_reg_conf(pdev);

You should be doing this in ASoC suspend/resume callbacks, not in PCI
ones.

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

* Re: [PATCH 3/3 v2] sound/soc/lapis: add platform driver for ML7213 IOH I2S
@ 2011-12-20 13:23     ` Mark Brown
  0 siblings, 0 replies; 29+ messages in thread
From: Mark Brown @ 2011-12-20 13:23 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: Dimitris Papastamos, alsa-devel, Lars-Peter Clausen,
	Mike Frysinger, qi.wang, Takashi Iwai, linux-kernel, yong.y.wang,
	kok.howg.ewe, Daniel Mack, Liam Girdwood, joel.clark

On Tue, Dec 20, 2011 at 11:45:44AM +0900, Tomoya MORINAGA wrote:

> +static int ignore_overrun = 1;
> +module_param(ignore_overrun, int, 0444);
> +MODULE_PARM_DESC(ignore_overrun, "ignore RX overruns (default=0)");

This shouldn't be a driver specific thing, and if we were to have such a
feature a module parameter doesn't seem like a great way of doing it.

> +/*****************************************************************************
> + *	I2S HAL (Hardware Abstruction Layer)
> + *****************************************************************************/

Please follow the kernel coding style in terms of comments.

> +static void ioh_i2s_enable_interrupts(int ch, enum dma_data_direction dir)
> +{
> +	unsigned int intr_lines;
> +
> +	if (dir)
> +		intr_lines = 1 << (I2S_IMASK_RX_BIT_START + ch);
> +
> +	else
> +		intr_lines = 1 << (I2S_IMASK_TX_BIT_START + ch);

I'd expect a switch statement corresponding to the enum, not an if
statement.

> +static void ioh_i2s_disable_interrupts(int ch, enum dma_data_direction dir)
> +{
> +	unsigned int intr_lines;
> +
> +	/*intr_lines&=I2S_ALL_INTERRUPT_BITS; */
> +	intr_lines = ioread32(i2s_data->iobase + I2SIMASK_OFFSET);

What's this commented out code for?  Also as a coding style thing
you should have a space between /* and the text in the comment (this
applies throughout the driver).

> +	/*disable interrupts for specified channel */
> +	if (dir)
> +		intr_lines |= 1 << (I2S_IMASK_RX_BIT_START + ch);
> +	else
> +		intr_lines |= 1 << (I2S_IMASK_TX_BIT_START + ch);
> +
> +	/*Mask the specific interrupt bits */
> +	iowrite32(intr_lines, i2s_data->iobase + I2SIMASK_OFFSET);

What ensures that this read/modify/write cycle can't race with another
caller?

> +/* Clear interrupt status */
> +static void ioh_i2s_clear_tx_sts_ir(int ch)
> +{
> +	int offset = ch * 0x800;
> +
> +	iowrite32(I2S_TX_FINT | I2S_TX_AFINT | I2S_TX_EINT | I2S_TX_AEINT,
> +		i2s_data->iobase + I2SISTTX_OFFSET + offset);
> +}

This appears to unconditionally acknowledge all interrupts, generally
you should only acknowledge interrupts that have been handled.
Otherwise it's possible that interrupts may be dropped if they're
flagged between the status read and acknowledgement write.

> +/*****************************************************************************
> + *	I2S Middle ware
> + *****************************************************************************/

I'm really not convinced of the value of these layers, reading the code
it really feels incredibly verbose in comparison with what I'd expect
from such a driver and I can't help that think that a lot of this
verbosity is down to muddling through these abstraction layers.

> +static bool filter(struct dma_chan *chan, void *slave)

This needs a better name.  It's not namespaced and it's not clear what
it's filtering.

> +static struct dma_chan *ioh_request_dma_channel(
> +		   int ch, struct ioh_i2s_dma *dma, enum dma_data_direction dir)
> +{
> +	dma_cap_mask_t mask;
> +	struct dma_chan *chan;
> +	struct pci_dev *dma_dev;
> +	dma_cap_zero(mask);
> +	dma_cap_set(DMA_SLAVE, mask);
> +
> +	dma_dev = pci_get_bus_and_slot(2, PCI_DEVFN(0, 1)); /* Get DMA's dev
> +								information */
> +
> +	if (dir == DMA_FROM_DEVICE) { /* Rx */

switch statement to select between multiple options in the enum.  This
applies throughout the code.

> +static void
> +ioh_i2s_write(struct snd_pcm_substream *substream, const void *data, int len)
> +{
> +	int rem1;
> +	int rem2;

What is this supposed to do?  There's an awful lot of it, and it looks
like it's supposed to be rewriting the data format for some reason which
isn't something that a driver ought to be doing (apart from anything
else it does rather defeat the point of DMA).

> +static void ioh_i2s_configure_i2s_regs(int ch, enum ioh_direction dir)
> +{
> +	int offset = ch * 0x800;

You should have a function to map the channel into a number.

> +
> +	if (dir) {
> +		/* Rx register */
> +		iowrite32(I2S_AFULL_THRESH / 2,
> +			  i2s_data->iobase + I2SAFRX_OFFSET + offset);
> +		iowrite32(0x1F, i2s_data->iobase + I2SAERX_OFFSET + offset);
> +		iowrite32(0x1F, i2s_data->iobase + I2SMSKRX_OFFSET + offset);
> +		iowrite32(0xC, i2s_data->iobase + I2SISTRX_OFFSET + offset);

Lots of magic numbers here and given that the function is called just
"configure" it's not clear what the intended purpose is.  This needs to
be clarified.

> +		/* Rx configuration */
> +		if (atomic_read(&dma->rx_busy)) {
> +			dev_err(i2s_data->dev, "rx i2s%dalready opened\n", ch);

...

> +	} else {
> +		/* Tx configuration */
> +		if (atomic_read(&dma->tx_busy)) {

There's a *lot* of duplicate code in the driver for TX and RX, it should
be possible to abstract out much more common code.

> +static void i2s_tx_almost_empty_ir(int ch)
> +{
> +	struct dma_async_tx_descriptor *desc;
> +	int num;
> +	int tx_comp_index;
> +	struct ioh_i2s_dma *dma = &dmadata[ch];
> +	struct scatterlist *sg = dma->sg_tx_p;
> +	void *cb_ch;
> +
> +	dev_dbg(i2s_data->dev, "%s: data_head=%p data_complete=%p\n", __func__,
> +		dma->tx_data_head, dma->tx_complete);
> +
> +	num = ((int)dma->tx_avail) / (I2S_AEMPTY_THRESH * 4);

That case looks *very* suspect.

> +	tx_comp_index = (((int)(dma->tx_complete - dma->tx_head))) /\
> +			(I2S_AEMPTY_THRESH * 4);

There is very rarely any need for line continuations outside of macros.

> +static inline void ioh_i2s_interrupt_sub_tx(int ch)
> +{
> +	unsigned int status;
> +	int offset = ch * 0x800;
> +
> +	status = ioread32(i2s_data->iobase + I2SISTTX_OFFSET + offset);
> +	if (status & I2S_TX_EINT)
> +		i2s_tx_empty_ir(ch);
> +	if (status & I2S_TX_AEINT)
> +		i2s_tx_almost_empty_ir(ch);

Given that all you're doing is logging just open code the log message.

> +	default:
> +		return -1;

Return error codes, you EPERM almost certainly isn't the error you meant
to report.

> +static int ioh_i2s_pci_suspend(struct pci_dev *pdev, pm_message_t state)
> +{
> +	int ret;
> +
> +	ioh_i2s_save_reg_conf(pdev);

You should be doing this in ASoC suspend/resume callbacks, not in PCI
ones.

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

* Re: [PATCH 3/3 v2] sound/soc/lapis: add platform driver for ML7213 IOH I2S
  2011-12-20 13:23     ` Mark Brown
@ 2011-12-21 11:22       ` Tomoya MORINAGA
  -1 siblings, 0 replies; 29+ messages in thread
From: Tomoya MORINAGA @ 2011-12-21 11:22 UTC (permalink / raw)
  To: Mark Brown
  Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai, Lars-Peter Clausen,
	Dimitris Papastamos, Mike Frysinger, Daniel Mack, alsa-devel,
	linux-kernel, qi.wang, yong.y.wang, joel.clark, kok.howg.ewe

2011/12/20 Mark Brown <broonie@opensource.wolfsonmicro.com>:
> On Tue, Dec 20, 2011 at 11:45:44AM +0900, Tomoya MORINAGA wrote:
>
>> +static int ignore_overrun = 1;
>> +module_param(ignore_overrun, int, 0444);
>> +MODULE_PARM_DESC(ignore_overrun, "ignore RX overruns (default=0)");
>
> This shouldn't be a driver specific thing, and if we were to have such a
> feature a module parameter doesn't seem like a great way of doing it.

I'll delete this parameter.

>
>> +/*****************************************************************************
>> + *   I2S HAL (Hardware Abstruction Layer)
>> + *****************************************************************************/
>
> Please follow the kernel coding style in terms of comments.
I' ll modify this.

>
>> +static void ioh_i2s_enable_interrupts(int ch, enum dma_data_direction dir)
>> +{
>> +     unsigned int intr_lines;
>> +
>> +     if (dir)
>> +             intr_lines = 1 << (I2S_IMASK_RX_BIT_START + ch);
>> +
>> +     else
>> +             intr_lines = 1 << (I2S_IMASK_TX_BIT_START + ch);
>
> I'd expect a switch statement corresponding to the enum, not an if
> statement.
"if" will replace "switch".

>
>> +static void ioh_i2s_disable_interrupts(int ch, enum dma_data_direction dir)
>> +{
>> +     unsigned int intr_lines;
>> +
>> +     /*intr_lines&=I2S_ALL_INTERRUPT_BITS; */
>> +     intr_lines = ioread32(i2s_data->iobase + I2SIMASK_OFFSET);
>
> What's this commented out code for?  Also as a coding style thing
> you should have a space between /* and the text in the comment (this
> applies throughout the driver).
I'll delete.

>
>> +     /*disable interrupts for specified channel */
>> +     if (dir)
>> +             intr_lines |= 1 << (I2S_IMASK_RX_BIT_START + ch);
>> +     else
>> +             intr_lines |= 1 << (I2S_IMASK_TX_BIT_START + ch);
>> +
>> +     /*Mask the specific interrupt bits */
>> +     iowrite32(intr_lines, i2s_data->iobase + I2SIMASK_OFFSET);
>
> What ensures that this read/modify/write cycle can't race with another
> caller?
I'll add exclusive processing like spin-lock.

>
>> +/* Clear interrupt status */
>> +static void ioh_i2s_clear_tx_sts_ir(int ch)
>> +{
>> +     int offset = ch * 0x800;
>> +
>> +     iowrite32(I2S_TX_FINT | I2S_TX_AFINT | I2S_TX_EINT | I2S_TX_AEINT,
>> +             i2s_data->iobase + I2SISTTX_OFFSET + offset);
>> +}
>
> This appears to unconditionally acknowledge all interrupts, generally
> you should only acknowledge interrupts that have been handled.
> Otherwise it's possible that interrupts may be dropped if they're
> flagged between the status read and acknowledgement write.

I can understand your saying.
If following your saying, only called by i2s_dma_tx_complete is enough.

However I think adding clearing interrupt status at both before and
after i2s communicating start/finish is better.


>
>> +/*****************************************************************************
>> + *   I2S Middle ware
>> + *****************************************************************************/
>
> I'm really not convinced of the value of these layers, reading the code
> it really feels incredibly verbose in comparison with what I'd expect
> from such a driver and I can't help that think that a lot of this
> verbosity is down to muddling through these abstraction layers.
>
>> +static bool filter(struct dma_chan *chan, void *slave)
>
> This needs a better name.  It's not namespaced and it's not clear what
> it's filtering.

In case of using Linux standard DMA API, function "filter" is the most
popular name.
Almost drivers which use standard use "filter".


>
>> +static struct dma_chan *ioh_request_dma_channel(
>> +                int ch, struct ioh_i2s_dma *dma, enum dma_data_direction dir)
>> +{
>> +     dma_cap_mask_t mask;
>> +     struct dma_chan *chan;
>> +     struct pci_dev *dma_dev;
>> +     dma_cap_zero(mask);
>> +     dma_cap_set(DMA_SLAVE, mask);
>> +
>> +     dma_dev = pci_get_bus_and_slot(2, PCI_DEVFN(0, 1)); /* Get DMA's dev
>> +                                                             information */
>> +
>> +     if (dir == DMA_FROM_DEVICE) { /* Rx */
>
> switch statement to select between multiple options in the enum.  This
> applies throughout the code.
"if" will replace "switch".

>
>> +static void
>> +ioh_i2s_write(struct snd_pcm_substream *substream, const void *data, int len)
>> +{
>> +     int rem1;
>> +     int rem2;
>
> What is this supposed to do?  There's an awful lot of it, and it looks
> like it's supposed to be rewriting the data format for some reason which
> isn't something that a driver ought to be doing (apart from anything
> else it does rather defeat the point of DMA).

This driver has buffer which is for preventing noisy sound  by jitter
So, this buffer is variable.

>
>> +static void ioh_i2s_configure_i2s_regs(int ch, enum ioh_direction dir)
>> +{
>> +     int offset = ch * 0x800;
>
> You should have a function to map the channel into a number.
understand.

>
>> +
>> +     if (dir) {
>> +             /* Rx register */
>> +             iowrite32(I2S_AFULL_THRESH / 2,
>> +                       i2s_data->iobase + I2SAFRX_OFFSET + offset);
>> +             iowrite32(0x1F, i2s_data->iobase + I2SAERX_OFFSET + offset);
>> +             iowrite32(0x1F, i2s_data->iobase + I2SMSKRX_OFFSET + offset);
>> +             iowrite32(0xC, i2s_data->iobase + I2SISTRX_OFFSET + offset);
>
> Lots of magic numbers here and given that the function is called just
> "configure" it's not clear what the intended purpose is.  This needs to
> be clarified.
understand.

>
>> +             /* Rx configuration */
>> +             if (atomic_read(&dma->rx_busy)) {
>> +                     dev_err(i2s_data->dev, "rx i2s%dalready opened\n", ch);
>
> ...
>
>> +     } else {
>> +             /* Tx configuration */
>> +             if (atomic_read(&dma->tx_busy)) {
>
> There's a *lot* of duplicate code in the driver for TX and RX, it should
> be possible to abstract out much more common code.
I'll study.

>
>> +static void i2s_tx_almost_empty_ir(int ch)
>> +{
>> +     struct dma_async_tx_descriptor *desc;
>> +     int num;
>> +     int tx_comp_index;
>> +     struct ioh_i2s_dma *dma = &dmadata[ch];
>> +     struct scatterlist *sg = dma->sg_tx_p;
>> +     void *cb_ch;
>> +
>> +     dev_dbg(i2s_data->dev, "%s: data_head=%p data_complete=%p\n", __func__,
>> +             dma->tx_data_head, dma->tx_complete);
>> +
>> +     num = ((int)dma->tx_avail) / (I2S_AEMPTY_THRESH * 4);
>
> That case looks *very* suspect.
This is correct.
Why do you think so ?

>
>> +     tx_comp_index = (((int)(dma->tx_complete - dma->tx_head))) /\
>> +                     (I2S_AEMPTY_THRESH * 4);
>
> There is very rarely any need for line continuations outside of macros.
>
>> +static inline void ioh_i2s_interrupt_sub_tx(int ch)
>> +{
>> +     unsigned int status;
>> +     int offset = ch * 0x800;
>> +
>> +     status = ioread32(i2s_data->iobase + I2SISTTX_OFFSET + offset);
>> +     if (status & I2S_TX_EINT)
>> +             i2s_tx_empty_ir(ch);
>> +     if (status & I2S_TX_AEINT)
>> +             i2s_tx_almost_empty_ir(ch);
>
> Given that all you're doing is logging just open code the log message.
Understand.

>
>> +     default:
>> +             return -1;
>
> Return error codes, you EPERM almost certainly isn't the error you meant
> to report.
I'll modify.

>
>> +static int ioh_i2s_pci_suspend(struct pci_dev *pdev, pm_message_t state)
>> +{
>> +     int ret;
>> +
>> +     ioh_i2s_save_reg_conf(pdev);
>
> You should be doing this in ASoC suspend/resume callbacks, not in PCI
> ones.
Understand.


thanks,
tomoya

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

* Re: [PATCH 3/3 v2] sound/soc/lapis: add platform driver for ML7213 IOH I2S
@ 2011-12-21 11:22       ` Tomoya MORINAGA
  0 siblings, 0 replies; 29+ messages in thread
From: Tomoya MORINAGA @ 2011-12-21 11:22 UTC (permalink / raw)
  To: Mark Brown
  Cc: Dimitris Papastamos, alsa-devel, Lars-Peter Clausen,
	Mike Frysinger, qi.wang, Takashi Iwai, linux-kernel, yong.y.wang,
	kok.howg.ewe, Daniel Mack, Liam Girdwood, joel.clark

2011/12/20 Mark Brown <broonie@opensource.wolfsonmicro.com>:
> On Tue, Dec 20, 2011 at 11:45:44AM +0900, Tomoya MORINAGA wrote:
>
>> +static int ignore_overrun = 1;
>> +module_param(ignore_overrun, int, 0444);
>> +MODULE_PARM_DESC(ignore_overrun, "ignore RX overruns (default=0)");
>
> This shouldn't be a driver specific thing, and if we were to have such a
> feature a module parameter doesn't seem like a great way of doing it.

I'll delete this parameter.

>
>> +/*****************************************************************************
>> + *   I2S HAL (Hardware Abstruction Layer)
>> + *****************************************************************************/
>
> Please follow the kernel coding style in terms of comments.
I' ll modify this.

>
>> +static void ioh_i2s_enable_interrupts(int ch, enum dma_data_direction dir)
>> +{
>> +     unsigned int intr_lines;
>> +
>> +     if (dir)
>> +             intr_lines = 1 << (I2S_IMASK_RX_BIT_START + ch);
>> +
>> +     else
>> +             intr_lines = 1 << (I2S_IMASK_TX_BIT_START + ch);
>
> I'd expect a switch statement corresponding to the enum, not an if
> statement.
"if" will replace "switch".

>
>> +static void ioh_i2s_disable_interrupts(int ch, enum dma_data_direction dir)
>> +{
>> +     unsigned int intr_lines;
>> +
>> +     /*intr_lines&=I2S_ALL_INTERRUPT_BITS; */
>> +     intr_lines = ioread32(i2s_data->iobase + I2SIMASK_OFFSET);
>
> What's this commented out code for?  Also as a coding style thing
> you should have a space between /* and the text in the comment (this
> applies throughout the driver).
I'll delete.

>
>> +     /*disable interrupts for specified channel */
>> +     if (dir)
>> +             intr_lines |= 1 << (I2S_IMASK_RX_BIT_START + ch);
>> +     else
>> +             intr_lines |= 1 << (I2S_IMASK_TX_BIT_START + ch);
>> +
>> +     /*Mask the specific interrupt bits */
>> +     iowrite32(intr_lines, i2s_data->iobase + I2SIMASK_OFFSET);
>
> What ensures that this read/modify/write cycle can't race with another
> caller?
I'll add exclusive processing like spin-lock.

>
>> +/* Clear interrupt status */
>> +static void ioh_i2s_clear_tx_sts_ir(int ch)
>> +{
>> +     int offset = ch * 0x800;
>> +
>> +     iowrite32(I2S_TX_FINT | I2S_TX_AFINT | I2S_TX_EINT | I2S_TX_AEINT,
>> +             i2s_data->iobase + I2SISTTX_OFFSET + offset);
>> +}
>
> This appears to unconditionally acknowledge all interrupts, generally
> you should only acknowledge interrupts that have been handled.
> Otherwise it's possible that interrupts may be dropped if they're
> flagged between the status read and acknowledgement write.

I can understand your saying.
If following your saying, only called by i2s_dma_tx_complete is enough.

However I think adding clearing interrupt status at both before and
after i2s communicating start/finish is better.


>
>> +/*****************************************************************************
>> + *   I2S Middle ware
>> + *****************************************************************************/
>
> I'm really not convinced of the value of these layers, reading the code
> it really feels incredibly verbose in comparison with what I'd expect
> from such a driver and I can't help that think that a lot of this
> verbosity is down to muddling through these abstraction layers.
>
>> +static bool filter(struct dma_chan *chan, void *slave)
>
> This needs a better name.  It's not namespaced and it's not clear what
> it's filtering.

In case of using Linux standard DMA API, function "filter" is the most
popular name.
Almost drivers which use standard use "filter".


>
>> +static struct dma_chan *ioh_request_dma_channel(
>> +                int ch, struct ioh_i2s_dma *dma, enum dma_data_direction dir)
>> +{
>> +     dma_cap_mask_t mask;
>> +     struct dma_chan *chan;
>> +     struct pci_dev *dma_dev;
>> +     dma_cap_zero(mask);
>> +     dma_cap_set(DMA_SLAVE, mask);
>> +
>> +     dma_dev = pci_get_bus_and_slot(2, PCI_DEVFN(0, 1)); /* Get DMA's dev
>> +                                                             information */
>> +
>> +     if (dir == DMA_FROM_DEVICE) { /* Rx */
>
> switch statement to select between multiple options in the enum.  This
> applies throughout the code.
"if" will replace "switch".

>
>> +static void
>> +ioh_i2s_write(struct snd_pcm_substream *substream, const void *data, int len)
>> +{
>> +     int rem1;
>> +     int rem2;
>
> What is this supposed to do?  There's an awful lot of it, and it looks
> like it's supposed to be rewriting the data format for some reason which
> isn't something that a driver ought to be doing (apart from anything
> else it does rather defeat the point of DMA).

This driver has buffer which is for preventing noisy sound  by jitter
So, this buffer is variable.

>
>> +static void ioh_i2s_configure_i2s_regs(int ch, enum ioh_direction dir)
>> +{
>> +     int offset = ch * 0x800;
>
> You should have a function to map the channel into a number.
understand.

>
>> +
>> +     if (dir) {
>> +             /* Rx register */
>> +             iowrite32(I2S_AFULL_THRESH / 2,
>> +                       i2s_data->iobase + I2SAFRX_OFFSET + offset);
>> +             iowrite32(0x1F, i2s_data->iobase + I2SAERX_OFFSET + offset);
>> +             iowrite32(0x1F, i2s_data->iobase + I2SMSKRX_OFFSET + offset);
>> +             iowrite32(0xC, i2s_data->iobase + I2SISTRX_OFFSET + offset);
>
> Lots of magic numbers here and given that the function is called just
> "configure" it's not clear what the intended purpose is.  This needs to
> be clarified.
understand.

>
>> +             /* Rx configuration */
>> +             if (atomic_read(&dma->rx_busy)) {
>> +                     dev_err(i2s_data->dev, "rx i2s%dalready opened\n", ch);
>
> ...
>
>> +     } else {
>> +             /* Tx configuration */
>> +             if (atomic_read(&dma->tx_busy)) {
>
> There's a *lot* of duplicate code in the driver for TX and RX, it should
> be possible to abstract out much more common code.
I'll study.

>
>> +static void i2s_tx_almost_empty_ir(int ch)
>> +{
>> +     struct dma_async_tx_descriptor *desc;
>> +     int num;
>> +     int tx_comp_index;
>> +     struct ioh_i2s_dma *dma = &dmadata[ch];
>> +     struct scatterlist *sg = dma->sg_tx_p;
>> +     void *cb_ch;
>> +
>> +     dev_dbg(i2s_data->dev, "%s: data_head=%p data_complete=%p\n", __func__,
>> +             dma->tx_data_head, dma->tx_complete);
>> +
>> +     num = ((int)dma->tx_avail) / (I2S_AEMPTY_THRESH * 4);
>
> That case looks *very* suspect.
This is correct.
Why do you think so ?

>
>> +     tx_comp_index = (((int)(dma->tx_complete - dma->tx_head))) /\
>> +                     (I2S_AEMPTY_THRESH * 4);
>
> There is very rarely any need for line continuations outside of macros.
>
>> +static inline void ioh_i2s_interrupt_sub_tx(int ch)
>> +{
>> +     unsigned int status;
>> +     int offset = ch * 0x800;
>> +
>> +     status = ioread32(i2s_data->iobase + I2SISTTX_OFFSET + offset);
>> +     if (status & I2S_TX_EINT)
>> +             i2s_tx_empty_ir(ch);
>> +     if (status & I2S_TX_AEINT)
>> +             i2s_tx_almost_empty_ir(ch);
>
> Given that all you're doing is logging just open code the log message.
Understand.

>
>> +     default:
>> +             return -1;
>
> Return error codes, you EPERM almost certainly isn't the error you meant
> to report.
I'll modify.

>
>> +static int ioh_i2s_pci_suspend(struct pci_dev *pdev, pm_message_t state)
>> +{
>> +     int ret;
>> +
>> +     ioh_i2s_save_reg_conf(pdev);
>
> You should be doing this in ASoC suspend/resume callbacks, not in PCI
> ones.
Understand.


thanks,
tomoya

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

* Re: [PATCH 3/3 v2] sound/soc/lapis: add platform driver for ML7213 IOH I2S
  2011-12-21 11:22       ` Tomoya MORINAGA
  (?)
@ 2011-12-22  0:58       ` Mark Brown
  2011-12-22  8:10         ` Tomoya MORINAGA
  -1 siblings, 1 reply; 29+ messages in thread
From: Mark Brown @ 2011-12-22  0:58 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai, Lars-Peter Clausen,
	Dimitris Papastamos, Mike Frysinger, Daniel Mack, alsa-devel,
	linux-kernel, qi.wang, yong.y.wang, joel.clark, kok.howg.ewe

On Wed, Dec 21, 2011 at 08:22:56PM +0900, Tomoya MORINAGA wrote:
> 2011/12/20 Mark Brown <broonie@opensource.wolfsonmicro.com>:
> > On Tue, Dec 20, 2011 at 11:45:44AM +0900, Tomoya MORINAGA wrote:

> >> +     iowrite32(I2S_TX_FINT | I2S_TX_AFINT | I2S_TX_EINT | I2S_TX_AEINT,
> >> +             i2s_data->iobase + I2SISTTX_OFFSET + offset);

> > This appears to unconditionally acknowledge all interrupts, generally
> > you should only acknowledge interrupts that have been handled.
> > Otherwise it's possible that interrupts may be dropped if they're
> > flagged between the status read and acknowledgement write.

> I can understand your saying.
> If following your saying, only called by i2s_dma_tx_complete is enough.

> However I think adding clearing interrupt status at both before and
> after i2s communicating start/finish is better.

No, the issue is that you're acknowleding a hard coded set of
interrupts, not the interrupts that were actually handled.  The ordering
isn't really the issue, the issue is that you could acknowledge things
that weren't handled.  For example:

   1. ISR runs, reads status A
   2. Condition B happens
   3. ISR handles condition A.
   4. ISR acknowledges both A and B.

would mean that B would just get ignored.

> >> +static bool filter(struct dma_chan *chan, void *slave)

> > This needs a better name.  It's not namespaced and it's not clear what
> > it's filtering.

> In case of using Linux standard DMA API, function "filter" is the most
> popular name.
> Almost drivers which use standard use "filter".

I suspect they may have the filter immediately next to the code that
uses it, which certainly isn't the case here.

> >> +ioh_i2s_write(struct snd_pcm_substream *substream, const void *data, int len)
> >> +{
> >> +     int rem1;
> >> +     int rem2;

> > What is this supposed to do?  There's an awful lot of it, and it looks
> > like it's supposed to be rewriting the data format for some reason which
> > isn't something that a driver ought to be doing (apart from anything
> > else it does rather defeat the point of DMA).

> This driver has buffer which is for preventing noisy sound  by jitter
> So, this buffer is variable.

You're implementing some sort of custom buffering in your driver?  That
sounds terribly unidiomatic - pretty much all DMA drivers are very thin
and manage to work well, I'd expect this is masking some problems in the
code rather than anything else.  Can you provide more detail on what
this is working around?

> >> +     num = ((int)dma->tx_avail) / (I2S_AEMPTY_THRESH * 4);

> > That case looks *very* suspect.

> This is correct.
> Why do you think so ?

I can't tell what it's supposed to do, and casting that isn't super
clear especially casting to integer types tends to be a big red flag.

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

* Re: [PATCH 3/3 v2] sound/soc/lapis: add platform driver for ML7213 IOH I2S
  2011-12-22  0:58       ` Mark Brown
@ 2011-12-22  8:10         ` Tomoya MORINAGA
  2011-12-22 10:39             ` Mark Brown
  0 siblings, 1 reply; 29+ messages in thread
From: Tomoya MORINAGA @ 2011-12-22  8:10 UTC (permalink / raw)
  To: Mark Brown
  Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai, Lars-Peter Clausen,
	Dimitris Papastamos, Mike Frysinger, Daniel Mack, alsa-devel,
	linux-kernel, qi.wang, yong.y.wang, joel.clark, kok.howg.ewe

2011/12/22 Mark Brown <broonie@opensource.wolfsonmicro.com>:
> On Wed, Dec 21, 2011 at 08:22:56PM +0900, Tomoya MORINAGA wrote:
>> 2011/12/20 Mark Brown <broonie@opensource.wolfsonmicro.com>:
>> > On Tue, Dec 20, 2011 at 11:45:44AM +0900, Tomoya MORINAGA wrote:
>
>> >> +     iowrite32(I2S_TX_FINT | I2S_TX_AFINT | I2S_TX_EINT | I2S_TX_AEINT,
>> >> +             i2s_data->iobase + I2SISTTX_OFFSET + offset);
>
>> > This appears to unconditionally acknowledge all interrupts, generally
>> > you should only acknowledge interrupts that have been handled.
>> > Otherwise it's possible that interrupts may be dropped if they're
>> > flagged between the status read and acknowledgement write.
>
>> I can understand your saying.
>> If following your saying, only called by i2s_dma_tx_complete is enough.
>
>> However I think adding clearing interrupt status at both before and
>> after i2s communicating start/finish is better.
>
> No, the issue is that you're acknowleding a hard coded set of
> interrupts, not the interrupts that were actually handled.  The ordering
> isn't really the issue, the issue is that you could acknowledge things
> that weren't handled.  For example:
>
>   1. ISR runs, reads status A
>   2. Condition B happens
>   3. ISR handles condition A.
>   4. ISR acknowledges both A and B.
>
> would mean that B would just get ignored.
I can understand your saying.
However, in case of Tx interrupt, only ALMOST-EMPTY occurs. in case of
Rx, only Rx ALMOST-FULL occurs.
So as long as a user uses ML7213 I2S, your mentioned behavior is never happened.


>
>> >> +static bool filter(struct dma_chan *chan, void *slave)
>
>> > This needs a better name.  It's not namespaced and it's not clear what
>> > it's filtering.
>
>> In case of using Linux standard DMA API, function "filter" is the most
>> popular name.
>> Almost drivers which use standard use "filter".
>
> I suspect they may have the filter immediately next to the code that
> uses it, which certainly isn't the case here.
They have the "filter" function immediately next to the code uses
"dma_request_channel".
Anyway, if you want me to change the name, I can change the name.
Please decide it.

>
>> >> +ioh_i2s_write(struct snd_pcm_substream *substream, const void *data, int len)
>> >> +{
>> >> +     int rem1;
>> >> +     int rem2;
>
>> > What is this supposed to do?  There's an awful lot of it, and it looks
>> > like it's supposed to be rewriting the data format for some reason which
>> > isn't something that a driver ought to be doing (apart from anything
>> > else it does rather defeat the point of DMA).
>
>> This driver has buffer which is for preventing noisy sound  by jitter
>> So, this buffer is variable.
>
> You're implementing some sort of custom buffering in your driver?  That
> sounds terribly unidiomatic - pretty much all DMA drivers are very thin
> and manage to work well, I'd expect this is masking some problems in the
> code rather than anything else.  Can you provide more detail on what
> this is working around?
Not sorted but queuing only.
In sound/voice control system, queuing is not rare, I think.
If necessary, though this method is very common, I can send the method
of the queue.

>
>> >> +     num = ((int)dma->tx_avail) / (I2S_AEMPTY_THRESH * 4);
>
>> > That case looks *very* suspect.
>
>> This is correct.
>> Why do you think so ?
>
> I can't tell what it's supposed to do, and casting that isn't super
> clear especially casting to integer types tends to be a big red flag.

Currently, "tx_avail" is "int" type. So, "(int)" is redundant.
Maybe, once type of "tx_avail" is not "int" but something pointer type.
I'll modify like below.
num = dma->tx_avail / (I2S_AEMPTY_THRESH * 4);


thanks,
tomoya

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

* Re: [PATCH 1/3 v5] sound/soc/codecs: add LAPIS Semiconductor ML26124
  2011-12-20 10:40   ` Mark Brown
@ 2011-12-22  8:31     ` Tomoya MORINAGA
  -1 siblings, 0 replies; 29+ messages in thread
From: Tomoya MORINAGA @ 2011-12-22  8:31 UTC (permalink / raw)
  To: Mark Brown
  Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai, Lars-Peter Clausen,
	Dimitris Papastamos, Mike Frysinger, Daniel Mack, alsa-devel,
	linux-kernel, qi.wang, yong.y.wang, joel.clark, kok.howg.ewe

2011/12/20 Mark Brown <broonie@opensource.wolfsonmicro.com>:
> How have you tested this code?
No, I haven't tested at all. (Of course, I tested before ASoC.)
I'll test the new these drivers after you say "Roughly, how to use
ASoC framework is OK"

thanks,
tomoya

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

* Re: [PATCH 1/3 v5] sound/soc/codecs: add LAPIS Semiconductor ML26124
@ 2011-12-22  8:31     ` Tomoya MORINAGA
  0 siblings, 0 replies; 29+ messages in thread
From: Tomoya MORINAGA @ 2011-12-22  8:31 UTC (permalink / raw)
  To: Mark Brown
  Cc: Dimitris Papastamos, alsa-devel, Lars-Peter Clausen,
	Mike Frysinger, qi.wang, Takashi Iwai, linux-kernel, yong.y.wang,
	kok.howg.ewe, Daniel Mack, Liam Girdwood, joel.clark

2011/12/20 Mark Brown <broonie@opensource.wolfsonmicro.com>:
> How have you tested this code?
No, I haven't tested at all. (Of course, I tested before ASoC.)
I'll test the new these drivers after you say "Roughly, how to use
ASoC framework is OK"

thanks,
tomoya

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

* Re: [PATCH 3/3 v2] sound/soc/lapis: add platform driver for ML7213 IOH I2S
  2011-12-22  8:10         ` Tomoya MORINAGA
@ 2011-12-22 10:39             ` Mark Brown
  0 siblings, 0 replies; 29+ messages in thread
From: Mark Brown @ 2011-12-22 10:39 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai, Lars-Peter Clausen,
	Dimitris Papastamos, Mike Frysinger, Daniel Mack, alsa-devel,
	linux-kernel, qi.wang, yong.y.wang, joel.clark, kok.howg.ewe

On Thu, Dec 22, 2011 at 05:10:24PM +0900, Tomoya MORINAGA wrote:

> I can understand your saying.
> However, in case of Tx interrupt, only ALMOST-EMPTY occurs. in case of
> Rx, only Rx ALMOST-FULL occurs.
> So as long as a user uses ML7213 I2S, your mentioned behavior is never happened.

That's not what your code says...  it may be that the other interrupts
are very rare but that's not the same thing.

> They have the "filter" function immediately next to the code uses
> "dma_request_channel".

That's a rather large function...

> Anyway, if you want me to change the name, I can change the name.
> Please decide it.

Rename.

> > You're implementing some sort of custom buffering in your driver?  That
> > sounds terribly unidiomatic - pretty much all DMA drivers are very thin
> > and manage to work well, I'd expect this is masking some problems in the
> > code rather than anything else.  Can you provide more detail on what
> > this is working around?

> Not sorted but queuing only.
> In sound/voice control system, queuing is not rare, I think.
> If necessary, though this method is very common, I can send the method
> of the queue.

No, please describe the problem you're trying to fix.  If nothing else
think about what you're saying here - if this is a common need then it's
something that's going to be handled in generic code, not open coded in
individual drivers.  Like I say I would expect that you have problems
elsewhere in your code which you are masking with this.

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

* Re: [PATCH 3/3 v2] sound/soc/lapis: add platform driver for ML7213 IOH I2S
@ 2011-12-22 10:39             ` Mark Brown
  0 siblings, 0 replies; 29+ messages in thread
From: Mark Brown @ 2011-12-22 10:39 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: Dimitris Papastamos, alsa-devel, Lars-Peter Clausen,
	Mike Frysinger, qi.wang, Takashi Iwai, linux-kernel, yong.y.wang,
	kok.howg.ewe, Daniel Mack, Liam Girdwood, joel.clark

On Thu, Dec 22, 2011 at 05:10:24PM +0900, Tomoya MORINAGA wrote:

> I can understand your saying.
> However, in case of Tx interrupt, only ALMOST-EMPTY occurs. in case of
> Rx, only Rx ALMOST-FULL occurs.
> So as long as a user uses ML7213 I2S, your mentioned behavior is never happened.

That's not what your code says...  it may be that the other interrupts
are very rare but that's not the same thing.

> They have the "filter" function immediately next to the code uses
> "dma_request_channel".

That's a rather large function...

> Anyway, if you want me to change the name, I can change the name.
> Please decide it.

Rename.

> > You're implementing some sort of custom buffering in your driver?  That
> > sounds terribly unidiomatic - pretty much all DMA drivers are very thin
> > and manage to work well, I'd expect this is masking some problems in the
> > code rather than anything else.  Can you provide more detail on what
> > this is working around?

> Not sorted but queuing only.
> In sound/voice control system, queuing is not rare, I think.
> If necessary, though this method is very common, I can send the method
> of the queue.

No, please describe the problem you're trying to fix.  If nothing else
think about what you're saying here - if this is a common need then it's
something that's going to be handled in generic code, not open coded in
individual drivers.  Like I say I would expect that you have problems
elsewhere in your code which you are masking with this.

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

* Re: [PATCH 1/3 v5] sound/soc/codecs: add LAPIS Semiconductor ML26124
  2011-12-22  8:31     ` Tomoya MORINAGA
@ 2011-12-22 10:50       ` Mark Brown
  -1 siblings, 0 replies; 29+ messages in thread
From: Mark Brown @ 2011-12-22 10:50 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai, Lars-Peter Clausen,
	Dimitris Papastamos, Mike Frysinger, Daniel Mack, alsa-devel,
	linux-kernel, qi.wang, yong.y.wang, joel.clark, kok.howg.ewe

On Thu, Dec 22, 2011 at 05:31:08PM +0900, Tomoya MORINAGA wrote:
> 2011/12/20 Mark Brown <broonie@opensource.wolfsonmicro.com>:

> > How have you tested this code?

> No, I haven't tested at all. (Of course, I tested before ASoC.)
> I'll test the new these drivers after you say "Roughly, how to use
> ASoC framework is OK"

Don't do this unless you really have to, and if for some reason it's
needed or useful then indicate that this is what you're doing.  People
shouldn't be finding issues on review that mean that the code will never
run.  If the testing is very light that's fine but you need to make some
effort to verify that things will work.

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

* Re: [PATCH 1/3 v5] sound/soc/codecs: add LAPIS Semiconductor ML26124
@ 2011-12-22 10:50       ` Mark Brown
  0 siblings, 0 replies; 29+ messages in thread
From: Mark Brown @ 2011-12-22 10:50 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: Dimitris Papastamos, alsa-devel, Lars-Peter Clausen,
	Mike Frysinger, qi.wang, Takashi Iwai, linux-kernel, yong.y.wang,
	kok.howg.ewe, Daniel Mack, Liam Girdwood, joel.clark

On Thu, Dec 22, 2011 at 05:31:08PM +0900, Tomoya MORINAGA wrote:
> 2011/12/20 Mark Brown <broonie@opensource.wolfsonmicro.com>:

> > How have you tested this code?

> No, I haven't tested at all. (Of course, I tested before ASoC.)
> I'll test the new these drivers after you say "Roughly, how to use
> ASoC framework is OK"

Don't do this unless you really have to, and if for some reason it's
needed or useful then indicate that this is what you're doing.  People
shouldn't be finding issues on review that mean that the code will never
run.  If the testing is very light that's fine but you need to make some
effort to verify that things will work.

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

* Re: [PATCH 3/3 v2] sound/soc/lapis: add platform driver for ML7213 IOH I2S
  2011-12-22 10:39             ` Mark Brown
@ 2011-12-26  6:33               ` Tomoya MORINAGA
  -1 siblings, 0 replies; 29+ messages in thread
From: Tomoya MORINAGA @ 2011-12-26  6:33 UTC (permalink / raw)
  To: Mark Brown
  Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai, Lars-Peter Clausen,
	Dimitris Papastamos, Mike Frysinger, Daniel Mack, alsa-devel,
	linux-kernel, qi.wang, yong.y.wang, joel.clark, kok.howg.ewe

2011/12/22 Mark Brown <broonie@opensource.wolfsonmicro.com>:
> On Thu, Dec 22, 2011 at 05:10:24PM +0900, Tomoya MORINAGA wrote:
>
>> I can understand your saying.
>> However, in case of Tx interrupt, only ALMOST-EMPTY occurs. in case of
>> Rx, only Rx ALMOST-FULL occurs.
>> So as long as a user uses ML7213 I2S, your mentioned behavior is never happened.
>
> That's not what your code says...  it may be that the other interrupts
> are very rare but that's not the same thing.

I'll delete these except initialize() and transfer_complete().

>
>> They have the "filter" function immediately next to the code uses
>> "dma_request_channel".
>
> That's a rather large function...
>
>> Anyway, if you want me to change the name, I can change the name.
>> Please decide it.
>
> Rename.
I'll obey your opinion.

>
>> > You're implementing some sort of custom buffering in your driver?  That
>> > sounds terribly unidiomatic - pretty much all DMA drivers are very thin
>> > and manage to work well, I'd expect this is masking some problems in the
>> > code rather than anything else.  Can you provide more detail on what
>> > this is working around?
>
>> Not sorted but queuing only.
>> In sound/voice control system, queuing is not rare, I think.
>> If necessary, though this method is very common, I can send the method
>> of the queue.
>
> No, please describe the problem you're trying to fix.

When CPU is heavy load, this buffer is useful.
The heavy load causes delaying receiving processing.
If there is no buffer, stream sound/voice can be broken.
If there is the buffer, it can prevent the broken sound.

thanks,
tomoya

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

* Re: [PATCH 3/3 v2] sound/soc/lapis: add platform driver for ML7213 IOH I2S
@ 2011-12-26  6:33               ` Tomoya MORINAGA
  0 siblings, 0 replies; 29+ messages in thread
From: Tomoya MORINAGA @ 2011-12-26  6:33 UTC (permalink / raw)
  To: Mark Brown
  Cc: Dimitris Papastamos, alsa-devel, Lars-Peter Clausen,
	Mike Frysinger, qi.wang, Takashi Iwai, linux-kernel, yong.y.wang,
	kok.howg.ewe, Daniel Mack, Liam Girdwood, joel.clark

2011/12/22 Mark Brown <broonie@opensource.wolfsonmicro.com>:
> On Thu, Dec 22, 2011 at 05:10:24PM +0900, Tomoya MORINAGA wrote:
>
>> I can understand your saying.
>> However, in case of Tx interrupt, only ALMOST-EMPTY occurs. in case of
>> Rx, only Rx ALMOST-FULL occurs.
>> So as long as a user uses ML7213 I2S, your mentioned behavior is never happened.
>
> That's not what your code says...  it may be that the other interrupts
> are very rare but that's not the same thing.

I'll delete these except initialize() and transfer_complete().

>
>> They have the "filter" function immediately next to the code uses
>> "dma_request_channel".
>
> That's a rather large function...
>
>> Anyway, if you want me to change the name, I can change the name.
>> Please decide it.
>
> Rename.
I'll obey your opinion.

>
>> > You're implementing some sort of custom buffering in your driver?  That
>> > sounds terribly unidiomatic - pretty much all DMA drivers are very thin
>> > and manage to work well, I'd expect this is masking some problems in the
>> > code rather than anything else.  Can you provide more detail on what
>> > this is working around?
>
>> Not sorted but queuing only.
>> In sound/voice control system, queuing is not rare, I think.
>> If necessary, though this method is very common, I can send the method
>> of the queue.
>
> No, please describe the problem you're trying to fix.

When CPU is heavy load, this buffer is useful.
The heavy load causes delaying receiving processing.
If there is no buffer, stream sound/voice can be broken.
If there is the buffer, it can prevent the broken sound.

thanks,
tomoya

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

* Re: [PATCH 3/3 v2] sound/soc/lapis: add platform driver for ML7213 IOH I2S
  2011-12-26  6:33               ` Tomoya MORINAGA
@ 2011-12-26 12:12                 ` Mark Brown
  -1 siblings, 0 replies; 29+ messages in thread
From: Mark Brown @ 2011-12-26 12:12 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai, Lars-Peter Clausen,
	Dimitris Papastamos, Mike Frysinger, Daniel Mack, alsa-devel,
	linux-kernel, qi.wang, yong.y.wang, joel.clark, kok.howg.ewe

On Mon, Dec 26, 2011 at 03:33:29PM +0900, Tomoya MORINAGA wrote:
> 2011/12/22 Mark Brown <broonie@opensource.wolfsonmicro.com>:

> >> Not sorted but queuing only.
> >> In sound/voice control system, queuing is not rare, I think.
> >> If necessary, though this method is very common, I can send the method
> >> of the queue.

> > No, please describe the problem you're trying to fix.

> When CPU is heavy load, this buffer is useful.
> The heavy load causes delaying receiving processing.
> If there is no buffer, stream sound/voice can be broken.
> If there is the buffer, it can prevent the broken sound.

So you're just talking about standard underflows if the application
can't keep up?  There's *no* reason for your driver to do anything about
this, it's a really basic thing that affects all audio hardware.  Just
write a driver for the hardware.

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

* Re: [PATCH 3/3 v2] sound/soc/lapis: add platform driver for ML7213 IOH I2S
@ 2011-12-26 12:12                 ` Mark Brown
  0 siblings, 0 replies; 29+ messages in thread
From: Mark Brown @ 2011-12-26 12:12 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: Dimitris Papastamos, alsa-devel, Lars-Peter Clausen,
	Mike Frysinger, qi.wang, Takashi Iwai, linux-kernel, yong.y.wang,
	kok.howg.ewe, Daniel Mack, Liam Girdwood, joel.clark

On Mon, Dec 26, 2011 at 03:33:29PM +0900, Tomoya MORINAGA wrote:
> 2011/12/22 Mark Brown <broonie@opensource.wolfsonmicro.com>:

> >> Not sorted but queuing only.
> >> In sound/voice control system, queuing is not rare, I think.
> >> If necessary, though this method is very common, I can send the method
> >> of the queue.

> > No, please describe the problem you're trying to fix.

> When CPU is heavy load, this buffer is useful.
> The heavy load causes delaying receiving processing.
> If there is no buffer, stream sound/voice can be broken.
> If there is the buffer, it can prevent the broken sound.

So you're just talking about standard underflows if the application
can't keep up?  There's *no* reason for your driver to do anything about
this, it's a really basic thing that affects all audio hardware.  Just
write a driver for the hardware.

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

* Re: [PATCH 3/3 v2] sound/soc/lapis: add platform driver for ML7213 IOH I2S
  2011-12-26 12:12                 ` Mark Brown
@ 2011-12-27  1:25                   ` Tomoya MORINAGA
  -1 siblings, 0 replies; 29+ messages in thread
From: Tomoya MORINAGA @ 2011-12-27  1:25 UTC (permalink / raw)
  To: Mark Brown
  Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai, Lars-Peter Clausen,
	Dimitris Papastamos, Mike Frysinger, Daniel Mack, alsa-devel,
	linux-kernel, qi.wang, yong.y.wang, joel.clark, kok.howg.ewe

2011/12/26 Mark Brown <broonie@opensource.wolfsonmicro.com>:
> On Mon, Dec 26, 2011 at 03:33:29PM +0900, Tomoya MORINAGA wrote:
>> 2011/12/22 Mark Brown <broonie@opensource.wolfsonmicro.com>:
>
>> >> Not sorted but queuing only.
>> >> In sound/voice control system, queuing is not rare, I think.
>> >> If necessary, though this method is very common, I can send the method
>> >> of the queue.
>
>> > No, please describe the problem you're trying to fix.
>
>> When CPU is heavy load, this buffer is useful.
>> The heavy load causes delaying receiving processing.
>> If there is no buffer, stream sound/voice can be broken.
>> If there is the buffer, it can prevent the broken sound.
>
> So you're just talking about standard underflows if the application
> can't keep up?
No. not only underflow but overflow.

> There's *no* reason for your driver to do anything about
> this, it's a really basic thing that affects all audio hardware.  Just
> write a driver for the hardware.
In case driver layer doesn't have the function,
which part works for the under/overflow ? Application ? or nothing ?

tomoya

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

* Re: [PATCH 3/3 v2] sound/soc/lapis: add platform driver for ML7213 IOH I2S
@ 2011-12-27  1:25                   ` Tomoya MORINAGA
  0 siblings, 0 replies; 29+ messages in thread
From: Tomoya MORINAGA @ 2011-12-27  1:25 UTC (permalink / raw)
  To: Mark Brown
  Cc: Dimitris Papastamos, alsa-devel, Lars-Peter Clausen,
	Mike Frysinger, qi.wang, Takashi Iwai, linux-kernel, yong.y.wang,
	kok.howg.ewe, Daniel Mack, Liam Girdwood, joel.clark

2011/12/26 Mark Brown <broonie@opensource.wolfsonmicro.com>:
> On Mon, Dec 26, 2011 at 03:33:29PM +0900, Tomoya MORINAGA wrote:
>> 2011/12/22 Mark Brown <broonie@opensource.wolfsonmicro.com>:
>
>> >> Not sorted but queuing only.
>> >> In sound/voice control system, queuing is not rare, I think.
>> >> If necessary, though this method is very common, I can send the method
>> >> of the queue.
>
>> > No, please describe the problem you're trying to fix.
>
>> When CPU is heavy load, this buffer is useful.
>> The heavy load causes delaying receiving processing.
>> If there is no buffer, stream sound/voice can be broken.
>> If there is the buffer, it can prevent the broken sound.
>
> So you're just talking about standard underflows if the application
> can't keep up?
No. not only underflow but overflow.

> There's *no* reason for your driver to do anything about
> this, it's a really basic thing that affects all audio hardware.  Just
> write a driver for the hardware.
In case driver layer doesn't have the function,
which part works for the under/overflow ? Application ? or nothing ?

tomoya

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

* Re: [PATCH 3/3 v2] sound/soc/lapis: add platform driver for ML7213 IOH I2S
  2011-12-27  1:25                   ` Tomoya MORINAGA
@ 2011-12-27 17:33                     ` Mark Brown
  -1 siblings, 0 replies; 29+ messages in thread
From: Mark Brown @ 2011-12-27 17:33 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai, Lars-Peter Clausen,
	Dimitris Papastamos, Mike Frysinger, Daniel Mack, alsa-devel,
	linux-kernel, qi.wang, yong.y.wang, joel.clark, kok.howg.ewe

On Tue, Dec 27, 2011 at 10:25:25AM +0900, Tomoya MORINAGA wrote:
> 2011/12/26 Mark Brown <broonie@opensource.wolfsonmicro.com>:

> > So you're just talking about standard underflows if the application
> > can't keep up?

> No. not only underflow but overflow.

Same difference.

> > There's *no* reason for your driver to do anything about
> > this, it's a really basic thing that affects all audio hardware.  Just
> > write a driver for the hardware.

> In case driver layer doesn't have the function,
> which part works for the under/overflow ? Application ? or nothing ?

Your driver has to accurately describe what the hardware supports and
then the application layer is responsible for making sure it keeps up
with things.

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

* Re: [PATCH 3/3 v2] sound/soc/lapis: add platform driver for ML7213 IOH I2S
@ 2011-12-27 17:33                     ` Mark Brown
  0 siblings, 0 replies; 29+ messages in thread
From: Mark Brown @ 2011-12-27 17:33 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: Dimitris Papastamos, alsa-devel, Lars-Peter Clausen,
	Mike Frysinger, qi.wang, Takashi Iwai, linux-kernel, yong.y.wang,
	kok.howg.ewe, Daniel Mack, Liam Girdwood, joel.clark

On Tue, Dec 27, 2011 at 10:25:25AM +0900, Tomoya MORINAGA wrote:
> 2011/12/26 Mark Brown <broonie@opensource.wolfsonmicro.com>:

> > So you're just talking about standard underflows if the application
> > can't keep up?

> No. not only underflow but overflow.

Same difference.

> > There's *no* reason for your driver to do anything about
> > this, it's a really basic thing that affects all audio hardware.  Just
> > write a driver for the hardware.

> In case driver layer doesn't have the function,
> which part works for the under/overflow ? Application ? or nothing ?

Your driver has to accurately describe what the hardware supports and
then the application layer is responsible for making sure it keeps up
with things.

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

end of thread, other threads:[~2011-12-27 17:33 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-12-20  2:45 [PATCH 1/3 v5] sound/soc/codecs: add LAPIS Semiconductor ML26124 Tomoya MORINAGA
2011-12-20  2:45 ` [PATCH 2/3 v4] sound/soc/lapis: add machine driver for ML7213 Carrier Board Tomoya MORINAGA
2011-12-20 10:45   ` Mark Brown
2011-12-20 10:45     ` Mark Brown
2011-12-20  2:45 ` [PATCH 3/3 v2] sound/soc/lapis: add platform driver for ML7213 IOH I2S Tomoya MORINAGA
2011-12-20 13:23   ` Mark Brown
2011-12-20 13:23     ` Mark Brown
2011-12-21 11:22     ` Tomoya MORINAGA
2011-12-21 11:22       ` Tomoya MORINAGA
2011-12-22  0:58       ` Mark Brown
2011-12-22  8:10         ` Tomoya MORINAGA
2011-12-22 10:39           ` Mark Brown
2011-12-22 10:39             ` Mark Brown
2011-12-26  6:33             ` Tomoya MORINAGA
2011-12-26  6:33               ` Tomoya MORINAGA
2011-12-26 12:12               ` Mark Brown
2011-12-26 12:12                 ` Mark Brown
2011-12-27  1:25                 ` Tomoya MORINAGA
2011-12-27  1:25                   ` Tomoya MORINAGA
2011-12-27 17:33                   ` Mark Brown
2011-12-27 17:33                     ` Mark Brown
2011-12-20 10:40 ` [PATCH 1/3 v5] sound/soc/codecs: add LAPIS Semiconductor ML26124 Mark Brown
2011-12-20 10:40   ` Mark Brown
2011-12-22  8:31   ` Tomoya MORINAGA
2011-12-22  8:31     ` Tomoya MORINAGA
2011-12-22 10:50     ` Mark Brown
2011-12-22 10:50       ` Mark Brown
2011-12-20 10:42 ` Mark Brown
2011-12-20 10:42   ` Mark Brown

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