ASoC: fsi: Add FSI DMAEngine support

ASoC: fsi: Add FSI DMAEngine support

[Kuninori Morimoto]

Hi Mark, Liam, and Paul

These are DMAEngine support for FSI driver

I created a lot of patches since FSI DMAEngine needed much modification.
I hope it was good for reviewing.
But some patches are just cleanup/tidyup/rename.
Last patch is main DMAEngine patch. it support Tx case only at this point.

FSI will use PIO version if CPU/platform doesn't have DMAEngine support.

>> Paul

Could you please check #16 patch which modify arch/arm/mach-shmobile and arch/sh


Kuninori Morimoto (19):
      ASoC: fsi: reduce runtime calculation by using pre-setting
      ASoC: fsi: tidyup: fsi_stream_xx() functions were gathered
      ASoC: fsi: data push/pop calculation part was divided
      ASoC: fsi: rename fsi_dma_soft_xxx() to fsi_pio_xxx()
      ASoC: fsi: tidyup: move fsi_fifo_init() onto fsi_hw_startup()
      ASoC: fsi: remove unnecessary parameter from fsi_hw_shutdown()
      ASoC: fsi: rename fsi_stream_push/pop() to fsi_stream_init/quit()
      ASoC: fsi: modify fsi_pio_get_area() parameter and using position
      ASoC: fsi: re-define fsi_is_play() and fsi_stream_is_play()
      ASoC: fsi: use fsi_stream in fsi_get_current_fifo_samples() parameter
      ASoC: fsi: add fsi_stream_handler and PIO handler
      ASoC: fsi: tidyup: fsi_pio_xxx() are gathered
      ASoC: fsi: don't use is_play as a parameter of fsi functions
      ASoC: fsi: add .start_stop handler to fsi_stream_handler
      ASoC: fsi: fsi_stream_is_working() care substream->runtime
      ASoC: fsi: PortA/B information was controlled by sh_fsi_port_info
      ASoC: fsi: add .init/.quit handler support
      ASoC: fsi: fixup fsi_pointer() calculation method
      ASoC: fsi: Add DMAEngine support

Best regards
---
Kuninori Morimoto

[PATCH 01/19] ASoC: fsi: reduce runtime calculation by using pre-setting

[Kuninori Morimoto]

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

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index 3241e5b..0d78740 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -167,6 +167,7 @@ struct fsi_stream {
 	int buff_sample_pos;	/* sample position of ALSA buffer */
 	int period_samples;	/* sample number / 1 period */
 	int period_pos;		/* current period position */
+	int sample_width;	/* sample width */
 
 	int uerr_num;
 	int oerr_num;
@@ -406,6 +407,7 @@ static void fsi_stream_push(struct fsi_priv *fsi,
 	io->buff_sample_pos	= 0;
 	io->period_samples	= fsi_frame2sample(fsi, runtime->period_size);
 	io->period_pos		= 0;
+	io->sample_width	= samples_to_bytes(runtime, 1);
 	io->oerr_num	= -1; /* ignore 1st err */
 	io->uerr_num	= -1; /* ignore 1st err */
 	spin_unlock_irqrestore(&master->lock, flags);
@@ -431,6 +433,7 @@ static void fsi_stream_pop(struct fsi_priv *fsi, int is_play)
 	io->buff_sample_pos	= 0;
 	io->period_samples	= 0;
 	io->period_pos		= 0;
+	io->sample_width	= 0;
 	io->oerr_num	= 0;
 	io->uerr_num	= 0;
 	spin_unlock_irqrestore(&master->lock, flags);
@@ -752,7 +755,6 @@ static int fsi_fifo_data_ctrl(struct fsi_priv *fsi, int stream)
 	int is_play = fsi_stream_is_play(stream);
 	struct fsi_stream *io = fsi_get_stream(fsi, is_play);
 	int sample_residues;
-	int sample_width;
 	int samples;
 	int samples_max;
 	int over_period;
@@ -780,9 +782,6 @@ static int fsi_fifo_data_ctrl(struct fsi_priv *fsi, int stream)
 			io->buff_sample_pos = 0;
 	}
 
-	/* get 1 sample data width */
-	sample_width = samples_to_bytes(runtime, 1);
-
 	/* get number of residue samples */
 	sample_residues = io->buff_sample_capa - io->buff_sample_pos;
 
@@ -798,7 +797,7 @@ static int fsi_fifo_data_ctrl(struct fsi_priv *fsi, int stream)
 
 		samples = sample_residues;
 
-		switch (sample_width) {
+		switch (io->sample_width) {
 		case 2:
 			fn = fsi_dma_soft_push16;
 			break;
@@ -818,7 +817,7 @@ static int fsi_fifo_data_ctrl(struct fsi_priv *fsi, int stream)
 		samples_max = sample_residues;
 		samples     = fsi_get_current_fifo_samples(fsi, is_play);
 
-		switch (sample_width) {
+		switch (io->sample_width) {
 		case 2:
 			fn = fsi_dma_soft_pop16;
 			break;
-- 
1.7.5.4

[PATCH 02/19] ASoC: fsi: tidyup: fsi_stream_xx() functions were gathered

[Kuninori Morimoto]

This patch gathered fsi_stream_xxx() functions in order to make it readable.

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

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index 0d78740..162416e 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -338,22 +338,6 @@ static u32 fsi_get_info_flags(struct fsi_priv *fsi)
 		master->info->portb_flags;
 }
 
-static inline int fsi_stream_is_play(int stream)
-{
-	return stream == SNDRV_PCM_STREAM_PLAYBACK;
-}
-
-static inline int fsi_is_play(struct snd_pcm_substream *substream)
-{
-	return fsi_stream_is_play(substream->stream);
-}
-
-static inline struct fsi_stream *fsi_get_stream(struct fsi_priv *fsi,
-						int is_play)
-{
-	return is_play ? &fsi->playback : &fsi->capture;
-}
-
 static u32 fsi_get_port_shift(struct fsi_priv *fsi, int is_play)
 {
 	int is_porta = fsi_is_port_a(fsi);
@@ -377,10 +361,60 @@ static int fsi_sample2frame(struct fsi_priv *fsi, int samples)
 	return samples / fsi->chan_num;
 }
 
+static int fsi_get_current_fifo_samples(struct fsi_priv *fsi, int is_play)
+{
+	u32 status;
+	int frames;
+
+	status = is_play ?
+		fsi_reg_read(fsi, DOFF_ST) :
+		fsi_reg_read(fsi, DIFF_ST);
+
+	frames = 0x1ff & (status >> 8);
+
+	return fsi_frame2sample(fsi, frames);
+}
+
+static void fsi_count_fifo_err(struct fsi_priv *fsi)
+{
+	u32 ostatus = fsi_reg_read(fsi, DOFF_ST);
+	u32 istatus = fsi_reg_read(fsi, DIFF_ST);
+
+	if (ostatus & ERR_OVER)
+		fsi->playback.oerr_num++;
+
+	if (ostatus & ERR_UNDER)
+		fsi->playback.uerr_num++;
+
+	if (istatus & ERR_OVER)
+		fsi->capture.oerr_num++;
+
+	if (istatus & ERR_UNDER)
+		fsi->capture.uerr_num++;
+
+	fsi_reg_write(fsi, DOFF_ST, 0);
+	fsi_reg_write(fsi, DIFF_ST, 0);
+}
+
+/*
+ *		fsi_stream_xx() function
+ */
+#define fsi_is_play(substream)	fsi_stream_is_play(substream->stream)
+static inline int fsi_stream_is_play(int stream)
+{
+	return stream == SNDRV_PCM_STREAM_PLAYBACK;
+}
+
+static inline struct fsi_stream *fsi_stream_get(struct fsi_priv *fsi,
+						int is_play)
+{
+	return is_play ? &fsi->playback : &fsi->capture;
+}
+
 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_stream *io = fsi_stream_get(fsi, is_play);
 	struct fsi_master *master = fsi_get_master(fsi);
 	unsigned long flags;
 	int ret;
@@ -396,7 +430,7 @@ static void fsi_stream_push(struct fsi_priv *fsi,
 			    int is_play,
 			    struct snd_pcm_substream *substream)
 {
-	struct fsi_stream *io = fsi_get_stream(fsi, is_play);
+	struct fsi_stream *io = fsi_stream_get(fsi, is_play);
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct fsi_master *master = fsi_get_master(fsi);
 	unsigned long flags;
@@ -415,7 +449,7 @@ static void fsi_stream_push(struct fsi_priv *fsi,
 
 static void fsi_stream_pop(struct fsi_priv *fsi, int is_play)
 {
-	struct fsi_stream *io = fsi_get_stream(fsi, is_play);
+	struct fsi_stream *io = fsi_stream_get(fsi, is_play);
 	struct snd_soc_dai *dai = fsi_get_dai(io->substream);
 	struct fsi_master *master = fsi_get_master(fsi);
 	unsigned long flags;
@@ -439,41 +473,6 @@ static void fsi_stream_pop(struct fsi_priv *fsi, int is_play)
 	spin_unlock_irqrestore(&master->lock, flags);
 }
 
-static int fsi_get_current_fifo_samples(struct fsi_priv *fsi, int is_play)
-{
-	u32 status;
-	int frames;
-
-	status = is_play ?
-		fsi_reg_read(fsi, DOFF_ST) :
-		fsi_reg_read(fsi, DIFF_ST);
-
-	frames = 0x1ff & (status >> 8);
-
-	return fsi_frame2sample(fsi, frames);
-}
-
-static void fsi_count_fifo_err(struct fsi_priv *fsi)
-{
-	u32 ostatus = fsi_reg_read(fsi, DOFF_ST);
-	u32 istatus = fsi_reg_read(fsi, DIFF_ST);
-
-	if (ostatus & ERR_OVER)
-		fsi->playback.oerr_num++;
-
-	if (ostatus & ERR_UNDER)
-		fsi->playback.uerr_num++;
-
-	if (istatus & ERR_OVER)
-		fsi->capture.oerr_num++;
-
-	if (istatus & ERR_UNDER)
-		fsi->capture.uerr_num++;
-
-	fsi_reg_write(fsi, DOFF_ST, 0);
-	fsi_reg_write(fsi, DIFF_ST, 0);
-}
-
 /*
  *		dma function
  */
@@ -481,7 +480,7 @@ static void fsi_count_fifo_err(struct fsi_priv *fsi)
 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 fsi_stream *io = fsi_stream_get(fsi, is_play);
 	struct snd_pcm_runtime *runtime = io->substream->runtime;
 
 	return runtime->dma_area +
@@ -698,7 +697,7 @@ static void fsi_fifo_init(struct fsi_priv *fsi,
 			  struct device *dev)
 {
 	struct fsi_master *master = fsi_get_master(fsi);
-	struct fsi_stream *io = fsi_get_stream(fsi, is_play);
+	struct fsi_stream *io = fsi_stream_get(fsi, is_play);
 	u32 shift, i;
 	int frame_capa;
 
@@ -753,7 +752,7 @@ static int fsi_fifo_data_ctrl(struct fsi_priv *fsi, int stream)
 	struct snd_pcm_runtime *runtime;
 	struct snd_pcm_substream *substream = NULL;
 	int is_play = fsi_stream_is_play(stream);
-	struct fsi_stream *io = fsi_get_stream(fsi, is_play);
+	struct fsi_stream *io = fsi_stream_get(fsi, is_play);
 	int sample_residues;
 	int samples;
 	int samples_max;
@@ -1150,7 +1149,7 @@ static int fsi_hw_free(struct snd_pcm_substream *substream)
 static snd_pcm_uframes_t fsi_pointer(struct snd_pcm_substream *substream)
 {
 	struct fsi_priv *fsi = fsi_get_priv(substream);
-	struct fsi_stream *io = fsi_get_stream(fsi, fsi_is_play(substream));
+	struct fsi_stream *io = fsi_stream_get(fsi, fsi_is_play(substream));
 	int samples_pos = io->buff_sample_pos - 1;
 
 	if (samples_pos < 0)
-- 
1.7.5.4

Re: [PATCH 02/19] ASoC: fsi: tidyup: fsi_stream_xx() functions were gathered

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 391 bytes --]

On Fri, Feb 03, 2012 at 12:50:35AM -0800, Kuninori Morimoto wrote:

> +/*
> + *		fsi_stream_xx() function
> + */
> +#define fsi_is_play(substream)	fsi_stream_is_play(substream->stream)
> +static inline int fsi_stream_is_play(int stream)
> +{
> +	return stream == SNDRV_PCM_STREAM_PLAYBACK;
> +}

That #define should be an inline function.  I'll apply this but please
fix this incrementally.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH 03/19] ASoC: fsi: data push/pop calculation part was divided

[Kuninori Morimoto]

Next transfer data size of "push" and "pop" had calculated on shared function.
But it was not readable code.
This patch divided it into for push, and for pop.

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

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index 162416e..cbb5643 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -747,17 +747,14 @@ static void fsi_fifo_init(struct fsi_priv *fsi,
 	}
 }
 
-static int fsi_fifo_data_ctrl(struct fsi_priv *fsi, int stream)
+static int fsi_fifo_data_ctrl(struct fsi_priv *fsi, struct fsi_stream *io,
+			      void (*run16)(struct fsi_priv *fsi, int size),
+			      void (*run32)(struct fsi_priv *fsi, int size),
+			      int samples)
 {
 	struct snd_pcm_runtime *runtime;
-	struct snd_pcm_substream *substream = NULL;
-	int is_play = fsi_stream_is_play(stream);
-	struct fsi_stream *io = fsi_stream_get(fsi, is_play);
-	int sample_residues;
-	int samples;
-	int samples_max;
+	struct snd_pcm_substream *substream;
 	int over_period;
-	void (*fn)(struct fsi_priv *fsi, int size);
 
 	if (!fsi			||
 	    !io->substream		||
@@ -781,57 +778,17 @@ static int fsi_fifo_data_ctrl(struct fsi_priv *fsi, int stream)
 			io->buff_sample_pos = 0;
 	}
 
-	/* get number of residue samples */
-	sample_residues = io->buff_sample_capa - io->buff_sample_pos;
-
-	if (is_play) {
-		/*
-		 * for play-back
-		 *
-		 * samples_max	: number of FSI fifo free samples space
-		 * samples	: number of ALSA residue samples
-		 */
-		samples_max  = io->fifo_sample_capa;
-		samples_max -= fsi_get_current_fifo_samples(fsi, is_play);
-
-		samples = sample_residues;
-
-		switch (io->sample_width) {
-		case 2:
-			fn = fsi_dma_soft_push16;
-			break;
-		case 4:
-			fn = fsi_dma_soft_push32;
-			break;
-		default:
-			return -EINVAL;
-		}
-	} else {
-		/*
-		 * for capture
-		 *
-		 * samples_max	: number of ALSA free samples space
-		 * samples	: number of samples in FSI fifo
-		 */
-		samples_max = sample_residues;
-		samples     = fsi_get_current_fifo_samples(fsi, is_play);
-
-		switch (io->sample_width) {
-		case 2:
-			fn = fsi_dma_soft_pop16;
-			break;
-		case 4:
-			fn = fsi_dma_soft_pop32;
-			break;
-		default:
-			return -EINVAL;
-		}
+	switch (io->sample_width) {
+	case 2:
+		run16(fsi, samples);
+		break;
+	case 4:
+		run32(fsi, samples);
+		break;
+	default:
+		return -EINVAL;
 	}
 
-	samples = min(samples, samples_max);
-
-	fn(fsi, samples);
-
 	/* update buff_sample_pos */
 	io->buff_sample_pos += samples;
 
@@ -843,12 +800,41 @@ static int fsi_fifo_data_ctrl(struct fsi_priv *fsi, int stream)
 
 static int fsi_data_pop(struct fsi_priv *fsi)
 {
-	return fsi_fifo_data_ctrl(fsi, SNDRV_PCM_STREAM_CAPTURE);
+	int is_play = fsi_stream_is_play(SNDRV_PCM_STREAM_CAPTURE);
+	int sample_residues;	/* samples in FSI fifo */
+	int sample_space;	/* ALSA free samples space */
+	int samples;
+	struct fsi_stream *io = fsi_stream_get(fsi, is_play);
+
+	sample_residues	= fsi_get_current_fifo_samples(fsi, is_play);
+	sample_space	= io->buff_sample_capa - io->buff_sample_pos;
+
+	samples = min(sample_residues, sample_space);
+
+	return fsi_fifo_data_ctrl(fsi, io,
+				  fsi_dma_soft_pop16,
+				  fsi_dma_soft_pop32,
+				  samples);
 }
 
 static int fsi_data_push(struct fsi_priv *fsi)
 {
-	return fsi_fifo_data_ctrl(fsi, SNDRV_PCM_STREAM_PLAYBACK);
+	int is_play = fsi_stream_is_play(SNDRV_PCM_STREAM_PLAYBACK);
+	int sample_residues;	/* ALSA residue samples */
+	int sample_space;	/* FSI fifo free samples space */
+	int samples;
+	struct fsi_stream *io = fsi_stream_get(fsi, is_play);
+
+	sample_residues	= io->buff_sample_capa - io->buff_sample_pos;
+	sample_space	= io->fifo_sample_capa -
+		fsi_get_current_fifo_samples(fsi, is_play);
+
+	samples = min(sample_residues, sample_space);
+
+	return fsi_fifo_data_ctrl(fsi, io,
+				  fsi_dma_soft_push16,
+				  fsi_dma_soft_push32,
+				  samples);
 }
 
 static irqreturn_t fsi_interrupt(int irq, void *data)
-- 
1.7.5.4

[PATCH 04/19] ASoC: fsi: rename fsi_dma_soft_xxx() to fsi_pio_xxx()

[Kuninori Morimoto]

This is preparation for DMAEngine support

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

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index cbb5643..05307ac 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -474,10 +474,10 @@ static void fsi_stream_pop(struct fsi_priv *fsi, int is_play)
 }
 
 /*
- *		dma function
+ *		pio function
  */
 
-static u8 *fsi_dma_get_area(struct fsi_priv *fsi, int stream)
+static u8 *fsi_pio_get_area(struct fsi_priv *fsi, int stream)
 {
 	int is_play = fsi_stream_is_play(stream);
 	struct fsi_stream *io = fsi_stream_get(fsi, is_play);
@@ -487,47 +487,47 @@ static u8 *fsi_dma_get_area(struct fsi_priv *fsi, int stream)
 		samples_to_bytes(runtime, io->buff_sample_pos);
 }
 
-static void fsi_dma_soft_push16(struct fsi_priv *fsi, int num)
+static void fsi_pio_push16(struct fsi_priv *fsi, int num)
 {
 	u16 *start;
 	int i;
 
-	start  = (u16 *)fsi_dma_get_area(fsi, SNDRV_PCM_STREAM_PLAYBACK);
+	start  = (u16 *)fsi_pio_get_area(fsi, SNDRV_PCM_STREAM_PLAYBACK);
 
 	for (i = 0; i < num; i++)
 		fsi_reg_write(fsi, DODT, ((u32)*(start + i) << 8));
 }
 
-static void fsi_dma_soft_pop16(struct fsi_priv *fsi, int num)
+static void fsi_pio_pop16(struct fsi_priv *fsi, int num)
 {
 	u16 *start;
 	int i;
 
-	start  = (u16 *)fsi_dma_get_area(fsi, SNDRV_PCM_STREAM_CAPTURE);
+	start  = (u16 *)fsi_pio_get_area(fsi, SNDRV_PCM_STREAM_CAPTURE);
 
 
 	for (i = 0; i < num; i++)
 		*(start + i) = (u16)(fsi_reg_read(fsi, DIDT) >> 8);
 }
 
-static void fsi_dma_soft_push32(struct fsi_priv *fsi, int num)
+static void fsi_pio_push32(struct fsi_priv *fsi, int num)
 {
 	u32 *start;
 	int i;
 
-	start  = (u32 *)fsi_dma_get_area(fsi, SNDRV_PCM_STREAM_PLAYBACK);
+	start  = (u32 *)fsi_pio_get_area(fsi, SNDRV_PCM_STREAM_PLAYBACK);
 
 
 	for (i = 0; i < num; i++)
 		fsi_reg_write(fsi, DODT, *(start + i));
 }
 
-static void fsi_dma_soft_pop32(struct fsi_priv *fsi, int num)
+static void fsi_pio_pop32(struct fsi_priv *fsi, int num)
 {
 	u32 *start;
 	int i;
 
-	start  = (u32 *)fsi_dma_get_area(fsi, SNDRV_PCM_STREAM_CAPTURE);
+	start  = (u32 *)fsi_pio_get_area(fsi, SNDRV_PCM_STREAM_CAPTURE);
 
 	for (i = 0; i < num; i++)
 		*(start + i) = fsi_reg_read(fsi, DIDT);
@@ -812,8 +812,8 @@ static int fsi_data_pop(struct fsi_priv *fsi)
 	samples = min(sample_residues, sample_space);
 
 	return fsi_fifo_data_ctrl(fsi, io,
-				  fsi_dma_soft_pop16,
-				  fsi_dma_soft_pop32,
+				  fsi_pio_pop16,
+				  fsi_pio_pop32,
 				  samples);
 }
 
@@ -832,8 +832,8 @@ static int fsi_data_push(struct fsi_priv *fsi)
 	samples = min(sample_residues, sample_space);
 
 	return fsi_fifo_data_ctrl(fsi, io,
-				  fsi_dma_soft_push16,
-				  fsi_dma_soft_push32,
+				  fsi_pio_push16,
+				  fsi_pio_push32,
 				  samples);
 }
 
-- 
1.7.5.4

[PATCH 05/19] ASoC: fsi: tidyup: move fsi_fifo_init() onto fsi_hw_startup()

[Kuninori Morimoto]

fsi_fifo_init() is called only from fsi_hw_startup()

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

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index 05307ac..79485ed 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -692,61 +692,6 @@ static void __fsi_port_clk_ctrl(struct fsi_priv *fsi, int is_play, int enable)
 /*
  *		ctrl function
  */
-static void fsi_fifo_init(struct fsi_priv *fsi,
-			  int is_play,
-			  struct device *dev)
-{
-	struct fsi_master *master = fsi_get_master(fsi);
-	struct fsi_stream *io = fsi_stream_get(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;
-	frame_capa = 256 << shift;
-	dev_dbg(dev, "fifo = %d words\n", frame_capa);
-
-	/*
-	 * The maximum number of sample data varies depending
-	 * on the number of channels selected for the format.
-	 *
-	 * FIFOs are used in 4-channel units in 3-channel mode
-	 * and in 8-channel units in 5- to 7-channel mode
-	 * meaning that more FIFOs than the required size of DPRAM
-	 * are used.
-	 *
-	 * ex) if 256 words of DP-RAM is connected
-	 * 1 channel:  256 (256 x 1 = 256)
-	 * 2 channels: 128 (128 x 2 = 256)
-	 * 3 channels:  64 ( 64 x 3 = 192)
-	 * 4 channels:  64 ( 64 x 4 = 256)
-	 * 5 channels:  32 ( 32 x 5 = 160)
-	 * 6 channels:  32 ( 32 x 6 = 192)
-	 * 7 channels:  32 ( 32 x 7 = 224)
-	 * 8 channels:  32 ( 32 x 8 = 256)
-	 */
-	for (i = 1; i < fsi->chan_num; i <<= 1)
-		frame_capa >>= 1;
-	dev_dbg(dev, "%d channel %d store\n",
-		fsi->chan_num, frame_capa);
-
-	io->fifo_sample_capa = fsi_frame2sample(fsi, frame_capa);
-
-	/*
-	 * set interrupt generation factor
-	 * clear FIFO
-	 */
-	if (is_play) {
-		fsi_reg_write(fsi,	DOFF_CTL, IRQ_HALF);
-		fsi_reg_mask_set(fsi,	DOFF_CTL, FIFO_CLR, FIFO_CLR);
-	} else {
-		fsi_reg_write(fsi,	DIFF_CTL, IRQ_HALF);
-		fsi_reg_mask_set(fsi,	DIFF_CTL, FIFO_CLR, FIFO_CLR);
-	}
-}
-
 static int fsi_fifo_data_ctrl(struct fsi_priv *fsi, struct fsi_stream *io,
 			      void (*run16)(struct fsi_priv *fsi, int size),
 			      void (*run32)(struct fsi_priv *fsi, int size),
@@ -867,6 +812,60 @@ static irqreturn_t fsi_interrupt(int irq, void *data)
 /*
  *		dai ops
  */
+static void fsi_fifo_init(struct fsi_priv *fsi,
+			  int is_play,
+			  struct device *dev)
+{
+	struct fsi_master *master = fsi_get_master(fsi);
+	struct fsi_stream *io = fsi_stream_get(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;
+	frame_capa = 256 << shift;
+	dev_dbg(dev, "fifo = %d words\n", frame_capa);
+
+	/*
+	 * The maximum number of sample data varies depending
+	 * on the number of channels selected for the format.
+	 *
+	 * FIFOs are used in 4-channel units in 3-channel mode
+	 * and in 8-channel units in 5- to 7-channel mode
+	 * meaning that more FIFOs than the required size of DPRAM
+	 * are used.
+	 *
+	 * ex) if 256 words of DP-RAM is connected
+	 * 1 channel:  256 (256 x 1 = 256)
+	 * 2 channels: 128 (128 x 2 = 256)
+	 * 3 channels:  64 ( 64 x 3 = 192)
+	 * 4 channels:  64 ( 64 x 4 = 256)
+	 * 5 channels:  32 ( 32 x 5 = 160)
+	 * 6 channels:  32 ( 32 x 6 = 192)
+	 * 7 channels:  32 ( 32 x 7 = 224)
+	 * 8 channels:  32 ( 32 x 8 = 256)
+	 */
+	for (i = 1; i < fsi->chan_num; i <<= 1)
+		frame_capa >>= 1;
+	dev_dbg(dev, "%d channel %d store\n",
+		fsi->chan_num, frame_capa);
+
+	io->fifo_sample_capa = fsi_frame2sample(fsi, frame_capa);
+
+	/*
+	 * set interrupt generation factor
+	 * clear FIFO
+	 */
+	if (is_play) {
+		fsi_reg_write(fsi,	DOFF_CTL, IRQ_HALF);
+		fsi_reg_mask_set(fsi,	DOFF_CTL, FIFO_CLR, FIFO_CLR);
+	} else {
+		fsi_reg_write(fsi,	DIFF_CTL, IRQ_HALF);
+		fsi_reg_mask_set(fsi,	DIFF_CTL, FIFO_CLR, FIFO_CLR);
+	}
+}
 
 static int fsi_hw_startup(struct fsi_priv *fsi,
 			  int is_play,
-- 
1.7.5.4

[PATCH 06/19] ASoC: fsi: remove unnecessary parameter from fsi_hw_shutdown()

[Kuninori Morimoto]

This is preparation for DMAEngine support

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

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index 79485ed..a0a9c36 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -927,7 +927,6 @@ static int fsi_hw_startup(struct fsi_priv *fsi,
 }
 
 static void fsi_hw_shutdown(struct fsi_priv *fsi,
-			    int is_play,
 			    struct device *dev)
 {
 	if (fsi_is_clk_master(fsi))
@@ -947,9 +946,8 @@ 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_hw_shutdown(fsi, is_play, dai->dev);
+	fsi_hw_shutdown(fsi, dai->dev);
 	fsi->rate = 0;
 }
 
@@ -1342,7 +1340,7 @@ static void __fsi_suspend(struct fsi_priv *fsi,
 		return;
 
 	fsi_port_stop(fsi, is_play);
-	fsi_hw_shutdown(fsi, is_play, dev);
+	fsi_hw_shutdown(fsi, dev);
 }
 
 static void __fsi_resume(struct fsi_priv *fsi,
-- 
1.7.5.4

[PATCH 07/19] ASoC: fsi: rename fsi_stream_push/pop() to fsi_stream_init/quit()

[Kuninori Morimoto]

This is preparation for DMAEngine support

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

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index a0a9c36..2e2663b 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -426,7 +426,7 @@ static int fsi_stream_is_working(struct fsi_priv *fsi,
 	return ret;
 }
 
-static void fsi_stream_push(struct fsi_priv *fsi,
+static void fsi_stream_init(struct fsi_priv *fsi,
 			    int is_play,
 			    struct snd_pcm_substream *substream)
 {
@@ -447,7 +447,7 @@ static void fsi_stream_push(struct fsi_priv *fsi,
 	spin_unlock_irqrestore(&master->lock, flags);
 }
 
-static void fsi_stream_pop(struct fsi_priv *fsi, int is_play)
+static void fsi_stream_quit(struct fsi_priv *fsi, int is_play)
 {
 	struct fsi_stream *io = fsi_stream_get(fsi, is_play);
 	struct snd_soc_dai *dai = fsi_get_dai(io->substream);
@@ -960,13 +960,13 @@ static int fsi_dai_trigger(struct snd_pcm_substream *substream, int cmd,
 
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
-		fsi_stream_push(fsi, is_play, substream);
+		fsi_stream_init(fsi, is_play, substream);
 		ret = is_play ? fsi_data_push(fsi) : fsi_data_pop(fsi);
 		fsi_port_start(fsi, is_play);
 		break;
 	case SNDRV_PCM_TRIGGER_STOP:
 		fsi_port_stop(fsi, is_play);
-		fsi_stream_pop(fsi, is_play);
+		fsi_stream_quit(fsi, is_play);
 		break;
 	}
 
-- 
1.7.5.4

[PATCH 08/19] ASoC: fsi: modify fsi_pio_get_area() parameter and using position

[Kuninori Morimoto]

This patch modify fsi_pio_get_area() parameter to use
struct fsi_stream, and used it on fsi_fifo_data_ctrl().
This is just prepare cleanup for DMAEngine support.

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

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index 2e2663b..c814d8a 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -477,58 +477,46 @@ static void fsi_stream_quit(struct fsi_priv *fsi, int is_play)
  *		pio function
  */
 
-static u8 *fsi_pio_get_area(struct fsi_priv *fsi, int stream)
+static u8 *fsi_pio_get_area(struct fsi_priv *fsi, struct fsi_stream *io)
 {
-	int is_play = fsi_stream_is_play(stream);
-	struct fsi_stream *io = fsi_stream_get(fsi, is_play);
 	struct snd_pcm_runtime *runtime = io->substream->runtime;
 
 	return runtime->dma_area +
 		samples_to_bytes(runtime, io->buff_sample_pos);
 }
 
-static void fsi_pio_push16(struct fsi_priv *fsi, int num)
+static void fsi_pio_push16(struct fsi_priv *fsi, u8 *_buf, int num)
 {
-	u16 *start;
+	u16 *start = (u16 *)_buf;
 	int i;
 
-	start  = (u16 *)fsi_pio_get_area(fsi, SNDRV_PCM_STREAM_PLAYBACK);
-
 	for (i = 0; i < num; i++)
 		fsi_reg_write(fsi, DODT, ((u32)*(start + i) << 8));
 }
 
-static void fsi_pio_pop16(struct fsi_priv *fsi, int num)
+static void fsi_pio_pop16(struct fsi_priv *fsi, u8 *_buf, int num)
 {
-	u16 *start;
+	u16 *start = (u16 *)_buf;
 	int i;
 
-	start  = (u16 *)fsi_pio_get_area(fsi, SNDRV_PCM_STREAM_CAPTURE);
-
-
 	for (i = 0; i < num; i++)
 		*(start + i) = (u16)(fsi_reg_read(fsi, DIDT) >> 8);
 }
 
-static void fsi_pio_push32(struct fsi_priv *fsi, int num)
+static void fsi_pio_push32(struct fsi_priv *fsi, u8 *_buf, int num)
 {
-	u32 *start;
+	u32 *start = (u32 *)_buf;
 	int i;
 
-	start  = (u32 *)fsi_pio_get_area(fsi, SNDRV_PCM_STREAM_PLAYBACK);
-
-
 	for (i = 0; i < num; i++)
 		fsi_reg_write(fsi, DODT, *(start + i));
 }
 
-static void fsi_pio_pop32(struct fsi_priv *fsi, int num)
+static void fsi_pio_pop32(struct fsi_priv *fsi, u8 *_buf, int num)
 {
-	u32 *start;
+	u32 *start = (u32 *)_buf;
 	int i;
 
-	start  = (u32 *)fsi_pio_get_area(fsi, SNDRV_PCM_STREAM_CAPTURE);
-
 	for (i = 0; i < num; i++)
 		*(start + i) = fsi_reg_read(fsi, DIDT);
 }
@@ -693,12 +681,13 @@ static void __fsi_port_clk_ctrl(struct fsi_priv *fsi, int is_play, int enable)
  *		ctrl function
  */
 static int fsi_fifo_data_ctrl(struct fsi_priv *fsi, struct fsi_stream *io,
-			      void (*run16)(struct fsi_priv *fsi, int size),
-			      void (*run32)(struct fsi_priv *fsi, int size),
-			      int samples)
+		void (*run16)(struct fsi_priv *fsi, u8 *buf, int samples),
+		void (*run32)(struct fsi_priv *fsi, u8 *buf, int samples),
+		int samples)
 {
 	struct snd_pcm_runtime *runtime;
 	struct snd_pcm_substream *substream;
+	u8 *buf;
 	int over_period;
 
 	if (!fsi			||
@@ -723,12 +712,14 @@ static int fsi_fifo_data_ctrl(struct fsi_priv *fsi, struct fsi_stream *io,
 			io->buff_sample_pos = 0;
 	}
 
+	buf = fsi_pio_get_area(fsi, io);
+
 	switch (io->sample_width) {
 	case 2:
-		run16(fsi, samples);
+		run16(fsi, buf, samples);
 		break;
 	case 4:
-		run32(fsi, samples);
+		run32(fsi, buf, samples);
 		break;
 	default:
 		return -EINVAL;
-- 
1.7.5.4

[PATCH 09/19] ASoC: fsi: re-define fsi_is_play() and fsi_stream_is_play()

[Kuninori Morimoto]

This patch re-define fsi_is_play() and fsi_stream_is_play().
fsi_data_pop/push() function keeps direct value of "is_play" at this point,
but it will be removed soon.
This is just prepare cleanup for DMAEngine support.

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

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index c814d8a..1cbe474 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -296,6 +296,11 @@ static int fsi_is_spdif(struct fsi_priv *fsi)
 	return fsi->spdif;
 }
 
+static int fsi_is_play(struct snd_pcm_substream *substream)
+{
+	return substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
+}
+
 static struct snd_soc_dai *fsi_get_dai(struct snd_pcm_substream *substream)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
@@ -399,10 +404,10 @@ static void fsi_count_fifo_err(struct fsi_priv *fsi)
 /*
  *		fsi_stream_xx() function
  */
-#define fsi_is_play(substream)	fsi_stream_is_play(substream->stream)
-static inline int fsi_stream_is_play(int stream)
+static inline int fsi_stream_is_play(struct fsi_priv *fsi,
+				     struct fsi_stream *io)
 {
-	return stream == SNDRV_PCM_STREAM_PLAYBACK;
+	return &fsi->playback == io;
 }
 
 static inline struct fsi_stream *fsi_stream_get(struct fsi_priv *fsi,
@@ -736,7 +741,7 @@ static int fsi_fifo_data_ctrl(struct fsi_priv *fsi, struct fsi_stream *io,
 
 static int fsi_data_pop(struct fsi_priv *fsi)
 {
-	int is_play = fsi_stream_is_play(SNDRV_PCM_STREAM_CAPTURE);
+	int is_play = 0;
 	int sample_residues;	/* samples in FSI fifo */
 	int sample_space;	/* ALSA free samples space */
 	int samples;
@@ -755,7 +760,7 @@ static int fsi_data_pop(struct fsi_priv *fsi)
 
 static int fsi_data_push(struct fsi_priv *fsi)
 {
-	int is_play = fsi_stream_is_play(SNDRV_PCM_STREAM_PLAYBACK);
+	int is_play = 1;
 	int sample_residues;	/* ALSA residue samples */
 	int sample_space;	/* FSI fifo free samples space */
 	int samples;
-- 
1.7.5.4

Re: [PATCH 09/19] ASoC: fsi: re-define fsi_is_play() and fsi_stream_is_play()

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 239 bytes --]

On Fri, Feb 03, 2012 at 12:54:02AM -0800, Kuninori Morimoto wrote:

> +static int fsi_is_play(struct snd_pcm_substream *substream)
> +{
> +	return substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
> +}
> +

Oh, good you fixed that up later.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH 10/19] ASoC: fsi: use fsi_stream in fsi_get_current_fifo_samples() parameter

[Kuninori Morimoto]

fsi_get_current_fifo_samples() uses fsi_stream instead of is_play.
This is just prepare cleanup for DMAEngine support.

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

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index 1cbe474..24dbe16 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -210,6 +210,8 @@ struct fsi_master {
 	spinlock_t lock;
 };
 
+static int fsi_stream_is_play(struct fsi_priv *fsi, struct fsi_stream *io);
+
 /*
  *		basic read write function
  */
@@ -366,8 +368,10 @@ static int fsi_sample2frame(struct fsi_priv *fsi, int samples)
 	return samples / fsi->chan_num;
 }
 
-static int fsi_get_current_fifo_samples(struct fsi_priv *fsi, int is_play)
+static int fsi_get_current_fifo_samples(struct fsi_priv *fsi,
+					struct fsi_stream *io)
 {
+	int is_play = fsi_stream_is_play(fsi, io);
 	u32 status;
 	int frames;
 
@@ -747,7 +751,7 @@ static int fsi_data_pop(struct fsi_priv *fsi)
 	int samples;
 	struct fsi_stream *io = fsi_stream_get(fsi, is_play);
 
-	sample_residues	= fsi_get_current_fifo_samples(fsi, is_play);
+	sample_residues	= fsi_get_current_fifo_samples(fsi, io);
 	sample_space	= io->buff_sample_capa - io->buff_sample_pos;
 
 	samples = min(sample_residues, sample_space);
@@ -768,7 +772,7 @@ static int fsi_data_push(struct fsi_priv *fsi)
 
 	sample_residues	= io->buff_sample_capa - io->buff_sample_pos;
 	sample_space	= io->fifo_sample_capa -
-		fsi_get_current_fifo_samples(fsi, is_play);
+		fsi_get_current_fifo_samples(fsi, io);
 
 	samples = min(sample_residues, sample_space);
 
-- 
1.7.5.4

[PATCH 11/19] ASoC: fsi: add fsi_stream_handler and PIO handler

[Kuninori Morimoto]

This patch adds struct fsi_stream_handler and defined fsi_pio_push/pop_handler.
these are controled by fsi_steam_xxx() function.

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

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index 24dbe16..b02886a 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -159,18 +159,27 @@ typedef int (*set_rate_func)(struct device *dev, int is_porta, int rate, int ena
  *		struct
  */
 
+struct fsi_stream_handler;
 struct fsi_stream {
-	struct snd_pcm_substream *substream;
 
+	/*
+	 * these are initialized by fsi_stream_init()
+	 */
+	struct snd_pcm_substream *substream;
 	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 sample_width;	/* sample width */
-
 	int uerr_num;
 	int oerr_num;
+
+	/*
+	 * thse are initialized by fsi_handler_init()
+	 */
+	struct fsi_stream_handler *handler;
+	struct fsi_priv		*priv;
 };
 
 struct fsi_priv {
@@ -190,6 +199,16 @@ struct fsi_priv {
 	long rate;
 };
 
+struct fsi_stream_handler {
+	int (*probe)(struct fsi_priv *fsi, struct fsi_stream *io);
+	int (*transfer)(struct fsi_priv *fsi, struct fsi_stream *io);
+	int (*remove)(struct fsi_priv *fsi, struct fsi_stream *io);
+};
+#define fsi_stream_handler_call(io, func, args...)	\
+	(!(io) ? -ENODEV :				\
+	 !((io)->handler->func) ? 0 :			\
+	 (io)->handler->func(args))
+
 struct fsi_core {
 	int ver;
 
@@ -435,6 +454,11 @@ static int fsi_stream_is_working(struct fsi_priv *fsi,
 	return ret;
 }
 
+static struct fsi_priv *fsi_stream_to_priv(struct fsi_stream *io)
+{
+	return io->priv;
+}
+
 static void fsi_stream_init(struct fsi_priv *fsi,
 			    int is_play,
 			    struct snd_pcm_substream *substream)
@@ -482,6 +506,53 @@ static void fsi_stream_quit(struct fsi_priv *fsi, int is_play)
 	spin_unlock_irqrestore(&master->lock, flags);
 }
 
+static int fsi_stream_transfer(struct fsi_stream *io)
+{
+	struct fsi_priv *fsi = fsi_stream_to_priv(io);
+	if (!fsi)
+		return -EIO;
+
+	return fsi_stream_handler_call(io, transfer, fsi, io);
+}
+
+static int fsi_stream_probe(struct fsi_priv *fsi)
+{
+	struct fsi_stream *io;
+	int ret1, ret2;
+
+	io = &fsi->playback;
+	ret1 = fsi_stream_handler_call(io, probe, fsi, io);
+
+	io = &fsi->capture;
+	ret2 = fsi_stream_handler_call(io, probe, fsi, io);
+
+	if (ret1 < 0)
+		return ret1;
+	if (ret2 < 0)
+		return ret2;
+
+	return 0;
+}
+
+static int fsi_stream_remove(struct fsi_priv *fsi)
+{
+	struct fsi_stream *io;
+	int ret1, ret2;
+
+	io = &fsi->playback;
+	ret1 = fsi_stream_handler_call(io, remove, fsi, io);
+
+	io = &fsi->capture;
+	ret2 = fsi_stream_handler_call(io, remove, fsi, io);
+
+	if (ret1 < 0)
+		return ret1;
+	if (ret2 < 0)
+		return ret2;
+
+	return 0;
+}
+
 /*
  *		pio function
  */
@@ -743,13 +814,11 @@ static int fsi_fifo_data_ctrl(struct fsi_priv *fsi, struct fsi_stream *io,
 	return 0;
 }
 
-static int fsi_data_pop(struct fsi_priv *fsi)
+static int fsi_pio_pop(struct fsi_priv *fsi, struct fsi_stream *io)
 {
-	int is_play = 0;
 	int sample_residues;	/* samples in FSI fifo */
 	int sample_space;	/* ALSA free samples space */
 	int samples;
-	struct fsi_stream *io = fsi_stream_get(fsi, is_play);
 
 	sample_residues	= fsi_get_current_fifo_samples(fsi, io);
 	sample_space	= io->buff_sample_capa - io->buff_sample_pos;
@@ -762,13 +831,11 @@ static int fsi_data_pop(struct fsi_priv *fsi)
 				  samples);
 }
 
-static int fsi_data_push(struct fsi_priv *fsi)
+static int fsi_pio_push(struct fsi_priv *fsi, struct fsi_stream *io)
 {
-	int is_play = 1;
 	int sample_residues;	/* ALSA residue samples */
 	int sample_space;	/* FSI fifo free samples space */
 	int samples;
-	struct fsi_stream *io = fsi_stream_get(fsi, is_play);
 
 	sample_residues	= io->buff_sample_capa - io->buff_sample_pos;
 	sample_space	= io->fifo_sample_capa -
@@ -782,6 +849,14 @@ static int fsi_data_push(struct fsi_priv *fsi)
 				  samples);
 }
 
+static struct fsi_stream_handler fsi_pio_push_handler = {
+	.transfer	= fsi_pio_push,
+};
+
+static struct fsi_stream_handler fsi_pio_pop_handler = {
+	.transfer	= fsi_pio_pop,
+};
+
 static irqreturn_t fsi_interrupt(int irq, void *data)
 {
 	struct fsi_master *master = data;
@@ -792,13 +867,13 @@ static irqreturn_t fsi_interrupt(int irq, void *data)
 	fsi_master_mask_set(master, SOFT_RST, IR, IR);
 
 	if (int_st & AB_IO(1, AO_SHIFT))
-		fsi_data_push(&master->fsia);
+		fsi_stream_transfer(&master->fsia.playback);
 	if (int_st & AB_IO(1, BO_SHIFT))
-		fsi_data_push(&master->fsib);
+		fsi_stream_transfer(&master->fsib.playback);
 	if (int_st & AB_IO(1, AI_SHIFT))
-		fsi_data_pop(&master->fsia);
+		fsi_stream_transfer(&master->fsia.capture);
 	if (int_st & AB_IO(1, BI_SHIFT))
-		fsi_data_pop(&master->fsib);
+		fsi_stream_transfer(&master->fsib.capture);
 
 	fsi_count_fifo_err(&master->fsia);
 	fsi_count_fifo_err(&master->fsib);
@@ -955,14 +1030,16 @@ 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 fsi_stream *io = fsi_stream_get(fsi, fsi_is_play(substream));
 	int is_play = fsi_is_play(substream);
 	int ret = 0;
 
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
 		fsi_stream_init(fsi, is_play, substream);
-		ret = is_play ? fsi_data_push(fsi) : fsi_data_pop(fsi);
-		fsi_port_start(fsi, is_play);
+		ret = fsi_stream_transfer(io);
+		if (0 == ret)
+			fsi_port_start(fsi, is_play);
 		break;
 	case SNDRV_PCM_TRIGGER_STOP:
 		fsi_port_stop(fsi, is_play);
@@ -1224,6 +1301,13 @@ static struct snd_soc_platform_driver fsi_soc_platform = {
 /*
  *		platform function
  */
+static void fsi_handler_init(struct fsi_priv *fsi)
+{
+	fsi->playback.handler	= &fsi_pio_push_handler; /* default PIO */
+	fsi->playback.priv	= fsi;
+	fsi->capture.handler	= &fsi_pio_pop_handler;  /* default PIO */
+	fsi->capture.priv	= fsi;
+}
 
 static int fsi_probe(struct platform_device *pdev)
 {
@@ -1270,10 +1354,22 @@ static int fsi_probe(struct platform_device *pdev)
 	/* FSI A setting */
 	master->fsia.base	= master->base;
 	master->fsia.master	= master;
+	fsi_handler_init(&master->fsia);
+	ret = fsi_stream_probe(&master->fsia);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "FSIA stream probe failed\n");
+		goto exit_iounmap;
+	}
 
 	/* FSI B setting */
 	master->fsib.base	= master->base + 0x40;
 	master->fsib.master	= master;
+	fsi_handler_init(&master->fsib);
+	ret = fsi_stream_probe(&master->fsib);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "FSIB stream probe failed\n");
+		goto exit_fsia;
+	}
 
 	pm_runtime_enable(&pdev->dev);
 	dev_set_drvdata(&pdev->dev, master);
@@ -1282,7 +1378,7 @@ static int fsi_probe(struct platform_device *pdev)
 			  id_entry->name, master);
 	if (ret) {
 		dev_err(&pdev->dev, "irq request err\n");
-		goto exit_iounmap;
+		goto exit_fsib;
 	}
 
 	ret = snd_soc_register_platform(&pdev->dev, &fsi_soc_platform);
@@ -1304,6 +1400,10 @@ exit_snd_soc:
 	snd_soc_unregister_platform(&pdev->dev);
 exit_free_irq:
 	free_irq(irq, master);
+exit_fsib:
+	fsi_stream_remove(&master->fsib);
+exit_fsia:
+	fsi_stream_remove(&master->fsia);
 exit_iounmap:
 	iounmap(master->base);
 	pm_runtime_disable(&pdev->dev);
@@ -1326,6 +1426,9 @@ static int fsi_remove(struct platform_device *pdev)
 	snd_soc_unregister_dais(&pdev->dev, ARRAY_SIZE(fsi_soc_dai));
 	snd_soc_unregister_platform(&pdev->dev);
 
+	fsi_stream_remove(&master->fsia);
+	fsi_stream_remove(&master->fsib);
+
 	iounmap(master->base);
 	kfree(master);
 
-- 
1.7.5.4

[PATCH 12/19] ASoC: fsi: tidyup: fsi_pio_xxx() are gathered

[Kuninori Morimoto]

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

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index b02886a..7c93b7c 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -554,54 +554,6 @@ static int fsi_stream_remove(struct fsi_priv *fsi)
 }
 
 /*
- *		pio function
- */
-
-static u8 *fsi_pio_get_area(struct fsi_priv *fsi, struct fsi_stream *io)
-{
-	struct snd_pcm_runtime *runtime = io->substream->runtime;
-
-	return runtime->dma_area +
-		samples_to_bytes(runtime, io->buff_sample_pos);
-}
-
-static void fsi_pio_push16(struct fsi_priv *fsi, u8 *_buf, int num)
-{
-	u16 *start = (u16 *)_buf;
-	int i;
-
-	for (i = 0; i < num; i++)
-		fsi_reg_write(fsi, DODT, ((u32)*(start + i) << 8));
-}
-
-static void fsi_pio_pop16(struct fsi_priv *fsi, u8 *_buf, int num)
-{
-	u16 *start = (u16 *)_buf;
-	int i;
-
-	for (i = 0; i < num; i++)
-		*(start + i) = (u16)(fsi_reg_read(fsi, DIDT) >> 8);
-}
-
-static void fsi_pio_push32(struct fsi_priv *fsi, u8 *_buf, int num)
-{
-	u32 *start = (u32 *)_buf;
-	int i;
-
-	for (i = 0; i < num; i++)
-		fsi_reg_write(fsi, DODT, *(start + i));
-}
-
-static void fsi_pio_pop32(struct fsi_priv *fsi, u8 *_buf, int num)
-{
-	u32 *start = (u32 *)_buf;
-	int i;
-
-	for (i = 0; i < num; i++)
-		*(start + i) = fsi_reg_read(fsi, DIDT);
-}
-
-/*
  *		irq function
  */
 
@@ -757,10 +709,55 @@ static void __fsi_port_clk_ctrl(struct fsi_priv *fsi, int is_play, int enable)
 		fsi_master_mask_set(master, CLK_RST, clk, (enable) ? clk : 0);
 }
 
+
 /*
- *		ctrl function
+ *		pio data transfer handler
  */
-static int fsi_fifo_data_ctrl(struct fsi_priv *fsi, struct fsi_stream *io,
+static void fsi_pio_push16(struct fsi_priv *fsi, u8 *_buf, int samples)
+{
+	u16 *buf = (u16 *)_buf;
+	int i;
+
+	for (i = 0; i < samples; i++)
+		fsi_reg_write(fsi, DODT, ((u32)*(buf + i) << 8));
+}
+
+static void fsi_pio_pop16(struct fsi_priv *fsi, u8 *_buf, int samples)
+{
+	u16 *buf = (u16 *)_buf;
+	int i;
+
+	for (i = 0; i < samples; i++)
+		*(buf + i) = (u16)(fsi_reg_read(fsi, DIDT) >> 8);
+}
+
+static void fsi_pio_push32(struct fsi_priv *fsi, u8 *_buf, int samples)
+{
+	u32 *buf = (u32 *)_buf;
+	int i;
+
+	for (i = 0; i < samples; i++)
+		fsi_reg_write(fsi, DODT, *(buf + i));
+}
+
+static void fsi_pio_pop32(struct fsi_priv *fsi, u8 *_buf, int samples)
+{
+	u32 *buf = (u32 *)_buf;
+	int i;
+
+	for (i = 0; i < samples; i++)
+		*(buf + i) = fsi_reg_read(fsi, DIDT);
+}
+
+static u8 *fsi_pio_get_area(struct fsi_priv *fsi, struct fsi_stream *io)
+{
+	struct snd_pcm_runtime *runtime = io->substream->runtime;
+
+	return runtime->dma_area +
+		samples_to_bytes(runtime, io->buff_sample_pos);
+}
+
+static int fsi_pio_transfer(struct fsi_priv *fsi, struct fsi_stream *io,
 		void (*run16)(struct fsi_priv *fsi, u8 *buf, int samples),
 		void (*run32)(struct fsi_priv *fsi, u8 *buf, int samples),
 		int samples)
@@ -825,7 +822,7 @@ static int fsi_pio_pop(struct fsi_priv *fsi, struct fsi_stream *io)
 
 	samples = min(sample_residues, sample_space);
 
-	return fsi_fifo_data_ctrl(fsi, io,
+	return fsi_pio_transfer(fsi, io,
 				  fsi_pio_pop16,
 				  fsi_pio_pop32,
 				  samples);
@@ -843,7 +840,7 @@ static int fsi_pio_push(struct fsi_priv *fsi, struct fsi_stream *io)
 
 	samples = min(sample_residues, sample_space);
 
-	return fsi_fifo_data_ctrl(fsi, io,
+	return fsi_pio_transfer(fsi, io,
 				  fsi_pio_push16,
 				  fsi_pio_push32,
 				  samples);
-- 
1.7.5.4

[PATCH 13/19] ASoC: fsi: don't use is_play as a parameter of fsi functions

[Kuninori Morimoto]

is_play should be kept as local valuable.
it prepare cleanup for DMAEngine support

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

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index 7c93b7c..7dec144 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -364,8 +364,9 @@ static u32 fsi_get_info_flags(struct fsi_priv *fsi)
 		master->info->portb_flags;
 }
 
-static u32 fsi_get_port_shift(struct fsi_priv *fsi, int is_play)
+static u32 fsi_get_port_shift(struct fsi_priv *fsi, struct fsi_stream *io)
 {
+	int is_play = fsi_stream_is_play(fsi, io);
 	int is_porta = fsi_is_port_a(fsi);
 	u32 shift;
 
@@ -434,15 +435,14 @@ static inline int fsi_stream_is_play(struct fsi_priv *fsi,
 }
 
 static inline struct fsi_stream *fsi_stream_get(struct fsi_priv *fsi,
-						int is_play)
+					struct snd_pcm_substream *substream)
 {
-	return is_play ? &fsi->playback : &fsi->capture;
+	return fsi_is_play(substream) ? &fsi->playback : &fsi->capture;
 }
 
 static int fsi_stream_is_working(struct fsi_priv *fsi,
-				  int is_play)
+				 struct fsi_stream *io)
 {
-	struct fsi_stream *io = fsi_stream_get(fsi, is_play);
 	struct fsi_master *master = fsi_get_master(fsi);
 	unsigned long flags;
 	int ret;
@@ -460,10 +460,9 @@ static struct fsi_priv *fsi_stream_to_priv(struct fsi_stream *io)
 }
 
 static void fsi_stream_init(struct fsi_priv *fsi,
-			    int is_play,
+			    struct fsi_stream *io,
 			    struct snd_pcm_substream *substream)
 {
-	struct fsi_stream *io = fsi_stream_get(fsi, is_play);
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct fsi_master *master = fsi_get_master(fsi);
 	unsigned long flags;
@@ -480,9 +479,8 @@ static void fsi_stream_init(struct fsi_priv *fsi,
 	spin_unlock_irqrestore(&master->lock, flags);
 }
 
-static void fsi_stream_quit(struct fsi_priv *fsi, int is_play)
+static void fsi_stream_quit(struct fsi_priv *fsi, struct fsi_stream *io)
 {
-	struct fsi_stream *io = fsi_stream_get(fsi, is_play);
 	struct snd_soc_dai *dai = fsi_get_dai(io->substream);
 	struct fsi_master *master = fsi_get_master(fsi);
 	unsigned long flags;
@@ -557,18 +555,18 @@ static int fsi_stream_remove(struct fsi_priv *fsi)
  *		irq function
  */
 
-static void fsi_irq_enable(struct fsi_priv *fsi, int is_play)
+static void fsi_irq_enable(struct fsi_priv *fsi, struct fsi_stream *io)
 {
-	u32 data = AB_IO(1, fsi_get_port_shift(fsi, is_play));
+	u32 data = AB_IO(1, fsi_get_port_shift(fsi, io));
 	struct fsi_master *master = fsi_get_master(fsi);
 
 	fsi_core_mask_set(master, imsk,  data, data);
 	fsi_core_mask_set(master, iemsk, data, data);
 }
 
-static void fsi_irq_disable(struct fsi_priv *fsi, int is_play)
+static void fsi_irq_disable(struct fsi_priv *fsi, struct fsi_stream *io)
 {
-	u32 data = AB_IO(1, fsi_get_port_shift(fsi, is_play));
+	u32 data = AB_IO(1, fsi_get_port_shift(fsi, io));
 	struct fsi_master *master = fsi_get_master(fsi);
 
 	fsi_core_mask_set(master, imsk,  data, 0);
@@ -585,8 +583,8 @@ static void fsi_irq_clear_status(struct fsi_priv *fsi)
 	u32 data = 0;
 	struct fsi_master *master = fsi_get_master(fsi);
 
-	data |= AB_IO(1, fsi_get_port_shift(fsi, 0));
-	data |= AB_IO(1, fsi_get_port_shift(fsi, 1));
+	data |= AB_IO(1, fsi_get_port_shift(fsi, &fsi->playback));
+	data |= AB_IO(1, fsi_get_port_shift(fsi, &fsi->capture));
 
 	/* clear interrupt factor */
 	fsi_core_mask_set(master, int_st, data, 0);
@@ -695,15 +693,16 @@ static int fsi_set_master_clk(struct device *dev, struct fsi_priv *fsi,
 
 #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)
+static void __fsi_port_clk_ctrl(struct fsi_priv *fsi, struct fsi_stream *io,
+				int enable)
 {
 	struct fsi_master *master = fsi_get_master(fsi);
 	u32 clk  = fsi_is_port_a(fsi) ? CRA  : CRB;
 
 	if (enable)
-		fsi_irq_enable(fsi, is_play);
+		fsi_irq_enable(fsi, io);
 	else
-		fsi_irq_disable(fsi, is_play);
+		fsi_irq_disable(fsi, io);
 
 	if (fsi_is_clk_master(fsi))
 		fsi_master_mask_set(master, CLK_RST, clk, (enable) ? clk : 0);
@@ -885,17 +884,17 @@ static irqreturn_t fsi_interrupt(int irq, void *data)
  *		dai ops
  */
 static void fsi_fifo_init(struct fsi_priv *fsi,
-			  int is_play,
+			  struct fsi_stream *io,
 			  struct device *dev)
 {
 	struct fsi_master *master = fsi_get_master(fsi);
-	struct fsi_stream *io = fsi_stream_get(fsi, is_play);
+	int is_play = fsi_stream_is_play(fsi, io);
 	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 >>= fsi_get_port_shift(fsi, io);
 	shift &= FIFO_SZ_MASK;
 	frame_capa = 256 << shift;
 	dev_dbg(dev, "fifo = %d words\n", frame_capa);
@@ -940,7 +939,7 @@ static void fsi_fifo_init(struct fsi_priv *fsi,
 }
 
 static int fsi_hw_startup(struct fsi_priv *fsi,
-			  int is_play,
+			  struct fsi_stream *io,
 			  struct device *dev)
 {
 	struct fsi_master *master = fsi_get_master(fsi);
@@ -989,11 +988,11 @@ static int fsi_hw_startup(struct fsi_priv *fsi,
 	}
 
 	/* irq clear */
-	fsi_irq_disable(fsi, is_play);
+	fsi_irq_disable(fsi, io);
 	fsi_irq_clear_status(fsi);
 
 	/* fifo init */
-	fsi_fifo_init(fsi, is_play, dev);
+	fsi_fifo_init(fsi, io, dev);
 
 	return 0;
 }
@@ -1009,9 +1008,8 @@ 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);
+	return fsi_hw_startup(fsi, fsi_stream_get(fsi, substream), dai->dev);
 }
 
 static void fsi_dai_shutdown(struct snd_pcm_substream *substream,
@@ -1027,20 +1025,19 @@ 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 fsi_stream *io = fsi_stream_get(fsi, fsi_is_play(substream));
-	int is_play = fsi_is_play(substream);
+	struct fsi_stream *io = fsi_stream_get(fsi, substream);
 	int ret = 0;
 
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
-		fsi_stream_init(fsi, is_play, substream);
+		fsi_stream_init(fsi, io, substream);
 		ret = fsi_stream_transfer(io);
 		if (0 == ret)
-			fsi_port_start(fsi, is_play);
+			fsi_port_start(fsi, io);
 		break;
 	case SNDRV_PCM_TRIGGER_STOP:
-		fsi_port_stop(fsi, is_play);
-		fsi_stream_quit(fsi, is_play);
+		fsi_port_stop(fsi, io);
+		fsi_stream_quit(fsi, io);
 		break;
 	}
 
@@ -1206,7 +1203,7 @@ static int fsi_hw_free(struct snd_pcm_substream *substream)
 static snd_pcm_uframes_t fsi_pointer(struct snd_pcm_substream *substream)
 {
 	struct fsi_priv *fsi = fsi_get_priv(substream);
-	struct fsi_stream *io = fsi_stream_get(fsi, fsi_is_play(substream));
+	struct fsi_stream *io = fsi_stream_get(fsi, substream);
 	int samples_pos = io->buff_sample_pos - 1;
 
 	if (samples_pos < 0)
@@ -1433,30 +1430,29 @@ static int fsi_remove(struct platform_device *pdev)
 }
 
 static void __fsi_suspend(struct fsi_priv *fsi,
-			  int is_play,
+			  struct fsi_stream *io,
 			  struct device *dev)
 {
-	if (!fsi_stream_is_working(fsi, is_play))
+	if (!fsi_stream_is_working(fsi, io))
 		return;
 
-	fsi_port_stop(fsi, is_play);
+	fsi_port_stop(fsi, io);
 	fsi_hw_shutdown(fsi, dev);
 }
 
 static void __fsi_resume(struct fsi_priv *fsi,
-			 int is_play,
+			 struct fsi_stream *io,
 			 struct device *dev)
 {
-	if (!fsi_stream_is_working(fsi, is_play))
+	if (!fsi_stream_is_working(fsi, io))
 		return;
 
-	fsi_hw_startup(fsi, is_play, dev);
+	fsi_hw_startup(fsi, io, dev);
 
 	if (fsi_is_clk_master(fsi) && fsi->rate)
 		fsi_set_master_clk(dev, fsi, fsi->rate, 1);
 
-	fsi_port_start(fsi, is_play);
-
+	fsi_port_start(fsi, io);
 }
 
 static int fsi_suspend(struct device *dev)
@@ -1465,11 +1461,11 @@ static int fsi_suspend(struct device *dev)
 	struct fsi_priv *fsia = &master->fsia;
 	struct fsi_priv *fsib = &master->fsib;
 
-	__fsi_suspend(fsia, 1, dev);
-	__fsi_suspend(fsia, 0, dev);
+	__fsi_suspend(fsia, &fsia->playback, dev);
+	__fsi_suspend(fsia, &fsia->capture, dev);
 
-	__fsi_suspend(fsib, 1, dev);
-	__fsi_suspend(fsib, 0, dev);
+	__fsi_suspend(fsib, &fsib->playback, dev);
+	__fsi_suspend(fsib, &fsib->capture, dev);
 
 	return 0;
 }
@@ -1480,11 +1476,11 @@ static int fsi_resume(struct device *dev)
 	struct fsi_priv *fsia = &master->fsia;
 	struct fsi_priv *fsib = &master->fsib;
 
-	__fsi_resume(fsia, 1, dev);
-	__fsi_resume(fsia, 0, dev);
+	__fsi_resume(fsia, &fsia->playback, dev);
+	__fsi_resume(fsia, &fsia->capture, dev);
 
-	__fsi_resume(fsib, 1, dev);
-	__fsi_resume(fsib, 0, dev);
+	__fsi_resume(fsib, &fsib->playback, dev);
+	__fsi_resume(fsib, &fsib->capture, dev);
 
 	return 0;
 }
-- 
1.7.5.4

[PATCH 14/19] ASoC: fsi: add .start_stop handler to fsi_stream_handler

[Kuninori Morimoto]

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

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index 7dec144..8d05e59 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -203,6 +203,8 @@ struct fsi_stream_handler {
 	int (*probe)(struct fsi_priv *fsi, struct fsi_stream *io);
 	int (*transfer)(struct fsi_priv *fsi, struct fsi_stream *io);
 	int (*remove)(struct fsi_priv *fsi, struct fsi_stream *io);
+	void (*start_stop)(struct fsi_priv *fsi, struct fsi_stream *io,
+			   int enable);
 };
 #define fsi_stream_handler_call(io, func, args...)	\
 	(!(io) ? -ENODEV :				\
@@ -513,6 +515,12 @@ static int fsi_stream_transfer(struct fsi_stream *io)
 	return fsi_stream_handler_call(io, transfer, fsi, io);
 }
 
+#define fsi_stream_start(fsi, io)\
+	fsi_stream_handler_call(io, start_stop, fsi, io, 1)
+
+#define fsi_stream_stop(fsi, io)\
+	fsi_stream_handler_call(io, start_stop, fsi, io, 0)
+
 static int fsi_stream_probe(struct fsi_priv *fsi)
 {
 	struct fsi_stream *io;
@@ -691,24 +699,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)
-#define fsi_port_stop(f, i)	__fsi_port_clk_ctrl(f, i, 0)
-static void __fsi_port_clk_ctrl(struct fsi_priv *fsi, struct fsi_stream *io,
-				int enable)
-{
-	struct fsi_master *master = fsi_get_master(fsi);
-	u32 clk  = fsi_is_port_a(fsi) ? CRA  : CRB;
-
-	if (enable)
-		fsi_irq_enable(fsi, io);
-	else
-		fsi_irq_disable(fsi, io);
-
-	if (fsi_is_clk_master(fsi))
-		fsi_master_mask_set(master, CLK_RST, clk, (enable) ? clk : 0);
-}
-
-
 /*
  *		pio data transfer handler
  */
@@ -845,12 +835,29 @@ static int fsi_pio_push(struct fsi_priv *fsi, struct fsi_stream *io)
 				  samples);
 }
 
+static void fsi_pio_start_stop(struct fsi_priv *fsi, struct fsi_stream *io,
+			       int enable)
+{
+	struct fsi_master *master = fsi_get_master(fsi);
+	u32 clk  = fsi_is_port_a(fsi) ? CRA  : CRB;
+
+	if (enable)
+		fsi_irq_enable(fsi, io);
+	else
+		fsi_irq_disable(fsi, io);
+
+	if (fsi_is_clk_master(fsi))
+		fsi_master_mask_set(master, CLK_RST, clk, (enable) ? clk : 0);
+}
+
 static struct fsi_stream_handler fsi_pio_push_handler = {
 	.transfer	= fsi_pio_push,
+	.start_stop	= fsi_pio_start_stop,
 };
 
 static struct fsi_stream_handler fsi_pio_pop_handler = {
 	.transfer	= fsi_pio_pop,
+	.start_stop	= fsi_pio_start_stop,
 };
 
 static irqreturn_t fsi_interrupt(int irq, void *data)
@@ -1033,10 +1040,10 @@ static int fsi_dai_trigger(struct snd_pcm_substream *substream, int cmd,
 		fsi_stream_init(fsi, io, substream);
 		ret = fsi_stream_transfer(io);
 		if (0 == ret)
-			fsi_port_start(fsi, io);
+			fsi_stream_start(fsi, io);
 		break;
 	case SNDRV_PCM_TRIGGER_STOP:
-		fsi_port_stop(fsi, io);
+		fsi_stream_stop(fsi, io);
 		fsi_stream_quit(fsi, io);
 		break;
 	}
@@ -1436,7 +1443,7 @@ static void __fsi_suspend(struct fsi_priv *fsi,
 	if (!fsi_stream_is_working(fsi, io))
 		return;
 
-	fsi_port_stop(fsi, io);
+	fsi_stream_stop(fsi, io);
 	fsi_hw_shutdown(fsi, dev);
 }
 
@@ -1452,7 +1459,7 @@ static void __fsi_resume(struct fsi_priv *fsi,
 	if (fsi_is_clk_master(fsi) && fsi->rate)
 		fsi_set_master_clk(dev, fsi, fsi->rate, 1);
 
-	fsi_port_start(fsi, io);
+	fsi_stream_start(fsi, io);
 }
 
 static int fsi_suspend(struct device *dev)
-- 
1.7.5.4

[PATCH 15/19] ASoC: fsi: fsi_stream_is_working() care substream->runtime

[Kuninori Morimoto]

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

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index 8d05e59..1e10184 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -450,7 +450,7 @@ static int fsi_stream_is_working(struct fsi_priv *fsi,
 	int ret;
 
 	spin_lock_irqsave(&master->lock, flags);
-	ret = !!io->substream;
+	ret = !!(io->substream && io->substream->runtime);
 	spin_unlock_irqrestore(&master->lock, flags);
 
 	return ret;
@@ -756,9 +756,7 @@ static int fsi_pio_transfer(struct fsi_priv *fsi, struct fsi_stream *io,
 	u8 *buf;
 	int over_period;
 
-	if (!fsi			||
-	    !io->substream		||
-	    !io->substream->runtime)
+	if (!fsi_stream_is_working(fsi, io))
 		return -EINVAL;
 
 	over_period	= 0;
-- 
1.7.5.4

[PATCH 16/19] ASoC: fsi: PortA/B information was controlled by sh_fsi_port_info

[Kuninori Morimoto]

Current FSI got each PortA/B parameter by porta_flags/portb_flags from platform.
And .set_rate function was shared for PortA/B.
This structure was not readable and not flexible.
This patch adds sh_fsi_port_info, and its own settings was added on each platform.
it is preparation for DMAEngine support

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
>> Paul

Could you please check this patch ?

 arch/arm/mach-shmobile/board-ap4evb.c   |   30 ++++++++++------------------
 arch/arm/mach-shmobile/board-mackerel.c |   18 +++++++---------
 arch/sh/boards/mach-ecovec24/setup.c    |    4 ++-
 arch/sh/boards/mach-se/7724/setup.c     |    4 ++-
 include/sound/sh_fsi.h                  |   10 ++++++--
 sound/soc/sh/fsi.c                      |   32 +++++++++++++++---------------
 6 files changed, 48 insertions(+), 50 deletions(-)

diff --git a/arch/arm/mach-shmobile/board-ap4evb.c b/arch/arm/mach-shmobile/board-ap4evb.c
index aab0a34..2ac08b2 100644
--- a/arch/arm/mach-shmobile/board-ap4evb.c
+++ b/arch/arm/mach-shmobile/board-ap4evb.c
@@ -745,26 +745,18 @@ fsi_set_rate_end:
 	return ret;
 }
 
-static int fsi_set_rate(struct device *dev, int is_porta, int rate, int enable)
-{
-	int ret;
-
-	if (is_porta)
-		ret = fsi_ak4642_set_rate(dev, rate, enable);
-	else
-		ret = fsi_hdmi_set_rate(dev, rate, enable);
-
-	return ret;
-}
-
 static struct sh_fsi_platform_info fsi_info = {
-	.porta_flags = SH_FSI_BRS_INV,
-
-	.portb_flags = SH_FSI_BRS_INV |
-		       SH_FSI_BRM_INV |
-		       SH_FSI_LRS_INV |
-		       SH_FSI_FMT_SPDIF,
-	.set_rate = fsi_set_rate,
+	.port_a = {
+		.flags		= SH_FSI_BRS_INV,
+		.set_rate	= fsi_ak4642_set_rate,
+	},
+	.port_b = {
+		.flags		= SH_FSI_BRS_INV |
+				  SH_FSI_BRM_INV |
+				  SH_FSI_LRS_INV |
+				  SH_FSI_FMT_SPDIF,
+		.set_rate	= fsi_hdmi_set_rate,
+	},
 };
 
 static struct resource fsi_resources[] = {
diff --git a/arch/arm/mach-shmobile/board-mackerel.c b/arch/arm/mach-shmobile/board-mackerel.c
index 9b42fbd..28d6d1f 100644
--- a/arch/arm/mach-shmobile/board-mackerel.c
+++ b/arch/arm/mach-shmobile/board-mackerel.c
@@ -901,7 +901,7 @@ static int __fsi_set_round_rate(struct clk *clk, long rate, int enable)
 	return clk_enable(clk);
 }
 
-static int fsi_set_rate(struct device *dev, int is_porta, int rate, int enable)
+static int fsi_b_set_rate(struct device *dev, int rate, int enable)
 {
 	struct clk *fsib_clk;
 	struct clk *fdiv_clk = &sh7372_fsidivb_clk;
@@ -910,10 +910,6 @@ static int fsi_set_rate(struct device *dev, int is_porta, int rate, int enable)
 	int ackmd_bpfmd;
 	int ret;
 
-	/* FSIA is slave mode. nothing to do here */
-	if (is_porta)
-		return 0;
-
 	/* clock start */
 	switch (rate) {
 	case 44100:
@@ -957,14 +953,16 @@ fsi_set_rate_end:
 }
 
 static struct sh_fsi_platform_info fsi_info = {
-	.porta_flags =	SH_FSI_BRS_INV,
-
-	.portb_flags =	SH_FSI_BRS_INV	|
+	.port_a = {
+		.flags = SH_FSI_BRS_INV,
+	},
+	.port_b = {
+		.flags = SH_FSI_BRS_INV	|
 			SH_FSI_BRM_INV	|
 			SH_FSI_LRS_INV	|
 			SH_FSI_FMT_SPDIF,
-
-	.set_rate = fsi_set_rate,
+		.set_rate = fsi_b_set_rate,
+	}
 };
 
 static struct resource fsi_resources[] = {
diff --git a/arch/sh/boards/mach-ecovec24/setup.c b/arch/sh/boards/mach-ecovec24/setup.c
index 033ef2b..1e146fc 100644
--- a/arch/sh/boards/mach-ecovec24/setup.c
+++ b/arch/sh/boards/mach-ecovec24/setup.c
@@ -767,7 +767,9 @@ static struct platform_device camera_devices[] = {
 
 /* FSI */
 static struct sh_fsi_platform_info fsi_info = {
-	.portb_flags = SH_FSI_BRS_INV,
+	.port_b = {
+		.flags = SH_FSI_BRS_INV,
+	},
 };
 
 static struct resource fsi_resources[] = {
diff --git a/arch/sh/boards/mach-se/7724/setup.c b/arch/sh/boards/mach-se/7724/setup.c
index 036fe1a..1fc5d47 100644
--- a/arch/sh/boards/mach-se/7724/setup.c
+++ b/arch/sh/boards/mach-se/7724/setup.c
@@ -277,7 +277,9 @@ static struct platform_device ceu1_device = {
 /* FSI */
 /* change J20, J21, J22 pin to 1-2 connection to use slave mode */
 static struct sh_fsi_platform_info fsi_info = {
-	.porta_flags = SH_FSI_BRS_INV,
+	.port_a = {
+		.flags = SH_FSI_BRS_INV,
+	},
 };
 
 static struct resource fsi_resources[] = {
diff --git a/include/sound/sh_fsi.h b/include/sound/sh_fsi.h
index 9b1aaca..78cd77a 100644
--- a/include/sound/sh_fsi.h
+++ b/include/sound/sh_fsi.h
@@ -72,10 +72,14 @@
 #define SH_FSI_BPFMD_32		(5 << 4)
 #define SH_FSI_BPFMD_16		(6 << 4)
 
+struct sh_fsi_port_info {
+	unsigned long flags;
+	int (*set_rate)(struct device *dev, int rate, int enable);
+};
+
 struct sh_fsi_platform_info {
-	unsigned long porta_flags;
-	unsigned long portb_flags;
-	int (*set_rate)(struct device *dev, int is_porta, int rate, int enable);
+	struct sh_fsi_port_info port_a;
+	struct sh_fsi_port_info port_b;
 };
 
 /*
diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index 1e10184..75d0cda 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -116,7 +116,7 @@
 
 #define FSI_FMTS (SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S16_LE)
 
-typedef int (*set_rate_func)(struct device *dev, int is_porta, int rate, int enable);
+typedef int (*set_rate_func)(struct device *dev, int rate, int enable);
 
 /*
  * FSI driver use below type name for variable
@@ -185,6 +185,7 @@ struct fsi_stream {
 struct fsi_priv {
 	void __iomem *base;
 	struct fsi_master *master;
+	struct sh_fsi_port_info *info;
 
 	struct fsi_stream playback;
 	struct fsi_stream capture;
@@ -227,7 +228,6 @@ struct fsi_master {
 	struct fsi_priv fsia;
 	struct fsi_priv fsib;
 	struct fsi_core *core;
-	struct sh_fsi_platform_info *info;
 	spinlock_t lock;
 };
 
@@ -346,24 +346,20 @@ static struct fsi_priv *fsi_get_priv(struct snd_pcm_substream *substream)
 	return fsi_get_priv_frm_dai(fsi_get_dai(substream));
 }
 
-static set_rate_func fsi_get_info_set_rate(struct fsi_master *master)
+static set_rate_func fsi_get_info_set_rate(struct fsi_priv *fsi)
 {
-	if (!master->info)
+	if (!fsi->info)
 		return NULL;
 
-	return master->info->set_rate;
+	return fsi->info->set_rate;
 }
 
 static u32 fsi_get_info_flags(struct fsi_priv *fsi)
 {
-	int is_porta = fsi_is_port_a(fsi);
-	struct fsi_master *master = fsi_get_master(fsi);
-
-	if (!master->info)
+	if (!fsi->info)
 		return 0;
 
-	return is_porta ? master->info->porta_flags :
-		master->info->portb_flags;
+	return fsi->info->flags;
 }
 
 static u32 fsi_get_port_shift(struct fsi_priv *fsi, struct fsi_stream *io)
@@ -628,11 +624,14 @@ 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);
+	set_rate_func set_rate = fsi_get_info_set_rate(fsi);
 	int fsi_ver = master->core->ver;
 	int ret;
 
-	ret = set_rate(dev, fsi_is_port_a(fsi), rate, enable);
+	if (!set_rate)
+		return 0;
+
+	ret = set_rate(dev, rate, enable);
 	if (ret < 0) /* error */
 		return ret;
 
@@ -1093,8 +1092,7 @@ static int fsi_set_fmt_spdif(struct fsi_priv *fsi)
 static int fsi_dai_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
 {
 	struct fsi_priv *fsi = fsi_get_priv_frm_dai(dai);
-	struct fsi_master *master = fsi_get_master(fsi);
-	set_rate_func set_rate = fsi_get_info_set_rate(master);
+	set_rate_func set_rate = fsi_get_info_set_rate(fsi);
 	u32 flags = fsi_get_info_flags(fsi);
 	int ret;
 
@@ -1312,6 +1310,7 @@ static int fsi_probe(struct platform_device *pdev)
 {
 	struct fsi_master *master;
 	const struct platform_device_id	*id_entry;
+	struct sh_fsi_platform_info *info = pdev->dev.platform_data;
 	struct resource *res;
 	unsigned int irq;
 	int ret;
@@ -1346,13 +1345,13 @@ static int fsi_probe(struct platform_device *pdev)
 
 	/* master setting */
 	master->irq		= irq;
-	master->info		= pdev->dev.platform_data;
 	master->core		= (struct fsi_core *)id_entry->driver_data;
 	spin_lock_init(&master->lock);
 
 	/* FSI A setting */
 	master->fsia.base	= master->base;
 	master->fsia.master	= master;
+	master->fsia.info	= &info->port_a;
 	fsi_handler_init(&master->fsia);
 	ret = fsi_stream_probe(&master->fsia);
 	if (ret < 0) {
@@ -1363,6 +1362,7 @@ static int fsi_probe(struct platform_device *pdev)
 	/* FSI B setting */
 	master->fsib.base	= master->base + 0x40;
 	master->fsib.master	= master;
+	master->fsib.info	= &info->port_b;
 	fsi_handler_init(&master->fsib);
 	ret = fsi_stream_probe(&master->fsib);
 	if (ret < 0) {
-- 
1.7.5.4

[PATCH 17/19] ASoC: fsi: add .init/.quit handler support

[Kuninori Morimoto]

This is preparation for DMAEngine support

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

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index 75d0cda..79a0afb 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -201,6 +201,8 @@ struct fsi_priv {
 };
 
 struct fsi_stream_handler {
+	int (*init)(struct fsi_priv *fsi, struct fsi_stream *io);
+	int (*quit)(struct fsi_priv *fsi, struct fsi_stream *io);
 	int (*probe)(struct fsi_priv *fsi, struct fsi_stream *io);
 	int (*transfer)(struct fsi_priv *fsi, struct fsi_stream *io);
 	int (*remove)(struct fsi_priv *fsi, struct fsi_stream *io);
@@ -474,6 +476,7 @@ static void fsi_stream_init(struct fsi_priv *fsi,
 	io->sample_width	= samples_to_bytes(runtime, 1);
 	io->oerr_num	= -1; /* ignore 1st err */
 	io->uerr_num	= -1; /* ignore 1st err */
+	fsi_stream_handler_call(io, init, fsi, io);
 	spin_unlock_irqrestore(&master->lock, flags);
 }
 
@@ -491,6 +494,7 @@ static void fsi_stream_quit(struct fsi_priv *fsi, struct fsi_stream *io)
 	if (io->uerr_num > 0)
 		dev_err(dai->dev, "under_run = %d\n", io->uerr_num);
 
+	fsi_stream_handler_call(io, quit, fsi, io);
 	io->substream	= NULL;
 	io->buff_sample_capa	= 0;
 	io->buff_sample_pos	= 0;
-- 
1.7.5.4

[PATCH 18/19] ASoC: fsi: fixup fsi_pointer() calculation method

[Kuninori Morimoto]

current fsi_pointer() calculation was not correct for FSI driver.
This patch fix it up.

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

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index 79a0afb..1374680 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -1211,12 +1211,8 @@ static snd_pcm_uframes_t fsi_pointer(struct snd_pcm_substream *substream)
 {
 	struct fsi_priv *fsi = fsi_get_priv(substream);
 	struct fsi_stream *io = fsi_stream_get(fsi, substream);
-	int samples_pos = io->buff_sample_pos - 1;
 
-	if (samples_pos < 0)
-		samples_pos = 0;
-
-	return fsi_sample2frame(fsi, samples_pos);
+	return fsi_sample2frame(fsi, io->buff_sample_pos);
 }
 
 static struct snd_pcm_ops fsi_pcm_ops = {
-- 
1.7.5.4

Re: [PATCH 18/19] ASoC: fsi: fixup fsi_pointer() calculation method

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 413 bytes --]

On Fri, Feb 03, 2012 at 12:59:15AM -0800, Kuninori Morimoto wrote:
> current fsi_pointer() calculation was not correct for FSI driver.
> This patch fix it up.

This looks like a bug fix which we should send for version 3.3 but it's
patch 18/19.  Is there any dependency?  If there is a dependency can you
provide a version that applies against version 3.3 so that we can get
the fix into the next release please.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 18/19] ASoC: fsi: fixup fsi_pointer() calculation method

[Kuninori Morimoto]

Hi Mark

Thank you for checking patch

> On Fri, Feb 03, 2012 at 12:59:15AM -0800, Kuninori Morimoto wrote:
> > current fsi_pointer() calculation was not correct for FSI driver.
> > This patch fix it up.
> 
> This looks like a bug fix which we should send for version 3.3 but it's
> patch 18/19.  Is there any dependency?  If there is a dependency can you
> provide a version that applies against version 3.3 so that we can get
> the fix into the next release please.

It depend on DMAEngine series.
This is bugfix patch for PIO/DMA, but PIO version FSI driver never
got bug since it was sleeping bug.

But this comment seems not understandable.
maybe it should be

"current fsi_pointer() calculation was not good much for fsi_stream_handler.
This patch fix it up."

I can send v2 patch if you want it.
Thank you.

Best regards
---
Kuninori Morimoto

Re: [PATCH 18/19] ASoC: fsi: fixup fsi_pointer() calculation method

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 332 bytes --]

On Sun, Feb 05, 2012 at 04:36:28PM -0800, Kuninori Morimoto wrote:

> It depend on DMAEngine series.
> This is bugfix patch for PIO/DMA, but PIO version FSI driver never
> got bug since it was sleeping bug.

I'm sorry but I'm having a hard time understanding what this means in
the context of your patch.  What is a "sleeping bug"?

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 18/19] ASoC: fsi: fixup fsi_pointer() calculation method

[Kuninori Morimoto]

Hi Mark

> > It depend on DMAEngine series.
> > This is bugfix patch for PIO/DMA, but PIO version FSI driver never
> > got bug since it was sleeping bug.
> 
> I'm sorry but I'm having a hard time understanding what this means in
> the context of your patch.  What is a "sleeping bug"?

Sorry for my Engilish.
It mean "(Maybe) old fsi_pointer() has bug, but this bug doesn't run a mischief"

Best regards
---
Kuninori Morimoto

Re: [PATCH 18/19] ASoC: fsi: fixup fsi_pointer() calculation method

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 703 bytes --]

On Mon, Feb 06, 2012 at 04:13:20PM -0800, Kuninori Morimoto wrote:

> > I'm sorry but I'm having a hard time understanding what this means in
> > the context of your patch.  What is a "sleeping bug"?

> Sorry for my Engilish.
> It mean "(Maybe) old fsi_pointer() has bug, but this bug doesn't run a mischief"

Ah, I see - so it probably was an existing bug but nobody reported it?
I suspect it's worth fixing in 3.3 then, there's often issues where
userspace applications don't work that don't get reported well or at
all, often these are due to issues in the pointer implementations.
People end up coming up with configurations that work around them and
never think that there might be a driver issue.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 18/19] ASoC: fsi: fixup fsi_pointer() calculation method

[Kuninori Morimoto]

Hi Mark

Thank you for your comment

> > > I'm sorry but I'm having a hard time understanding what this means in
> > > the context of your patch.  What is a "sleeping bug"?
> 
> > Sorry for my Engilish.
> > It mean "(Maybe) old fsi_pointer() has bug, but this bug doesn't run a mischief"
> 
> Ah, I see - so it probably was an existing bug but nobody reported it?

Yes

> I suspect it's worth fixing in 3.3 then, there's often issues where
> userspace applications don't work that don't get reported well or at
> all, often these are due to issues in the pointer implementations.
> People end up coming up with configurations that work around them and
> never think that there might be a driver issue.

I'm not good at detail of user side applications,
but aplay was working correctly on old/PIO FSI.
So, I didn't notice it.

Best regards
---
Kuninori Morimoto

Re: [PATCH 18/19] ASoC: fsi: fixup fsi_pointer() calculation method

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 905 bytes --]

On Tue, Feb 07, 2012 at 04:08:52PM -0800, Kuninori Morimoto wrote:

> > I suspect it's worth fixing in 3.3 then, there's often issues where
> > userspace applications don't work that don't get reported well or at
> > all, often these are due to issues in the pointer implementations.
> > People end up coming up with configurations that work around them and
> > never think that there might be a driver issue.

> I'm not good at detail of user side applications,
> but aplay was working correctly on old/PIO FSI.
> So, I didn't notice it.

aplay is (un)fortunately pretty forgiving here - PulseAudio is the most
difficult test, though some other applications do run into issues too.
Anyway, can you please generate a version of this for 3.3 (or confirm if
the current patch is OK for there) so we can integrate the change there?

The change itself seems good, it's just that it'd be better done as a
fix.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH 18/19 v2] ASoC: fsi: fixup fsi_pointer() calculation method

[Kuninori Morimoto]

current fsi_pointer() calculation was not correct for FSI driver.
This patch fix it up.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
v1 -> v2

- this patch is based on latest linus/master (=v3.3.0-rc2)

>> Mark

I guess you will got some conflict around this fsi_pointer() when you merge.

 sound/soc/sh/fsi.c |    6 +-----
 1 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index db6c89a..ea4a82d0 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -1152,12 +1152,8 @@ static snd_pcm_uframes_t fsi_pointer(struct snd_pcm_substream *substream)
 {
 	struct fsi_priv *fsi = fsi_get_priv(substream);
 	struct fsi_stream *io = fsi_get_stream(fsi, fsi_is_play(substream));
-	int samples_pos = io->buff_sample_pos - 1;
 
-	if (samples_pos < 0)
-		samples_pos = 0;
-
-	return fsi_sample2frame(fsi, samples_pos);
+	return fsi_sample2frame(fsi, io->buff_sample_pos);
 }
 
 static struct snd_pcm_ops fsi_pcm_ops = {
-- 
1.7.5.4

Re: [PATCH 18/19 v2] ASoC: fsi: fixup fsi_pointer() calculation method

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 328 bytes --]

On Wed, Feb 08, 2012 at 04:57:29PM -0800, Kuninori Morimoto wrote:

> I guess you will got some conflict around this fsi_pointer() when you merge.

Applied, thanks.  I'll try to make sure that the conflict resolution
looks like your original patch whenever I merge up from Linus' tree so
hopefully shouldn't be too much hassle.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH 19/19] ASoC: fsi: Add DMAEngine support

[Kuninori Morimoto]

This patch supports DMAEngine to FSI driver.
It supports only Tx case at this point.
If platform/cpu doesn't support DMAEngine, FSI driver will
use PIO transfer.

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

diff --git a/include/sound/sh_fsi.h b/include/sound/sh_fsi.h
index 78cd77a..b457e87 100644
--- a/include/sound/sh_fsi.h
+++ b/include/sound/sh_fsi.h
@@ -74,6 +74,8 @@
 
 struct sh_fsi_port_info {
 	unsigned long flags;
+	int tx_id;
+	int rx_id;
 	int (*set_rate)(struct device *dev, int rate, int enable);
 };
 
diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index 1374680..378cc5b 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -13,8 +13,11 @@
  */
 
 #include <linux/delay.h>
+#include <linux/dma-mapping.h>
 #include <linux/pm_runtime.h>
 #include <linux/io.h>
+#include <linux/scatterlist.h>
+#include <linux/sh_dma.h>
 #include <linux/slab.h>
 #include <linux/module.h>
 #include <sound/soc.h>
@@ -53,6 +56,7 @@
 
 /* DO_FMT */
 /* DI_FMT */
+#define CR_BWS_MASK	(0x3 << 20) /* FSI2 */
 #define CR_BWS_24	(0x0 << 20) /* FSI2 */
 #define CR_BWS_16	(0x1 << 20) /* FSI2 */
 #define CR_BWS_20	(0x2 << 20) /* FSI2 */
@@ -68,6 +72,15 @@
 #define CR_TDM		(0x4 << 4)
 #define CR_TDM_D	(0x5 << 4)
 
+/* OUT_DMAC */
+/* IN_DMAC */
+#define VDMD_MASK	(0x3 << 4)
+#define VDMD_FRONT	(0x0 << 4) /* Package in front */
+#define VDMD_BACK	(0x1 << 4) /* Package in back */
+#define VDMD_STREAM	(0x2 << 4) /* Stream mode(16bit * 2) */
+
+#define DMA_ON		(0x1 << 0)
+
 /* DOFF_CTL */
 /* DIFF_CTL */
 #define IRQ_HALF	0x00100000
@@ -180,6 +193,14 @@ struct fsi_stream {
 	 */
 	struct fsi_stream_handler *handler;
 	struct fsi_priv		*priv;
+
+	/*
+	 * these are for DMAEngine
+	 */
+	struct dma_chan		*chan;
+	struct sh_dmae_slave	slave; /* see fsi_handler_init() */
+	struct tasklet_struct	tasklet;
+	dma_addr_t		dma;
 };
 
 struct fsi_priv {
@@ -889,6 +910,212 @@ static irqreturn_t fsi_interrupt(int irq, void *data)
 }
 
 /*
+ *		dma data transfer handler
+ */
+static int fsi_dma_init(struct fsi_priv *fsi, struct fsi_stream *io)
+{
+	struct snd_pcm_runtime *runtime = io->substream->runtime;
+	struct snd_soc_dai *dai = fsi_get_dai(io->substream);
+	enum dma_data_direction dir = fsi_stream_is_play(fsi, io) ?
+				DMA_TO_DEVICE : DMA_FROM_DEVICE;
+
+	io->dma = dma_map_single(dai->dev, runtime->dma_area,
+				 snd_pcm_lib_buffer_bytes(io->substream), dir);
+	return 0;
+}
+
+static int fsi_dma_quit(struct fsi_priv *fsi, struct fsi_stream *io)
+{
+	struct snd_soc_dai *dai = fsi_get_dai(io->substream);
+	enum dma_data_direction dir = fsi_stream_is_play(fsi, io) ?
+		DMA_TO_DEVICE : DMA_FROM_DEVICE;
+
+	dma_unmap_single(dai->dev, io->dma,
+			 snd_pcm_lib_buffer_bytes(io->substream), dir);
+	return 0;
+}
+
+static void fsi_dma_complete(void *data)
+{
+	struct fsi_stream *io = (struct fsi_stream *)data;
+	struct fsi_priv *fsi = fsi_stream_to_priv(io);
+	struct snd_pcm_runtime *runtime = io->substream->runtime;
+	struct snd_soc_dai *dai = fsi_get_dai(io->substream);
+	enum dma_data_direction dir = fsi_stream_is_play(fsi, io) ?
+		DMA_TO_DEVICE : DMA_FROM_DEVICE;
+
+	dma_sync_single_for_cpu(dai->dev, io->dma,
+			samples_to_bytes(runtime, io->period_samples), dir);
+
+	io->buff_sample_pos += io->period_samples;
+	io->period_pos++;
+
+	if (io->period_pos >= runtime->periods) {
+		io->period_pos = 0;
+		io->buff_sample_pos = 0;
+	}
+
+	fsi_count_fifo_err(fsi);
+	fsi_stream_transfer(io);
+
+	snd_pcm_period_elapsed(io->substream);
+}
+
+static dma_addr_t fsi_dma_get_area(struct fsi_stream *io)
+{
+	struct snd_pcm_runtime *runtime = io->substream->runtime;
+
+	return io->dma + samples_to_bytes(runtime, io->buff_sample_pos);
+}
+
+static void fsi_dma_do_tasklet(unsigned long data)
+{
+	struct fsi_stream *io = (struct fsi_stream *)data;
+	struct fsi_priv *fsi = fsi_stream_to_priv(io);
+	struct dma_chan *chan;
+	struct snd_soc_dai *dai;
+	struct dma_async_tx_descriptor *desc;
+	struct scatterlist sg;
+	struct snd_pcm_runtime *runtime;
+	enum dma_data_direction dir;
+	dma_cookie_t cookie;
+	int is_play = fsi_stream_is_play(fsi, io);
+	int len;
+	dma_addr_t buf;
+
+	if (!fsi_stream_is_working(fsi, io))
+		return;
+
+	dai	= fsi_get_dai(io->substream);
+	chan	= io->chan;
+	runtime	= io->substream->runtime;
+	dir	= is_play ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
+	len	= samples_to_bytes(runtime, io->period_samples);
+	buf	= fsi_dma_get_area(io);
+
+	dma_sync_single_for_device(dai->dev, io->dma, len, dir);
+
+	sg_init_table(&sg, 1);
+	sg_set_page(&sg, pfn_to_page(PFN_DOWN(buf)),
+		    len , offset_in_page(buf));
+	sg_dma_address(&sg) = buf;
+	sg_dma_len(&sg) = len;
+
+	desc = chan->device->device_prep_slave_sg(chan, &sg, 1, dir,
+						  DMA_PREP_INTERRUPT |
+						  DMA_CTRL_ACK);
+	if (!desc) {
+		dev_err(dai->dev, "device_prep_slave_sg() fail\n");
+		return;
+	}
+
+	desc->callback		= fsi_dma_complete;
+	desc->callback_param	= io;
+
+	cookie = desc->tx_submit(desc);
+	if (cookie < 0) {
+		dev_err(dai->dev, "tx_submit() fail\n");
+		return;
+	}
+
+	dma_async_issue_pending(chan);
+
+	/*
+	 * FIXME
+	 *
+	 * In DMAEngine case, codec and FSI cannot be started simultaneously
+	 * since FSI is using tasklet.
+	 * Therefore, in capture case, probably FSI FIFO will have got
+	 * overflow error in this point.
+	 * in that case, DMA cannot start transfer until error was cleared.
+	 */
+	if (!is_play) {
+		if (ERR_OVER & fsi_reg_read(fsi, DIFF_ST)) {
+			fsi_reg_mask_set(fsi, DIFF_CTL, FIFO_CLR, FIFO_CLR);
+			fsi_reg_write(fsi, DIFF_ST, 0);
+		}
+	}
+}
+
+static bool fsi_dma_filter(struct dma_chan *chan, void *param)
+{
+	struct sh_dmae_slave *slave = param;
+
+	chan->private = slave;
+
+	return true;
+}
+
+static int fsi_dma_transfer(struct fsi_priv *fsi, struct fsi_stream *io)
+{
+	tasklet_schedule(&io->tasklet);
+
+	return 0;
+}
+
+static void fsi_dma_push_start_stop(struct fsi_priv *fsi, struct fsi_stream *io,
+				 int start)
+{
+	u32 bws;
+	u32 dma;
+
+	switch (io->sample_width * start) {
+	case 2:
+		bws = CR_BWS_16;
+		dma = VDMD_STREAM | DMA_ON;
+		break;
+	case 4:
+		bws = CR_BWS_24;
+		dma = VDMD_BACK | DMA_ON;
+		break;
+	default:
+		bws = 0;
+		dma = 0;
+	}
+
+	fsi_reg_mask_set(fsi, DO_FMT, CR_BWS_MASK, bws);
+	fsi_reg_write(fsi, OUT_DMAC, dma);
+}
+
+static int fsi_dma_probe(struct fsi_priv *fsi, struct fsi_stream *io)
+{
+	dma_cap_mask_t mask;
+
+	dma_cap_zero(mask);
+	dma_cap_set(DMA_SLAVE, mask);
+
+	io->chan = dma_request_channel(mask, fsi_dma_filter, &io->slave);
+	if (!io->chan)
+		return -EIO;
+
+	tasklet_init(&io->tasklet, fsi_dma_do_tasklet, (unsigned long)io);
+
+	return 0;
+}
+
+static int fsi_dma_remove(struct fsi_priv *fsi, struct fsi_stream *io)
+{
+	tasklet_kill(&io->tasklet);
+
+	fsi_stream_stop(fsi, io);
+
+	if (io->chan)
+		dma_release_channel(io->chan);
+
+	io->chan = NULL;
+	return 0;
+}
+
+static struct fsi_stream_handler fsi_dma_push_handler = {
+	.init		= fsi_dma_init,
+	.quit		= fsi_dma_quit,
+	.probe		= fsi_dma_probe,
+	.transfer	= fsi_dma_transfer,
+	.remove		= fsi_dma_remove,
+	.start_stop	= fsi_dma_push_start_stop,
+};
+
+/*
  *		dai ops
  */
 static void fsi_fifo_init(struct fsi_priv *fsi,
@@ -1304,6 +1531,11 @@ static void fsi_handler_init(struct fsi_priv *fsi)
 	fsi->playback.priv	= fsi;
 	fsi->capture.handler	= &fsi_pio_pop_handler;  /* default PIO */
 	fsi->capture.priv	= fsi;
+
+	if (fsi->info->tx_id) {
+		fsi->playback.slave.slave_id	= fsi->info->tx_id;
+		fsi->playback.handler		= &fsi_dma_push_handler;
+	}
 }
 
 static int fsi_probe(struct platform_device *pdev)
-- 
1.7.5.4

Re: [PATCH 19/19] ASoC: fsi: Add DMAEngine support

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 846 bytes --]

On Fri, Feb 03, 2012 at 12:59:33AM -0800, Kuninori Morimoto wrote:

> +static void fsi_dma_do_tasklet(unsigned long data)
> +{

So, we're getting more and more drivers converted over to dmaengine
(I've CCed a random selection of people working on relevant drivers and
Vinod).  This means we're getting a bunch of drivers that talk to the
ALSA API on the top and the dmaengine API on the bottom and seem to need
to implement very similar code patterns (like this tasklet to push the
DMA through).  This feels like we need some sort of abstraction to
factor out some code here, either library code or perhaps a framework
driver with callbacks that fill in the platform specifics - if the APIs
are providing useful abstractions then presumably a large part of this
is just translating between them.

If that's not doable then what are the problems?

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 19/19] ASoC: fsi: Add DMAEngine support

[Lars-Peter Clausen]

On 02/03/2012 02:48 PM, Mark Brown wrote:
> So, we're getting more and more drivers converted over to dmaengine
> (I've CCed a random selection of people working on relevant drivers and
> Vinod).  This means we're getting a bunch of drivers that talk to the
> ALSA API on the top and the dmaengine API on the bottom and seem to need
> to implement very similar code patterns (like this tasklet to push the
> DMA through).  This feels like we need some sort of abstraction to
> factor out some code here, either library code or perhaps a framework
> driver with callbacks that fill in the platform specifics - if the APIs
> are providing useful abstractions then presumably a large part of this
> is just translating between them.
> 
> If that's not doable then what are the problems?


Yes, I was faced with the task of writing yet another dmaengine based ASoC
PCM driver last week. At first I thought it might be a good idea to come up
with a common ASoC PCM driver which could be used by all platforms using DMA
engine. Unfortunately this turned out to be not feasible due to how
dmaengine works. Each dmaengine driver has it's custom API for configuring
the DMA controller. :/

So what I have come up with so far is a set of helper functions which can be
used to implement a dmaengine based ASoC PCM driver. Right now I'm in the
process of converting imx, mxs and ep93xx to this set of functions.

These platforms have in common that their dmaengine driver implements
device_prep_dma_cyclic function, which prepares a continuous transfer for
cyclic buffer.
The other ASoC platforms using dmaengine for their PCM driver (including the
one in this patch series) don't have this function implemented and so the
PCM drivers manually have to setup the cyclic SG lists. So while we could
add a common set up functions for this to ASoC or ALSA I think the better
place for this is in the dmaengine framework and implement
device_prep_dma_cyclic for those drivers which don't do yet.

I'll try to send patches and some further explanations next week.

- Lars

Re: [PATCH 19/19] ASoC: fsi: Add DMAEngine support

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1217 bytes --]

On Fri, Feb 03, 2012 at 03:05:12PM +0100, Lars-Peter Clausen wrote:

> Yes, I was faced with the task of writing yet another dmaengine based ASoC
> PCM driver last week. At first I thought it might be a good idea to come up
> with a common ASoC PCM driver which could be used by all platforms using DMA
> engine. Unfortunately this turned out to be not feasible due to how
> dmaengine works. Each dmaengine driver has it's custom API for configuring
> the DMA controller. :/

That doesn't sound insurmountible - you need a callback to allow the
individual drivers to set the parameters but hopefully the data flow is
more generic?  There will probably be some other setup type stuff that
needs doing.

> The other ASoC platforms using dmaengine for their PCM driver (including the
> one in this patch series) don't have this function implemented and so the
> PCM drivers manually have to setup the cyclic SG lists. So while we could
> add a common set up functions for this to ASoC or ALSA I think the better
> place for this is in the dmaengine framework and implement
> device_prep_dma_cyclic for those drivers which don't do yet.

That seems like a good plan, that way other users of cyclic DMA will
also benefit.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 19/19] ASoC: fsi: Add DMAEngine support

[Vinod Koul]

On Fri, 2012-02-03 at 14:14 +0000, Mark Brown wrote:
> On Fri, Feb 03, 2012 at 03:05:12PM +0100, Lars-Peter Clausen wrote:
> 
> > Yes, I was faced with the task of writing yet another dmaengine based ASoC
> > PCM driver last week. At first I thought it might be a good idea to come up
> > with a common ASoC PCM driver which could be used by all platforms using DMA
> > engine. Unfortunately this turned out to be not feasible due to how
> > dmaengine works. Each dmaengine driver has it's custom API for configuring
> > the DMA controller. :/
Not really, if the driver is written properly its doable..
> 
> That doesn't sound insurmountible - you need a callback to allow the
> individual drivers to set the parameters but hopefully the data flow is
> more generic?  There will probably be some other setup type stuff that
> needs doing.
I had though about this sometime back, and if we add dmaengine support
into asoc, we wouldn't need to add this kind of support in every driver.

Typically, finding the right channel involves some handshaking between
client and dma driver, so this is something the PCM driver should do as
part of setup.
The dma_chan can be part of the DAI, including the dma_add_t of the
peripheral. Any required dmac parametrs should be set using
dma_slave_config at setup.
Once the stream is opened, asoc core on finding dma_chan in DAI should
call the dmaengine APIs to get cyclic descriptor (in prepare) and then
submit. The trigger START should invoke issue_pending callback of DMA.
Similarly the trigger PAUSE, RESUME maps to dmanegine PAUSE and RESUME
respectively (if implemented by dmac).

This way PCM driver would just need to setup the dma and not worry about
data transfer as long as dma driver is properly done. 
> 
> > The other ASoC platforms using dmaengine for their PCM driver (including the
> > one in this patch series) don't have this function implemented and so the
> > PCM drivers manually have to setup the cyclic SG lists. So while we could
> > add a common set up functions for this to ASoC or ALSA I think the better
> > place for this is in the dmaengine framework and implement
> > device_prep_dma_cyclic for those drivers which don't do yet.
> 
> That seems like a good plan, that way other users of cyclic DMA will
> also benefit.


-- 
~Vinod

Re: [PATCH 19/19] ASoC: fsi: Add DMAEngine support

[Lars-Peter Clausen]

On 02/04/2012 05:31 PM, Vinod Koul wrote:
> On Fri, 2012-02-03 at 14:14 +0000, Mark Brown wrote:
>> On Fri, Feb 03, 2012 at 03:05:12PM +0100, Lars-Peter Clausen wrote:
>>
>>> Yes, I was faced with the task of writing yet another dmaengine based ASoC
>>> PCM driver last week. At first I thought it might be a good idea to come up
>>> with a common ASoC PCM driver which could be used by all platforms using DMA
>>> engine. Unfortunately this turned out to be not feasible due to how
>>> dmaengine works. Each dmaengine driver has it's custom API for configuring
>>> the DMA controller. :/
> Not really, if the driver is written properly its doable..

yes, _if_. Though with the current state of the dmaengine drivers you need at
least custom code for configuring the DMA controller and for matching the channel.

>>
>> That doesn't sound insurmountible - you need a callback to allow the
>> individual drivers to set the parameters but hopefully the data flow is
>> more generic?  There will probably be some other setup type stuff that
>> needs doing.
> I had though about this sometime back, and if we add dmaengine support
> into asoc, we wouldn't need to add this kind of support in every driver.
> 
> Typically, finding the right channel involves some handshaking between
> client and dma driver, so this is something the PCM driver should do as
> part of setup.
> The dma_chan can be part of the DAI, including the dma_add_t of the
> peripheral. Any required dmac parametrs should be set using
> dma_slave_config at setup.
> Once the stream is opened, asoc core on finding dma_chan in DAI should
> call the dmaengine APIs to get cyclic descriptor (in prepare) and then
> submit. The trigger START should invoke issue_pending callback of DMA.
> Similarly the trigger PAUSE, RESUME maps to dmanegine PAUSE and RESUME
> respectively (if implemented by dmac).

This is sort of what I have implemented right now. Although I also prepare and
submit the dma descriptor in trigger START instead of prepare. Mainly beacuse I
mapped trigger STOP to dmaengine_terminate_all. Or is issue_pending supposed to
just restart the transfer if it is circular, even though terminate_all has been
called?

I also have a helper function which maps a hw_params to a dma_slave_config.
Individual drivers still have to implement their own hw_params callback since
not all fields of a dma_slave_config can be deduced from a hw_params (e.g. the
burst size).

- Lars

Re: [PATCH 19/19] ASoC: fsi: Add DMAEngine support

[Vinod Koul]

On Sat, 2012-02-04 at 18:00 +0100, Lars-Peter Clausen wrote:
> yes, _if_. Though with the current state of the dmaengine drivers you
> need at
> least custom code for configuring the DMA controller and for matching
> the channel. 
And that is why the dma_chan allocation should be done in setup code by
PCM driver. Rest all the work in runtime should be moved into soc-pcm.

-- 
~Vinod

Re: [PATCH 19/19] ASoC: fsi: Add DMAEngine support

[Vinod Koul]

On Sat, 2012-02-04 at 18:00 +0100, Lars-Peter Clausen wrote:
> This is sort of what I have implemented right now. Although I also prepare and
> submit the dma descriptor in trigger START instead of prepare. Mainly beacuse I
> mapped trigger STOP to dmaengine_terminate_all. Or is issue_pending supposed to
> just restart the transfer if it is circular, even though terminate_all has been
> called?
trigger START should not be used for preparing descriptors. You want to
start to actually start the descriptor (which is issue_pending and NOT
submit!!). You should do this is prepare callback.
> 
> I also have a helper function which maps a hw_params to a dma_slave_config.
> Individual drivers still have to implement their own hw_params callback since
> not all fields of a dma_slave_config can be deduced from a hw_params (e.g. the
> burst size). 
yes, since dma_slave_config is dependent upon what you want to DMA and
match that with peripheral, it can be done by PCM drivers only. Core
cannot take care of this. That would also help, as channel is set
rightly before core calls dmaengine APIs in prepare :-)

-- 
~Vinod

Re: [PATCH 19/19] ASoC: fsi: Add DMAEngine support

[Lars-Peter Clausen]

On 02/04/2012 07:06 PM, Vinod Koul wrote:
> On Sat, 2012-02-04 at 18:00 +0100, Lars-Peter Clausen wrote:
>> This is sort of what I have implemented right now. Although I also prepare and
>> submit the dma descriptor in trigger START instead of prepare. Mainly beacuse I
>> mapped trigger STOP to dmaengine_terminate_all. Or is issue_pending supposed to
>> just restart the transfer if it is circular, even though terminate_all has been
>> called?
> trigger START should not be used for preparing descriptors. You want to
> start to actually start the descriptor (which is issue_pending and NOT
> submit!!). You should do this is prepare callback.

I'm doing both, prepare+submit and issue_pending, because of what I wrote
above. Or can we assume that each call to the ALSA/ASoC prepare callback will
only be followed by at maximum one call to trigger START?

Re: [PATCH 19/19] ASoC: fsi: Add DMAEngine support

[Vinod Koul]

On Sat, 2012-02-04 at 19:25 +0100, Lars-Peter Clausen wrote:
> On 02/04/2012 07:06 PM, Vinod Koul wrote:
> > On Sat, 2012-02-04 at 18:00 +0100, Lars-Peter Clausen wrote:
> >> This is sort of what I have implemented right now. Although I also prepare and
> >> submit the dma descriptor in trigger START instead of prepare. Mainly beacuse I
> >> mapped trigger STOP to dmaengine_terminate_all. Or is issue_pending supposed to
> >> just restart the transfer if it is circular, even though terminate_all has been
> >> called?
> > trigger START should not be used for preparing descriptors. You want to
> > start to actually start the descriptor (which is issue_pending and NOT
> > submit!!). You should do this is prepare callback.
> 
> I'm doing both, prepare+submit and issue_pending, because of what I wrote
> above. Or can we assume that each call to the ALSA/ASoC prepare callback will
> only be followed by at maximum one call to trigger START?
terminate_all is designed to abort all transfers, so that would mean the
dmac will move the descriptors to free_list. So that way your
implementation is the right one.

And while at it, I believe for this use, terminate_all is too strong an
API to be used, perhaps we should introduce a STOP call, which just
aborts the current descriptor but doesn't free it, or possibly move to
submitted state/queue. And yes terminate_all should used in scenarios
like free callback.
That way it should map properly to ALSA PCM operations.


-- 
~Vinod

Re: [PATCH 19/19] ASoC: fsi: Add DMAEngine support

[Lars-Peter Clausen]

On 02/04/2012 07:40 PM, Vinod Koul wrote:
>[...]
> 
> And while at it, I believe for this use, terminate_all is too strong an
> API to be used, perhaps we should introduce a STOP call, which just
> aborts the current descriptor but doesn't free it, or possibly move to
> submitted state/queue. And yes terminate_all should used in scenarios
> like free callback.
> That way it should map properly to ALSA PCM operations.
> 

Yes, a call that would allow us to reset/restart a transfer would be nice.
While we are at it: Another nice addition, which would be good for sound
drivers is a callback, which reports the transfer's current position in the
datastream. This allows us to implement a pointer callback with sub-period
granularity and also allow us to implement the 'no period wakeup' feature. A
lot of DMA controllers support reporting this, but as far as I can see there
is currently no way to export this information using dmaengine.

- Lars

Re: [PATCH 19/19] ASoC: fsi: Add DMAEngine support

[Vinod Koul]

On Mon, 2012-02-06 at 09:47 +0100, Lars-Peter Clausen wrote:
> On 02/04/2012 07:40 PM, Vinod Koul wrote:
> >[...]
> > 
> > And while at it, I believe for this use, terminate_all is too strong an
> > API to be used, perhaps we should introduce a STOP call, which just
> > aborts the current descriptor but doesn't free it, or possibly move to
> > submitted state/queue. And yes terminate_all should used in scenarios
> > like free callback.
> > That way it should map properly to ALSA PCM operations.
> > 
> 
> Yes, a call that would allow us to reset/restart a transfer would be nice.
> While we are at it: Another nice addition, which would be good for sound
> drivers is a callback, which reports the transfer's current position in the
> datastream. This allows us to implement a pointer callback with sub-period
> granularity and also allow us to implement the 'no period wakeup' feature. A
> lot of DMA controllers support reporting this, but as far as I can see there
> is currently no way to export this information using dmaengine.
What makes you think it's missing :-)

Pls see device_tx_status callback. You can return the descriptor
position using this and enable "no wake mode"
> 
> - Lars


-- 
~Vinod

Re: [PATCH 19/19] ASoC: fsi: Add DMAEngine support

[Lars-Peter Clausen]

On 02/06/2012 03:46 PM, Vinod Koul wrote:
> On Mon, 2012-02-06 at 09:47 +0100, Lars-Peter Clausen wrote:
>> On 02/04/2012 07:40 PM, Vinod Koul wrote:
>>> [...]
>>>
>>> And while at it, I believe for this use, terminate_all is too strong an
>>> API to be used, perhaps we should introduce a STOP call, which just
>>> aborts the current descriptor but doesn't free it, or possibly move to
>>> submitted state/queue. And yes terminate_all should used in scenarios
>>> like free callback.
>>> That way it should map properly to ALSA PCM operations.
>>>
>>
>> Yes, a call that would allow us to reset/restart a transfer would be nice.
>> While we are at it: Another nice addition, which would be good for sound
>> drivers is a callback, which reports the transfer's current position in the
>> datastream. This allows us to implement a pointer callback with sub-period
>> granularity and also allow us to implement the 'no period wakeup' feature. A
>> lot of DMA controllers support reporting this, but as far as I can see there
>> is currently no way to export this information using dmaengine.
> What makes you think it's missing :-)
> 
> Pls see device_tx_status callback. You can return the descriptor
> position using this and enable "no wake mode"

Ah nice, I somehow missed it, thanks.

- Lars

Re: [PATCH 19/19] ASoC: fsi: Add DMAEngine support

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 828 bytes --]

On Sat, Feb 04, 2012 at 10:01:32PM +0530, Vinod Koul wrote:
> On Fri, 2012-02-03 at 14:14 +0000, Mark Brown wrote:

> > That doesn't sound insurmountible - you need a callback to allow the
> > individual drivers to set the parameters but hopefully the data flow is
> > more generic?  There will probably be some other setup type stuff that
> > needs doing.

> I had though about this sometime back, and if we add dmaengine support
> into asoc, we wouldn't need to add this kind of support in every driver.

OK, this all sounds good and it seems like Lars-Peter has already been
working on this.  It doesn't seem like it's worth holding Morimoto-san's
patch up for the infrastructure, though so once we work out what's going
on with the bugfix patch I'll go ahead with that and we can convert it
over to use the framework later.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 19/19] ASoC: fsi: Add DMAEngine support

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 394 bytes --]

On Fri, Feb 03, 2012 at 12:59:33AM -0800, Kuninori Morimoto wrote:
> This patch supports DMAEngine to FSI driver.
> It supports only Tx case at this point.
> If platform/cpu doesn't support DMAEngine, FSI driver will
> use PIO transfer.

Applied, thanks!  I have resolved the conflict with your bugfix patch
which is now merged into Linus' tree so please check that I have done
this correctly.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 19/19] ASoC: fsi: Add DMAEngine support

[Kuninori Morimoto]

Hi Mark

> [1  <text/plain; us-ascii (7bit)>]
> On Fri, Feb 03, 2012 at 12:59:33AM -0800, Kuninori Morimoto wrote:
> > This patch supports DMAEngine to FSI driver.
> > It supports only Tx case at this point.
> > If platform/cpu doesn't support DMAEngine, FSI driver will
> > use PIO transfer.
> 
> Applied, thanks!  I have resolved the conflict with your bugfix patch
> which is now merged into Linus' tree so please check that I have done
> this correctly.

Thank you for your hardwork.
Yes. It seems correct.

Best regards
---
Kuninori Morimoto

Re: ASoC: fsi: Add FSI DMAEngine support

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 273 bytes --]

On Fri, Feb 03, 2012 at 12:48:46AM -0800, Kuninori Morimoto wrote:

> These are DMAEngine support for FSI driver

Applied patches 1-17, thanks.

> FSI will use PIO version if CPU/platform doesn't have DMAEngine support.

Is this a regression compared to the existing code?

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: ASoC: fsi: Add FSI DMAEngine support

[Kuninori Morimoto]

Hi Mark

Thank you for checking patch

> > These are DMAEngine support for FSI driver
> 
> Applied patches 1-17, thanks.

Thank you.

> > FSI will use PIO version if CPU/platform doesn't have DMAEngine support.
> 
> Is this a regression compared to the existing code?

It mean compatible for old platform (which needs PIO transfer)
and new platform (which can use DMA transfer).

Best regards
---
Kuninori Morimoto