ASoC: rsnd: protect mod->status
authorKuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Thu, 27 May 2021 02:41:42 +0000 (11:41 +0900)
committerMark Brown <broonie@kernel.org>
Thu, 27 May 2021 10:15:31 +0000 (11:15 +0100)
Renesas Sound uses many modules (SSI/SSIU/SRC/CTU/MIX/DVC/DMA),
and supports complex connections/path.
Thus each modules needs to save its status to correctly control it.
This status is updated when by .trigger, and .hw_params/.hw_free.

Renesas Sound is protecting modules by using lock when .trigger,
but it was not enough to protecting each modules "status" if it was
used from many paths.

1) .hw_params/.hw_free update status
2) another doesn't update status, but overwrites by same value

This patch do
1) protects .hw_params/.hw_free by lock
2) do nothing if no status update

Without this patch, protected mod->status (= .trigger) might be
overwrote by non protected mod->status (= .hw_params / .hw_free),
and in such case, CTU/MIX/DVC/SSIU/SSI which are used from
many paths might get damage.

If above issue happens, Renesas Sound will be hung (= silence)
and never be recoverd.
I could reproduce this issue by continue playing very short sound
with loop very long term (3-4 hours) through 2 inputs (= MIXer).

For updating rsnd_status_update(), this patch removes rsnd_dai_call()
debug message. Because we already have debugfs support, and is not
good match to new code.

Reported-by: Linh Phung T. Y <linh.phung.jy@renesas.com>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Message-Id: <87wnrklwyh.wl-kuninori.morimoto.gx@renesas.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
sound/soc/sh/rcar/core.c
sound/soc/sh/rcar/rsnd.h

index 28d119d3dd48343cfb411d7f4ae5486be4ba7889..2dc8aee4ac124a4f3d8c6f4830812fa7d5ff6164 100644 (file)
  *
  */
 
-/*
- * you can enable below define if you don't need
- * DAI status debug message when debugging
- * see rsnd_dbg_dai_call()
- *
- * #define RSND_DEBUG_NO_DAI_CALL 1
- */
-
 #include <linux/pm_runtime.h>
 #include "rsnd.h"
 
@@ -534,14 +526,20 @@ static enum rsnd_mod_type rsnd_mod_sequence[][RSND_MOD_MAX] = {
        },
 };
 
-static int rsnd_status_update(u32 *status,
+static int rsnd_status_update(struct rsnd_dai_stream *io,
+                             struct rsnd_mod *mod, enum rsnd_mod_type type,
                              int shift, int add, int timing)
 {
+       u32 *status     = mod->ops->get_status(mod, io, type);
        u32 mask        = 0xF << shift;
        u8 val          = (*status >> shift) & 0xF;
        u8 next_val     = (val + add) & 0xF;
        int func_call   = (val == timing);
 
+       /* no status update */
+       if (add == 0 || shift == 28)
+               return 1;
+
        if (next_val == 0xF) /* underflow case */
                func_call = -1;
        else
@@ -559,14 +557,10 @@ static int rsnd_status_update(u32 *status,
        enum rsnd_mod_type *types = rsnd_mod_sequence[is_play];         \
        for_each_rsnd_mod_arrays(i, mod, io, types, RSND_MOD_MAX) {     \
                int tmp = 0;                                            \
-               u32 *status = mod->ops->get_status(mod, io, types[i]);  \
-               int func_call = rsnd_status_update(status,              \
+               int func_call = rsnd_status_update(io, mod, types[i],   \
                                                __rsnd_mod_shift_##fn,  \
                                                __rsnd_mod_add_##fn,    \
                                                __rsnd_mod_call_##fn);  \
-               rsnd_dbg_dai_call(dev, "%s\t0x%08x %s\n",               \
-                       rsnd_mod_name(mod), *status,    \
-                       (func_call && (mod)->ops->fn) ? #fn : "");      \
                if (func_call > 0 && (mod)->ops->fn)                    \
                        tmp = (mod)->ops->fn(mod, io, param);           \
                if (unlikely(func_call < 0) ||                          \
@@ -1390,6 +1384,26 @@ static int rsnd_dai_probe(struct rsnd_priv *priv)
 /*
  *             pcm ops
  */
+static int rsnd_hw_update(struct snd_pcm_substream *substream,
+                         struct snd_pcm_hw_params *hw_params)
+{
+       struct snd_soc_dai *dai = rsnd_substream_to_dai(substream);
+       struct rsnd_dai *rdai = rsnd_dai_to_rdai(dai);
+       struct rsnd_dai_stream *io = rsnd_rdai_to_io(rdai, substream);
+       struct rsnd_priv *priv = rsnd_io_to_priv(io);
+       unsigned long flags;
+       int ret;
+
+       spin_lock_irqsave(&priv->lock, flags);
+       if (hw_params)
+               ret = rsnd_dai_call(hw_params, io, substream, hw_params);
+       else
+               ret = rsnd_dai_call(hw_free, io, substream);
+       spin_unlock_irqrestore(&priv->lock, flags);
+
+       return ret;
+}
+
 static int rsnd_hw_params(struct snd_soc_component *component,
                          struct snd_pcm_substream *substream,
                          struct snd_pcm_hw_params *hw_params)
@@ -1497,17 +1511,13 @@ static int rsnd_hw_params(struct snd_soc_component *component,
                }
        }
 
-       return rsnd_dai_call(hw_params, io, substream, hw_params);
+       return rsnd_hw_update(substream, hw_params);
 }
 
 static int rsnd_hw_free(struct snd_soc_component *component,
                        struct snd_pcm_substream *substream)
 {
-       struct snd_soc_dai *dai = rsnd_substream_to_dai(substream);
-       struct rsnd_dai *rdai = rsnd_dai_to_rdai(dai);
-       struct rsnd_dai_stream *io = rsnd_rdai_to_io(rdai, substream);
-
-       return rsnd_dai_call(hw_free, io, substream);
+       return rsnd_hw_update(substream, NULL);
 }
 
 static snd_pcm_uframes_t rsnd_pointer(struct snd_soc_component *component,
index 0527aa8e139c612cbf3b855b56e9d58c11abc191..159754b7bb536a4a4c69d0f9da132409412f629e 100644 (file)
@@ -397,12 +397,12 @@ struct rsnd_mod {
 #define __rsnd_mod_add_remove          0
 #define __rsnd_mod_add_prepare         0
 #define __rsnd_mod_add_cleanup         0
-#define __rsnd_mod_add_init             1
-#define __rsnd_mod_add_quit            -1
-#define __rsnd_mod_add_start            1
-#define __rsnd_mod_add_stop            -1
-#define __rsnd_mod_add_hw_params       1
-#define __rsnd_mod_add_hw_free         -1
+#define __rsnd_mod_add_init             1 /* needs protect */
+#define __rsnd_mod_add_quit            -1 /* needs protect */
+#define __rsnd_mod_add_start            1 /* needs protect */
+#define __rsnd_mod_add_stop            -1 /* needs protect */
+#define __rsnd_mod_add_hw_params        1 /* needs protect */
+#define __rsnd_mod_add_hw_free         -1 /* needs protect */
 #define __rsnd_mod_add_irq             0
 #define __rsnd_mod_add_pcm_new         0
 #define __rsnd_mod_add_fallback                0