[PATCH 0/8] ASoC: fsi: fixup patches

[PATCH 0/8] ASoC: fsi: fixup patches

[Kuninori Morimoto]

Dear Mark

These patches are fixup for FSI driver

Kuninori Morimoto (8):
      ASoC: fsi: modify format area definition on flags
      ASoC: fsi: fixup clock inversion operation
      ASoC: fsi: remove un-used variable on fsi_dai_startup
      ASoC: fsi: remove noisy CR_FMT macro
      ASoC: fsi: modify wrong value setting order of TDM
      ASoC: fsi: Change struct fsi_regs to fsi_core
      ASoC: fsi: Add pr_err for noticing unsupported access
      ASoC: fsi: Fixup for master mode

Some of these are not so important for now.
But it is used from "FSI - HDMI" patches which I will send in future

[PATCH 1/8] ASoC: fsi: modify format area definition on flags

[Kuninori Morimoto]

There is no necessity that each bit in this area has the meaning.
This patch modify it to sequence number

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 include/sound/sh_fsi.h |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/sound/sh_fsi.h b/include/sound/sh_fsi.h
index c022736..837dad4 100644
--- a/include/sound/sh_fsi.h
+++ b/include/sound/sh_fsi.h
@@ -56,11 +56,11 @@
 #define SH_FSI_GET_OFMT(x)	((x >> 0) & SH_FSI_FMT_MASK)
 
 #define SH_FSI_FMT_MONO		(1 << 0)
-#define SH_FSI_FMT_MONO_DELAY	(1 << 1)
-#define SH_FSI_FMT_PCM		(1 << 2)
-#define SH_FSI_FMT_I2S		(1 << 3)
-#define SH_FSI_FMT_TDM		(1 << 4)
-#define SH_FSI_FMT_TDM_DELAY	(1 << 5)
+#define SH_FSI_FMT_MONO_DELAY	(2 << 0)
+#define SH_FSI_FMT_PCM		(3 << 0)
+#define SH_FSI_FMT_I2S		(4 << 0)
+#define SH_FSI_FMT_TDM		(5 << 0)
+#define SH_FSI_FMT_TDM_DELAY	(6 << 0)
 
 #define SH_FSI_IFMT_TDM_CH(x) \
 	(SH_FSI_IFMT(TDM)	| SH_FSI_SET_CH_I(x))
-- 
1.7.0.4

[PATCH 2/8] ASoC: fsi: fixup clock inversion operation

[Kuninori Morimoto, Kuninori Morimoto]

Clock inversion should be specified by each flags bit.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/sh/fsi.c |   23 +++++++++--------------
 1 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index 30765ab..814512f 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -674,20 +674,15 @@ static int fsi_dai_startup(struct snd_pcm_substream *substream,
 
 	/* clock inversion (CKG2) */
 	data = 0;
-	switch (SH_FSI_INVERSION_MASK & flags) {
-	case SH_FSI_LRM_INV:
-		data = 1 << 12;
-		break;
-	case SH_FSI_BRM_INV:
-		data = 1 << 8;
-		break;
-	case SH_FSI_LRS_INV:
-		data = 1 << 4;
-		break;
-	case SH_FSI_BRS_INV:
-		data = 1 << 0;
-		break;
-	}
+	if (SH_FSI_LRM_INV & flags)
+		data |= 1 << 12;
+	if (SH_FSI_BRM_INV & flags)
+		data |= 1 << 8;
+	if (SH_FSI_LRS_INV & flags)
+		data |= 1 << 4;
+	if (SH_FSI_BRS_INV & flags)
+		data |= 1 << 0;
+
 	fsi_reg_write(fsi, CKG2, data);
 
 	/* do fmt, di fmt */
-- 
1.7.0.4

[PATCH 3/8] ASoC: fsi: remove un-used variable on fsi_dai_startup

[Kuninori Morimoto, Kuninori Morimoto]

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/sh/fsi.c |    7 -------
 1 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index 814512f..db21f42 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -653,7 +653,6 @@ static int fsi_dai_startup(struct snd_pcm_substream *substream,
 			   struct snd_soc_dai *dai)
 {
 	struct fsi_priv *fsi = fsi_get_priv(substream);
-	const char *msg;
 	u32 flags = fsi_get_info_flags(fsi);
 	u32 fmt;
 	u32 reg;
@@ -691,33 +690,27 @@ static int fsi_dai_startup(struct snd_pcm_substream *substream,
 	fmt = is_play ? SH_FSI_GET_OFMT(flags) : SH_FSI_GET_IFMT(flags);
 	switch (fmt) {
 	case SH_FSI_FMT_MONO:
-		msg = "MONO";
 		data = CR_FMT(CR_MONO);
 		fsi->chan = 1;
 		break;
 	case SH_FSI_FMT_MONO_DELAY:
-		msg = "MONO Delay";
 		data = CR_FMT(CR_MONO_D);
 		fsi->chan = 1;
 		break;
 	case SH_FSI_FMT_PCM:
-		msg = "PCM";
 		data = CR_FMT(CR_PCM);
 		fsi->chan = 2;
 		break;
 	case SH_FSI_FMT_I2S:
-		msg = "I2S";
 		data = CR_FMT(CR_I2S);
 		fsi->chan = 2;
 		break;
 	case SH_FSI_FMT_TDM:
-		msg = "TDM";
 		data = CR_FMT(CR_TDM) | (fsi->chan - 1);
 		fsi->chan = is_play ?
 			SH_FSI_GET_CH_O(flags) : SH_FSI_GET_CH_I(flags);
 		break;
 	case SH_FSI_FMT_TDM_DELAY:
-		msg = "TDM Delay";
 		data = CR_FMT(CR_TDM_D) | (fsi->chan - 1);
 		fsi->chan = is_play ?
 			SH_FSI_GET_CH_O(flags) : SH_FSI_GET_CH_I(flags);
-- 
1.7.0.4

[PATCH 4/8] ASoC: fsi: remove noisy CR_FMT macro

[Kuninori Morimoto, Kuninori Morimoto]

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/sh/fsi.c |   25 ++++++++++++-------------
 1 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index db21f42..efc0299 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -48,13 +48,12 @@
 
 /* DO_FMT */
 /* DI_FMT */
-#define CR_FMT(param) ((param) << 4)
-# define CR_MONO	0x0
-# define CR_MONO_D	0x1
-# define CR_PCM		0x2
-# define CR_I2S		0x3
-# define CR_TDM		0x4
-# define CR_TDM_D	0x5
+#define CR_MONO		(0x0 << 4)
+#define CR_MONO_D	(0x1 << 4)
+#define CR_PCM		(0x2 << 4)
+#define CR_I2S		(0x3 << 4)
+#define CR_TDM		(0x4 << 4)
+#define CR_TDM_D	(0x5 << 4)
 
 /* DOFF_CTL */
 /* DIFF_CTL */
@@ -690,28 +689,28 @@ static int fsi_dai_startup(struct snd_pcm_substream *substream,
 	fmt = is_play ? SH_FSI_GET_OFMT(flags) : SH_FSI_GET_IFMT(flags);
 	switch (fmt) {
 	case SH_FSI_FMT_MONO:
-		data = CR_FMT(CR_MONO);
+		data = CR_MONO;
 		fsi->chan = 1;
 		break;
 	case SH_FSI_FMT_MONO_DELAY:
-		data = CR_FMT(CR_MONO_D);
+		data = CR_MONO_D;
 		fsi->chan = 1;
 		break;
 	case SH_FSI_FMT_PCM:
-		data = CR_FMT(CR_PCM);
+		data = CR_PCM;
 		fsi->chan = 2;
 		break;
 	case SH_FSI_FMT_I2S:
-		data = CR_FMT(CR_I2S);
+		data = CR_I2S;
 		fsi->chan = 2;
 		break;
 	case SH_FSI_FMT_TDM:
-		data = CR_FMT(CR_TDM) | (fsi->chan - 1);
+		data = CR_TDM | (fsi->chan - 1);
 		fsi->chan = is_play ?
 			SH_FSI_GET_CH_O(flags) : SH_FSI_GET_CH_I(flags);
 		break;
 	case SH_FSI_FMT_TDM_DELAY:
-		data = CR_FMT(CR_TDM_D) | (fsi->chan - 1);
+		data = CR_TDM_D | (fsi->chan - 1);
 		fsi->chan = is_play ?
 			SH_FSI_GET_CH_O(flags) : SH_FSI_GET_CH_I(flags);
 		break;
-- 
1.7.0.4

[PATCH 5/8] ASoC: fsi: modify wrong value setting order of TDM

[Kuninori Morimoto, Kuninori Morimoto]

channel size should be set before setting register value

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/sh/fsi.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index efc0299..28aae0d 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -705,14 +705,14 @@ static int fsi_dai_startup(struct snd_pcm_substream *substream,
 		fsi->chan = 2;
 		break;
 	case SH_FSI_FMT_TDM:
-		data = CR_TDM | (fsi->chan - 1);
 		fsi->chan = is_play ?
 			SH_FSI_GET_CH_O(flags) : SH_FSI_GET_CH_I(flags);
+		data = CR_TDM | (fsi->chan - 1);
 		break;
 	case SH_FSI_FMT_TDM_DELAY:
-		data = CR_TDM_D | (fsi->chan - 1);
 		fsi->chan = is_play ?
 			SH_FSI_GET_CH_O(flags) : SH_FSI_GET_CH_I(flags);
+		data = CR_TDM_D | (fsi->chan - 1);
 		break;
 	default:
 		dev_err(dai->dev, "unknown format.\n");
-- 
1.7.0.4

[PATCH 6/8] ASoC: fsi: Change struct fsi_regs to fsi_core

[Kuninori Morimoto, Kuninori Morimoto]

Many registers which were grouped by category were added in FSI2.
To make easy to switch FSI/FSI2, fsi_core was added instead of fsi_regs.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/sh/fsi.c |   36 ++++++++++++++++++++++--------------
 1 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index 28aae0d..1693be4 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -111,7 +111,9 @@ struct fsi_priv {
 	int periods;
 };
 
-struct fsi_regs {
+struct fsi_core {
+	int ver;
+
 	u32 int_st;
 	u32 iemsk;
 	u32 imsk;
@@ -122,7 +124,7 @@ struct fsi_master {
 	int irq;
 	struct fsi_priv fsia;
 	struct fsi_priv fsib;
-	struct fsi_regs *regs;
+	struct fsi_core *core;
 	struct sh_fsi_platform_info *info;
 	spinlock_t lock;
 };
@@ -339,8 +341,8 @@ static void fsi_irq_enable(struct fsi_priv *fsi, int is_play)
 	u32 data = fsi_port_ab_io_bit(fsi, is_play);
 	struct fsi_master *master = fsi_get_master(fsi);
 
-	fsi_master_mask_set(master, master->regs->imsk,  data, data);
-	fsi_master_mask_set(master, master->regs->iemsk, data, data);
+	fsi_master_mask_set(master, master->core->imsk,  data, data);
+	fsi_master_mask_set(master, master->core->iemsk, data, data);
 }
 
 static void fsi_irq_disable(struct fsi_priv *fsi, int is_play)
@@ -348,18 +350,18 @@ static void fsi_irq_disable(struct fsi_priv *fsi, int is_play)
 	u32 data = fsi_port_ab_io_bit(fsi, is_play);
 	struct fsi_master *master = fsi_get_master(fsi);
 
-	fsi_master_mask_set(master, master->regs->imsk,  data, 0);
-	fsi_master_mask_set(master, master->regs->iemsk, data, 0);
+	fsi_master_mask_set(master, master->core->imsk,  data, 0);
+	fsi_master_mask_set(master, master->core->iemsk, data, 0);
 }
 
 static u32 fsi_irq_get_status(struct fsi_master *master)
 {
-	return fsi_master_read(master, master->regs->int_st);
+	return fsi_master_read(master, master->core->int_st);
 }
 
 static void fsi_irq_clear_all_status(struct fsi_master *master)
 {
-	fsi_master_write(master, master->regs->int_st, 0x0000000);
+	fsi_master_write(master, master->core->int_st, 0);
 }
 
 static void fsi_irq_clear_status(struct fsi_priv *fsi)
@@ -371,7 +373,7 @@ static void fsi_irq_clear_status(struct fsi_priv *fsi)
 	data |= fsi_port_ab_io_bit(fsi, 1);
 
 	/* clear interrupt factor */
-	fsi_master_mask_set(master, master->regs->int_st, data, 0);
+	fsi_master_mask_set(master, master->core->int_st, data, 0);
 }
 
 /************************************************************************
@@ -987,7 +989,7 @@ static int fsi_probe(struct platform_device *pdev)
 	master->fsia.master	= master;
 	master->fsib.base	= master->base + 0x40;
 	master->fsib.master	= master;
-	master->regs		= (struct fsi_regs *)id_entry->driver_data;
+	master->core		= (struct fsi_core *)id_entry->driver_data;
 	spin_lock_init(&master->lock);
 
 	pm_runtime_enable(&pdev->dev);
@@ -1068,21 +1070,27 @@ static struct dev_pm_ops fsi_pm_ops = {
 	.runtime_resume		= fsi_runtime_nop,
 };
 
-static struct fsi_regs fsi_regs = {
+static struct fsi_core fsi1_core = {
+	.ver	= 1,
+
+	/* Interrupt */
 	.int_st	= INT_ST,
 	.iemsk	= IEMSK,
 	.imsk	= IMSK,
 };
 
-static struct fsi_regs fsi2_regs = {
+static struct fsi_core fsi2_core = {
+	.ver	= 2,
+
+	/* Interrupt */
 	.int_st	= CPU_INT_ST,
 	.iemsk	= CPU_IEMSK,
 	.imsk	= CPU_IMSK,
 };
 
 static struct platform_device_id fsi_id_table[] = {
-	{ "sh_fsi",	(kernel_ulong_t)&fsi_regs },
-	{ "sh_fsi2",	(kernel_ulong_t)&fsi2_regs },
+	{ "sh_fsi",	(kernel_ulong_t)&fsi1_core },
+	{ "sh_fsi2",	(kernel_ulong_t)&fsi2_core },
 };
 
 static struct platform_driver fsi_driver = {
-- 
1.7.0.4

[PATCH 7/8] ASoC: fsi: Add pr_err for noticing unsupported access

[Kuninori Morimoto, Kuninori Morimoto]

This patch didn't use dev_err,
because it is difficult to get struct device here.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/sh/fsi.c |   24 ++++++++++++++++++------
 1 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index 1693be4..e551ca4 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -161,24 +161,30 @@ static void __fsi_reg_mask_set(u32 reg, u32 mask, u32 data)
 
 static void fsi_reg_write(struct fsi_priv *fsi, u32 reg, u32 data)
 {
-	if (reg > REG_END)
+	if (reg > REG_END) {
+		pr_err("fsi: register access err (%s)\n", __func__);
 		return;
+	}
 
 	__fsi_reg_write((u32)(fsi->base + reg), data);
 }
 
 static u32 fsi_reg_read(struct fsi_priv *fsi, u32 reg)
 {
-	if (reg > REG_END)
+	if (reg > REG_END) {
+		pr_err("fsi: register access err (%s)\n", __func__);
 		return 0;
+	}
 
 	return __fsi_reg_read((u32)(fsi->base + reg));
 }
 
 static void fsi_reg_mask_set(struct fsi_priv *fsi, u32 reg, u32 mask, u32 data)
 {
-	if (reg > REG_END)
+	if (reg > REG_END) {
+		pr_err("fsi: register access err (%s)\n", __func__);
 		return;
+	}
 
 	__fsi_reg_mask_set((u32)(fsi->base + reg), mask, data);
 }
@@ -188,8 +194,10 @@ static void fsi_master_write(struct fsi_master *master, u32 reg, u32 data)
 	unsigned long flags;
 
 	if ((reg < MREG_START) ||
-	    (reg > MREG_END))
+	    (reg > MREG_END)) {
+		pr_err("fsi: register access err (%s)\n", __func__);
 		return;
+	}
 
 	spin_lock_irqsave(&master->lock, flags);
 	__fsi_reg_write((u32)(master->base + reg), data);
@@ -202,8 +210,10 @@ static u32 fsi_master_read(struct fsi_master *master, u32 reg)
 	unsigned long flags;
 
 	if ((reg < MREG_START) ||
-	    (reg > MREG_END))
+	    (reg > MREG_END)) {
+		pr_err("fsi: register access err (%s)\n", __func__);
 		return 0;
+	}
 
 	spin_lock_irqsave(&master->lock, flags);
 	ret = __fsi_reg_read((u32)(master->base + reg));
@@ -218,8 +228,10 @@ static void fsi_master_mask_set(struct fsi_master *master,
 	unsigned long flags;
 
 	if ((reg < MREG_START) ||
-	    (reg > MREG_END))
+	    (reg > MREG_END)) {
+		pr_err("fsi: register access err (%s)\n", __func__);
 		return;
+	}
 
 	spin_lock_irqsave(&master->lock, flags);
 	__fsi_reg_mask_set((u32)(master->base + reg), mask, data);
-- 
1.7.0.4

[PATCH 8/8] ASoC: fsi: Fixup for master mode

[Kuninori Morimoto]

This patch add hw_params to snd_soc_dai_ops,
because board specific set_rate is needed
when FSI was used as master mode.

This patch remove fsi_clk_ctrl from fsi_dai_startup,
because clock should be disabled before set_rate.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 include/sound/sh_fsi.h |   32 ++++++++++++++++
 sound/soc/sh/fsi.c     |   98 +++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 124 insertions(+), 6 deletions(-)

diff --git a/include/sound/sh_fsi.h b/include/sound/sh_fsi.h
index 837dad4..9d436e1 100644
--- a/include/sound/sh_fsi.h
+++ b/include/sound/sh_fsi.h
@@ -72,9 +72,41 @@
 #define SH_FSI_OFMT_TDM_DELAY_CH(x) \
 	(SH_FSI_OFMT(TDM_DELAY)	| SH_FSI_SET_CH_O(x))
 
+
+/*
+ * set_rate return value
+ *
+ * see ACKMD/BPFMD on
+ *     ACK_MD (FSI2)
+ *     CKG1   (FSI)
+ *
+ * err:  return value < 0
+ *
+ * 0x-00000AB
+ *
+ * A:  ACKMD value
+ * B:  BPFMD value
+ */
+
+#define SH_FSI_ACKMD_MASK	(0xF << 0)
+#define SH_FSI_ACKMD_512	(1 << 0)
+#define SH_FSI_ACKMD_256	(2 << 0)
+#define SH_FSI_ACKMD_128	(3 << 0)
+#define SH_FSI_ACKMD_64		(4 << 0)
+#define SH_FSI_ACKMD_32		(5 << 0)
+
+#define SH_FSI_BPFMD_MASK	(0xF << 4)
+#define SH_FSI_BPFMD_512	(1 << 4)
+#define SH_FSI_BPFMD_256	(2 << 4)
+#define SH_FSI_BPFMD_128	(3 << 4)
+#define SH_FSI_BPFMD_64		(4 << 4)
+#define SH_FSI_BPFMD_32		(5 << 4)
+#define SH_FSI_BPFMD_16		(6 << 4)
+
 struct sh_fsi_platform_info {
 	unsigned long porta_flags;
 	unsigned long portb_flags;
+	int (*set_rate)(int is_porta, int rate); /* for master mode */
 };
 
 extern struct snd_soc_dai fsi_soc_dai[2];
diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index 2377307..aab0d87 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -74,6 +74,10 @@
 #define ERR_UNDER	0x00000001
 #define ST_ERR		(ERR_OVER | ERR_UNDER)
 
+/* CKG1 */
+#define ACKMD_MASK	0x00007000
+#define BPFMD_MASK	0x00000700
+
 /* CLK_RST */
 #define B_CLK		0x00000010
 #define A_CLK		0x00000001
@@ -743,12 +747,6 @@ static int fsi_dai_startup(struct snd_pcm_substream *substream,
 	}
 	fsi_reg_write(fsi, reg, data);
 
-	/*
-	 * clear clk reset if master mode
-	 */
-	if (is_master)
-		fsi_clk_ctrl(fsi, 1);
-
 	/* irq clear */
 	fsi_irq_disable(fsi, is_play);
 	fsi_irq_clear_status(fsi);
@@ -795,10 +793,98 @@ static int fsi_dai_trigger(struct snd_pcm_substream *substream, int cmd,
 	return ret;
 }
 
+static int fsi_dai_hw_params(struct snd_pcm_substream *substream,
+			     struct snd_pcm_hw_params *params,
+			     struct snd_soc_dai *dai)
+{
+	struct fsi_priv *fsi = fsi_get_priv(substream);
+	struct fsi_master *master = fsi_get_master(fsi);
+	int (*set_rate)(int is_porta, int rate) = master->info->set_rate;
+	int fsi_ver = master->core->ver;
+	int is_play = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK);
+	int ret;
+
+	/* if slave mode, set_rate is not needed */
+	if (!fsi_is_master_mode(fsi, is_play))
+		return 0;
+
+	/* it is error if no set_rate */
+	if (!set_rate)
+		return -EIO;
+
+	/* clock stop */
+	pm_runtime_put_sync(dai->dev);
+	fsi_clk_ctrl(fsi, 0);
+
+	ret = set_rate(fsi_is_port_a(fsi), params_rate(params));
+	if (ret > 0) {
+		u32 data = 0;
+
+		switch (ret & SH_FSI_ACKMD_MASK) {
+		default:
+			/* FALL THROUGH */
+		case SH_FSI_ACKMD_512:
+			data |= (0x0 << 12);
+			break;
+		case SH_FSI_ACKMD_256:
+			data |= (0x1 << 12);
+			break;
+		case SH_FSI_ACKMD_128:
+			data |= (0x2 << 12);
+			break;
+		case SH_FSI_ACKMD_64:
+			data |= (0x3 << 12);
+			break;
+		case SH_FSI_ACKMD_32:
+			if (fsi_ver < 2)
+				dev_err(dai->dev, "unsupported ACKMD\n");
+			else
+				data |= (0x4 << 12);
+			break;
+		}
+
+		switch (ret & SH_FSI_BPFMD_MASK) {
+		default:
+			/* FALL THROUGH */
+		case SH_FSI_BPFMD_32:
+			data |= (0x0 << 8);
+			break;
+		case SH_FSI_BPFMD_64:
+			data |= (0x1 << 8);
+			break;
+		case SH_FSI_BPFMD_128:
+			data |= (0x2 << 8);
+			break;
+		case SH_FSI_BPFMD_256:
+			data |= (0x3 << 8);
+			break;
+		case SH_FSI_BPFMD_512:
+			data |= (0x4 << 8);
+			break;
+		case SH_FSI_BPFMD_16:
+			if (fsi_ver < 2)
+				dev_err(dai->dev, "unsupported ACKMD\n");
+			else
+				data |= (0x7 << 8);
+			break;
+		}
+
+		fsi_reg_mask_set(fsi, CKG1, (ACKMD_MASK | BPFMD_MASK) , data);
+		udelay(10);
+		fsi_clk_ctrl(fsi, 1);
+		ret = 0;
+	}
+	pm_runtime_get_sync(dai->dev);
+
+	return ret;
+
+}
+
 static struct snd_soc_dai_ops fsi_dai_ops = {
 	.startup	= fsi_dai_startup,
 	.shutdown	= fsi_dai_shutdown,
 	.trigger	= fsi_dai_trigger,
+	.hw_params	= fsi_dai_hw_params,
 };
 
 /************************************************************************
-- 
1.7.0.4

Re: [PATCH 1/8] ASoC: fsi: modify format area definition on flags

[Mark Brown]

On Mon, Jul 12, 2010 at 02:55:53PM +0900, Kuninori Morimoto wrote:
> +#define SH_FSI_FMT_MONO_DELAY	(2 << 0)
> +#define SH_FSI_FMT_PCM		(3 << 0)
> +#define SH_FSI_FMT_I2S		(4 << 0)
> +#define SH_FSI_FMT_TDM		(5 << 0)
> +#define SH_FSI_FMT_TDM_DELAY	(6 << 0)

Wouldn't it be clearer to just define these as straight numbers?

Re: [PATCH 5/8] ASoC: fsi: modify wrong value setting order of TDM

[Mark Brown]

On Mon, Jul 12, 2010 at 02:56:17PM +0900, Kuninori Morimoto wrote:
> channel size should be set before setting register value

So, this looks like a bug fix which should go into 2.6.35 but...

>  	case SH_FSI_FMT_TDM:
> -		data = CR_TDM | (fsi->chan - 1);
>  		fsi->chan = is_play ?
>  			SH_FSI_GET_CH_O(flags) : SH_FSI_GET_CH_I(flags);
> +		data = CR_TDM | (fsi->chan - 1);
>  		break;

...it depends on your earlier patch to change the CR_ macros which is
just a coding style tweak and so should wait for 2.6.36.

When preparing patches it's good to separate out things like cleanups
and new features from bug fixes - the bug fixes we want to get into the
next release if possible, but other things are supposed to wait for the
next kernel development cycle.

Re: [PATCH 1/8] ASoC: fsi: modify format area definition on flags

[Kuninori Morimoto]

Dear Mark

Thank you for your checking patch.

At first, I will re-send v2 patches as "bug fix" and "add XXX".

> > +#define SH_FSI_FMT_MONO_DELAY	(2 << 0)
> > +#define SH_FSI_FMT_PCM		(3 << 0)
> > +#define SH_FSI_FMT_I2S		(4 << 0)
> > +#define SH_FSI_FMT_TDM		(5 << 0)
> > +#define SH_FSI_FMT_TDM_DELAY	(6 << 0)
> 
> Wouldn't it be clearer to just define these as straight numbers?

Sorry. Do you mean (1 << 0) is needed ?
(1 << 0) is not changed from old one
 
--------------------------------
 #define SH_FSI_FMT_MONO		(1 << 0)  <===
-#define SH_FSI_FMT_MONO_DELAY	(1 << 1)
-#define SH_FSI_FMT_PCM		(1 << 2)
-#define SH_FSI_FMT_I2S		(1 << 3)
-#define SH_FSI_FMT_TDM		(1 << 4)
-#define SH_FSI_FMT_TDM_DELAY	(1 << 5)
+#define SH_FSI_FMT_MONO_DELAY	(2 << 0)
+#define SH_FSI_FMT_PCM		(3 << 0)
+#define SH_FSI_FMT_I2S		(4 << 0)
+#define SH_FSI_FMT_TDM		(5 << 0)
+#define SH_FSI_FMT_TDM_DELAY	(6 << 0)
--------------------------------


Best regards
--
Kuninori Morimoto

Re: [PATCH 1/8] ASoC: fsi: modify format area definition on flags

[Mark Brown]

On Tue, Jul 13, 2010 at 10:22:35AM +0900, Kuninori Morimoto wrote:

> > > +#define SH_FSI_FMT_TDM_DELAY	(6 << 0)

> > Wouldn't it be clearer to just define these as straight numbers?

> Sorry. Do you mean (1 << 0) is needed ?
> (1 << 0) is not changed from old one

I mean the << 0 is not needed - a shift by zero bits is just the
original number.

Re: [PATCH 1/8] ASoC: fsi: modify format area definition on flags

[Kuninori Morimoto]

Dear Mark

> > Sorry. Do you mean (1 << 0) is needed ?
> > (1 << 0) is not changed from old one
> 
> I mean the << 0 is not needed - a shift by zero bits is just the
> original number.

I understand. Thank you
I sent v2 patch of this. but please drop it.
I send v3 patch.

Best regards
--
Kuninori Morimoto