[PATCH 0/9] ASoC: sh: fsi: tidyup and cleanup FSI

[PATCH 0/9] ASoC: sh: fsi: tidyup and cleanup FSI

[Kuninori Morimoto]

Dear Mark, Liam

Cc Simon, yoshii-san, Magnus

These are tidyup and cleanup patches for FSI

Kuninori Morimoto (9):
      ASoC: sh: fsi: tidyup parameter of fsi_stream_push
      ASoC: sh: fsi: tidyup unclear variable naming
      ASoC: sh: fsi: remove pm_runtime from fsi_dai_set_fmt.
      ASoC: sh: fsi: make sure fsi_stream_push/pop access by spin lock
      ASoC: sh: fsi: Add fsi_set_master_clk
      ASoC: sh: fsi: irq control moves to fsi_port_start/stop
      ASoC: sh: fsi: Add fsi_hw_startup/shutdown
      ASoC: sh: fsi: remove fsi_module_init/kill
      ASoC: sh: fsi: cleanup suspend/resume

#1 and #2 are tidyup for fsi_fifo_data_ctrl.
The variables and sub-functions were unclear nameing.
So, it was difficult to understand the detail of calculation.
Thank you for pointing it yoshii-san.

#3 - #9 are clean up for RuntimePM.
Current FSI driver was wrong for RuntimePM.
these patches fixup for RuntimePM and
clean up for suspend/resume.
last patch remove noisy saved_xxx variable.

Best regards
--
Kuninori Morimoto
 

[PATCH 1/9] ASoC: sh: fsi: tidyup parameter of fsi_stream_push

[Kuninori Morimoto]

It is possible to create buff_len and period_len
from substream->runtime.
This patch is preparation of tidyup unclear variable naming patch.

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

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index 4a9da6b..7025885 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -344,16 +344,15 @@ static u32 fsi_get_port_shift(struct fsi_priv *fsi, int is_play)
 
 static void fsi_stream_push(struct fsi_priv *fsi,
 			    int is_play,
-			    struct snd_pcm_substream *substream,
-			    u32 buffer_len,
-			    u32 period_len)
+			    struct snd_pcm_substream *substream)
 {
 	struct fsi_stream *io = fsi_get_stream(fsi, is_play);
+	struct snd_pcm_runtime *runtime = substream->runtime;
 
 	io->substream	= substream;
-	io->buff_len	= buffer_len;
+	io->buff_len	= frames_to_bytes(runtime, runtime->buffer_size);
 	io->buff_offset	= 0;
-	io->period_len	= period_len;
+	io->period_len	= frames_to_bytes(runtime, runtime->period_size);
 	io->period_num	= 0;
 	io->oerr_num	= -1; /* ignore 1st err */
 	io->uerr_num	= -1; /* ignore 1st err */
@@ -844,15 +843,12 @@ static int fsi_dai_trigger(struct snd_pcm_substream *substream, int cmd,
 			   struct snd_soc_dai *dai)
 {
 	struct fsi_priv *fsi = fsi_get_priv(substream);
-	struct snd_pcm_runtime *runtime = substream->runtime;
 	int is_play = fsi_is_play(substream);
 	int ret = 0;
 
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
-		fsi_stream_push(fsi, is_play, substream,
-				frames_to_bytes(runtime, runtime->buffer_size),
-				frames_to_bytes(runtime, runtime->period_size));
+		fsi_stream_push(fsi, is_play, substream);
 		ret = is_play ? fsi_data_push(fsi) : fsi_data_pop(fsi);
 		fsi_irq_enable(fsi, is_play);
 		fsi_port_start(fsi);
-- 
1.7.1

[PATCH 2/9] ASoC: sh: fsi: tidyup unclear variable naming

[Kuninori Morimoto]

Some variables on this driver were a unclear naming,
and were different unit (byte, frame, sample).
And some functions had wrong name
(ex. it returned "sample width" but name was "fsi_get_frame_width").
This patch tidy-up this issue, and the minimum unit become "sample".
Special thanks to Takashi YOSHII.

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

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index 7025885..14fed47 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -118,10 +118,38 @@ typedef int (*set_rate_func)(struct device *dev, int is_porta, int rate, int ena
 /*
  * FSI driver use below type name for variable
  *
- * xxx_len	: data length
- * xxx_width	: data width
- * xxx_offset	: data offset
  * xxx_num	: number of data
+ * xxx_pos	: position of data
+ * xxx_capa	: capacity of data
+ */
+
+/*
+ *	period/frame/sample image
+ *
+ * ex) PCM (2ch)
+ *
+ * period pos					   period pos
+ *   [n]					     [n + 1]
+ *   |<-------------------- period--------------------->|
+ * ==|============================================ ... =|==
+ *   |							|
+ *   ||<-----  frame ----->|<------ frame ----->|  ...	|
+ *   |+--------------------+--------------------+- ...	|
+ *   ||[ sample ][ sample ]|[ sample ][ sample ]|  ...	|
+ *   |+--------------------+--------------------+- ...	|
+ * ==|============================================ ... =|==
+ */
+
+/*
+ *	FSI FIFO image
+ *
+ *	|	     |
+ *	|	     |
+ *	| [ sample ] |
+ *	| [ sample ] |
+ *	| [ sample ] |
+ *	| [ sample ] |
+ *		--> go to codecs
  */
 
 /*
@@ -131,12 +159,11 @@ typedef int (*set_rate_func)(struct device *dev, int is_porta, int rate, int ena
 struct fsi_stream {
 	struct snd_pcm_substream *substream;
 
-	int fifo_max_num;
-
-	int buff_offset;
-	int buff_len;
-	int period_len;
-	int period_num;
+	int fifo_sample_capa;	/* sample capacity of FSI FIFO */
+	int buff_sample_capa;	/* sample capacity of ALSA buffer */
+	int buff_sample_pos;	/* sample position of ALSA buffer */
+	int period_samples;	/* sample number / 1 period */
+	int period_pos;		/* current period position */
 
 	int uerr_num;
 	int oerr_num;
@@ -342,6 +369,16 @@ static u32 fsi_get_port_shift(struct fsi_priv *fsi, int is_play)
 	return shift;
 }
 
+static int fsi_frame2sample(struct fsi_priv *fsi, int frames)
+{
+	return frames * fsi->chan_num;
+}
+
+static int fsi_sample2frame(struct fsi_priv *fsi, int samples)
+{
+	return samples / fsi->chan_num;
+}
+
 static void fsi_stream_push(struct fsi_priv *fsi,
 			    int is_play,
 			    struct snd_pcm_substream *substream)
@@ -350,10 +387,10 @@ static void fsi_stream_push(struct fsi_priv *fsi,
 	struct snd_pcm_runtime *runtime = substream->runtime;
 
 	io->substream	= substream;
-	io->buff_len	= frames_to_bytes(runtime, runtime->buffer_size);
-	io->buff_offset	= 0;
-	io->period_len	= frames_to_bytes(runtime, runtime->period_size);
-	io->period_num	= 0;
+	io->buff_sample_capa	= fsi_frame2sample(fsi, runtime->buffer_size);
+	io->buff_sample_pos	= 0;
+	io->period_samples	= fsi_frame2sample(fsi, runtime->period_size);
+	io->period_pos		= 0;
 	io->oerr_num	= -1; /* ignore 1st err */
 	io->uerr_num	= -1; /* ignore 1st err */
 }
@@ -371,47 +408,26 @@ static void fsi_stream_pop(struct fsi_priv *fsi, int is_play)
 		dev_err(dai->dev, "under_run = %d\n", io->uerr_num);
 
 	io->substream	= NULL;
-	io->buff_len	= 0;
-	io->buff_offset	= 0;
-	io->period_len	= 0;
-	io->period_num	= 0;
+	io->buff_sample_capa	= 0;
+	io->buff_sample_pos	= 0;
+	io->period_samples	= 0;
+	io->period_pos		= 0;
 	io->oerr_num	= 0;
 	io->uerr_num	= 0;
 }
 
-static int fsi_get_fifo_data_num(struct fsi_priv *fsi, int is_play)
+static int fsi_get_current_fifo_samples(struct fsi_priv *fsi, int is_play)
 {
 	u32 status;
-	int data_num;
+	int frames;
 
 	status = is_play ?
 		fsi_reg_read(fsi, DOFF_ST) :
 		fsi_reg_read(fsi, DIFF_ST);
 
-	data_num = 0x1ff & (status >> 8);
-	data_num *= fsi->chan_num;
-
-	return data_num;
-}
-
-static int fsi_len2num(int len, int width)
-{
-	return len / width;
-}
+	frames = 0x1ff & (status >> 8);
 
-#define fsi_num2offset(a, b) fsi_num2len(a, b)
-static int fsi_num2len(int num, int width)
-{
-	return num * width;
-}
-
-static int fsi_get_frame_width(struct fsi_priv *fsi, int is_play)
-{
-	struct fsi_stream *io = fsi_get_stream(fsi, is_play);
-	struct snd_pcm_substream *substream = io->substream;
-	struct snd_pcm_runtime *runtime = substream->runtime;
-
-	return frames_to_bytes(runtime, 1) / fsi->chan_num;
+	return fsi_frame2sample(fsi, frames);
 }
 
 static void fsi_count_fifo_err(struct fsi_priv *fsi)
@@ -443,8 +459,10 @@ static u8 *fsi_dma_get_area(struct fsi_priv *fsi, int stream)
 {
 	int is_play = fsi_stream_is_play(stream);
 	struct fsi_stream *io = fsi_get_stream(fsi, is_play);
+	struct snd_pcm_runtime *runtime = io->substream->runtime;
 
-	return io->substream->runtime->dma_area + io->buff_offset;
+	return runtime->dma_area +
+		samples_to_bytes(runtime, io->buff_sample_pos);
 }
 
 static void fsi_dma_soft_push16(struct fsi_priv *fsi, int num)
@@ -602,13 +620,14 @@ static void fsi_fifo_init(struct fsi_priv *fsi,
 	struct fsi_master *master = fsi_get_master(fsi);
 	struct fsi_stream *io = fsi_get_stream(fsi, is_play);
 	u32 shift, i;
+	int frame_capa;
 
 	/* get on-chip RAM capacity */
 	shift = fsi_master_read(master, FIFO_SZ);
 	shift >>= fsi_get_port_shift(fsi, is_play);
 	shift &= FIFO_SZ_MASK;
-	io->fifo_max_num = 256 << shift;
-	dev_dbg(dai->dev, "fifo = %d words\n", io->fifo_max_num);
+	frame_capa = 256 << shift;
+	dev_dbg(dai->dev, "fifo = %d words\n", frame_capa);
 
 	/*
 	 * The maximum number of sample data varies depending
@@ -630,9 +649,11 @@ static void fsi_fifo_init(struct fsi_priv *fsi,
 	 * 8 channels:  32 ( 32 x 8 = 256)
 	 */
 	for (i = 1; i < fsi->chan_num; i <<= 1)
-		io->fifo_max_num >>= 1;
+		frame_capa >>= 1;
 	dev_dbg(dai->dev, "%d channel %d store\n",
-		fsi->chan_num, io->fifo_max_num);
+		fsi->chan_num, frame_capa);
+
+	io->fifo_sample_capa = fsi_frame2sample(fsi, frame_capa);
 
 	/*
 	 * set interrupt generation factor
@@ -653,10 +674,10 @@ static int fsi_fifo_data_ctrl(struct fsi_priv *fsi, int stream)
 	struct snd_pcm_substream *substream = NULL;
 	int is_play = fsi_stream_is_play(stream);
 	struct fsi_stream *io = fsi_get_stream(fsi, is_play);
-	int data_residue_num;
-	int data_num;
-	int data_num_max;
-	int ch_width;
+	int sample_residues;
+	int sample_width;
+	int samples;
+	int samples_max;
 	int over_period;
 	void (*fn)(struct fsi_priv *fsi, int size);
 
@@ -672,36 +693,35 @@ static int fsi_fifo_data_ctrl(struct fsi_priv *fsi, int stream)
 	/* FSI FIFO has limit.
 	 * So, this driver can not send periods data at a time
 	 */
-	if (io->buff_offset >=
-	    fsi_num2offset(io->period_num + 1, io->period_len)) {
+	if (io->buff_sample_pos >=
+	    io->period_samples * (io->period_pos + 1)) {
 
 		over_period = 1;
-		io->period_num = (io->period_num + 1) % runtime->periods;
+		io->period_pos = (io->period_pos + 1) % runtime->periods;
 
-		if (0 == io->period_num)
-			io->buff_offset = 0;
+		if (0 == io->period_pos)
+			io->buff_sample_pos = 0;
 	}
 
-	/* get 1 channel data width */
-	ch_width = fsi_get_frame_width(fsi, is_play);
+	/* get 1 sample data width */
+	sample_width = samples_to_bytes(runtime, 1);
 
-	/* get residue data number of alsa */
-	data_residue_num = fsi_len2num(io->buff_len - io->buff_offset,
-				       ch_width);
+	/* get number of residue samples */
+	sample_residues = io->buff_sample_capa - io->buff_sample_pos;
 
 	if (is_play) {
 		/*
 		 * for play-back
 		 *
-		 * data_num_max	: number of FSI fifo free space
-		 * data_num	: number of ALSA residue data
+		 * samples_max	: number of FSI fifo free samples space
+		 * samples	: number of ALSA residue samples
 		 */
-		data_num_max  = io->fifo_max_num * fsi->chan_num;
-		data_num_max -= fsi_get_fifo_data_num(fsi, is_play);
+		samples_max  = io->fifo_sample_capa;
+		samples_max -= fsi_get_current_fifo_samples(fsi, is_play);
 
-		data_num = data_residue_num;
+		samples = sample_residues;
 
-		switch (ch_width) {
+		switch (sample_width) {
 		case 2:
 			fn = fsi_dma_soft_push16;
 			break;
@@ -715,13 +735,13 @@ static int fsi_fifo_data_ctrl(struct fsi_priv *fsi, int stream)
 		/*
 		 * for capture
 		 *
-		 * data_num_max	: number of ALSA free space
-		 * data_num	: number of data in FSI fifo
+		 * samples_max	: number of ALSA free samples space
+		 * samples	: number of samples in FSI fifo
 		 */
-		data_num_max = data_residue_num;
-		data_num     = fsi_get_fifo_data_num(fsi, is_play);
+		samples_max = sample_residues;
+		samples     = fsi_get_current_fifo_samples(fsi, is_play);
 
-		switch (ch_width) {
+		switch (sample_width) {
 		case 2:
 			fn = fsi_dma_soft_pop16;
 			break;
@@ -733,12 +753,12 @@ static int fsi_fifo_data_ctrl(struct fsi_priv *fsi, int stream)
 		}
 	}
 
-	data_num = min(data_num, data_num_max);
+	samples = min(samples, samples_max);
 
-	fn(fsi, data_num);
+	fn(fsi, samples);
 
-	/* update buff_offset */
-	io->buff_offset += fsi_num2offset(data_num, ch_width);
+	/* update buff_sample_pos */
+	io->buff_sample_pos += samples;
 
 	if (over_period)
 		snd_pcm_period_elapsed(substream);
@@ -1093,16 +1113,14 @@ static int fsi_hw_free(struct snd_pcm_substream *substream)
 
 static snd_pcm_uframes_t fsi_pointer(struct snd_pcm_substream *substream)
 {
-	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct fsi_priv *fsi = fsi_get_priv(substream);
 	struct fsi_stream *io = fsi_get_stream(fsi, fsi_is_play(substream));
-	long location;
+	int samples_pos = io->buff_sample_pos - 1;
 
-	location = (io->buff_offset - 1);
-	if (location < 0)
-		location = 0;
+	if (samples_pos < 0)
+		samples_pos = 0;
 
-	return bytes_to_frames(runtime, location);
+	return fsi_sample2frame(fsi, samples_pos);
 }
 
 static struct snd_pcm_ops fsi_pcm_ops = {
-- 
1.7.1

[PATCH 3/9] ASoC: sh: fsi: remove pm_runtime from fsi_dai_set_fmt.

[Kuninori Morimoto]

pm_runtime_get/put_sync were used to access FSI register in fsi_dai_set_fmt
which is called when ALSA probe.
But this register value will disappear after pm_runtime_put_sync
if platform is supporting RuntimePM.
To solve this issue, this patch adds new variable for format,
and remove pm_runtime_get/put_sync from fsi_dai_set_fmt.

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

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index 14fed47..bdf2d65 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -176,8 +176,12 @@ struct fsi_priv {
 	struct fsi_stream playback;
 	struct fsi_stream capture;
 
+	u32 do_fmt;
+	u32 di_fmt;
+
 	int chan_num:16;
 	int clk_master:1;
+	int spdif:1;
 
 	long rate;
 
@@ -298,6 +302,11 @@ static int fsi_is_port_a(struct fsi_priv *fsi)
 	return fsi->master->base == fsi->base;
 }
 
+static int fsi_is_spdif(struct fsi_priv *fsi)
+{
+	return fsi->spdif;
+}
+
 static struct snd_soc_dai *fsi_get_dai(struct snd_pcm_substream *substream)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
@@ -812,11 +821,16 @@ static int fsi_dai_startup(struct snd_pcm_substream *substream,
 {
 	struct fsi_priv *fsi = fsi_get_priv(substream);
 	u32 flags = fsi_get_info_flags(fsi);
-	u32 data;
+	u32 data = 0;
 	int is_play = fsi_is_play(substream);
 
 	pm_runtime_get_sync(dai->dev);
 
+	/* clock setting */
+	if (fsi_is_clk_master(fsi))
+		data = DIMD | DOMD;
+
+	fsi_reg_mask_set(fsi, CKG1, (DIMD | DOMD), data);
 
 	/* clock inversion (CKG2) */
 	data = 0;
@@ -831,6 +845,16 @@ static int fsi_dai_startup(struct snd_pcm_substream *substream,
 
 	fsi_reg_write(fsi, CKG2, data);
 
+	/* set format */
+	fsi_reg_write(fsi, DO_FMT, fsi->do_fmt);
+	fsi_reg_write(fsi, DI_FMT, fsi->di_fmt);
+
+	/* spdif ? */
+	if (fsi_is_spdif(fsi)) {
+		fsi_spdif_clk_ctrl(fsi, 1);
+		fsi_reg_mask_set(fsi, OUT_SEL, DMMD, DMMD);
+	}
+
 	/* irq clear */
 	fsi_irq_disable(fsi, is_play);
 	fsi_irq_clear_status(fsi);
@@ -900,8 +924,8 @@ static int fsi_set_fmt_dai(struct fsi_priv *fsi, unsigned int fmt)
 		return -EINVAL;
 	}
 
-	fsi_reg_write(fsi, DO_FMT, data);
-	fsi_reg_write(fsi, DI_FMT, data);
+	fsi->do_fmt = data;
+	fsi->di_fmt = data;
 
 	return 0;
 }
@@ -916,11 +940,10 @@ static int fsi_set_fmt_spdif(struct fsi_priv *fsi)
 
 	data = CR_BWS_16 | CR_DTMD_SPDIF_PCM | CR_PCM;
 	fsi->chan_num = 2;
-	fsi_spdif_clk_ctrl(fsi, 1);
-	fsi_reg_mask_set(fsi, OUT_SEL, DMMD, DMMD);
+	fsi->spdif = 1;
 
-	fsi_reg_write(fsi, DO_FMT, data);
-	fsi_reg_write(fsi, DI_FMT, data);
+	fsi->do_fmt = data;
+	fsi->di_fmt = data;
 
 	return 0;
 }
@@ -931,32 +954,24 @@ static int fsi_dai_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
 	struct fsi_master *master = fsi_get_master(fsi);
 	set_rate_func set_rate = fsi_get_info_set_rate(master);
 	u32 flags = fsi_get_info_flags(fsi);
-	u32 data = 0;
 	int ret;
 
-	pm_runtime_get_sync(dai->dev);
-
 	/* set master/slave audio interface */
 	switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
 	case SND_SOC_DAIFMT_CBM_CFM:
-		data = DIMD | DOMD;
 		fsi->clk_master = 1;
 		break;
 	case SND_SOC_DAIFMT_CBS_CFS:
 		break;
 	default:
-		ret = -EINVAL;
-		goto set_fmt_exit;
+		return -EINVAL;
 	}
 
 	if (fsi_is_clk_master(fsi) && !set_rate) {
 		dev_err(dai->dev, "platform doesn't have set_rate\n");
-		ret = -EINVAL;
-		goto set_fmt_exit;
+		return -EINVAL;
 	}
 
-	fsi_reg_mask_set(fsi, CKG1, (DIMD | DOMD), data);
-
 	/* set format */
 	switch (flags & SH_FSI_FMT_MASK) {
 	case SH_FSI_FMT_DAI:
@@ -969,9 +984,6 @@ static int fsi_dai_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
 		ret = -EINVAL;
 	}
 
-set_fmt_exit:
-	pm_runtime_put_sync(dai->dev);
-
 	return ret;
 }
 
-- 
1.7.1

[PATCH 4/9] ASoC: sh: fsi: make sure fsi_stream_push/pop access by spin lock

[Kuninori Morimoto]

fsi_stream_push/pop might be called in same time.
This patch protect it.

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

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index bdf2d65..33a6bd0 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -394,7 +394,10 @@ static void fsi_stream_push(struct fsi_priv *fsi,
 {
 	struct fsi_stream *io = fsi_get_stream(fsi, is_play);
 	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct fsi_master *master = fsi_get_master(fsi);
+	unsigned long flags;
 
+	spin_lock_irqsave(&master->lock, flags);
 	io->substream	= substream;
 	io->buff_sample_capa	= fsi_frame2sample(fsi, runtime->buffer_size);
 	io->buff_sample_pos	= 0;
@@ -402,13 +405,17 @@ static void fsi_stream_push(struct fsi_priv *fsi,
 	io->period_pos		= 0;
 	io->oerr_num	= -1; /* ignore 1st err */
 	io->uerr_num	= -1; /* ignore 1st err */
+	spin_unlock_irqrestore(&master->lock, flags);
 }
 
 static void fsi_stream_pop(struct fsi_priv *fsi, int is_play)
 {
 	struct fsi_stream *io = fsi_get_stream(fsi, is_play);
 	struct snd_soc_dai *dai = fsi_get_dai(io->substream);
+	struct fsi_master *master = fsi_get_master(fsi);
+	unsigned long flags;
 
+	spin_lock_irqsave(&master->lock, flags);
 
 	if (io->oerr_num > 0)
 		dev_err(dai->dev, "over_run = %d\n", io->oerr_num);
@@ -423,6 +430,7 @@ static void fsi_stream_pop(struct fsi_priv *fsi, int is_play)
 	io->period_pos		= 0;
 	io->oerr_num	= 0;
 	io->uerr_num	= 0;
+	spin_unlock_irqrestore(&master->lock, flags);
 }
 
 static int fsi_get_current_fifo_samples(struct fsi_priv *fsi, int is_play)
-- 
1.7.1

[PATCH 5/9] ASoC: sh: fsi: add fsi_set_master_clk

[Kuninori Morimoto]

Current FSI driver is using set_rate call back function which is for
master mode.
By this patch, it is used from fsi_set_master_clk.
This patch is preparation of cleanup suspend/resume patch.

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

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index 33a6bd0..ae05d8f 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -593,6 +593,82 @@ static void fsi_spdif_clk_ctrl(struct fsi_priv *fsi, int enable)
 /*
  *		clock function
  */
+static int fsi_set_master_clk(struct device *dev, struct fsi_priv *fsi,
+			      long rate, int enable)
+{
+	struct fsi_master *master = fsi_get_master(fsi);
+	set_rate_func set_rate = fsi_get_info_set_rate(master);
+	int fsi_ver = master->core->ver;
+	int ret;
+
+	ret = set_rate(dev, fsi_is_port_a(fsi), rate, enable);
+	if (ret < 0) /* error */
+		return ret;
+
+	if (!enable)
+		return 0;
+
+	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(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(dev, "unsupported ACKMD\n");
+			else
+				data |= (0x7 << 8);
+			break;
+		}
+
+		fsi_reg_mask_set(fsi, CKG1, (ACKMD_MASK | BPFMD_MASK) , data);
+		udelay(10);
+		ret = 0;
+	}
+
+	return ret;
+
+}
+
 #define fsi_module_init(m, d)	__fsi_module_clk_ctrl(m, d, 1)
 #define fsi_module_kill(m, d)	__fsi_module_clk_ctrl(m, d, 0)
 static void __fsi_module_clk_ctrl(struct fsi_master *master,
@@ -878,13 +954,11 @@ static void fsi_dai_shutdown(struct snd_pcm_substream *substream,
 {
 	struct fsi_priv *fsi = fsi_get_priv(substream);
 	int is_play = fsi_is_play(substream);
-	struct fsi_master *master = fsi_get_master(fsi);
-	set_rate_func set_rate = fsi_get_info_set_rate(master);
 
 	fsi_irq_disable(fsi, is_play);
 
 	if (fsi_is_clk_master(fsi))
-		set_rate(dai->dev, fsi_is_port_a(fsi), fsi->rate, 0);
+		fsi_set_master_clk(dai->dev, fsi, fsi->rate, 0);
 
 	fsi->rate = 0;
 
@@ -1000,79 +1074,19 @@ static int fsi_dai_hw_params(struct snd_pcm_substream *substream,
 			     struct snd_soc_dai *dai)
 {
 	struct fsi_priv *fsi = fsi_get_priv(substream);
-	struct fsi_master *master = fsi_get_master(fsi);
-	set_rate_func set_rate = fsi_get_info_set_rate(master);
-	int fsi_ver = master->core->ver;
 	long rate = params_rate(params);
 	int ret;
 
 	if (!fsi_is_clk_master(fsi))
 		return 0;
 
-	ret = set_rate(dai->dev, fsi_is_port_a(fsi), rate, 1);
-	if (ret < 0) /* error */
+	ret = fsi_set_master_clk(dai->dev, fsi, rate, 1);
+	if (ret < 0)
 		return ret;
 
 	fsi->rate = rate;
-	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);
-		ret = 0;
-	}
 
 	return ret;
-
 }
 
 static struct snd_soc_dai_ops fsi_dai_ops = {
@@ -1339,8 +1353,7 @@ static int fsi_remove(struct platform_device *pdev)
 }
 
 static void __fsi_suspend(struct fsi_priv *fsi,
-			  struct device *dev,
-			  set_rate_func set_rate)
+			  struct device *dev)
 {
 	fsi->saved_do_fmt	= fsi_reg_read(fsi, DO_FMT);
 	fsi->saved_di_fmt	= fsi_reg_read(fsi, DI_FMT);
@@ -1349,12 +1362,11 @@ static void __fsi_suspend(struct fsi_priv *fsi,
 	fsi->saved_out_sel	= fsi_reg_read(fsi, OUT_SEL);
 
 	if (fsi_is_clk_master(fsi))
-		set_rate(dev, fsi_is_port_a(fsi), fsi->rate, 0);
+		fsi_set_master_clk(dev, fsi, fsi->rate, 0);
 }
 
 static void __fsi_resume(struct fsi_priv *fsi,
-			 struct device *dev,
-			 set_rate_func set_rate)
+			 struct device *dev)
 {
 	fsi_reg_write(fsi, DO_FMT,	fsi->saved_do_fmt);
 	fsi_reg_write(fsi, DI_FMT,	fsi->saved_di_fmt);
@@ -1363,18 +1375,17 @@ static void __fsi_resume(struct fsi_priv *fsi,
 	fsi_reg_write(fsi, OUT_SEL,	fsi->saved_out_sel);
 
 	if (fsi_is_clk_master(fsi))
-		set_rate(dev, fsi_is_port_a(fsi), fsi->rate, 1);
+		fsi_set_master_clk(dev, fsi, fsi->rate, 1);
 }
 
 static int fsi_suspend(struct device *dev)
 {
 	struct fsi_master *master = dev_get_drvdata(dev);
-	set_rate_func set_rate = fsi_get_info_set_rate(master);
 
 	pm_runtime_get_sync(dev);
 
-	__fsi_suspend(&master->fsia, dev, set_rate);
-	__fsi_suspend(&master->fsib, dev, set_rate);
+	__fsi_suspend(&master->fsia, dev);
+	__fsi_suspend(&master->fsib, dev);
 
 	master->saved_a_mclk	= fsi_core_read(master, a_mclk);
 	master->saved_b_mclk	= fsi_core_read(master, b_mclk);
@@ -1393,7 +1404,6 @@ static int fsi_suspend(struct device *dev)
 static int fsi_resume(struct device *dev)
 {
 	struct fsi_master *master = dev_get_drvdata(dev);
-	set_rate_func set_rate = fsi_get_info_set_rate(master);
 
 	pm_runtime_get_sync(dev);
 
@@ -1406,8 +1416,8 @@ static int fsi_resume(struct device *dev)
 	fsi_core_mask_set(master, iemsk, 0xffff, master->saved_iemsk);
 	fsi_core_mask_set(master, imsk, 0xffff, master->saved_imsk);
 
-	__fsi_resume(&master->fsia, dev, set_rate);
-	__fsi_resume(&master->fsib, dev, set_rate);
+	__fsi_resume(&master->fsia, dev);
+	__fsi_resume(&master->fsib, dev);
 
 	pm_runtime_put_sync(dev);
 
-- 
1.7.1

[PATCH 6/9] ASoC: sh: fsi: irq control moves to fsi_port_start/stop

[Kuninori Morimoto]

Using fsi_irq_enable/disable in fsi_port_start/stop is very natural.
This patch is preparation of cleanup suspend/resume patch.

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

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index ae05d8f..a00fe37 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -689,15 +689,20 @@ static void __fsi_module_clk_ctrl(struct fsi_master *master,
 	pm_runtime_put_sync(dev);
 }
 
-#define fsi_port_start(f)	__fsi_port_clk_ctrl(f, 1)
-#define fsi_port_stop(f)	__fsi_port_clk_ctrl(f, 0)
-static void __fsi_port_clk_ctrl(struct fsi_priv *fsi, int enable)
+#define fsi_port_start(f, i)	__fsi_port_clk_ctrl(f, i, 1)
+#define fsi_port_stop(f, i)	__fsi_port_clk_ctrl(f, i, 0)
+static void __fsi_port_clk_ctrl(struct fsi_priv *fsi, int is_play, int enable)
 {
 	struct fsi_master *master = fsi_get_master(fsi);
 	u32 soft = fsi_is_port_a(fsi) ? PASR : PBSR;
 	u32 clk  = fsi_is_port_a(fsi) ? CRA  : CRB;
 	int is_master = fsi_is_clk_master(fsi);
 
+	if (enable)
+		fsi_irq_enable(fsi, is_play);
+	else
+		fsi_irq_disable(fsi, is_play);
+
 	fsi_master_mask_set(master, SOFT_RST, soft, (enable) ? soft : 0);
 	if (is_master)
 		fsi_master_mask_set(master, CLK_RST, clk, (enable) ? clk : 0);
@@ -953,9 +958,6 @@ static void fsi_dai_shutdown(struct snd_pcm_substream *substream,
 			     struct snd_soc_dai *dai)
 {
 	struct fsi_priv *fsi = fsi_get_priv(substream);
-	int is_play = fsi_is_play(substream);
-
-	fsi_irq_disable(fsi, is_play);
 
 	if (fsi_is_clk_master(fsi))
 		fsi_set_master_clk(dai->dev, fsi, fsi->rate, 0);
@@ -976,12 +978,10 @@ static int fsi_dai_trigger(struct snd_pcm_substream *substream, int cmd,
 	case SNDRV_PCM_TRIGGER_START:
 		fsi_stream_push(fsi, is_play, substream);
 		ret = is_play ? fsi_data_push(fsi) : fsi_data_pop(fsi);
-		fsi_irq_enable(fsi, is_play);
-		fsi_port_start(fsi);
+		fsi_port_start(fsi, is_play);
 		break;
 	case SNDRV_PCM_TRIGGER_STOP:
-		fsi_port_stop(fsi);
-		fsi_irq_disable(fsi, is_play);
+		fsi_port_stop(fsi, is_play);
 		fsi_stream_pop(fsi, is_play);
 		break;
 	}
-- 
1.7.1

[PATCH 7/9] ASoC: sh: fsi: add fsi_hw_startup/shutdown

[Kuninori Morimoto]

This patch is preparation of cleanup suspend/resume patch.

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

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index a00fe37..c755d21 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -713,7 +713,7 @@ static void __fsi_port_clk_ctrl(struct fsi_priv *fsi, int is_play, int enable)
  */
 static void fsi_fifo_init(struct fsi_priv *fsi,
 			  int is_play,
-			  struct snd_soc_dai *dai)
+			  struct device *dev)
 {
 	struct fsi_master *master = fsi_get_master(fsi);
 	struct fsi_stream *io = fsi_get_stream(fsi, is_play);
@@ -725,7 +725,7 @@ static void fsi_fifo_init(struct fsi_priv *fsi,
 	shift >>= fsi_get_port_shift(fsi, is_play);
 	shift &= FIFO_SZ_MASK;
 	frame_capa = 256 << shift;
-	dev_dbg(dai->dev, "fifo = %d words\n", frame_capa);
+	dev_dbg(dev, "fifo = %d words\n", frame_capa);
 
 	/*
 	 * The maximum number of sample data varies depending
@@ -748,7 +748,7 @@ static void fsi_fifo_init(struct fsi_priv *fsi,
 	 */
 	for (i = 1; i < fsi->chan_num; i <<= 1)
 		frame_capa >>= 1;
-	dev_dbg(dai->dev, "%d channel %d store\n",
+	dev_dbg(dev, "%d channel %d store\n",
 		fsi->chan_num, frame_capa);
 
 	io->fifo_sample_capa = fsi_frame2sample(fsi, frame_capa);
@@ -905,15 +905,14 @@ static irqreturn_t fsi_interrupt(int irq, void *data)
  *		dai ops
  */
 
-static int fsi_dai_startup(struct snd_pcm_substream *substream,
-			   struct snd_soc_dai *dai)
+static int fsi_hw_startup(struct fsi_priv *fsi,
+			  int is_play,
+			  struct device *dev)
 {
-	struct fsi_priv *fsi = fsi_get_priv(substream);
 	u32 flags = fsi_get_info_flags(fsi);
 	u32 data = 0;
-	int is_play = fsi_is_play(substream);
 
-	pm_runtime_get_sync(dai->dev);
+	pm_runtime_get_sync(dev);
 
 	/* clock setting */
 	if (fsi_is_clk_master(fsi))
@@ -949,22 +948,38 @@ static int fsi_dai_startup(struct snd_pcm_substream *substream,
 	fsi_irq_clear_status(fsi);
 
 	/* fifo init */
-	fsi_fifo_init(fsi, is_play, dai);
+	fsi_fifo_init(fsi, is_play, dev);
 
 	return 0;
 }
 
+static void fsi_hw_shutdown(struct fsi_priv *fsi,
+			    int is_play,
+			    struct device *dev)
+{
+	if (fsi_is_clk_master(fsi))
+		fsi_set_master_clk(dev, fsi, fsi->rate, 0);
+
+	pm_runtime_put_sync(dev);
+}
+
+static int fsi_dai_startup(struct snd_pcm_substream *substream,
+			   struct snd_soc_dai *dai)
+{
+	struct fsi_priv *fsi = fsi_get_priv(substream);
+	int is_play = fsi_is_play(substream);
+
+	return fsi_hw_startup(fsi, is_play, dai->dev);
+}
+
 static void fsi_dai_shutdown(struct snd_pcm_substream *substream,
 			     struct snd_soc_dai *dai)
 {
 	struct fsi_priv *fsi = fsi_get_priv(substream);
+	int is_play = fsi_is_play(substream);
 
-	if (fsi_is_clk_master(fsi))
-		fsi_set_master_clk(dai->dev, fsi, fsi->rate, 0);
-
+	fsi_hw_shutdown(fsi, is_play, dai->dev);
 	fsi->rate = 0;
-
-	pm_runtime_put_sync(dai->dev);
 }
 
 static int fsi_dai_trigger(struct snd_pcm_substream *substream, int cmd,
-- 
1.7.1

[PATCH 8/9] ASoC: sh: fsi: remove fsi_module_init/kill

[Kuninori Morimoto]

FSIA/B ports is enabled by default when power-on,
and current FSI is supporting RuntimePM.
In addition, current fsi_module_init/kill doesn't care
simultaneous playback/recorde.
This mean FSI port control is not needed.
This patch remove fsi_module_init/kill

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

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index c755d21..a457e5b 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -669,32 +669,11 @@ static int fsi_set_master_clk(struct device *dev, struct fsi_priv *fsi,
 
 }
 
-#define fsi_module_init(m, d)	__fsi_module_clk_ctrl(m, d, 1)
-#define fsi_module_kill(m, d)	__fsi_module_clk_ctrl(m, d, 0)
-static void __fsi_module_clk_ctrl(struct fsi_master *master,
-				  struct device *dev,
-				  int enable)
-{
-	pm_runtime_get_sync(dev);
-
-	if (enable) {
-		/* enable only SR */
-		fsi_master_mask_set(master, SOFT_RST, FSISR, FSISR);
-		fsi_master_mask_set(master, SOFT_RST, PASR | PBSR, 0);
-	} else {
-		/* clear all registers */
-		fsi_master_mask_set(master, SOFT_RST, FSISR, 0);
-	}
-
-	pm_runtime_put_sync(dev);
-}
-
 #define fsi_port_start(f, i)	__fsi_port_clk_ctrl(f, i, 1)
 #define fsi_port_stop(f, i)	__fsi_port_clk_ctrl(f, i, 0)
 static void __fsi_port_clk_ctrl(struct fsi_priv *fsi, int is_play, int enable)
 {
 	struct fsi_master *master = fsi_get_master(fsi);
-	u32 soft = fsi_is_port_a(fsi) ? PASR : PBSR;
 	u32 clk  = fsi_is_port_a(fsi) ? CRA  : CRB;
 	int is_master = fsi_is_clk_master(fsi);
 
@@ -703,7 +682,6 @@ static void __fsi_port_clk_ctrl(struct fsi_priv *fsi, int is_play, int enable)
 	else
 		fsi_irq_disable(fsi, is_play);
 
-	fsi_master_mask_set(master, SOFT_RST, soft, (enable) ? soft : 0);
 	if (is_master)
 		fsi_master_mask_set(master, CLK_RST, clk, (enable) ? clk : 0);
 }
@@ -1309,8 +1287,6 @@ static int fsi_probe(struct platform_device *pdev)
 	pm_runtime_enable(&pdev->dev);
 	dev_set_drvdata(&pdev->dev, master);
 
-	fsi_module_init(master, &pdev->dev);
-
 	ret = request_irq(irq, &fsi_interrupt, IRQF_DISABLED,
 			  id_entry->name, master);
 	if (ret) {
@@ -1353,8 +1329,6 @@ static int fsi_remove(struct platform_device *pdev)
 
 	master = dev_get_drvdata(&pdev->dev);
 
-	fsi_module_kill(master, &pdev->dev);
-
 	free_irq(master->irq, master);
 	pm_runtime_disable(&pdev->dev);
 
@@ -1409,8 +1383,6 @@ static int fsi_suspend(struct device *dev)
 	master->saved_clk_rst	= fsi_master_read(master, CLK_RST);
 	master->saved_soft_rst	= fsi_master_read(master, SOFT_RST);
 
-	fsi_module_kill(master, dev);
-
 	pm_runtime_put_sync(dev);
 
 	return 0;
@@ -1422,8 +1394,6 @@ static int fsi_resume(struct device *dev)
 
 	pm_runtime_get_sync(dev);
 
-	fsi_module_init(master, dev);
-
 	fsi_master_mask_set(master, SOFT_RST, 0xffff, master->saved_soft_rst);
 	fsi_master_mask_set(master, CLK_RST, 0xffff, master->saved_clk_rst);
 	fsi_core_mask_set(master, a_mclk, 0xffff, master->saved_a_mclk);
-- 
1.7.1

[PATCH 9/9] ASoC: sh: fsi: cleanup suspend/resume

[Kuninori Morimoto]

Current FSI driver was using saved_xxx variable for suspend/resume.
OTOH, the start and stop of power/clock are controlled by
fsi_hw_startup/fsi_hw_shutdown in current FSI driver.
The all necessary registers value are set by fsi_hw_startup.

So, if fsi_hw_shutdown is called when "suspend" is generated,
and fsi_hw_startup is called at "resume",
the saved_xxx are not needed.

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

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index a457e5b..d2f17ce 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -184,13 +184,6 @@ struct fsi_priv {
 	int spdif:1;
 
 	long rate;
-
-	/* for suspend/resume */
-	u32 saved_do_fmt;
-	u32 saved_di_fmt;
-	u32 saved_ckg1;
-	u32 saved_ckg2;
-	u32 saved_out_sel;
 };
 
 struct fsi_core {
@@ -211,14 +204,6 @@ struct fsi_master {
 	struct fsi_core *core;
 	struct sh_fsi_platform_info *info;
 	spinlock_t lock;
-
-	/* for suspend/resume */
-	u32 saved_a_mclk;
-	u32 saved_b_mclk;
-	u32 saved_iemsk;
-	u32 saved_imsk;
-	u32 saved_clk_rst;
-	u32 saved_soft_rst;
 };
 
 /*
@@ -388,6 +373,21 @@ static int fsi_sample2frame(struct fsi_priv *fsi, int samples)
 	return samples / fsi->chan_num;
 }
 
+static int fsi_stream_is_working(struct fsi_priv *fsi,
+				  int is_play)
+{
+	struct fsi_stream *io = fsi_get_stream(fsi, is_play);
+	struct fsi_master *master = fsi_get_master(fsi);
+	unsigned long flags;
+	int ret;
+
+	spin_lock_irqsave(&master->lock, flags);
+	ret = !!io->substream;
+	spin_unlock_irqrestore(&master->lock, flags);
+
+	return ret;
+}
+
 static void fsi_stream_push(struct fsi_priv *fsi,
 			    int is_play,
 			    struct snd_pcm_substream *substream)
@@ -666,7 +666,6 @@ static int fsi_set_master_clk(struct device *dev, struct fsi_priv *fsi,
 	}
 
 	return ret;
-
 }
 
 #define fsi_port_start(f, i)	__fsi_port_clk_ctrl(f, i, 1)
@@ -675,14 +674,13 @@ static void __fsi_port_clk_ctrl(struct fsi_priv *fsi, int is_play, int enable)
 {
 	struct fsi_master *master = fsi_get_master(fsi);
 	u32 clk  = fsi_is_port_a(fsi) ? CRA  : CRB;
-	int is_master = fsi_is_clk_master(fsi);
 
 	if (enable)
 		fsi_irq_enable(fsi, is_play);
 	else
 		fsi_irq_disable(fsi, is_play);
 
-	if (is_master)
+	if (fsi_is_clk_master(fsi))
 		fsi_master_mask_set(master, CLK_RST, clk, (enable) ? clk : 0);
 }
 
@@ -1342,48 +1340,43 @@ static int fsi_remove(struct platform_device *pdev)
 }
 
 static void __fsi_suspend(struct fsi_priv *fsi,
+			  int is_play,
 			  struct device *dev)
 {
-	fsi->saved_do_fmt	= fsi_reg_read(fsi, DO_FMT);
-	fsi->saved_di_fmt	= fsi_reg_read(fsi, DI_FMT);
-	fsi->saved_ckg1		= fsi_reg_read(fsi, CKG1);
-	fsi->saved_ckg2		= fsi_reg_read(fsi, CKG2);
-	fsi->saved_out_sel	= fsi_reg_read(fsi, OUT_SEL);
+	if (!fsi_stream_is_working(fsi, is_play))
+		return;
 
-	if (fsi_is_clk_master(fsi))
-		fsi_set_master_clk(dev, fsi, fsi->rate, 0);
+	fsi_port_stop(fsi, is_play);
+	fsi_hw_shutdown(fsi, is_play, dev);
 }
 
 static void __fsi_resume(struct fsi_priv *fsi,
+			 int is_play,
 			 struct device *dev)
 {
-	fsi_reg_write(fsi, DO_FMT,	fsi->saved_do_fmt);
-	fsi_reg_write(fsi, DI_FMT,	fsi->saved_di_fmt);
-	fsi_reg_write(fsi, CKG1,	fsi->saved_ckg1);
-	fsi_reg_write(fsi, CKG2,	fsi->saved_ckg2);
-	fsi_reg_write(fsi, OUT_SEL,	fsi->saved_out_sel);
+	if (!fsi_stream_is_working(fsi, is_play))
+		return;
 
-	if (fsi_is_clk_master(fsi))
+	fsi_hw_startup(fsi, is_play, dev);
+
+	if (fsi_is_clk_master(fsi) && fsi->rate)
 		fsi_set_master_clk(dev, fsi, fsi->rate, 1);
+
+	fsi_port_start(fsi, is_play);
+
 }
 
 static int fsi_suspend(struct device *dev)
 {
 	struct fsi_master *master = dev_get_drvdata(dev);
+	struct fsi_priv *fsia = &master->fsia;
+	struct fsi_priv *fsib = &master->fsib;
 
-	pm_runtime_get_sync(dev);
-
-	__fsi_suspend(&master->fsia, dev);
-	__fsi_suspend(&master->fsib, dev);
+	__fsi_suspend(fsia, 1, dev);
+	__fsi_suspend(fsia, 0, dev);
 
-	master->saved_a_mclk	= fsi_core_read(master, a_mclk);
-	master->saved_b_mclk	= fsi_core_read(master, b_mclk);
-	master->saved_iemsk	= fsi_core_read(master, iemsk);
-	master->saved_imsk	= fsi_core_read(master, imsk);
-	master->saved_clk_rst	= fsi_master_read(master, CLK_RST);
-	master->saved_soft_rst	= fsi_master_read(master, SOFT_RST);
-
-	pm_runtime_put_sync(dev);
+	__fsi_suspend(fsib, 1, dev);
+	__fsi_suspend(fsib, 0, dev);
 
 	return 0;
 }
@@ -1391,20 +1384,14 @@ static int fsi_suspend(struct device *dev)
 static int fsi_resume(struct device *dev)
 {
 	struct fsi_master *master = dev_get_drvdata(dev);
+	struct fsi_priv *fsia = &master->fsia;
+	struct fsi_priv *fsib = &master->fsib;
 
-	pm_runtime_get_sync(dev);
-
-	fsi_master_mask_set(master, SOFT_RST, 0xffff, master->saved_soft_rst);
-	fsi_master_mask_set(master, CLK_RST, 0xffff, master->saved_clk_rst);
-	fsi_core_mask_set(master, a_mclk, 0xffff, master->saved_a_mclk);
-	fsi_core_mask_set(master, b_mclk, 0xffff, master->saved_b_mclk);
-	fsi_core_mask_set(master, iemsk, 0xffff, master->saved_iemsk);
-	fsi_core_mask_set(master, imsk, 0xffff, master->saved_imsk);
+	__fsi_resume(fsia, 1, dev);
+	__fsi_resume(fsia, 0, dev);
 
-	__fsi_resume(&master->fsia, dev);
-	__fsi_resume(&master->fsib, dev);
-
-	pm_runtime_put_sync(dev);
+	__fsi_resume(fsib, 1, dev);
+	__fsi_resume(fsib, 0, dev);
 
 	return 0;
 }
-- 
1.7.1

Re: [PATCH 0/9] ASoC: sh: fsi: tidyup and cleanup FSI

[Liam Girdwood]

On 23/05/11 12:45, Kuninori Morimoto wrote:
> 
> Dear Mark, Liam
> 
> Cc Simon, yoshii-san, Magnus
> 
> These are tidyup and cleanup patches for FSI
> 
> Kuninori Morimoto (9):
>       ASoC: sh: fsi: tidyup parameter of fsi_stream_push
>       ASoC: sh: fsi: tidyup unclear variable naming
>       ASoC: sh: fsi: remove pm_runtime from fsi_dai_set_fmt.
>       ASoC: sh: fsi: make sure fsi_stream_push/pop access by spin lock
>       ASoC: sh: fsi: Add fsi_set_master_clk
>       ASoC: sh: fsi: irq control moves to fsi_port_start/stop
>       ASoC: sh: fsi: Add fsi_hw_startup/shutdown
>       ASoC: sh: fsi: remove fsi_module_init/kill
>       ASoC: sh: fsi: cleanup suspend/resume
> 
> #1 and #2 are tidyup for fsi_fifo_data_ctrl.
> The variables and sub-functions were unclear nameing.
> So, it was difficult to understand the detail of calculation.
> Thank you for pointing it yoshii-san.
> 
> #3 - #9 are clean up for RuntimePM.
> Current FSI driver was wrong for RuntimePM.
> these patches fixup for RuntimePM and
> clean up for suspend/resume.
> last patch remove noisy saved_xxx variable.
> 

All

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

Re: [PATCH 4/9] ASoC: sh: fsi: make sure fsi_stream_push/pop access by spin lock

[Mark Brown]

On Mon, May 23, 2011 at 08:46:13PM +0900, Kuninori Morimoto wrote:
> fsi_stream_push/pop might be called in same time.
> This patch protect it.
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

This one looks like a bug fix which should be done in 2.6.40 also?  If
that is the case could you please also provide a version of this patch
against 2.6.40.

Re: [PATCH 0/9] ASoC: sh: fsi: tidyup and cleanup FSI

[Mark Brown]

On Mon, May 23, 2011 at 08:45:26PM +0900, Kuninori Morimoto wrote:
> 
> Dear Mark, Liam
> 
> Cc Simon, yoshii-san, Magnus
> 
> These are tidyup and cleanup patches for FSI
> 
> Kuninori Morimoto (9):

Applied all, thanks.

Re: [PATCH 4/9] ASoC: sh: fsi: make sure fsi_stream_push/pop access by spin lock

[Kuninori Morimoto]

Dear Mark

> > fsi_stream_push/pop might be called in same time.
> > This patch protect it.
> > 
> > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> This one looks like a bug fix which should be done in 2.6.40 also?  If
> that is the case could you please also provide a version of this patch
> against 2.6.40.

Thank you for pointing it.

Sorry. the order of patch series (and explain) was a little bit wrong.

My point was "snd_soc_dai_ops :: trigger" (which call fsi_stream_push/pop)
and "suspend/resume" (which call fsi_stream_is_working)
might be called in same time.
This patch was preparation of cleanup suspend/resume patch.

Best regards
--
Kuninori Morimoto