Unverified Commit b43b8ae8 authored by Kuninori Morimoto's avatar Kuninori Morimoto Committed by Mark Brown
Browse files

ASoC: rsnd: protect mod->status 



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: default avatarLinh Phung T. Y <linh.phung.jy@renesas.com>
Signed-off-by: default avatarKuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Message-Id: <87wnrklwyh.wl-kuninori.morimoto.gx@renesas.com>
Signed-off-by: default avatarMark Brown <broonie@kernel.org>
parent 1f9c82b5

sound/soc/sh/rcar/core.c

+30 −20
Original line number Diff line number Diff line
@@ -90,14 +90,6 @@ 
 *

 */



/*

 * 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,

sound/soc/sh/rcar/rsnd.h

+6 −6
Original line number Diff line number Diff line
@@ -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