ASoC: renesas: msiof: tidyup to remove each errors

Merge series from Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>:

Current Renesas MSIOF driver might get some errors. This patch-set try to
reduce/remove them.

1 files changed, 131 insertions, 30 deletions

diff --git a/sound/soc/renesas/rcar/msiof.c b/sound/soc/renesas/rcar/msiof.c
index f5338bbb037c..f2addfbac923 100644
--- a/sound/soc/renesas/rcar/msiof.c
+++ b/sound/soc/renesas/rcar/msiof.c
@@ -7,7 +7,7 @@
 //
 
 /*
- * [NOTE]
+ * [NOTE-CLOCK-MODE]
  *
  * This driver doesn't support Clock/Frame Provider Mode
  *
@@ -24,12 +24,64 @@
  * Clock/Frame Consumer Mode.
  */
 
+/*
+ * [NOTE-RESET]
+ *
+ * MSIOF has TXRST/RXRST to reset FIFO, but it shouldn't be used during SYNC signal was asserted,
+ * because it will be cause of HW issue.
+ *
+ * When MSIOF is used as Sound driver, this driver is assuming it is used as clock consumer mode
+ * (= Codec is clock provider). This means, it can't control SYNC signal by itself.
+ *
+ * We need to use SW reset (= reset_control_xxx()) instead of TXRST/RXRST.
+ */
+
+/*
+ * [NOTE-BOTH-SETTING]
+ *
+ * SITMDRn / SIRMDRn and some other registers should not be updated during working even though it
+ * was not related the target direction (for example, do TX settings during RX is working),
+ * otherwise it cause a FSERR.
+ *
+ * Setup both direction (Playback/Capture) in the same time.
+ */
+
+/*
+ * [NOTE-R/L]
+ *
+ * The data of Captured might be R/L opposite.
+ *
+ * This driver is assuming MSIOF is used as Clock/Frame Consumer Mode, and there is a case that some
+ * Codec (= Clock/Frame Provider) might output Clock/Frame before setup MSIOF. It depends on Codec
+ * driver implementation.
+ *
+ * MSIOF will capture data without checking SYNC signal Hi/Low (= R/L).
+ *
+ * This means, if MSIOF RXE bit was set as 1 in case of SYNC signal was Hi (= R) timing, it will
+ * start capture data since next SYNC low singla (= L). Because Linux assumes sound data is lined
+ * up as R->L->R->L->..., the data R/L will be opposite.
+ *
+ * The only solution in this case is start CLK/SYNC *after* MSIOF settings, but it depends when and
+ * how Codec driver start it.
+ */
+
+/*
+ * [NOTE-FSERR]
+ *
+ * We can't remove all FSERR.
+ *
+ * Renesas have tried to minimize the occurrence of FSERR errors as much as possible, but
+ * unfortunately we cannot remove them completely, because MSIOF might setup its register during
+ * CLK/SYNC are inputed. It can be happen because MSIOF is working as Clock/Frame Consumer.
+ */
+
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_dma.h>
 #include <linux/of_graph.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/reset.h>
 #include <linux/spi/sh_msiof.h>
 #include <sound/dmaengine_pcm.h>
 #include <sound/soc.h>
@@ -60,10 +112,13 @@
 struct msiof_priv {
 	struct device *dev;
 	struct snd_pcm_substream *substream[SNDRV_PCM_STREAM_LAST + 1];
+	struct reset_control *reset;
 	spinlock_t lock;
 	void __iomem *base;
 	resource_size_t phy_addr;
 
+	int count;
+
 	/* for error */
 	int err_syc[SNDRV_PCM_STREAM_LAST + 1];
 	int err_ovf[SNDRV_PCM_STREAM_LAST + 1];
@@ -121,7 +176,7 @@ static int msiof_hw_start(struct snd_soc_component *component,
 
 	/*
 	 * see
-	 *	[NOTE] on top of this driver
+	 *	[NOTE-CLOCK-MODE] on top of this driver
 	 */
 	/*
 	 * see
@@ -131,42 +186,63 @@ static int msiof_hw_start(struct snd_soc_component *component,
 	 *	RX: Fig 109.15
 	 */
 
-	/* reset errors */
-	priv->err_syc[substream->stream] =
+	/*
+	 * Use reset_control_xx() instead of TXRST/RXRST.
+	 * see
+	 *	[NOTE-RESET]
+	 */
+	if (!priv->count)
+		reset_control_deassert(priv->reset);
+
+	priv->count++;
+
+	/*
+	 * Reset errors. ignore 1st FSERR
+	 *
+	 * see
+	 *	[NOTE-FSERR]
+	 */
+	priv->err_syc[substream->stream] = -1;
 	priv->err_ovf[substream->stream] =
 	priv->err_udf[substream->stream] = 0;
 
 	/* Start DMAC */
 	snd_dmaengine_pcm_trigger(substream, cmd);
 
+	/*
+	 * setup both direction (Playback/Capture) in the same time.
+	 * see
+	 *	above [NOTE-BOTH-SETTING]
+	 */
+
 	/* SITMDRx */
-	if (is_play) {
-		val = SITMDR1_PCON |
-		      FIELD_PREP(SIMDR1_SYNCMD, SIMDR1_SYNCMD_LR) |
-		      SIMDR1_SYNCAC | SIMDR1_XXSTP;
-		if (msiof_flag_has(priv, MSIOF_FLAGS_NEED_DELAY))
-			val |= FIELD_PREP(SIMDR1_DTDL, 1);
+	val = SITMDR1_PCON | SIMDR1_SYNCAC | SIMDR1_XXSTP |
+		FIELD_PREP(SIMDR1_SYNCMD, SIMDR1_SYNCMD_LR);
+	if (msiof_flag_has(priv, MSIOF_FLAGS_NEED_DELAY))
+		val |= FIELD_PREP(SIMDR1_DTDL, 1);
 
-		msiof_write(priv, SITMDR1, val);
+	msiof_write(priv, SITMDR1, val);
 
-		val = FIELD_PREP(SIMDR2_BITLEN1, width - 1);
-		msiof_write(priv, SITMDR2, val | FIELD_PREP(SIMDR2_GRP, 1));
-		msiof_write(priv, SITMDR3, val);
+	val = FIELD_PREP(SIMDR2_BITLEN1, width - 1);
+	msiof_write(priv, SITMDR2, val | FIELD_PREP(SIMDR2_GRP, 1));
+	msiof_write(priv, SITMDR3, val);
 
-	}
 	/* SIRMDRx */
-	else {
-		val = FIELD_PREP(SIMDR1_SYNCMD, SIMDR1_SYNCMD_LR) |
-		      SIMDR1_SYNCAC;
-		if (msiof_flag_has(priv, MSIOF_FLAGS_NEED_DELAY))
-			val |= FIELD_PREP(SIMDR1_DTDL, 1);
+	val = SIMDR1_SYNCAC |
+		FIELD_PREP(SIMDR1_SYNCMD, SIMDR1_SYNCMD_LR);
+	if (msiof_flag_has(priv, MSIOF_FLAGS_NEED_DELAY))
+		val |= FIELD_PREP(SIMDR1_DTDL, 1);
 
-		msiof_write(priv, SIRMDR1, val);
+	msiof_write(priv, SIRMDR1, val);
 
-		val = FIELD_PREP(SIMDR2_BITLEN1, width - 1);
-		msiof_write(priv, SIRMDR2, val | FIELD_PREP(SIMDR2_GRP, 1));
-		msiof_write(priv, SIRMDR3, val);
-	}
+	val = FIELD_PREP(SIMDR2_BITLEN1, width - 1);
+	msiof_write(priv, SIRMDR2, val | FIELD_PREP(SIMDR2_GRP, 1));
+	msiof_write(priv, SIRMDR3, val);
+
+	/* SIFCTR */
+	msiof_write(priv, SIFCTR,
+		    FIELD_PREP(SIFCTR_TFWM, SIFCTR_TFWM_1) |
+		    FIELD_PREP(SIFCTR_RFWM, SIFCTR_RFWM_1));
 
 	/* SIIER */
 	if (is_play)
@@ -183,10 +259,11 @@ static int msiof_hw_start(struct snd_soc_component *component,
 	msiof_update(priv, SISTR, val, val);
 
 	/* SICTR */
+	val = SICTR_TEDG | SICTR_REDG;
 	if (is_play)
-		val = SICTR_TXE | SICTR_TEDG;
+		val |= SICTR_TXE;
 	else
-		val = SICTR_RXE | SICTR_REDG;
+		val |= SICTR_RXE;
 	msiof_update_and_wait(priv, SICTR, val, val, val);
 
 	return 0;
@@ -207,9 +284,6 @@ static int msiof_hw_stop(struct snd_soc_component *component,
 		val = SIIER_RDREQE | SIIER_RDMAE | SISTR_ERR_RX;
 	msiof_update(priv, SIIER, val, 0);
 
-	/* Stop DMAC */
-	snd_dmaengine_pcm_trigger(substream, cmd);
-
 	/* SICTR */
 	if (is_play)
 		val = SICTR_TXE;
@@ -217,6 +291,18 @@ static int msiof_hw_stop(struct snd_soc_component *component,
 		val = SICTR_RXE;
 	msiof_update_and_wait(priv, SICTR, val, 0, 0);
 
+	/* Stop DMAC */
+	snd_dmaengine_pcm_trigger(substream, cmd);
+
+	/*
+	 * Ignore 1st FSERR
+	 *
+	 * see
+	 *	[NOTE-FSERR]
+	 */
+	if (priv->err_syc[substream->stream] < 0)
+		priv->err_syc[substream->stream] = 0;
+
 	/* indicate error status if exist */
 	if (priv->err_syc[substream->stream] ||
 	    priv->err_ovf[substream->stream] ||
@@ -227,6 +313,11 @@ static int msiof_hw_stop(struct snd_soc_component *component,
 			 priv->err_ovf[substream->stream],
 			 priv->err_udf[substream->stream]);
 
+	priv->count--;
+
+	if (!priv->count)
+		reset_control_assert(priv->reset);
+
 	return 0;
 }
 
@@ -302,6 +393,9 @@ static struct snd_soc_dai_driver msiof_dai_driver = {
 		.channels_max	= 2,
 	},
 	.ops = &msiof_dai_ops,
+	.symmetric_rate		= 1,
+	.symmetric_channels	= 1,
+	.symmetric_sample_bits	= 1,
 };
 
 static struct snd_pcm_hardware msiof_pcm_hardware = {
@@ -490,12 +584,19 @@ static int msiof_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->base))
 		return PTR_ERR(priv->base);
 
+	priv->reset = devm_reset_control_get_exclusive(dev, NULL);
+	if (IS_ERR(priv->reset))
+		return PTR_ERR(priv->reset);
+
+	reset_control_assert(priv->reset);
+
 	ret = devm_request_irq(dev, irq, msiof_interrupt, 0, dev_name(dev), priv);
 	if (ret)
 		return ret;
 
 	priv->dev	= dev;
 	priv->phy_addr	= res->start;
+	priv->count	= 0;
 
 	spin_lock_init(&priv->lock);
 	platform_set_drvdata(pdev, priv);