| Message ID | 87382qtwto.wl%kuninori.morimoto.gx@renesas.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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?
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
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?
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
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.
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; }