[PATCH 0/4 v2] ASoC: rsnd: fixup interrupt / spin lock issue

[PATCH 0/4 v2] ASoC: rsnd: fixup interrupt / spin lock issue

[Kuninori Morimoto]

Hi Mark, Vinod
Cc Laurent, Hiep-san

These are v2 of fixup unknown interrupt issue patch which was reported by
Hiep-san in v4.1-rcX kernel. It was not happened in v4.0 because ARM
platform side couldn't use DMA transfer.

We tested these same as v1 patches, and confirmed these solved problem

Hiep-san, can you please re-test these patches ?

Kuninori Morimoto (4):
      1) dmaengine: rcar-dmac: fixup spinlock in rcar-dmac
      2) ASoC: rsnd: indicate unknown HW start
      3) ASoC: rsnd: add rsnd_dai_stream_quit()
      4) ASoC: rsnd: spin lock for interrupt handler

 drivers/dma/sh/rcar-dmac.c | 55 +++++++++++++++++++++++++++++++------------------------
 sound/soc/sh/rcar/core.c   | 25 ++++++++++++++++++-------
 sound/soc/sh/rcar/rsnd.h   |  3 +--
 sound/soc/sh/rcar/src.c    | 11 ++++++++---
 sound/soc/sh/rcar/ssi.c    | 18 ++++++++++++++----
 5 files changed, 72 insertions(+), 40 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/4 v2] dmaengine: rcar-dmac: fixup spinlock in rcar-dmac

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Current rcar-dmac driver is using spin_lock_irq() / spin_unlock_irq()
in some functions. But, some other driver might call DMAEngine API
during interrupt disabled. In such case, rcar-dmac side spin_unlock_irq()
forcefully allows all interrupts. Therefore, other driver receives
unexpected interruption, and its exclusive access control will be broken.
This patch replaces spin_lock_irq() to spin_lock_irqsave(),
and spin_unlock_irq() to spin_unlock_irqrestore().

Reported-by: Cao Minh Hiep <cm-hiep@jinso.co.jp>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>
---
v1 -> v2

 - .device_free_chan_resources() doesn't use spin_lock_irqsave()
 - rcar_dmac_isr_channel_thread() doesn't use spin_lock_irqsave()

 drivers/dma/sh/rcar-dmac.c | 37 +++++++++++++++++++++----------------
 1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
index a18d16c..e0302c7 100644
--- a/drivers/dma/sh/rcar-dmac.c
+++ b/drivers/dma/sh/rcar-dmac.c
@@ -465,6 +465,7 @@ static dma_cookie_t rcar_dmac_tx_submit(struct dma_async_tx_descriptor *tx)
 static int rcar_dmac_desc_alloc(struct rcar_dmac_chan *chan, gfp_t gfp)
 {
 	struct rcar_dmac_desc_page *page;
+	unsigned long flags;
 	LIST_HEAD(list);
 	unsigned int i;
 
@@ -482,10 +483,10 @@ static int rcar_dmac_desc_alloc(struct rcar_dmac_chan *chan, gfp_t gfp)
 		list_add_tail(&desc->node, &list);
 	}
 
-	spin_lock_irq(&chan->lock);
+	spin_lock_irqsave(&chan->lock, flags);
 	list_splice_tail(&list, &chan->desc.free);
 	list_add_tail(&page->node, &chan->desc.pages);
-	spin_unlock_irq(&chan->lock);
+	spin_unlock_irqrestore(&chan->lock, flags);
 
 	return 0;
 }
@@ -516,6 +517,7 @@ static void rcar_dmac_desc_put(struct rcar_dmac_chan *chan,
 static void rcar_dmac_desc_recycle_acked(struct rcar_dmac_chan *chan)
 {
 	struct rcar_dmac_desc *desc, *_desc;
+	unsigned long flags;
 	LIST_HEAD(list);
 
 	/*
@@ -524,9 +526,9 @@ static void rcar_dmac_desc_recycle_acked(struct rcar_dmac_chan *chan)
 	 * list_for_each_entry_safe, isn't safe if we release the channel lock
 	 * around the rcar_dmac_desc_put() call.
 	 */
-	spin_lock_irq(&chan->lock);
+	spin_lock_irqsave(&chan->lock, flags);
 	list_splice_init(&chan->desc.wait, &list);
-	spin_unlock_irq(&chan->lock);
+	spin_unlock_irqrestore(&chan->lock, flags);
 
 	list_for_each_entry_safe(desc, _desc, &list, node) {
 		if (async_tx_test_ack(&desc->async_tx)) {
@@ -539,9 +541,9 @@ static void rcar_dmac_desc_recycle_acked(struct rcar_dmac_chan *chan)
 		return;
 
 	/* Put the remaining descriptors back in the wait list. */
-	spin_lock_irq(&chan->lock);
+	spin_lock_irqsave(&chan->lock, flags);
 	list_splice(&list, &chan->desc.wait);
-	spin_unlock_irq(&chan->lock);
+	spin_unlock_irqrestore(&chan->lock, flags);
 }
 
 /*
@@ -556,12 +558,13 @@ static void rcar_dmac_desc_recycle_acked(struct rcar_dmac_chan *chan)
 static struct rcar_dmac_desc *rcar_dmac_desc_get(struct rcar_dmac_chan *chan)
 {
 	struct rcar_dmac_desc *desc;
+	unsigned long flags;
 	int ret;
 
 	/* Recycle acked descriptors before attempting allocation. */
 	rcar_dmac_desc_recycle_acked(chan);
 
-	spin_lock_irq(&chan->lock);
+	spin_lock_irqsave(&chan->lock, flags);
 
 	while (list_empty(&chan->desc.free)) {
 		/*
@@ -570,17 +573,17 @@ static struct rcar_dmac_desc *rcar_dmac_desc_get(struct rcar_dmac_chan *chan)
 		 * allocated descriptors. If the allocation fails return an
 		 * error.
 		 */
-		spin_unlock_irq(&chan->lock);
+		spin_unlock_irqrestore(&chan->lock, flags);
 		ret = rcar_dmac_desc_alloc(chan, GFP_NOWAIT);
 		if (ret < 0)
 			return NULL;
-		spin_lock_irq(&chan->lock);
+		spin_lock_irqsave(&chan->lock, flags);
 	}
 
 	desc = list_first_entry(&chan->desc.free, struct rcar_dmac_desc, node);
 	list_del(&desc->node);
 
-	spin_unlock_irq(&chan->lock);
+	spin_unlock_irqrestore(&chan->lock, flags);
 
 	return desc;
 }
@@ -593,6 +596,7 @@ static struct rcar_dmac_desc *rcar_dmac_desc_get(struct rcar_dmac_chan *chan)
 static int rcar_dmac_xfer_chunk_alloc(struct rcar_dmac_chan *chan, gfp_t gfp)
 {
 	struct rcar_dmac_desc_page *page;
+	unsigned long flags;
 	LIST_HEAD(list);
 	unsigned int i;
 
@@ -606,10 +610,10 @@ static int rcar_dmac_xfer_chunk_alloc(struct rcar_dmac_chan *chan, gfp_t gfp)
 		list_add_tail(&chunk->node, &list);
 	}
 
-	spin_lock_irq(&chan->lock);
+	spin_lock_irqsave(&chan->lock, flags);
 	list_splice_tail(&list, &chan->desc.chunks_free);
 	list_add_tail(&page->node, &chan->desc.pages);
-	spin_unlock_irq(&chan->lock);
+	spin_unlock_irqrestore(&chan->lock, flags);
 
 	return 0;
 }
@@ -627,9 +631,10 @@ static struct rcar_dmac_xfer_chunk *
 rcar_dmac_xfer_chunk_get(struct rcar_dmac_chan *chan)
 {
 	struct rcar_dmac_xfer_chunk *chunk;
+	unsigned long flags;
 	int ret;
 
-	spin_lock_irq(&chan->lock);
+	spin_lock_irqsave(&chan->lock, flags);
 
 	while (list_empty(&chan->desc.chunks_free)) {
 		/*
@@ -638,18 +643,18 @@ rcar_dmac_xfer_chunk_get(struct rcar_dmac_chan *chan)
 		 * allocated descriptors. If the allocation fails return an
 		 * error.
 		 */
-		spin_unlock_irq(&chan->lock);
+		spin_unlock_irqrestore(&chan->lock, flags);
 		ret = rcar_dmac_xfer_chunk_alloc(chan, GFP_NOWAIT);
 		if (ret < 0)
 			return NULL;
-		spin_lock_irq(&chan->lock);
+		spin_lock_irqsave(&chan->lock, flags);
 	}
 
 	chunk = list_first_entry(&chan->desc.chunks_free,
 				 struct rcar_dmac_xfer_chunk, node);
 	list_del(&chunk->node);
 
-	spin_unlock_irq(&chan->lock);
+	spin_unlock_irqrestore(&chan->lock, flags);
 
 	return chunk;
 }
-- 
1.9.1

[PATCH 2/4 v2] ASoC: rsnd: indicate unknown HW start

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

rsnd_ssi_hw_stop() should be called after rsnd_ssi_hw_start().
This patch indicates unknown hw_stop as error

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>
Tested by: Cao Minh Hiep <cm-hiep@jinso.co.jp>
---
v1 -> v2

 - add Hiep's Test-by

 sound/soc/sh/rcar/ssi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/sound/soc/sh/rcar/ssi.c b/sound/soc/sh/rcar/ssi.c
index 5b89723..927ac52 100644
--- a/sound/soc/sh/rcar/ssi.c
+++ b/sound/soc/sh/rcar/ssi.c
@@ -232,8 +232,10 @@ static void rsnd_ssi_hw_stop(struct rsnd_ssi *ssi)
 	struct device *dev = rsnd_priv_to_dev(priv);
 	u32 cr;
 
-	if (0 == ssi->usrcnt) /* stop might be called without start */
+	if (0 == ssi->usrcnt) {
+		dev_err(dev, "%s called without starting\n", __func__);
 		return;
+	}
 
 	ssi->usrcnt--;
 
-- 
1.9.1

[PATCH 3/4 v2] ASoC: rsnd: add rsnd_dai_stream_quit()

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Current Renesas R-Car sound driver calls rsnd_dai_stream_init() when
start, but it didn't call paired function. This patch adds
rsnd_dai_stream_quit() for it. This is prepare for interrupt error
status check feature support.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>
Tested by: Cao Minh Hiep <cm-hiep@jinso.co.jp>
---
v1 -> v2

 - no change, but added Hiep's Test-by

 sound/soc/sh/rcar/core.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/sound/soc/sh/rcar/core.c b/sound/soc/sh/rcar/core.c
index 405cacd..2b7323c 100644
--- a/sound/soc/sh/rcar/core.c
+++ b/sound/soc/sh/rcar/core.c
@@ -315,7 +315,7 @@ void rsnd_dai_pointer_update(struct rsnd_dai_stream *io, int byte)
 	}
 }
 
-static int rsnd_dai_stream_init(struct rsnd_dai_stream *io,
+static void rsnd_dai_stream_init(struct rsnd_dai_stream *io,
 				struct snd_pcm_substream *substream)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
@@ -327,8 +327,11 @@ static int rsnd_dai_stream_init(struct rsnd_dai_stream *io,
 				  runtime->channels *
 				  samples_to_bytes(runtime, 1);
 	io->next_period_byte	= io->byte_per_period;
+}
 
-	return 0;
+static void rsnd_dai_stream_quit(struct rsnd_dai_stream *io)
+{
+	io->substream		= NULL;
 }
 
 static
@@ -363,9 +366,7 @@ static int rsnd_soc_dai_trigger(struct snd_pcm_substream *substream, int cmd,
 
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
-		ret = rsnd_dai_stream_init(io, substream);
-		if (ret < 0)
-			goto dai_trigger_end;
+		rsnd_dai_stream_init(io, substream);
 
 		ret = rsnd_platform_call(priv, dai, start, ssi_id);
 		if (ret < 0)
@@ -391,6 +392,8 @@ static int rsnd_soc_dai_trigger(struct snd_pcm_substream *substream, int cmd,
 		ret = rsnd_platform_call(priv, dai, stop, ssi_id);
 		if (ret < 0)
 			goto dai_trigger_end;
+
+		rsnd_dai_stream_quit(io);
 		break;
 	default:
 		ret = -EINVAL;
-- 
1.9.1

[PATCH 4/4 v2] ASoC: rsnd: spin lock for interrupt handler

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Renesas R-Car driver interrupt handler was not locked before.
But now, SSI/SRC interrupt handler calls restart function
which should be called under spin lock.
Below error might happen witout this patch.

Unable to handle kernel NULL pointer dereference at virtual address 00000048
pgd = edfac000
[00000048] *pgd=6e0f0831, *pte=00000000, *ppte=00000000
Internal error: Oops: 17 [#1] SMP ARM
CPU: 0 PID: 2009 Comm: aplay Not tainted 4.1.0-rc2-dirty #4
Hardware name: Generic R8A7790 (Flattened Device Tree)
task: eeac9040 ti: eebe8000 task.ti: eebe8000
PC is at rsnd_get_adinr+0x28/0x60
LR is at rsnd_src_ssiu_start+0xdc/0x19c
pc : [<c0409790>]    lr : [<c040c068>]    psr: a0000193
sp : eebe9e58  ip : eebe9e68  fp : eebe9e64
r10: c06ed9d0  r9 : ee919d10  r8 : 00000001
r7 : 00000001  r6 : ee1cb090  r5 : 00000000  r4 : edcaa418
r3 : 00000000  r2 : eea8ce00  r1 : 80000193  r0 : edcaa418
...

Reported-by: Cao Minh Hiep <cm-hiep@jinso.co.jp>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>
Tested by: Cao Minh Hiep <cm-hiep@jinso.co.jp>
---
v1 -> v2

 - no change, but added Hiep's Test-by

 sound/soc/sh/rcar/core.c | 12 ++++++++++--
 sound/soc/sh/rcar/rsnd.h |  3 +--
 sound/soc/sh/rcar/src.c  | 11 ++++++++---
 sound/soc/sh/rcar/ssi.c  | 14 +++++++++++---
 4 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/sound/soc/sh/rcar/core.c b/sound/soc/sh/rcar/core.c
index 2b7323c..d460d2a 100644
--- a/sound/soc/sh/rcar/core.c
+++ b/sound/soc/sh/rcar/core.c
@@ -170,6 +170,14 @@ void rsnd_mod_quit(struct rsnd_mod *mod)
 		clk_unprepare(mod->clk);
 }
 
+int rsnd_mod_is_working(struct rsnd_mod *mod)
+{
+	struct rsnd_dai_stream *io = rsnd_mod_to_io(mod);
+
+	/* see rsnd_dai_stream_init/quit() */
+	return !!io->substream;
+}
+
 /*
  *	settting function
  */
@@ -362,7 +370,7 @@ static int rsnd_soc_dai_trigger(struct snd_pcm_substream *substream, int cmd,
 	int ret;
 	unsigned long flags;
 
-	rsnd_lock(priv, flags);
+	spin_lock_irqsave(&priv->lock, flags);
 
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
@@ -400,7 +408,7 @@ static int rsnd_soc_dai_trigger(struct snd_pcm_substream *substream, int cmd,
 	}
 
 dai_trigger_end:
-	rsnd_unlock(priv, flags);
+	spin_unlock_irqrestore(&priv->lock, flags);
 
 	return ret;
 }
diff --git a/sound/soc/sh/rcar/rsnd.h b/sound/soc/sh/rcar/rsnd.h
index 4e6de68..03ff071 100644
--- a/sound/soc/sh/rcar/rsnd.h
+++ b/sound/soc/sh/rcar/rsnd.h
@@ -303,6 +303,7 @@ int rsnd_mod_init(struct rsnd_mod *mod,
 		   int id);
 void rsnd_mod_quit(struct rsnd_mod *mod);
 char *rsnd_mod_name(struct rsnd_mod *mod);
+int rsnd_mod_is_working(struct rsnd_mod *mod);
 struct dma_chan *rsnd_mod_dma_req(struct rsnd_mod *mod);
 
 /*
@@ -449,8 +450,6 @@ struct rsnd_priv {
 #define rsnd_priv_to_pdev(priv)	((priv)->pdev)
 #define rsnd_priv_to_dev(priv)	(&(rsnd_priv_to_pdev(priv)->dev))
 #define rsnd_priv_to_info(priv)	((priv)->info)
-#define rsnd_lock(priv, flags) spin_lock_irqsave(&priv->lock, flags)
-#define rsnd_unlock(priv, flags) spin_unlock_irqrestore(&priv->lock, flags)
 
 /*
  *	rsnd_kctrl
diff --git a/sound/soc/sh/rcar/src.c b/sound/soc/sh/rcar/src.c
index 3beb32e..fbe9166 100644
--- a/sound/soc/sh/rcar/src.c
+++ b/sound/soc/sh/rcar/src.c
@@ -673,10 +673,13 @@ static int _rsnd_src_stop_gen2(struct rsnd_mod *mod)
 static irqreturn_t rsnd_src_interrupt_gen2(int irq, void *data)
 {
 	struct rsnd_mod *mod = data;
-	struct rsnd_dai_stream *io = rsnd_mod_to_io(mod);
+	struct rsnd_priv *priv = rsnd_mod_to_priv(mod);
+
+	spin_lock(&priv->lock);
 
-	if (!io)
-		return IRQ_NONE;
+	/* ignore all cases if not working */
+	if (!rsnd_mod_is_working(mod))
+		goto rsnd_src_interrupt_gen2_out;
 
 	if (rsnd_src_error_record_gen2(mod)) {
 		struct rsnd_priv *priv = rsnd_mod_to_priv(mod);
@@ -692,6 +695,8 @@ static irqreturn_t rsnd_src_interrupt_gen2(int irq, void *data)
 		else
 			dev_warn(dev, "no more SRC restart\n");
 	}
+rsnd_src_interrupt_gen2_out:
+	spin_unlock(&priv->lock);
 
 	return IRQ_HANDLED;
 }
diff --git a/sound/soc/sh/rcar/ssi.c b/sound/soc/sh/rcar/ssi.c
index 927ac52..50fa392 100644
--- a/sound/soc/sh/rcar/ssi.c
+++ b/sound/soc/sh/rcar/ssi.c
@@ -423,10 +423,15 @@ static irqreturn_t rsnd_ssi_interrupt(int irq, void *data)
 	struct rsnd_priv *priv = rsnd_mod_to_priv(mod);
 	struct rsnd_dai_stream *io = rsnd_mod_to_io(mod);
 	int is_dma = rsnd_ssi_is_dma_mode(mod);
-	u32 status = rsnd_mod_read(mod, SSISR);
+	u32 status;
+
+	spin_lock(&priv->lock);
 
-	if (!io)
-		return IRQ_NONE;
+	/* ignore all cases if not working */
+	if (!rsnd_mod_is_working(mod))
+		goto rsnd_ssi_interrupt_out;
+
+	status = rsnd_mod_read(mod, SSISR);
 
 	/* PIO only */
 	if (!is_dma && (status & DIRQ)) {
@@ -466,6 +471,9 @@ static irqreturn_t rsnd_ssi_interrupt(int irq, void *data)
 
 	rsnd_ssi_record_error(ssi, status);
 
+rsnd_ssi_interrupt_out:
+	spin_unlock(&priv->lock);
+
 	return IRQ_HANDLED;
 }
 
-- 
1.9.1

Re: [PATCH 0/4 v2] ASoC: rsnd: fixup interrupt / spin lock issue

[Cao Minh Hiep]

Hi Morimoto-san,

On 2015年05月21日 12:47, Kuninori Morimoto wrote:
> Hi Mark, Vinod
> Cc Laurent, Hiep-san
>
> These are v2 of fixup unknown interrupt issue patch which was reported by
> Hiep-san in v4.1-rcX kernel. It was not happened in v4.0 because ARM
> platform side couldn't use DMA transfer.
>
> We tested these same as v1 patches, and confirmed these solved problem
>
> Hiep-san, can you please re-test these patches ?
>
> Kuninori Morimoto (4):
>        1) dmaengine: rcar-dmac: fixup spinlock in rcar-dmac
>        2) ASoC: rsnd: indicate unknown HW start
>        3) ASoC: rsnd: add rsnd_dai_stream_quit()
>        4) ASoC: rsnd: spin lock for interrupt handler
Thanks for your patch!
I'll re-test these patches now.

Best Regards,
Hiep.

Re: [PATCH 2/4 v2] ASoC: rsnd: indicate unknown HW start

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 269 bytes --]

On Thu, May 21, 2015 at 03:49:13AM +0000, Kuninori Morimoto wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> rsnd_ssi_hw_stop() should be called after rsnd_ssi_hw_start().
> This patch indicates unknown hw_stop as error

Applied, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 3/4 v2] ASoC: rsnd: add rsnd_dai_stream_quit()

[Mark Brown]

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

On Thu, May 21, 2015 at 03:49:54AM +0000, Kuninori Morimoto wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> Current Renesas R-Car sound driver calls rsnd_dai_stream_init() when
> start, but it didn't call paired function. This patch adds
> rsnd_dai_stream_quit() for it. This is prepare for interrupt error
> status check feature support.

Applied, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 4/4 v2] ASoC: rsnd: spin lock for interrupt handler

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 446 bytes --]

On Thu, May 21, 2015 at 03:50:23AM +0000, Kuninori Morimoto wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> Renesas R-Car driver interrupt handler was not locked before.
> But now, SSI/SRC interrupt handler calls restart function
> which should be called under spin lock.
> Below error might happen witout this patch.

This looks fine but I guess it depends on patch 1 otherwise more errors
will be introduced?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 4/4 v2] ASoC: rsnd: spin lock for interrupt handler

[Vinod Koul]

On Thu, May 21, 2015 at 11:59:40AM +0100, Mark Brown wrote:
> On Thu, May 21, 2015 at 03:50:23AM +0000, Kuninori Morimoto wrote:
> > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > 
> > Renesas R-Car driver interrupt handler was not locked before.
> > But now, SSI/SRC interrupt handler calls restart function
> > which should be called under spin lock.
> > Below error might happen witout this patch.
> 
> This looks fine but I guess it depends on patch 1 otherwise more errors
> will be introduced?
The 1st one looks good to me and was about to apply, but saw this and to
sync would you the 1st patch thru ASoC tree or fine with me applying that
one

-- 
~Vinod

Re: [PATCH 4/4 v2] ASoC: rsnd: spin lock for interrupt handler

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 637 bytes --]

On Thu, May 21, 2015 at 09:45:52PM +0530, Vinod Koul wrote:
> On Thu, May 21, 2015 at 11:59:40AM +0100, Mark Brown wrote:

> > This looks fine but I guess it depends on patch 1 otherwise more errors
> > will be introduced?

> The 1st one looks good to me and was about to apply, but saw this and to
> sync would you the 1st patch thru ASoC tree or fine with me applying that
> one

Morimoto-san, do the two patches need to go through the same tree?  I
guess it's better to have them both in the ASoC tree given how much work
you're doing on these drivers.  Vinod, do you want to apply and send a
pull request or do you just want to ack?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 4/4 v2] ASoC: rsnd: spin lock for interrupt handler

[Kuninori Morimoto]

Hi Mark

> > The 1st one looks good to me and was about to apply, but saw this and to
> > sync would you the 1st patch thru ASoC tree or fine with me applying that
> > one
> 
> Morimoto-san, do the two patches need to go through the same tree?  I
> guess it's better to have them both in the ASoC tree given how much work
> you're doing on these drivers.  Vinod, do you want to apply and send a
> pull request or do you just want to ack?

Thank you. Yes in same tree is understandable.

Best regards
---
Kuninori Morimoto

Re: [PATCH 0/4 v2] ASoC: rsnd: fixup interrupt / spin lock issue

[Cao Minh Hiep]

Hi Morimoto-san,
Sorry for late replying.

On 2015年05月21日 12:47, Kuninori Morimoto wrote:
> Hi Mark, Vinod
> Cc Laurent, Hiep-san
>
> These are v2 of fixup unknown interrupt issue patch which was reported by
> Hiep-san in v4.1-rcX kernel. It was not happened in v4.0 because ARM
> platform side couldn't use DMA transfer.
>
> We tested these same as v1 patches, and confirmed these solved problem
>
> Hiep-san, can you please re-test these patches ?
>
> Kuninori Morimoto (4):
>        1) dmaengine: rcar-dmac: fixup spinlock in rcar-dmac
>        2) ASoC: rsnd: indicate unknown HW start
>        3) ASoC: rsnd: add rsnd_dai_stream_quit()
>        4) ASoC: rsnd: spin lock for interrupt handler
>
>   drivers/dma/sh/rcar-dmac.c | 55 +++++++++++++++++++++++++++++++------------------------
>   sound/soc/sh/rcar/core.c   | 25 ++++++++++++++++++-------
>   sound/soc/sh/rcar/rsnd.h   |  3 +--
>   sound/soc/sh/rcar/src.c    | 11 ++++++++---
>   sound/soc/sh/rcar/ssi.c    | 18 ++++++++++++++----
>   5 files changed, 72 insertions(+), 40 deletions(-)
We re-tested these patches on our Lager board.
(30 times playback + 10 minutes recode and playback)
It works very well without any problems,

Best Regards,
Cao Minh Hiep.

Re: [PATCH 1/4 v2] dmaengine: rcar-dmac: fixup spinlock in rcar-dmac

[Vinod Koul]

On Thu, May 21, 2015 at 03:48:38AM +0000, Kuninori Morimoto wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> Current rcar-dmac driver is using spin_lock_irq() / spin_unlock_irq()
> in some functions. But, some other driver might call DMAEngine API
> during interrupt disabled. In such case, rcar-dmac side spin_unlock_irq()
> forcefully allows all interrupts. Therefore, other driver receives
> unexpected interruption, and its exclusive access control will be broken.
> This patch replaces spin_lock_irq() to spin_lock_irqsave(),
> and spin_unlock_irq() to spin_unlock_irqrestore().
> 
> Reported-by: Cao Minh Hiep <cm-hiep@jinso.co.jp>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>
Acked-by: Vinod Koul <vinod.koul@intel.com>

Mark, please apply this thru your tree

Thanks
-- 
~Vinod

Re: [PATCH 1/4 v2] dmaengine: rcar-dmac: fixup spinlock in rcar-dmac

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 297 bytes --]

On Thu, May 21, 2015 at 03:48:38AM +0000, Kuninori Morimoto wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> Current rcar-dmac driver is using spin_lock_irq() / spin_unlock_irq()
> in some functions. But, some other driver might call DMAEngine API

Applied, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 4/4 v2] ASoC: rsnd: spin lock for interrupt handler

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 370 bytes --]

On Thu, May 21, 2015 at 03:50:23AM +0000, Kuninori Morimoto wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> Renesas R-Car driver interrupt handler was not locked before.
> But now, SSI/SRC interrupt handler calls restart function
> which should be called under spin lock.
> Below error might happen witout this patch.

Applied, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]