On Thu, Mar 19, 2015 at 04:15:01AM +0000, Kuninori Morimoto wrote:
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
clk_prepare_enable/unprepare() uses mutex inside, and it uses __schedule(). Then, raw_spin_lock/unlock_irq() is called, and it breaks Renesas sound driver's spin lock irq. This patch moves clk_prepare_enable/unprepare to out of spin lock area. Special thanks to Das Biju.
This is definitely a bug which should be fixed but this means we should send a fix to Linus rather than having something that depends on -next. Also...
+int rsnd_mod_clk(struct rsnd_mod *mod, int enable) +{
- struct rsnd_priv *priv = rsnd_mod_to_priv(mod);
- struct device *dev = rsnd_priv_to_dev(priv);
- /*
* clk_prepare_enable/unprepare() should not be called* from inside spin lock*/- if (enable)
clk_prepare_enable(mod->clk);- else
clk_disable_unprepare(mod->clk);
It's not entirely clear what this wrapper function is doing (and note that we don't check the error code from clk_prepare_enable()).
@@ -345,6 +366,9 @@ static int rsnd_soc_dai_trigger(struct snd_pcm_substream *substream, int cmd, int ret; unsigned long flags;
- if (cmd == SNDRV_PCM_TRIGGER_START)
rsnd_dai_call(clk, io, 1);- rsnd_lock(priv, flags);
The trigger function is called from atomic context so this seems to leave us with the same problem this was supposed to be fixing?
+static int rsnd_ssi_clk(struct rsnd_mod *mod, int enable) +{
- struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
- struct rsnd_dai_stream *io = rsnd_mod_to_io(mod);
- struct rsnd_dai *rdai = rsnd_io_to_rdai(io);
- rsnd_mod_clk(mod, enable);
- if (rsnd_rdai_is_clk_master(rdai)) {
if (rsnd_ssi_clk_from_parent(ssi))rsnd_mod_clk(&ssi->parent->mod, enable);- }
This would be clearer with a single if statement with an &&, however it seems like something that the clock API should be doing itself - it's already got support for handling enabling of parents. If we need to manually handle dependencies (which is what this looks like) that seems like a problem.